Conversation
|
This is very useful! We had cases where we needed to do that, we had to slice the meshInstances array, then modify it and assign again, but that leads to unnecessary re-initializations and layers stuff, is just not efficient to do at runtime as it could be. |
|
We need this! |
|
@mvaligursky Could you please review and approve this PR? It's sorely needed. |
|
It's definitely on my list, sorry this is taking a while. |
There was a problem hiding this comment.
Pull request overview
Adds a public API to dynamically add/remove MeshInstance objects on RenderComponent, addressing the need for incremental procedural mesh management without resetting the whole meshInstances array.
Changes:
- Introduces
RenderComponent#addMeshInstance()/RenderComponent#removeMeshInstance()and refactors internal per-mesh instance update logic. - Extends
Layerwith single-instance add/remove helpers used by the component. - Adds unit tests covering add/remove behavior and layer membership updates.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| test/framework/components/render/component.test.mjs | Adds tests for the new RenderComponent mesh instance add/remove API and layer integration. |
| src/scene/layer.js | Refactors mesh/shadow caster add/remove to support single-instance operations and shared shader-variant clearing. |
| src/framework/components/render/component.js | Adds the new public API, refactors mesh instance property updates, and updates layer add/remove helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR adds a public API on RenderComponent to dynamically add and remove individual MeshInstances without needing to replace the whole meshInstances array, addressing the workflow described in #6680.
Changes:
- Added
RenderComponent#addMeshInstanceandRenderComponent#removeMeshInstance(with optional destruction behavior). - Extended
Layerwith single-instance add/remove helpers and refactored multi-instance paths to reuse them. - Added unit tests covering add/remove behavior and layer membership updates.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/framework/components/render/component.js |
Implements the new RenderComponent API and refactors shared mesh-instance update/cleanup logic. |
src/scene/layer.js |
Adds single-instance layer/shadow-caster helpers and consolidates shader-variant clearing. |
test/framework/components/render/component.test.mjs |
Adds tests validating add/remove behavior and that layers are updated accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #6680
Adds new API:
Tests included.
I confirm I have read the contributing guidelines and signed the Contributor License Agreement.