forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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 commit is contained in:
Patrick Walton 2025-02-21 22:29:00 -08:00 committed by GitHub
parent 9046859ca8
commit fffe623297
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 29 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
crates/bevy_pbr/src/material_bind_groups.rs
View File

@ -342,9 +342,15 @@ where
/// A single bind group that the [`MaterialBindGroupNonBindlessAllocator`] /// A single bind group that the [`MaterialBindGroupNonBindlessAllocator`]
/// manages. /// manages.
struct MaterialNonBindlessSlab<'a, M>(&'a PreparedBindGroup<M::Data>) enum MaterialNonBindlessSlab<'a, M>
where where
M: Material; M: Material,
{
/// A slab that has a bind group.
Prepared(&'a PreparedBindGroup<M::Data>),
/// A slab that doesn't yet have a bind group.
Unprepared(&'a UnpreparedBindGroup<M::Data>),
}
impl From<u32> for MaterialBindGroupSlot { impl From<u32> for MaterialBindGroupSlot {
fn from(value: u32) -> Self { fn from(value: u32) -> Self {
@ -1615,7 +1621,7 @@ where
/// Deallocates the bind group with the given binding ID. /// Deallocates the bind group with the given binding ID.
fn free(&mut self, binding_id: MaterialBindingId) { fn free(&mut self, binding_id: MaterialBindingId) {
debug_assert_eq!(binding_id.slot, MaterialBindGroupSlot(0)); debug_assert_eq!(binding_id.slot, MaterialBindGroupSlot(0));
debug_assert!(self.bind_groups[*binding_id.group as usize].is_none()); debug_assert!(self.bind_groups[*binding_id.group as usize].is_some());
self.bind_groups[*binding_id.group as usize] = None; self.bind_groups[*binding_id.group as usize] = None;
self.to_prepare.remove(&binding_id.group); self.to_prepare.remove(&binding_id.group);
self.free_indices.push(binding_id.group); self.free_indices.push(binding_id.group);
@ -1623,12 +1629,16 @@ where
/// Returns a wrapper around the bind group with the given index. /// Returns a wrapper around the bind group with the given index.
fn get(&self, group: MaterialBindGroupIndex) -> Option<MaterialNonBindlessSlab<M>> { fn get(&self, group: MaterialBindGroupIndex) -> Option<MaterialNonBindlessSlab<M>> {
match self.bind_groups[group.0 as usize] { self.bind_groups[group.0 as usize]
Some(MaterialNonBindlessAllocatedBindGroup::Prepared(ref prepared_bind_group)) => { .as_ref()
Some(MaterialNonBindlessSlab(prepared_bind_group)) .map(|bind_group| match bind_group {
} MaterialNonBindlessAllocatedBindGroup::Prepared(prepared_bind_group) => {
Some(MaterialNonBindlessAllocatedBindGroup::Unprepared { .. }) | None => None, MaterialNonBindlessSlab::Prepared(prepared_bind_group)
} }
MaterialNonBindlessAllocatedBindGroup::Unprepared { bind_group, .. } => {
MaterialNonBindlessSlab::Unprepared(bind_group)
}
})
} }
/// Prepares any as-yet unprepared bind groups that this allocator is /// Prepares any as-yet unprepared bind groups that this allocator is
@ -1683,7 +1693,12 @@ where
MaterialSlabImpl::Bindless(material_bindless_slab) => { MaterialSlabImpl::Bindless(material_bindless_slab) => {
material_bindless_slab.get_extra_data(slot) material_bindless_slab.get_extra_data(slot)
} }
MaterialSlabImpl::NonBindless(ref prepared_bind_group) => &prepared_bind_group.0.data, MaterialSlabImpl::NonBindless(MaterialNonBindlessSlab::Prepared(
prepared_bind_group,
)) => &prepared_bind_group.data,
MaterialSlabImpl::NonBindless(MaterialNonBindlessSlab::Unprepared(
unprepared_bind_group,
)) => &unprepared_bind_group.data,
} }
} }
@ -1698,9 +1713,10 @@ where
MaterialSlabImpl::Bindless(material_bindless_slab) => { MaterialSlabImpl::Bindless(material_bindless_slab) => {
material_bindless_slab.bind_group() material_bindless_slab.bind_group()
} }
MaterialSlabImpl::NonBindless(ref prepared_bind_group) => { MaterialSlabImpl::NonBindless(MaterialNonBindlessSlab::Prepared(
Some(&prepared_bind_group.0.bind_group) prepared_bind_group,
} )) => Some(&prepared_bind_group.bind_group),
MaterialSlabImpl::NonBindless(MaterialNonBindlessSlab::Unprepared(_)) => None,
} }
} }
} }