Skip to content

Fix material slot overflow in bindless slab#23554

Open
kristoff3r wants to merge 1 commit intobevyengine:mainfrom
kristoff3r:ks/material-overflow
Open

Fix material slot overflow in bindless slab#23554
kristoff3r wants to merge 1 commit intobevyengine:mainfrom
kristoff3r:ks/material-overflow

Conversation

@kristoff3r
Copy link
Copy Markdown
Contributor

Objective

Fixes #23128

When allocating in MaterialBindlessSlab it can hold up to 32 bits of allocations. However, when used on the GPU the slot is packed into 16 bits and combined with the lightmap slot, and when it overflows both slots end up being wrong.

Solution

Add some debug asserts to validate that the bit packing succeeds, and limit the slab to 16 bits of allocations.

I'm not super well versed in this part of the codebase, so I don't know if this is the best spot to put the check, since the limitation isn't really on the slab itself. And I don't know if this has other ramifications that need to be handled.

Testing

Test using the reproducer from the issue.

When allocating in MaterialBindlessSlab it can hold up to 32 bits of
allocations. However, when used on the GPU the slot is packed into 16 bits
and combined with the lightmap slot.

To fix we set add some debug asserts to validate that the bit packing
succeeds, and limit the slab to 16 bits of allocations.
@kristoff3r kristoff3r force-pushed the ks/material-overflow branch from f962eeb to fff3b9a Compare March 28, 2026 16:46
@JMS55 JMS55 requested a review from pcwalton March 28, 2026 16:53
@kristoff3r kristoff3r added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 28, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in Rendering Mar 28, 2026
@IceSentry
Copy link
Copy Markdown
Contributor

Isn't this just going to make the bug louder with the asserts and error? I'm not sure how that fixes the bug.

@kristoff3r
Copy link
Copy Markdown
Contributor Author

Isn't this just going to make the bug louder with the asserts and error? I'm not sure how that fixes the bug.

When running the example with this PR I don't hit the asserts. My understanding is that the slab saying it's full will cause a new slab to be created instead, but I could be wrong.

@IceSentry
Copy link
Copy Markdown
Contributor

Ah, right, that makes sense, I didn't realize the Err branch was in try_allocate. And yeah, I tested the example and it does appear to work.

@IceSentry IceSentry 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 Mar 28, 2026
@kristoff3r kristoff3r added this to the 0.19 milestone Mar 29, 2026
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: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

Incorrect materials applied when over 65535 instances

3 participants