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
feat: 633 use loop #673
Conversation
Co-authored-by: Tino Koch <17991193+Tinoooo@users.noreply.github.com>
There was a problem hiding this 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
I agree with @Tinoooo . |
Awesome @andretchen0 , I already made the changes on the return and documented the |
src/composables/useLoop/index.ts
Outdated
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 }) | ||
} |
There was a problem hiding this comment.
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(() => { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 passedrender
,invalidate
,advance
, andrenderMode
– 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.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to reduce dependencies in useRenderer
, but I won't block the PR because of it. It's your call.
@@ -185,6 +193,31 @@ export function useTresContextProvider({ | |||
root: ctx, | |||
} | |||
|
|||
// The loop | |||
|
|||
ctx.loop.register(() => { |
There was a problem hiding this comment.
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
?
Co-authored-by: Tino Koch <17991193+Tinoooo@users.noreply.github.com>
Co-authored-by: Tino Koch <17991193+Tinoooo@users.noreply.github.com>
docs/directives/v-always-look-at.md
Outdated
@@ -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. |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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
src/composables/useLoop/index.ts
Outdated
|
||
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 }) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
src/composables/useRenderer/index.ts
Outdated
@@ -274,13 +250,33 @@ export function useRenderer( | |||
} | |||
}) | |||
|
|||
// Register loop |
There was a problem hiding this comment.
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
.
src/composables/useLoop/index.ts
Outdated
function render(cb: Fn) { | ||
const wrappedCallback = wrapCallback(cb) | ||
const { off } = loop.register(wrappedCallback, 'render') | ||
return { off } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't be off
ed.
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>
There was a problem hiding this comment.
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
…to `useRender`" This reverts commit 24cec65.
…er with defaultFn fallback
BREAKING_CHANGE: Directives `vAlwaysLookAt` and `vRotate` due incompatibility with new `useLoop` and the refactor of the render loop logic.
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 aTresCanvas
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
Take over the render loop
The user can take over the render loop callback by using the
render
methodNote
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
Priority mechanism
Both
useBeforeRender
anduseAfteRender
provide an optional priority index. This indexes could be any from[Number.NEGATIVE_INFINITY ... 0 ... Number.NEGATIVE_INFINITY]
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
andresume
methods:Pausing and resuming the render
You can use
pauseRender
andresumeRender
methods:Pending