Skip to content

Make buffer binding arrays emit bind group layout entries and bindless resource descriptors again.#18125

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
pcwalton:fix-buffer-binding-arrays
Mar 10, 2025
Merged

Make buffer binding arrays emit bind group layout entries and bindless resource descriptors again.#18125
alice-i-cecile merged 1 commit intobevyengine:mainfrom
pcwalton:fix-buffer-binding-arrays

Conversation

@pcwalton
Copy link
Copy Markdown
Contributor

@pcwalton pcwalton commented Mar 3, 2025

PR #17965 mistakenly made the AsBindGroup macro no longer emit a bind group layout entry and a resource descriptor for buffers. This commit adds that functionality back, fixing the shader_material_bindless example.

Closes #18124.

resource descriptors again.

PR bevyengine#17965 mistakenly made the `AsBindGroup` macro no longer emit a bind
group layout entry and a resource descriptor for buffers. This commit
adds that functionality back, fixing the `shader_material_bindless`
example.

Closes bevyengine#18124.
@pcwalton pcwalton added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes P-Regression Functionality that used to work but no longer does. Add a test for this! C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Mar 3, 2025
@pcwalton pcwalton added P-Crash A sudden unexpected crash S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 3, 2025
@JMS55
Copy link
Copy Markdown
Contributor

JMS55 commented Mar 3, 2025

I don't know enough about AsBindGroup to review this, sorry.

@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Mar 3, 2025
_ => {}
}

let binding_array_binding = binding_array_binding.unwrap_or(0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

0? Why is 0 implicitly correct? I thought the data is automatically serialised into a buffer at binding 0?

Copy link
Copy Markdown
Contributor Author

@pcwalton pcwalton Mar 3, 2025

Choose a reason for hiding this comment

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

It's not. I just need a dummy value to insert into bindless_binding_layouts, because we populate that array anyway even in the non-bindless case, for simplicity. Maybe that should be refactored though. (This macro is a mess in general, unfortunately.)

Copy link
Copy Markdown
Member

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

Tested on macOS.

@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 7, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 9, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 10, 2025
Merged via the queue into bevyengine:main with commit b46d9f0 Mar 10, 2025
38 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 D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes P-Crash A sudden unexpected crash 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.

Example shader_material_bindless crashes

6 participants