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

BatchedMesh: add getColorAt and setColorAt #28255

Merged
merged 6 commits into from
May 17, 2024

Conversation

Kikedao
Copy link
Contributor

@Kikedao Kikedao commented May 1, 2024

Following the discussion at #28151, I'm creating this PR to add per-instance get/setColorAt() for BatchedMesh.

I took special care in doing it in a way similar to how setColorAt() is implemented for InstancedMesh so no additional computation happens in the shaders in case no color is ever set.

Copy link

github-actions bot commented May 1, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
676.8 kB (167.9 kB) 678.7 kB (168.2 kB) +1.88 kB

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
455.9 kB (110.1 kB) 456.9 kB (110.3 kB) +1.01 kB

Copy link
Collaborator

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

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

It might be worth updating the batched mesh example to show color rendering, as well.

src/objects/BatchedMesh.js Outdated Show resolved Hide resolved
src/objects/BatchedMesh.js Outdated Show resolved Hide resolved
src/objects/BatchedMesh.js Show resolved Hide resolved
src/renderers/WebGLRenderer.js Show resolved Hide resolved
docs/api/en/objects/BatchedMesh.html Outdated Show resolved Hide resolved
@Kikedao
Copy link
Contributor Author

Kikedao commented May 10, 2024

It might be worth updating the batched mesh example to show color rendering, as well.

For InstancedMesh we have several examples, one with a normal material ("instancing / performance") and one where the color setting and raycasting is showcased ("instancing / raycast").
Do you want me to create a new example similar to the raycasting one for BatchedMesh to not mess with the existing one ("mesh / batch")?

src/renderers/WebGLRenderer.js Outdated Show resolved Hide resolved
src/objects/BatchedMesh.js Show resolved Hide resolved
Copy link
Collaborator

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

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

Do you want me to create a new example similar to the raycasting one for BatchedMesh to not mess with the existing one ("mesh / batch")?

cc @Mugen87 do you think an example update is needed at all?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 13, 2024

Not necessarily.

@mrdoob mrdoob added this to the r165 milestone May 16, 2024
@gkjohnson
Copy link
Collaborator

Not necessarily.

Sounds good to me, then. Once the other couple comments are addressed I think this is ready

@Mugen87 Mugen87 merged commit 3b28707 into mrdoob:dev May 17, 2024
12 checks passed
@Kikedao Kikedao deleted the batchedMesh-setColorAt branch May 17, 2024 09:01
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.

5 participants