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

Sweep bins after queuing so as to only sweep them once. #17787

Merged
merged 3 commits into from
Feb 10, 2025

Conversation

pcwalton
Copy link
Contributor

Currently, we sweep, or remove entities from bins when those entities became invisible or changed phases, during queue_material_meshes and similar phases. This, however, is wrong, because queue_material_meshes executes once per material type, not once per phase. This could result in sweeping bins multiple times per phase, which can corrupt the bins. This commit fixes the issue by moving sweeping to a separate system that runs after queuing.

This manifested itself as entities appearing and disappearing seemingly at random.

Closes #17759.

Currently, we *sweep*, or remove entities from bins when those entities
became invisible or changed phases, during `queue_material_meshes` and
similar phases. This, however, is wrong, because `queue_material_meshes`
executes once per material type, not once per phase. This could result
in sweeping bins multiple times per phase, which can corrupt the bins.
This commit fixes the issue by moving sweeping to a separate system that
runs after queuing.

This manifested itself as entities appearing and disappearing seemingly
at random.

Closes bevyengine#17759.
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen P-High This is particularly urgent, and deserves immediate attention S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 10, 2025
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Feb 10, 2025
@alice-i-cecile
Copy link
Member

I spotted this over in #17769 too, for anyone looking for a reproduction.

crates/bevy_pbr/src/render/light.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

Approved aside from @tychedelia 's comment.

@superdump superdump enabled auto-merge February 10, 2025 22:59
@superdump superdump added this pull request to the merge queue Feb 10, 2025
Merged via the queue into bevyengine:main with commit 69db29e Feb 10, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-High This is particularly urgent, and deserves immediate attention S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Entities that should be visible are being culled
4 participants