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

Renderer: Call ID #27101

Merged
merged 12 commits into from Nov 4, 2023
Merged

Renderer: Call ID #27101

merged 12 commits into from Nov 4, 2023

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Nov 2, 2023

Related issue:
#26673 (comment),
https://discourse.threejs.org/t/has-anyone-noticed-any-unusual-behavior-in-the-uniform-node/56657/2

Description

This PR adds info.call, info.compute.call, info.render.call and remove the current behavior of info.render.frame.

info.render.frame is incremented every time we call render() but in theory one frame is the lapse of one keyframe for another, in a single frame we can have several RTT, for example. This caused confusion in the cache system and had duplicate calls in the NodeUpdateType.FRAME settings causing overloads and in other cases had problems with bindings updates.

@sunag sunag marked this pull request as ready for review November 3, 2023 18:58
@sunag sunag added this to the r159 milestone Nov 3, 2023
@sunag sunag merged commit 6d7566e into mrdoob:dev Nov 4, 2023
19 checks passed
@sunag sunag deleted the dev-callid branch November 4, 2023 21:13
if ( this.isAnimating === true || this.animationLoop === null || this.nodes === null ) return;

this.isAnimating = true;
_init() {

const update = ( time, frame ) => {

this.requestId = self.requestAnimationFrame( update );
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually... is this correct that this function is called every frame using rAF even if animationLoop isn't set? /ping @sunag

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was intentional, nodeFrame.update() cannot be called in render() to work correctly, this fix cases where no animationLoop was added.

Copy link
Contributor

@LeviPesin LeviPesin Jan 12, 2024

Choose a reason for hiding this comment

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

Why it cannot be called there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to know what frame we are in to trigger NodeUpdateType.FRAME frame events, otherwise we will only have NodeUpdateType.RENDER call events. The frame counter makes sure which frame is in order to avoid duplicate events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants