Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 633 use loop #673

Merged
merged 53 commits into from May 14, 2024
Merged

feat: 633 use loop #673

merged 53 commits into from May 14, 2024

Conversation

alvarosabu
Copy link
Member

@alvarosabu alvarosabu commented May 4, 2024

Hopefully Closes #633

Based on the team discussion. This PR introduces a new API

useLoop

This composable allows you to execute a callback on every frame, similar to useRenderLoop but unique to each TresCanvas instance and with access to the context.

Warning

useLoop can only be used inside of a TresCanvas since this component acts as the provider for the context data.

Usage

Register update callbacks

The user can register update callbacks (such as animations, fbo, etc) using the onBeforeRender

<script setup>
import { useLoop } from '@tresjs/core'

const boxRef = ref()

const { onBeforeRender } = useLoop()

onBeforeRender(({ delta }) => {
  boxRef.value.rotation.y += delta
})
</script>

Take over the render loop

The user can take over the render loop callback by using the render method

const { render } = useLoop()

render(({ renderer, scene, camera }) => {
  renderer.render(scene, camera)
})

Note

To run this example please run the playground and go to http://localhost:5173/advanced/take-over-loop

Register after render callbacks (ex physics calculations)

The user can also register after rendering callbacks using the onAfterRender

const { onAfterRender } = useLoop()

onAfterRender(({ renderer }) => {
  // Calculations
})

Priority mechanism

Both useBeforeRender and useAfteRender provide an optional priority index. This indexes could be any from [Number.NEGATIVE_INFINITY ... 0 ... Number.NEGATIVE_INFINITY]

const { onBeforeRender, pause, resume } = useLoop()

onBeforeRender((state) => {
  if (!sphereRef.value) { return }
  console.log('updating sphere')
  sphereRef.value.position.y += Math.sin(state.elapsed) * 0.01
})

onBeforeRender(() => {
   console.log('this should happen before updating the sphere')
}, -1)

For example, to use Frame Object Buffer (FBO) is convenient to use a very big number to ensure that all updates happen before the FBO callback and the last one just before the scene render.

Note

To run this example (FBO) please run the playground and go to http://localhost:5173/advanced/fbo`

Pausing and resuming the loop

The end user can use pause and resume methods:

const { onBeforeRender, pause, resume } = useLoop()

onBeforeRender(({ elapse }) => {
  sphereRef.value.position.y += Math.sin(elapsed) * 0.01
})

pause() // This stops the clock, delta = 0 and freezes elapsed.

Diagram onLoop

Pausing and resuming the render

You can use pauseRender and resumeRender methods:

const { pauseRender, resumeRender } = useLoop()

onBeforeRender(({ elapse }) => {
  sphereRef.value.position.y += Math.sin(elapsed) * 0.01
})

pauseRender() // This will pause the renderer
resumeRender() // This will resume the renderer

Pending

  • Tests
  • Documentation

@alvarosabu alvarosabu added feature p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) p3-significant High-priority enhancement (priority) labels May 4, 2024
@alvarosabu alvarosabu self-assigned this May 4, 2024
@alvarosabu alvarosabu requested a review from Tinoooo May 4, 2024 15:14
@alvarosabu alvarosabu marked this pull request as draft May 4, 2024 15:14
Co-authored-by: Tino Koch <17991193+Tinoooo@users.noreply.github.com>
@alvarosabu alvarosabu requested a review from Tinoooo May 8, 2024 15:08
Copy link
Member

@JaimeTorrealba JaimeTorrealba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see problems, thanks for this big work Alvaro

@andretchen0
Copy link
Contributor

I agree with @Tinoooo . off should be documented. on and trigger shouldn't be returned.

@alvarosabu
Copy link
Member Author

alvarosabu commented May 9, 2024

I agree with @Tinoooo . off should be documented. on and trigger shouldn't be returned.

Awesome @andretchen0 , I already made the changes on the return and documented the off method

docs/api/composables.md Outdated Show resolved Hide resolved
docs/api/composables.md Outdated Show resolved Hide resolved
src/components/TresCanvas.vue Outdated Show resolved Hide resolved
Comment on lines 50 to 52
const wrappedCallback = (params: LoopCallback) => {
cb({ ...params, camera: unref(camera) as TresCamera, scene: unref(scene), renderer: unref(renderer), raycaster: unref(raycaster), controls: unref(controls), invalidate, advance })
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: I suggest creating a method that creates this method to avoid code duplication like mentioned here

@@ -185,6 +193,31 @@ export function useTresContextProvider({
root: ctx,
}

// The loop

ctx.loop.register(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Can this be moved to useRenderer? This would be beneficial for the seperation of concerns and the dependencies would be more obvious.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I had commented the opposite.

Making this case for that:

If we move the question "should the scene be rendered?" to useRenderer that means:

  • useRenderer has to be passed render, invalidate, advance, and renderMode – 4 dependencies it otherwise doesn't require
  • it has to know and implement the update policies, which are orthogonal to what WebGLRenderer otherwise does.

If we want to keep the update in useRenderer, I think it'd be better off encapulated in a single dependency like onRender() – which it would simply call. But I'd personally prefer that it be removed from useRenderer altogether. The render policy can be part of the loop:

if (renderPolicy.shouldRender()) {
    renderer.render()
}

In this way, we eliminate the dependencies in useRenderer altogether and put them behind an interface – nothing else needs to muck around with the implementation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already moved it.... can we please agree on one way @Tinoooo @andretchen0 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alvarosabu

I'd prefer to reduce dependencies in useRenderer, but I won't block the PR because of it. It's your call.

src/composables/useTresContextProvider/index.ts Outdated Show resolved Hide resolved
@@ -185,6 +193,31 @@ export function useTresContextProvider({
root: ctx,
}

// The loop

ctx.loop.register(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Can/Should we make sure that this is resored once a user replaced the rendering with a custom callback and removed it afterwards via off?

@@ -2,6 +2,10 @@

With the new directive v-always-look-at provided by **TresJS**, you can add easily command an [Object3D](https://threejs.org/docs/index.html?q=object#api/en/core/Object3D) to always look at a specific position, this could be passed as a Vector3 or an Array.

::: warning
This directive can be only be used inside of a `TresCanvas` since this component acts as the provider for the context data and it uses `onLoop` under the hood.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? I'm getting

Error: useTresContext must be used together with useTresContextProvider

vAlwaysLookAt only runs if updated – I think that's a bug – so you'll have to update it in order to get the error to throw.

<script setup lang="ts">
import { shallowRef } from 'vue'
import { vAlwaysLookAt, vLightHelper } from '@tresjs/core'

const other = shallowRef([1,1,1])

setTimeout(() => other.value = [2,1,1], 1000)
</script>

<template>
  <TresDirectionalLight v-light-helper v-always-look-at="other" :position="[3, 3, 3]" :intensity="1" />
</template>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna remove the directives with loop. They are not adding enough value for the troubles


function wrapCallback(cb: Fn) {
return (params: LoopCallback) => {
cb({ ...params, camera: unref(camera) as TresCamera, scene: unref(scene), renderer: unref(renderer), raycaster: unref(raycaster), controls: unref(controls), invalidate, advance })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, it's probably be best to avoid wrapping the callback.

Currently, for every callback function in every frame, this code is recreating the argument object.

The argument object only needs to be created once.

Copy link
Member Author

@alvarosabu alvarosabu May 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andretchen0 how do we get the latest context if we are returning the plain values? if we called once on the onLoop is only going to return the snapshot of the context at that specific moment.

@@ -274,13 +250,33 @@ export function useRenderer(
}
})

// Register loop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For separation of concerns, I'd move this somewhere else.

Almost all core code has access to the renderer, so this could live just about anywhere.

On the other hand, useRenderer has to be passed render, loop, invalidate, advance, in order to do loop.register.

function render(cb: Fn) {
const wrappedCallback = wrapCallback(cb)
const { off } = loop.register(wrappedCallback, 'render')
return { off }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be offed.

playground/src/components/TakeOverLoopExperience.vue

<script setup lang="ts">
import { useLoop } from '@tresjs/core'

import { OrbitControls } from '@tresjs/cientos'
import { useControls } from '@tresjs/leches'

const { render, pauseRender, resumeRender } = useLoop()

const off = render(({ renderer, scene, camera }) => {
  console.log('on')
  renderer.render(scene, camera)
}).off

setTimeout(() => {off(); console.log('offing')}, 1000)

const { isRenderPaused } = useControls({
  isRenderPaused: {
    value: false,
    type: 'boolean',
    label: 'Pause Render',
  },
})

watchEffect(() => {
  if (isRenderPaused.value) {
    pauseRender()
  }
  else {
    resumeRender()
  }
})
</script>

<template>
  <TresPerspectiveCamera :position="[3, 3, 3]" />
  <OrbitControls />
  <AnimatedObjectUseUpdate />
  <TresAmbientLight :intensity="1" /> />
  <TresGridHelper />
</template>

Copy link
Member Author

@alvarosabu alvarosabu May 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @andretchen0 please check the discord discussion

BREAKING_CHANGE: Directives `vAlwaysLookAt` and `vRotate` due incompatibility with new `useLoop` and the refactor of the render loop logic.
@alvarosabu alvarosabu merged commit 1b2fa70 into v4 May 14, 2024
3 checks passed
@alvarosabu alvarosabu deleted the feature/633-use-loop-proposal-2 branch May 14, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) p3-significant High-priority enhancement (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants