Retain RenderMaterialInstances and RenderMeshMaterialIds from frame to frame.
#16985
+207
−89
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
to frame. This commit makes Bevy use change detection to only update `RenderMaterialInstances` and `RenderMeshMaterialIds` when meshes have been added, changed, or removed. `extract_mesh_materials`, the system that extracts these, now follows the pattern that `extract_meshes_for_gpu_building` established. This improves frame time of `many_cubes` from 3.9ms to approximately 3.1ms, which slightly surpasses the performance of Bevy 0.14.
pcwalton
added
A-Rendering
BenjaminBrienen
added
the
D-Straightforward
IceSentry
approved these changes
Jan 4, 2025
tychedelia
suggested changes
Jan 4, 2025
There was a problem hiding this comment.
I noticed that in practice the query for Changed<ViewVisibility> was firing every frame. It seems to be an issue in our lighting code see pcwalton#7.
|
@tychedelia Cherry-picked over, thanks! |
tychedelia
approved these changes
Jan 5, 2025
tychedelia
added
S-Ready-For-Final-Review
pcwalton
added
S-Needs-Review
|
There are some fairly serious bugs that have required a rewrite of a good deal of this patch. |
IceSentry
approved these changes
Jan 7, 2025
Elabajaba
approved these changes
Jan 7, 2025
pcwalton
added
S-Ready-For-Final-Review
|
Running another example test run; I'll merge this in if that comes up clean :) |
|
The colors in the examples in this PR seem messed up; are materials being batched correctly? See TheBevyFlock/bevy-example-runner#87 (comment) |
alice-i-cecile
added
S-Waiting-on-Author
|
Hmm, I'm having trouble reproducing the failures. It may be some kind of race... |
|
OK, looks like that fixed the race. I'll look at the regressions. |
|
All of the failures look either like things that were broken already (the pre-existing wireframe problem having to do with the fact that it breaks the 1:1 entity:material relationship), or are false positives. So I think this is ready to go. |
BenjaminBrienen
added
S-Needs-Review
alice-i-cecile
added
S-Ready-For-Final-Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit makes Bevy use change detection to only update
RenderMaterialInstancesandRenderMeshMaterialIdswhen meshes have been added, changed, or removed.extract_mesh_materials, the system that extracts these, now follows the pattern thatextract_meshes_for_gpu_buildingestablished.This improves frame time of
many_cubesfrom 3.9ms to approximately 3.1ms, which slightly surpasses the performance of Bevy 0.14.(Resubmitted from #16878 to clean up history.)