Skip to content

Fix bugs in the new non-bindless mesh material allocator.#17980

Merged
superdump merged 1 commit intobevyengine:mainfrom
pcwalton:fix-non-bindless-allocator
Feb 22, 2025
Merged

Fix bugs in the new non-bindless mesh material allocator.#17980
superdump merged 1 commit intobevyengine:mainfrom
pcwalton:fix-non-bindless-allocator

Conversation

@pcwalton
Copy link
Copy Markdown
Contributor

This patch fixes two bugs in the new non-bindless material allocator that landed in PR #17898:

  1. A debug assertion to prevent double frees had been flipped: we checked to see whether the slot was empty before freeing, while we should have checked to see whether the slot was full.

  2. The non-bindless allocator returned None when querying a slab that hadn't been prepared yet instead of returning a handle to that slab. This resulted in a 1-frame delay when modifying materials. In the animated_material example, this resulted in the meshes never showing up at all, because that example changes every material every frame.

Together with #17979, this patch locally fixes the problems with animated_material on macOS that were reported in #17970.

This patch fixes two bugs in the new non-bindless material allocator
that landed in PR bevyengine#17898:

1. A debug assertion to prevent double frees had been flipped: we
   checked to see whether the slot was empty before freeing, while we
   should have checked to see whether the slot was full.

2. The non-bindless allocator returned `None` when querying a slab that
   hadn't been prepared yet instead of returning a handle to that slab.
   This resulted in a 1-frame delay when modifying materials. In the
   `animated_material` example, this resulted in the meshes never
   showing up at all, because that example changes every material every
   frame.

Together with bevyengine#17979, this patch locally fixes the problems with
`animated_material` on macOS that were reported in bevyengine#17970.
@pcwalton pcwalton added A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-Regression Functionality that used to work but no longer does. Add a test for this! labels Feb 21, 2025
Copy link
Copy Markdown
Contributor

@rparrett rparrett left a comment

Choose a reason for hiding this comment

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

Mostly over my head, but seems to make sense and look good. Applying this and 17979 seems to fix most of #17970.

Of the stuff that didn't get documented there immediately, it seems that the texture example is still broken, so there may yet be one more lingering issue.

@JMS55 JMS55 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 22, 2025
@superdump superdump added this pull request to the merge queue Feb 22, 2025
Merged via the queue into bevyengine:main with commit fffe623 Feb 22, 2025
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-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants