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

Object3D: minimize rotation-quaternion conversions #29108

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

CodyJasonBennett
Copy link
Contributor

Description

Breaks up rotation/quaternion optimizations from #28534, where the conversion is only performed both when data has changed and on the next read if at all (instead of unconditionally on member write which may be much higher frequency).

Copy link

github-actions bot commented Aug 10, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
685.1 kB (169.6 kB) 684.4 kB (169.4 kB) -691 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
462 kB (111.4 kB) 461.3 kB (111.3 kB) -691 B

@@ -28,6 +28,36 @@ const _removedEvent = { type: 'removed' };
const _childaddedEvent = { type: 'childadded', child: null };
const _childremovedEvent = { type: 'childremoved', child: null };

function listenToProperties( object, properties, onReadCallback, onWriteCallback ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be moved to math classes' getters/setters, but I thought inlining here would make them easier to maintain.

Most of the diff from this PR is removing the indirection and callbacks from math classes.

get x() {
get _x() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_x, _y, _z fields etc. are preserved as getters/setters to not break user code (library code no longer references these).

If these are considered internal, we can exclude them without deprecation.

@CodyJasonBennett CodyJasonBennett marked this pull request as draft August 10, 2024 11:28
@CodyJasonBennett CodyJasonBennett marked this pull request as ready for review August 10, 2024 11:34
@CodyJasonBennett CodyJasonBennett marked this pull request as draft August 10, 2024 11:41
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.

1 participant