Skip to content

Commit

Permalink
Make #[bindless] in ExtendedMaterial actually enable bindless mode.
Browse files Browse the repository at this point in the history
I forgot to set `BINDLESS_SLOT_COUNT` in `ExtendedMaterial`'s
implementation of `AsBindGroup`, so it didn't actually become bindless.
In fact, it would usually crash with a shader/bind group layout
mismatch, because some parts of Bevy's renderer thought that the
resulting material was bindless while other parts didn't. This commit
corrects the situation.

I had to make `BINDLESS_SLOT_COUNT` a function instead of a constant
because the `ExtendedMaterial` version needs some logic. Unfortunately,
trait methods can't be `const fn`s, so it has to be a runtime function.
  • Loading branch information
pcwalton committed Dec 14, 2024
1 parent 30bd641 commit 57e608d
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 10 deletions.
17 changes: 11 additions & 6 deletions crates/bevy_pbr/src/extended_material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,15 @@ impl<B: Material, E: MaterialExtension> AsBindGroup for ExtendedMaterial<B, E> {
type Data = (<B as AsBindGroup>::Data, <E as AsBindGroup>::Data);
type Param = (<B as AsBindGroup>::Param, <E as AsBindGroup>::Param);

fn bindless_slot_count() -> Option<u32> {
match (B::bindless_slot_count(), E::bindless_slot_count()) {
(Some(base_bindless_slot_count), Some(extension_bindless_slot_count)) => {
Some(base_bindless_slot_count.min(extension_bindless_slot_count))
}
_ => None,
}
}

fn unprepared_bind_group(
&self,
layout: &BindGroupLayout,
Expand All @@ -159,9 +168,7 @@ impl<B: Material, E: MaterialExtension> AsBindGroup for ExtendedMaterial<B, E> {
) -> Result<UnpreparedBindGroup<Self::Data>, AsBindGroupError> {
// Only allow bindless mode if both the base material and the extension
// support it.
force_no_bindless = force_no_bindless
|| B::BINDLESS_SLOT_COUNT.is_none()
|| E::BINDLESS_SLOT_COUNT.is_none();
force_no_bindless = force_no_bindless || Self::bindless_slot_count().is_none();

// add together the bindings of the base material and the user material
let UnpreparedBindGroup {
Expand Down Expand Up @@ -199,9 +206,7 @@ impl<B: Material, E: MaterialExtension> AsBindGroup for ExtendedMaterial<B, E> {
{
// Only allow bindless mode if both the base material and the extension
// support it.
force_no_bindless = force_no_bindless
|| B::BINDLESS_SLOT_COUNT.is_none()
|| E::BINDLESS_SLOT_COUNT.is_none();
force_no_bindless = force_no_bindless || Self::bindless_slot_count().is_none();

// add together the bindings of the standard material and the user material
let mut entries = B::bind_group_layout_entries(render_device, force_no_bindless);
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/material_bind_groups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ where
{
/// Returns a new bind group.
fn new() -> MaterialBindlessBindGroup<M> {
let count = M::BINDLESS_SLOT_COUNT.unwrap_or(1);
let count = M::bindless_slot_count().unwrap_or(1);

MaterialBindlessBindGroup {
bind_group: None,
Expand Down Expand Up @@ -789,7 +789,7 @@ pub fn material_uses_bindless_resources<M>(render_device: &RenderDevice) -> bool
where
M: Material,
{
M::BINDLESS_SLOT_COUNT.is_some()
M::bindless_slot_count().is_some()
&& render_device
.features()
.contains(WgpuFeatures::BUFFER_BINDING_ARRAY | WgpuFeatures::TEXTURE_BINDING_ARRAY)
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_render/macros/src/as_bind_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,11 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result<TokenStream> {
// limitations into account.
let (bindless_slot_count, actual_bindless_slot_count_declaration) = match attr_bindless_count {
Some(bindless_count) => (
quote! { const BINDLESS_SLOT_COUNT: Option<u32> = Some(#bindless_count); },
quote! {
fn bindless_slot_count() -> Option<u32> {
Some(#bindless_count)
}
},
quote! {
let #actual_bindless_slot_count = if render_device.features().contains(
#render_path::settings::WgpuFeatures::BUFFER_BINDING_ARRAY |
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_render/src/render_resource/bind_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,9 @@ pub trait AsBindGroup {
/// Note that the *actual* slot count may be different from this value, due
/// to platform limitations. For example, if bindless resources aren't
/// supported on this platform, the actual slot count will be 1.
const BINDLESS_SLOT_COUNT: Option<u32> = None;
fn bindless_slot_count() -> Option<u32> {
None
}

/// label
fn label() -> Option<&'static str> {
Expand Down

0 comments on commit 57e608d

Please sign in to comment.