From d3241c4f8d1ea20353ae4d48d863b2e9032ed58b Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 5 Dec 2024 13:22:14 -0800 Subject: [PATCH] Fix the `texture_binding_array`, `specialized_mesh_pipeline`, and `custom_shader_instancing` examples after the bindless change. (#16641) The bindless PR (#16368) broke some examples: * `specialized_mesh_pipeline` and `custom_shader_instancing` failed because they expect to be able to render a mesh with no material, by overriding enough of the render pipeline to be able to do so. This PR fixes the issue by restoring the old behavior in which we extract meshes even if they have no material. * `texture_binding_array` broke because it doesn't implement `AsBindGroup::unprepared_bind_group`. This was tricky to fix because there's a very good reason why `texture_binding_array` doesn't implement that method: there's no sensible way to do so with `wgpu`'s current bindless API, due to its multiple levels of borrowed references. To fix the example, I split `MaterialBindGroup` into `MaterialBindlessBindGroup` and `MaterialNonBindlessBindGroup`, and allow direct custom implementations of `AsBindGroup::as_bind_group` for the latter type of bind groups. To opt in to the new behavior, return the `AsBindGroupError::CreateBindGroupDirectly` error from your `AsBindGroup::unprepared_bind_group` implementation, and Bevy will call your custom `AsBindGroup::as_bind_group` method as before. ## Migration Guide * Bevy will now unconditionally call `AsBindGroup::unprepared_bind_group` for your materials, so you must no longer panic in that function. Instead, return the new `AsBindGroupError::CreateBindGroupDirectly` error, and Bevy will fall back to calling `AsBindGroup::as_bind_group` as before. --- crates/bevy_pbr/src/material.rs | 66 +++- crates/bevy_pbr/src/material_bind_groups.rs | 341 ++++++++++++++++-- crates/bevy_pbr/src/render/mesh.rs | 51 ++- .../src/render_resource/bind_group.rs | 7 +- examples/shader/texture_binding_array.rs | 8 +- 5 files changed, 399 insertions(+), 74 deletions(-) diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index 4cf50432e2741..bb1b11bd115a8 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -1038,24 +1038,24 @@ impl RenderAsset for PreparedMaterial { return Err(PrepareAssetError::RetryNextUpdate(material)); }; + let method = match material.opaque_render_method() { + OpaqueRendererMethod::Forward => OpaqueRendererMethod::Forward, + OpaqueRendererMethod::Deferred => OpaqueRendererMethod::Deferred, + OpaqueRendererMethod::Auto => default_opaque_render_method.0, + }; + let mut mesh_pipeline_key_bits = MeshPipelineKey::empty(); + mesh_pipeline_key_bits.set( + MeshPipelineKey::READS_VIEW_TRANSMISSION_TEXTURE, + material.reads_view_transmission_texture(), + ); + match material.unprepared_bind_group( &pipeline.material_layout, render_device, material_param, ) { Ok(unprepared) => { - let method = match material.opaque_render_method() { - OpaqueRendererMethod::Forward => OpaqueRendererMethod::Forward, - OpaqueRendererMethod::Deferred => OpaqueRendererMethod::Deferred, - OpaqueRendererMethod::Auto => default_opaque_render_method.0, - }; - let mut mesh_pipeline_key_bits = MeshPipelineKey::empty(); - mesh_pipeline_key_bits.set( - MeshPipelineKey::READS_VIEW_TRANSMISSION_TEXTURE, - material.reads_view_transmission_texture(), - ); - - bind_group_allocator.init(*material_binding_id, unprepared); + bind_group_allocator.init(render_device, *material_binding_id, unprepared); Ok(PreparedMaterial { binding: *material_binding_id, @@ -1070,9 +1070,51 @@ impl RenderAsset for PreparedMaterial { phantom: PhantomData, }) } + Err(AsBindGroupError::RetryNextUpdate) => { Err(PrepareAssetError::RetryNextUpdate(material)) } + + Err(AsBindGroupError::CreateBindGroupDirectly) => { + // This material has opted out of automatic bind group creation + // and is requesting a fully-custom bind group. Invoke + // `as_bind_group` as requested, and store the resulting bind + // group in the slot. + match material.as_bind_group( + &pipeline.material_layout, + render_device, + material_param, + ) { + Ok(prepared_bind_group) => { + // Store the resulting bind group directly in the slot. + bind_group_allocator.init_custom( + *material_binding_id, + prepared_bind_group.bind_group, + prepared_bind_group.data, + ); + + Ok(PreparedMaterial { + binding: *material_binding_id, + properties: MaterialProperties { + alpha_mode: material.alpha_mode(), + depth_bias: material.depth_bias(), + reads_view_transmission_texture: mesh_pipeline_key_bits + .contains(MeshPipelineKey::READS_VIEW_TRANSMISSION_TEXTURE), + render_method: method, + mesh_pipeline_key_bits, + }, + phantom: PhantomData, + }) + } + + Err(AsBindGroupError::RetryNextUpdate) => { + Err(PrepareAssetError::RetryNextUpdate(material)) + } + + Err(other) => Err(PrepareAssetError::AsBindGroupError(other)), + } + } + Err(other) => Err(PrepareAssetError::AsBindGroupError(other)), } } diff --git a/crates/bevy_pbr/src/material_bind_groups.rs b/crates/bevy_pbr/src/material_bind_groups.rs index a19cd10916eca..b64ca339519ab 100644 --- a/crates/bevy_pbr/src/material_bind_groups.rs +++ b/crates/bevy_pbr/src/material_bind_groups.rs @@ -38,7 +38,7 @@ where M: Material, { /// The data that the allocator keeps about each bind group. - pub bind_groups: Vec>, + bind_groups: Vec>, /// Stores IDs of material bind groups that have at least one slot /// available. @@ -60,7 +60,22 @@ where } /// Information that the allocator keeps about each bind group. -pub struct MaterialBindGroup +pub enum MaterialBindGroup +where + M: Material, +{ + /// Information that the allocator keeps about each bind group with bindless + /// textures in use. + Bindless(MaterialBindlessBindGroup), + + /// Information that the allocator keeps about each bind group for which + /// bindless textures are not in use. + NonBindless(MaterialNonBindlessBindGroup), +} + +/// Information that the allocator keeps about each bind group with bindless +/// textures in use. +pub struct MaterialBindlessBindGroup where M: Material, { @@ -81,6 +96,33 @@ where used_slot_bitmap: u32, } +/// Information that the allocator keeps about each bind group for which +/// bindless textures are not in use. +/// +/// When a bindless texture isn't in use, bind groups and material instances are +/// in 1:1 correspondence, and therefore there's only a single slot for extra +/// material data here. +pub struct MaterialNonBindlessBindGroup +where + M: Material, +{ + /// The single allocation in a non-bindless bind group. + allocation: MaterialNonBindlessBindGroupAllocation, +} + +/// The single allocation in a non-bindless bind group. +enum MaterialNonBindlessBindGroupAllocation +where + M: Material, +{ + /// The allocation is free. + Unallocated, + /// The allocation has been allocated, but not yet initialized. + Allocated, + /// The allocation is full and contains both a bind group and extra data. + Initialized(BindGroup, M::Data), +} + /// Where the GPU data for a material is located. /// /// In bindless mode, materials are gathered into bind groups, and the slot is @@ -170,7 +212,6 @@ where fallback_image, fallback_resources, &self.fallback_buffers, - self.bindless_enabled, ); } } @@ -207,11 +248,31 @@ where /// [`MaterialBindingId`]. pub(crate) fn init( &mut self, + render_device: &RenderDevice, material_binding_id: MaterialBindingId, unprepared_bind_group: UnpreparedBindGroup, + ) { + self.bind_groups[material_binding_id.group.0 as usize].init( + render_device, + &self.bind_group_layout, + material_binding_id.slot, + unprepared_bind_group, + ); + } + + /// Fills a slot directly with a custom bind group. + /// + /// This is only a meaningful operation for non-bindless bind groups. It's + /// rarely used, but see the `texture_binding_array` example for an example + /// demonstrating how this feature might see use in practice. + pub(crate) fn init_custom( + &mut self, + material_binding_id: MaterialBindingId, + bind_group: BindGroup, + bind_group_data: M::Data, ) { self.bind_groups[material_binding_id.group.0 as usize] - .init(material_binding_id.slot, unprepared_bind_group); + .init_custom(bind_group, bind_group_data); } /// Marks the slot corresponding to the given [`MaterialBindingId`] as free. @@ -235,15 +296,156 @@ impl MaterialBindGroup where M: Material, { - /// Returns a new bind group. - fn new(bindless_enabled: bool) -> MaterialBindGroup { - let count = if !bindless_enabled { - 1 + /// Creates a new material bind group. + fn new(bindless: bool) -> MaterialBindGroup { + if bindless { + MaterialBindGroup::Bindless(MaterialBindlessBindGroup::new()) } else { - M::BINDLESS_SLOT_COUNT.unwrap_or(1) - }; + MaterialBindGroup::NonBindless(MaterialNonBindlessBindGroup::new()) + } + } + + /// Allocates a new binding slot and returns its ID. + fn allocate(&mut self) -> MaterialBindGroupSlot { + match *self { + MaterialBindGroup::Bindless(ref mut material_bindless_bind_group) => { + material_bindless_bind_group.allocate() + } + MaterialBindGroup::NonBindless(ref mut material_non_bindless_bind_group) => { + material_non_bindless_bind_group.allocate() + } + } + } + + /// Assigns an unprepared bind group to the group and slot specified in the + /// [`MaterialBindingId`]. + fn init( + &mut self, + render_device: &RenderDevice, + bind_group_layout: &BindGroupLayout, + slot: MaterialBindGroupSlot, + unprepared_bind_group: UnpreparedBindGroup, + ) { + match *self { + MaterialBindGroup::Bindless(ref mut material_bindless_bind_group) => { + material_bindless_bind_group.init( + render_device, + bind_group_layout, + slot, + unprepared_bind_group, + ); + } + MaterialBindGroup::NonBindless(ref mut material_non_bindless_bind_group) => { + material_non_bindless_bind_group.init( + render_device, + bind_group_layout, + slot, + unprepared_bind_group, + ); + } + } + } + + /// Fills a slot directly with a custom bind group. + /// + /// This is only a meaningful operation for non-bindless bind groups. It's + /// rarely used, but see the `texture_binding_array` example for an example + /// demonstrating how this feature might see use in practice. + fn init_custom(&mut self, bind_group: BindGroup, extra_data: M::Data) { + match *self { + MaterialBindGroup::Bindless(_) => { + error!("Custom bind groups aren't supported in bindless mode"); + } + MaterialBindGroup::NonBindless(ref mut material_non_bindless_bind_group) => { + material_non_bindless_bind_group.init_custom(bind_group, extra_data); + } + } + } + + /// Marks the slot corresponding to the given [`MaterialBindGroupSlot`] as + /// free. + fn free(&mut self, material_bind_group_slot: MaterialBindGroupSlot) { + match *self { + MaterialBindGroup::Bindless(ref mut material_bindless_bind_group) => { + material_bindless_bind_group.free(material_bind_group_slot); + } + MaterialBindGroup::NonBindless(ref mut material_non_bindless_bind_group) => { + material_non_bindless_bind_group.free(material_bind_group_slot); + } + } + } + + /// Returns the actual bind group, or `None` if it hasn't been created yet. + pub fn get_bind_group(&self) -> Option<&BindGroup> { + match *self { + MaterialBindGroup::Bindless(ref material_bindless_bind_group) => { + material_bindless_bind_group.get_bind_group() + } + MaterialBindGroup::NonBindless(ref material_non_bindless_bind_group) => { + material_non_bindless_bind_group.get_bind_group() + } + } + } + + /// Returns true if all the slots are full or false if at least one slot in + /// this bind group is free. + fn is_full(&self) -> bool { + match *self { + MaterialBindGroup::Bindless(ref material_bindless_bind_group) => { + material_bindless_bind_group.is_full() + } + MaterialBindGroup::NonBindless(ref material_non_bindless_bind_group) => { + material_non_bindless_bind_group.is_full() + } + } + } + + /// Recreates the bind group for this material bind group containing the + /// data for every material in it. + fn rebuild_bind_group_if_necessary( + &mut self, + render_device: &RenderDevice, + bind_group_layout: &BindGroupLayout, + fallback_image: &FallbackImage, + fallback_bindless_resources: &FallbackBindlessResources, + fallback_buffers: &MaterialFallbackBuffers, + ) { + match *self { + MaterialBindGroup::Bindless(ref mut material_bindless_bind_group) => { + material_bindless_bind_group.rebuild_bind_group_if_necessary( + render_device, + bind_group_layout, + fallback_image, + fallback_bindless_resources, + fallback_buffers, + ); + } + MaterialBindGroup::NonBindless(_) => {} + } + } + + /// Returns the associated extra data for the material with the given slot. + pub fn get_extra_data(&self, slot: MaterialBindGroupSlot) -> &M::Data { + match *self { + MaterialBindGroup::Bindless(ref material_bindless_bind_group) => { + material_bindless_bind_group.get_extra_data(slot) + } + MaterialBindGroup::NonBindless(ref material_non_bindless_bind_group) => { + material_non_bindless_bind_group.get_extra_data(slot) + } + } + } +} - MaterialBindGroup { +impl MaterialBindlessBindGroup +where + M: Material, +{ + /// Returns a new bind group. + fn new() -> MaterialBindlessBindGroup { + let count = M::BINDLESS_SLOT_COUNT.unwrap_or(1); + + MaterialBindlessBindGroup { bind_group: None, unprepared_bind_groups: iter::repeat_with(|| None).take(count as usize).collect(), used_slot_bitmap: 0, @@ -266,6 +468,8 @@ where /// Assigns the given unprepared bind group to the given slot. fn init( &mut self, + _: &RenderDevice, + _: &BindGroupLayout, slot: MaterialBindGroupSlot, unprepared_bind_group: UnpreparedBindGroup, ) { @@ -291,7 +495,7 @@ where } /// Returns the actual bind group, or `None` if it hasn't been created yet. - pub fn get_bind_group(&self) -> Option<&BindGroup> { + fn get_bind_group(&self) -> Option<&BindGroup> { self.bind_group.as_ref() } @@ -304,7 +508,6 @@ where fallback_image: &FallbackImage, fallback_bindless_resources: &FallbackBindlessResources, fallback_buffers: &MaterialFallbackBuffers, - bindless_enabled: bool, ) { if self.bind_group.is_some() { return; @@ -318,23 +521,6 @@ where return; }; - // If bindless isn't enabled, create a trivial bind group containing - // only one material's worth of data. - if !bindless_enabled { - let entries = first_bind_group - .bindings - .iter() - .map(|(index, binding)| BindGroupEntry { - binding: *index, - resource: binding.get_binding(), - }) - .collect::>(); - - self.bind_group = - Some(render_device.create_bind_group(M::label(), bind_group_layout, &entries)); - return; - } - // Creates the intermediate binding resource vectors. let Some(binding_resource_arrays) = self.recreate_binding_resource_arrays( first_bind_group, @@ -473,7 +659,7 @@ where } /// Returns the associated extra data for the material with the given slot. - pub fn get_extra_data(&self, slot: MaterialBindGroupSlot) -> &M::Data { + fn get_extra_data(&self, slot: MaterialBindGroupSlot) -> &M::Data { &self.unprepared_bind_groups[slot.0 as usize] .as_ref() .unwrap() @@ -481,6 +667,99 @@ where } } +impl MaterialNonBindlessBindGroup +where + M: Material, +{ + /// Creates a new material bind group. + fn new() -> MaterialNonBindlessBindGroup { + MaterialNonBindlessBindGroup { + allocation: MaterialNonBindlessBindGroupAllocation::Unallocated, + } + } + + /// Allocates a new slot and returns its index. + /// + /// This bind group must not be full. + fn allocate(&mut self) -> MaterialBindGroupSlot { + debug_assert!(!self.is_full()); + self.allocation = MaterialNonBindlessBindGroupAllocation::Allocated; + MaterialBindGroupSlot(0) + } + + /// Assigns an unprepared bind group to the group and slot specified in the + /// [`MaterialBindingId`]. + /// + /// For non-bindless bind groups, we go ahead and create the bind group + /// immediately. + fn init( + &mut self, + render_device: &RenderDevice, + bind_group_layout: &BindGroupLayout, + _: MaterialBindGroupSlot, + unprepared_bind_group: UnpreparedBindGroup, + ) { + let entries = unprepared_bind_group + .bindings + .iter() + .map(|(index, binding)| BindGroupEntry { + binding: *index, + resource: binding.get_binding(), + }) + .collect::>(); + + self.allocation = MaterialNonBindlessBindGroupAllocation::Initialized( + render_device.create_bind_group(M::label(), bind_group_layout, &entries), + unprepared_bind_group.data, + ); + } + + /// Fills the slot directly with a custom bind group. + /// + /// This is only a meaningful operation for non-bindless bind groups. It's + /// rarely used, but see the `texture_binding_array` example for an example + /// demonstrating how this feature might see use in practice. + fn init_custom(&mut self, bind_group: BindGroup, extra_data: M::Data) { + self.allocation = + MaterialNonBindlessBindGroupAllocation::Initialized(bind_group, extra_data); + } + + /// Deletes the stored bind group. + fn free(&mut self, _: MaterialBindGroupSlot) { + self.allocation = MaterialNonBindlessBindGroupAllocation::Unallocated; + } + + /// Returns true if the slot is full or false if it's free. + fn is_full(&self) -> bool { + !matches!( + self.allocation, + MaterialNonBindlessBindGroupAllocation::Unallocated + ) + } + + /// Returns the actual bind group, or `None` if it hasn't been created yet. + fn get_bind_group(&self) -> Option<&BindGroup> { + match self.allocation { + MaterialNonBindlessBindGroupAllocation::Unallocated + | MaterialNonBindlessBindGroupAllocation::Allocated => None, + MaterialNonBindlessBindGroupAllocation::Initialized(ref bind_group, _) => { + Some(bind_group) + } + } + } + + /// Returns the associated extra data for the material. + fn get_extra_data(&self, _: MaterialBindGroupSlot) -> &M::Data { + match self.allocation { + MaterialNonBindlessBindGroupAllocation::Initialized(_, ref extra_data) => extra_data, + MaterialNonBindlessBindGroupAllocation::Unallocated + | MaterialNonBindlessBindGroupAllocation::Allocated => { + panic!("Bind group not initialized") + } + } + } +} + impl FromWorld for MaterialBindGroupAllocator where M: Material, diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 996dfe7e2b5e2..29c7ce3ca3107 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/crates/bevy_pbr/src/render/mesh.rs @@ -679,6 +679,25 @@ pub struct RenderMeshMaterialIds { pub(crate) material_to_binding: HashMap, } +impl RenderMeshMaterialIds { + /// Returns the mesh material ID for the entity with the given mesh, or a + /// dummy mesh material ID if the mesh has no material ID. + /// + /// Meshes almost always have materials, but in very specific circumstances + /// involving custom pipelines they won't. (See the + /// `specialized_mesh_pipelines` example.) + fn mesh_material_binding_id(&self, entity: MainEntity) -> MaterialBindingId { + self.mesh_to_material + .get(&entity) + .and_then(|mesh_material_asset_id| { + self.material_to_binding + .get(mesh_material_asset_id) + .cloned() + }) + .unwrap_or_default() + } +} + impl RenderMeshInstances { /// Creates a new [`RenderMeshInstances`] instance. fn new(use_gpu_instance_buffer_builder: bool) -> RenderMeshInstances { @@ -1100,18 +1119,8 @@ pub fn extract_meshes_for_cpu_building( return; } - let Some(mesh_material_asset_id) = mesh_material_ids - .mesh_to_material - .get(&MainEntity::from(entity)) - else { - return; - }; - let Some(mesh_material_binding_id) = mesh_material_ids - .material_to_binding - .get(mesh_material_asset_id) - else { - return; - }; + let mesh_material_binding_id = + mesh_material_ids.mesh_material_binding_id(MainEntity::from(entity)); let mut lod_index = None; if visibility_range { @@ -1128,7 +1137,7 @@ pub fn extract_meshes_for_cpu_building( let shared = RenderMeshInstanceShared::from_components( previous_transform, mesh, - *mesh_material_binding_id, + mesh_material_binding_id, not_shadow_caster, no_automatic_batching, ); @@ -1257,18 +1266,8 @@ pub fn extract_meshes_for_gpu_building( return; } - let Some(mesh_material_asset_id) = mesh_material_ids - .mesh_to_material - .get(&MainEntity::from(entity)) - else { - return; - }; - let Some(mesh_material_binding_id) = mesh_material_ids - .material_to_binding - .get(mesh_material_asset_id) - else { - return; - }; + let mesh_material_binding_id = + mesh_material_ids.mesh_material_binding_id(MainEntity::from(entity)); let mut lod_index = None; if visibility_range { @@ -1285,7 +1284,7 @@ pub fn extract_meshes_for_gpu_building( let shared = RenderMeshInstanceShared::from_components( previous_transform, mesh, - *mesh_material_binding_id, + mesh_material_binding_id, not_shadow_caster, no_automatic_batching, ); diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index ccda757c532f8..290923b62d934 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -373,8 +373,9 @@ pub trait AsBindGroup { /// Returns a vec of (binding index, `OwnedBindingResource`). /// In cases where `OwnedBindingResource` is not available (as for bindless texture arrays currently), - /// an implementor may define `as_bind_group` directly. This may prevent certain features - /// from working correctly. + /// an implementor may return `AsBindGroupError::CreateBindGroupDirectly` + /// from this function and instead define `as_bind_group` directly. This may + /// prevent certain features, such as bindless mode, from working correctly. fn unprepared_bind_group( &self, layout: &BindGroupLayout, @@ -405,6 +406,8 @@ pub enum AsBindGroupError { /// The bind group could not be generated. Try again next frame. #[display("The bind group could not be generated")] RetryNextUpdate, + #[display("Create the bind group via `as_bind_group()` instead")] + CreateBindGroupDirectly, #[display("At binding index {_0}, the provided image sampler `{_1}` does not match the required sampler type(s) `{_2}`.")] InvalidSamplerType(u32, String, String), } diff --git a/examples/shader/texture_binding_array.rs b/examples/shader/texture_binding_array.rs index 5e1c2e7ed4dad..61cb6403e6e93 100644 --- a/examples/shader/texture_binding_array.rs +++ b/examples/shader/texture_binding_array.rs @@ -145,9 +145,11 @@ impl AsBindGroup for BindlessMaterial { _render_device: &RenderDevice, _param: &mut SystemParamItem<'_, '_, Self::Param>, ) -> Result, AsBindGroupError> { - // we implement as_bind_group directly because - panic!("bindless texture arrays can't be owned") - // or rather, they can be owned, but then you can't make a `&'a [&'a TextureView]` from a vec of them in get_binding(). + // We implement `as_bind_group`` directly because bindless texture + // arrays can't be owned. + // Or rather, they can be owned, but then you can't make a `&'a [&'a + // TextureView]` from a vec of them in `get_binding()`. + Err(AsBindGroupError::CreateBindGroupDirectly) } fn bind_group_layout_entries(_: &RenderDevice) -> Vec