Skip to content

Conversation

tychedelia
Copy link
Member

Objective

Fixes #18550.

Because bin state for unbatchable meshes wasn't being cleared each frame, the buffer indices for unbatchable meshes would demote from sparse to dense storage and aggressively leak memory, with all kinds of weird consequences downstream, namely supplying invalid instance ranges for render.

Solution

Clear out the unbatchable mesh bin state when we start a new frame.

@tychedelia tychedelia requested a review from pcwalton April 8, 2025 10:04
@tychedelia tychedelia added the A-Rendering Drawing game state to the screen label Apr 8, 2025
@tychedelia tychedelia added C-Bug An unexpected or incorrect behavior S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 8, 2025
@tychedelia tychedelia added this to the 0.16 milestone Apr 8, 2025
@greeble-dev
Copy link
Contributor

I'm not confident enough to review the code, but can confirm this fixes the stress in #18536 breaking when meshes are frustum culled (Win10/Vulkan/Nvidia).

@mockersf
Copy link
Member

mockersf commented Apr 8, 2025

I've pushed this fix to a branch on top of the 0.16.0-rc.3 if anyone want to test it before merging: https://github.com/mockersf/bevy/tree/16.0-rc.3-with-18761

@extrawurst
Copy link
Contributor

Tested against my issue in #18736 and happy to report this is fixing it even with the gltf using shape-keys! Thank you so much!

Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

Not really familiar with the code, but it looks fine and was shown to fix the problem.

Copy link
Contributor

@pcwalton pcwalton left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. The sparse/dense stuff is some of my least favorite parts of the renderer and can be removed once we drop WebGL 2.

// but let's go ahead and do the sensible thing anyhow: demote
// the compressed `NoDynamicOffsets` field to the full
// `DynamicOffsets` array.
warn!(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just make this an error!

@tychedelia tychedelia added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 9, 2025
@superdump superdump added this pull request to the merge queue Apr 9, 2025
Merged via the queue into bevyengine:main with commit e8fd750 Apr 9, 2025
41 checks passed
@github-project-automation github-project-automation bot moved this to Done in Rendering Apr 9, 2025
mockersf pushed a commit that referenced this pull request Apr 9, 2025
# Objective

Fixes #18550.

Because bin state for unbatchable meshes wasn't being cleared each
frame, the buffer indices for unbatchable meshes would demote from
sparse to dense storage and aggressively leak memory, with all kinds of
weird consequences downstream, namely supplying invalid instance ranges
for render.

## Solution

Clear out the unbatchable mesh bin state when we start a new frame.
jf908 pushed a commit to jf908/bevy that referenced this pull request May 12, 2025
# Objective

Fixes bevyengine#18550.

Because bin state for unbatchable meshes wasn't being cleared each
frame, the buffer indices for unbatchable meshes would demote from
sparse to dense storage and aggressively leak memory, with all kinds of
weird consequences downstream, namely supplying invalid instance ranges
for render.

## Solution

Clear out the unbatchable mesh bin state when we start a new frame.
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 S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Some meshes not rendering in 0.16-rc.1
7 participants