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

Animation: Rename frame parameter to xrFrame. #30558

Merged
merged 2 commits into from
Feb 19, 2025
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Feb 19, 2025

Related issue: -

Description

The PR removes the frame parameter from the renderer's internal animation loop since it is always undefined. requestAnimationFrame() only has a single parameter.

The PR renames frame to xrFrame for better clarity.

Copy link

github-actions bot commented Feb 19, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.51
78.38
336.51
78.38
+0 B
+0 B
WebGPU 521.78
144.83
521.78
144.83
+0 B
+0 B
WebGPU Nodes 521.25
144.72
521.25
144.72
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 465.51
112.23
465.51
112.23
+0 B
+0 B
WebGPU 592.15
160.44
592.15
160.44
+0 B
+0 B
WebGPU Nodes 547.28
149.88
547.28
149.88
+0 B
+0 B

@mrxz
Copy link
Contributor

mrxz commented Feb 19, 2025

Isn't the frame parameter used in case of WebXR?

function onAnimationFrame( time, frame ) {

@Mugen87 Mugen87 closed this Feb 19, 2025
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Feb 19, 2025

Thanks for the heads up. I've mixed XRFrame and NodeFrame.

@Mugen87 Mugen87 reopened this Feb 19, 2025
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Feb 19, 2025

Renaming the parameter so the signature gets more clear 😇 .

@Mugen87 Mugen87 added this to the r174 milestone Feb 19, 2025
@Mugen87 Mugen87 changed the title Animation: Remove obsolete parameter. Animation: Rename frame parameter to xrFrame. Feb 19, 2025
@Mugen87 Mugen87 merged commit 0e8894c into mrdoob:dev Feb 19, 2025
12 checks passed
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.

2 participants