Skip to content

Remove DUMMY_MESH_MATERIAL#22981

Open
greeble-dev wants to merge 1 commit intobevyengine:mainfrom
greeble-dev:remove-dummy-mesh-material
Open

Remove DUMMY_MESH_MATERIAL#22981
greeble-dev wants to merge 1 commit intobevyengine:mainfrom
greeble-dev:remove-dummy-mesh-material

Conversation

@greeble-dev
Copy link
Contributor

@greeble-dev greeble-dev commented Feb 16, 2026

Objective

Progress #19024 by avoiding AssetId::invalid. This helps assets-as-entities, and may help with future render optimizations as it's one step towards reducing AssetId to an Entity.

Solution

Previously, RenderMeshInstances::mesh_material would return an UntypedAssetId. The magic value DUMMY_MESH_MATERIAL would signal "not found". DUMMY_MESH_MATERIAL was equal to AssetId::<StandardMaterial>::invalid().

This PR makes RenderMeshInstances::mesh_material return Option<UntypedAssetId>, so "not found" becomes None.

Testing

Tested on Native/Win10 and WASM/Chrome/Win10/WebGL. This covers both the CPU and GPU mesh paths.

cargo run --example specialized_mesh_pipeline
cargo run --example testbed_3d
cargo run --example meshlet --features "meshlet meshlet_processor https free_camera"

Performance

Tested many_foxes with CPU and GPU mesh paths. Difference was below the noise floor (<5%). Probably a very minor optimization if it replaces a full UntypedAssetId comparison with an is_none.

…l` now returns an `Option<UntypedAssetId>`
@greeble-dev greeble-dev added A-Rendering Drawing game state to the screen A-Assets Load files from disk to use for things like images, models, and sounds S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Refinement Improves output quality, without fixing a clear bug or adding new functionality. labels Feb 16, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in Rendering Feb 16, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in Assets Feb 16, 2026
Comment on lines +1437 to +1438
let material_bindings_index = mesh_material
.and_then(|mesh_material| render_material_bindings.get(&mesh_material).copied())
Copy link
Contributor Author

@greeble-dev greeble-dev Feb 16, 2026

Choose a reason for hiding this comment

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

There's potential for a subtle behavior change if render_material_bindings somehow contained a DUMMY_MESH_MATERIAL with a non-default material binding. But that seems unlikely, and the new behavior is arguably more correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

We explicitly state that assigning to ::invalid() results in "undefined behavior" (I don't like the wording but the words are correct). So if that's what you mean by "non-default material binding" I think it's ok.

@andriyDev andriyDev requested a review from IceSentry February 16, 2026 18:54
Copy link
Contributor

@andriyDev andriyDev left a comment

Choose a reason for hiding this comment

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

@IceSentry you've been working on rendering perf, so you might care to look at this? Not sure if we're down to counting branches there :P

Comment on lines +1437 to +1438
let material_bindings_index = mesh_material
.and_then(|mesh_material| render_material_bindings.get(&mesh_material).copied())
Copy link
Contributor

Choose a reason for hiding this comment

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

We explicitly state that assigning to ::invalid() results in "undefined behavior" (I don't like the wording but the words are correct). So if that's what you mean by "non-default material binding" I think it's ok.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Feb 16, 2026
@alice-i-cecile
Copy link
Member

Waiting on a full rendering review before merging.

@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change and removed C-Refinement Improves output quality, without fixing a clear bug or adding new functionality. labels Feb 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Assets Load files from disk to use for things like images, models, and sounds A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

Status: Needs SME Triage
Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

3 participants