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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pcwalton
Copy link
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 P-Crash A sudden unexpected crash S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 3, 2025
@JMS55
Copy link
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
@@ -161,6 +161,29 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result<TokenStream> {
}
_ => {}
}

let binding_array_binding = binding_array_binding.unwrap_or(0);
Copy link
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
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.)

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-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Example shader_material_bindless crashes
4 participants