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

Cleanup: Update references to BatchedMesh.maxGeometryCount #28694

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

Methuselah96
Copy link
Contributor

@Methuselah96 Methuselah96 commented Jun 18, 2024

Related issue: N/A

Description

BatchMeshed.maxGeometryCount was renamed to maxInstanceCount in #28462, but not all references were updated. This PR updates those references.

@@ -752,7 +752,7 @@ class Object3D extends EventDispatcher {
sphereCenter: bound.sphere.center.toArray()
} ) );

object.maxGeometryCount = this._maxGeometryCount;
object.maxInstanceCount = this._maxInstanceCount;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how versioning works for toJSON(), does this require a major version bump of metadata.version on the outputted json?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would ignore this aspect for now. If someone complains, we can consider to apply a fallback on maxGeometryCount.

Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
679.4 kB (168.3 kB) 679.4 kB (168.3 kB) +0 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
457.4 kB (110.4 kB) 457.4 kB (110.4 kB) +0 B

@Mugen87 Mugen87 added this to the r166 milestone Jun 18, 2024
@Mugen87 Mugen87 merged commit 4601056 into mrdoob:dev Jun 18, 2024
12 checks passed
@Methuselah96 Methuselah96 deleted the batched-mesh-max-instance-count branch June 18, 2024 18:35
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