-
Notifications
You must be signed in to change notification settings - Fork 751
Rendering starts too late with frameloop demand and CameraControls transitions #2005
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
Comments
The CameraControls update code looks pretty suspicious to me. It only calls Something like this would fix it, but I'm not sure if it is actually wanted here. diff --git a/src/core/CameraControls.tsx b/src/core/CameraControls.tsx
index 082b967..b2dcab7 100644
--- a/src/core/CameraControls.tsx
+++ b/src/core/CameraControls.tsx
@@ -1,5 +1,6 @@
import {
Box3,
+ Clock,
EventDispatcher,
MathUtils,
Matrix4,
@@ -16,7 +17,7 @@ import {
import * as React from 'react'
import { forwardRef, useMemo, useEffect } from 'react'
-import { extend, useFrame, useThree, ReactThreeFiber, EventManager } from '@react-three/fiber'
+import { extend, useThree, ReactThreeFiber, EventManager } from '@react-three/fiber'
import CameraControlsImpl from 'camera-controls'
import { ForwardRefComponent } from '../helpers/ts-utils'
@@ -81,18 +82,24 @@ export const CameraControls: ForwardRefComponent<CameraControlsProps, CameraCont
const controls = useMemo(() => new CameraControlsImpl(explCamera), [explCamera])
- useFrame((state, delta) => {
- if (controls.enabled) controls.update(delta)
- }, -1)
-
useEffect(() => {
controls.connect(explDomElement)
return () => void controls.disconnect()
}, [explDomElement, controls])
useEffect(() => {
+ const clock = new Clock()
+ let animationFrame: number
+ const update = () => {
+ animationFrame = requestAnimationFrame(update)
+ const delta = clock.getDelta()
+ const needFrame = controls.update(delta)
+ if (needFrame) {
+ invalidate()
+ }
+ }
+
const callback = (e) => {
- invalidate()
if (regress) performance.regress()
if (onChange) onChange(e)
}
@@ -111,6 +118,7 @@ export const CameraControls: ForwardRefComponent<CameraControlsProps, CameraCont
controls.addEventListener('control', callback)
controls.addEventListener('transitionstart', callback)
controls.addEventListener('wake', callback)
+ animationFrame = requestAnimationFrame(update)
return () => {
controls.removeEventListener('update', callback)
@@ -119,6 +127,7 @@ export const CameraControls: ForwardRefComponent<CameraControlsProps, CameraCont
controls.removeEventListener('control', callback)
controls.removeEventListener('transitionstart', callback)
controls.removeEventListener('wake', callback)
+ cancelAnimationFrame(animationFrame)
}
}, [controls, onStart, onEnd, invalidate, setEvents, regress, onChange]) |
Is there a reason that const onStartCb: CameraControlsProps["onStart"] = (e) => {
callback(e); // trigger callback here as well
if (onStart) onStart(e);
}; EDIT: |
I am still looking into other solutions but can't find any, could your solution have a big performance impact? |
It's a bit ugly, and it has an extra delay of one frame if the requestAnimationFrame callback ends up being called after rendering. A better way to implement it would probably be to somehow include it in fiber's frame loop, but I'm not sure how that should be done. |
useFrame is the ticker/frameloop. opening up new requestAnimFrame-loops can only hurt performance, you should imo never call raf and always use useFrame instead. what is const needFrame = controls.update(delta) ? |
quick fix <mesh
onClick={() => {
invalidate()
requestAnimationFrame(() => controls.dolly(1, true))
}} imo the problem is that invalidate, by design, doesn't render. it will schedule a render. by that time .dolly, if it is sync, has already begun running. invalidate() will pre-emptively schedule render. requestAnimationFrame(() => ... would execute the code next frame, so render and dolly are now in sync. on demand rendering can pose challenges, since render has to be a centralized effort. but being able to control the flow with useland invalidate & raf seems acceptable. |
I tried it in some of the projects where I had this problem and it does work well, thanks a lot! It was the only issue that kept me from fully committing to on-demand rendering, so this might not be the only case where this fix is useful. |
will add a small example for keeping things in sync. |
I just realized that no frames are actually skipped, but the
ensures that a frame has been rendered just before starting the animation, so it keeps the delta small enough.
|
interesting! so cc thinks it will always render. this might be a limitation of the library. regardless, the raf trick would also take care of animations that truly start immediately. i added the gotcha to the docs |
three
version: 0.0165@react-three/fiber
version: 8.16.8@react-three/drei
version: 9.107.0Problem description:
When setting the canvas frameloop to
demand
, the<CameraControls />
functions triggerinvalidate()
too late.When the canvas is not being rendered and you do an action like this, you will notice the problem:
When the transition is set to
false
, it is not noticeable. But with the transition, the first few frames are skipped.When de Canvas is already rendering, this problem does not happen, it is only the initial trigger.
Relevant code:
In the codesandbox below you will notice that if you give the Canvas time to stop rendering, and you then click the box, the initial transition of the camera controls is not smooth. If you keep clicking it, the following transitions (when the canvas is still running) are smooth.
https://codesandbox.io/p/sandbox/use-controls-bug-2mtn6l
Suggested solution:
I am unsure if this is a @react-three/fiber problem or something in the CameraControls component. It might even have something to do with how the camera-controls library works, but it would be nice to see this fixed, or have a workaround.
I have tried to use
invalidate
to start rendering earlier but did not have any success with this.Thanks!
The text was updated successfully, but these errors were encountered: