From b501d5ebb2bab3ce0c4f3cf76e5a6b01d74aa2c0 Mon Sep 17 00:00:00 2001 From: Charlotte McElwain Date: Sat, 24 Aug 2024 13:52:58 -0700 Subject: [PATCH 1/5] Refactor `AsBindGroup` to use a associated `SystemParam`. --- crates/bevy_pbr/src/extended_material.rs | 20 +++++++------------ crates/bevy_pbr/src/material.rs | 13 +++--------- .../bevy_render/macros/src/as_bind_group.rs | 13 ++++++++---- .../src/render_resource/bind_group.rs | 17 ++++++++-------- crates/bevy_sprite/src/mesh2d/material.rs | 17 +++------------- .../src/render/ui_material_pipeline.rs | 13 ++++-------- examples/shader/texture_binding_array.rs | 18 +++++++++-------- 7 files changed, 45 insertions(+), 66 deletions(-) diff --git a/crates/bevy_pbr/src/extended_material.rs b/crates/bevy_pbr/src/extended_material.rs index ff21314f36a81..f54f8d0361012 100644 --- a/crates/bevy_pbr/src/extended_material.rs +++ b/crates/bevy_pbr/src/extended_material.rs @@ -1,14 +1,13 @@ use bevy_asset::{Asset, Handle}; +use bevy_ecs::system::SystemParamItem; use bevy_reflect::{impl_type_path, Reflect}; use bevy_render::{ mesh::MeshVertexBufferLayoutRef, - render_asset::RenderAssets, render_resource::{ AsBindGroup, AsBindGroupError, BindGroupLayout, RenderPipelineDescriptor, Shader, ShaderRef, SpecializedMeshPipelineError, UnpreparedBindGroup, }, renderer::RenderDevice, - texture::{FallbackImage, GpuImage}, }; use crate::{Material, MaterialPipeline, MaterialPipelineKey, MeshPipeline, MeshPipelineKey}; @@ -147,26 +146,21 @@ impl_type_path!((in bevy_pbr::extended_material) ExtendedMaterial AsBindGroup for ExtendedMaterial { type Data = (::Data, ::Data); + type Param = (::Param, ::Param); - fn unprepared_bind_group( + fn unprepared_bind_group<'w>( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - images: &RenderAssets, - fallback_image: &FallbackImage, + (base_param, extended_param): &mut SystemParamItem<'w, '_, Self::Param>, ) -> Result, AsBindGroupError> { // add together the bindings of the base material and the user material let UnpreparedBindGroup { mut bindings, data: base_data, - } = B::unprepared_bind_group(&self.base, layout, render_device, images, fallback_image)?; - let extended_bindgroup = E::unprepared_bind_group( - &self.extension, - layout, - render_device, - images, - fallback_image, - )?; + } = B::unprepared_bind_group(&self.base, layout, render_device, base_param)?; + let extended_bindgroup = + E::unprepared_bind_group(&self.extension, layout, render_device, extended_param)?; bindings.extend(extended_bindgroup.bindings); diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index a000b19a2c663..d5b601d6515cc 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -30,7 +30,6 @@ use bevy_render::{ render_phase::*, render_resource::*, renderer::RenderDevice, - texture::FallbackImage, view::{ExtractedView, Msaa, RenderVisibilityRanges, VisibleEntities, WithMesh}, }; use bevy_utils::tracing::error; @@ -908,22 +907,16 @@ impl RenderAsset for PreparedMaterial { type Param = ( SRes, - SRes>, - SRes, SRes>, SRes, + M::Param, ); fn prepare_asset( material: Self::SourceAsset, - (render_device, images, fallback_image, pipeline, default_opaque_render_method): &mut SystemParamItem, + (render_device, pipeline, default_opaque_render_method, ref mut material_param): &mut SystemParamItem, ) -> Result> { - match material.as_bind_group( - &pipeline.material_layout, - render_device, - images, - fallback_image, - ) { + match material.as_bind_group(&pipeline.material_layout, render_device, material_param) { Ok(prepared) => { let method = match material.opaque_render_method() { OpaqueRendererMethod::Forward => OpaqueRendererMethod::Forward, diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 8fa69d29f1a06..91d5c4bfbaeea 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -42,6 +42,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { let manifest = BevyManifest::default(); let render_path = manifest.get_path("bevy_render"); let asset_path = manifest.get_path("bevy_asset"); + let ecs_path = manifest.get_path("bevy_ecs"); let mut binding_states: Vec = Vec::new(); let mut binding_impls = Vec::new(); @@ -62,7 +63,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { binding_impls.push(quote! {{ use #render_path::render_resource::AsBindGroupShaderType; let mut buffer = #render_path::render_resource::encase::UniformBuffer::new(Vec::new()); - let converted: #converted_shader_type = self.as_bind_group_shader_type(images); + let converted: #converted_shader_type = self.as_bind_group_shader_type(&images); buffer.write(&converted).unwrap(); ( #binding_index, @@ -523,16 +524,20 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { impl #impl_generics #render_path::render_resource::AsBindGroup for #struct_name #ty_generics #where_clause { type Data = #prepared_data; + type Param = ( + #ecs_path::system::lifetimeless::SRes<#render_path::render_asset::RenderAssets<#render_path::texture::GpuImage>>, + #ecs_path::system::lifetimeless::SRes<#render_path::texture::FallbackImage>, + ); + fn label() -> Option<&'static str> { Some(#struct_name_literal) } - fn unprepared_bind_group( + fn unprepared_bind_group<'w>( &self, layout: &#render_path::render_resource::BindGroupLayout, render_device: &#render_path::renderer::RenderDevice, - images: &#render_path::render_asset::RenderAssets<#render_path::texture::GpuImage>, - fallback_image: &#render_path::texture::FallbackImage, + (images, fallback_image): &mut #ecs_path::system::SystemParamItem<'w, '_, Self::Param>, ) -> Result<#render_path::render_resource::UnpreparedBindGroup, #render_path::render_resource::AsBindGroupError> { let bindings = vec![#(#binding_impls,)*]; diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index 6371678e48b3d..4b7478dc7db70 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -3,8 +3,9 @@ use crate::{ render_asset::RenderAssets, render_resource::{resource_macros::*, BindGroupLayout, Buffer, Sampler, TextureView}, renderer::RenderDevice, - texture::{FallbackImage, GpuImage}, + texture::GpuImage, }; +use bevy_ecs::system::{SystemParam, SystemParamItem}; pub use bevy_render_macros::AsBindGroup; use encase::ShaderType; use std::ops::Deref; @@ -284,21 +285,22 @@ pub trait AsBindGroup { /// Data that will be stored alongside the "prepared" bind group. type Data: Send + Sync; + type Param: SystemParam + 'static; + /// label fn label() -> Option<&'static str> { None } /// Creates a bind group for `self` matching the layout defined in [`AsBindGroup::bind_group_layout`]. - fn as_bind_group( + fn as_bind_group<'w>( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - images: &RenderAssets, - fallback_image: &FallbackImage, + param: &mut SystemParamItem<'w, '_, Self::Param>, ) -> Result, AsBindGroupError> { let UnpreparedBindGroup { bindings, data } = - Self::unprepared_bind_group(self, layout, render_device, images, fallback_image)?; + Self::unprepared_bind_group(self, layout, render_device, param)?; let entries = bindings .iter() @@ -321,12 +323,11 @@ pub trait AsBindGroup { /// 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. - fn unprepared_bind_group( + fn unprepared_bind_group<'w>( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - images: &RenderAssets, - fallback_image: &FallbackImage, + param: &mut SystemParamItem<'w, '_, Self::Param>, ) -> Result, AsBindGroupError>; /// Creates the bind group layout matching all bind groups returned by [`AsBindGroup::as_bind_group`] diff --git a/crates/bevy_sprite/src/mesh2d/material.rs b/crates/bevy_sprite/src/mesh2d/material.rs index 849fd980fbc91..17743f5aab693 100644 --- a/crates/bevy_sprite/src/mesh2d/material.rs +++ b/crates/bevy_sprite/src/mesh2d/material.rs @@ -28,7 +28,6 @@ use bevy_render::{ SpecializedMeshPipeline, SpecializedMeshPipelineError, SpecializedMeshPipelines, }, renderer::RenderDevice, - texture::{FallbackImage, GpuImage}, view::{ExtractedView, InheritedVisibility, Msaa, ViewVisibility, Visibility, VisibleEntities}, Extract, ExtractSchedule, Render, RenderApp, RenderSet, }; @@ -581,23 +580,13 @@ impl PreparedMaterial2d { impl RenderAsset for PreparedMaterial2d { type SourceAsset = M; - type Param = ( - SRes, - SRes>, - SRes, - SRes>, - ); + type Param = (SRes, SRes>, M::Param); fn prepare_asset( material: Self::SourceAsset, - (render_device, images, fallback_image, pipeline): &mut SystemParamItem, + (render_device, pipeline, material_param): &mut SystemParamItem, ) -> Result> { - match material.as_bind_group( - &pipeline.material2d_layout, - render_device, - images, - fallback_image, - ) { + match material.as_bind_group(&pipeline.material2d_layout, render_device, material_param) { Ok(prepared) => { let mut mesh_pipeline_key_bits = Mesh2dPipelineKey::empty(); mesh_pipeline_key_bits.insert(alpha_mode_pipeline_key(material.alpha_mode())); diff --git a/crates/bevy_ui/src/render/ui_material_pipeline.rs b/crates/bevy_ui/src/render/ui_material_pipeline.rs index 2eaa2f583bc4d..bcdccf15fc1c8 100644 --- a/crates/bevy_ui/src/render/ui_material_pipeline.rs +++ b/crates/bevy_ui/src/render/ui_material_pipeline.rs @@ -16,7 +16,7 @@ use bevy_render::{ render_phase::*, render_resource::{binding_types::uniform_buffer, *}, renderer::{RenderDevice, RenderQueue}, - texture::{BevyDefault, FallbackImage, GpuImage}, + texture::BevyDefault, view::*, Extract, ExtractSchedule, Render, RenderSet, }; @@ -604,18 +604,13 @@ pub struct PreparedUiMaterial { impl RenderAsset for PreparedUiMaterial { type SourceAsset = M; - type Param = ( - SRes, - SRes>, - SRes, - SRes>, - ); + type Param = (SRes, SRes>, M::Param); fn prepare_asset( material: Self::SourceAsset, - (render_device, images, fallback_image, pipeline): &mut SystemParamItem, + (render_device, pipeline, ref mut material_param): &mut SystemParamItem, ) -> Result> { - match material.as_bind_group(&pipeline.ui_layout, render_device, images, fallback_image) { + match material.as_bind_group(&pipeline.ui_layout, render_device, material_param) { Ok(prepared) => Ok(PreparedUiMaterial { bindings: prepared.bindings, bind_group: prepared.bind_group, diff --git a/examples/shader/texture_binding_array.rs b/examples/shader/texture_binding_array.rs index 0698c488bf32e..178ec5dbbc8be 100644 --- a/examples/shader/texture_binding_array.rs +++ b/examples/shader/texture_binding_array.rs @@ -1,6 +1,8 @@ //! A shader that binds several textures onto one //! `binding_array>` shader binding slot and sample non-uniformly. +use bevy::ecs::system::lifetimeless::SRes; +use bevy::ecs::system::SystemParamItem; use bevy::{ prelude::*, reflect::TypePath, @@ -97,12 +99,13 @@ struct BindlessMaterial { impl AsBindGroup for BindlessMaterial { type Data = (); - fn as_bind_group( + type Param = (SRes>, SRes); + + fn as_bind_group<'w>( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - image_assets: &RenderAssets, - fallback_image: &FallbackImage, + (image_assets, fallback_image): &mut SystemParamItem<'w, '_, Self::Param>, ) -> Result, AsBindGroupError> { // retrieve the render resources from handles let mut images = vec![]; @@ -138,12 +141,11 @@ impl AsBindGroup for BindlessMaterial { }) } - fn unprepared_bind_group( + fn unprepared_bind_group<'w>( &self, - _: &BindGroupLayout, - _: &RenderDevice, - _: &RenderAssets, - _: &FallbackImage, + _layout: &BindGroupLayout, + _render_device: &RenderDevice, + _param: &mut SystemParamItem<'w, '_, Self::Param>, ) -> Result, AsBindGroupError> { // we implement as_bind_group directly because panic!("bindless texture arrays can't be owned") From bf36df69c7c6aedda3e97ac93a26d18c2c659666 Mon Sep 17 00:00:00 2001 From: Charlotte McElwain Date: Sat, 24 Aug 2024 14:03:29 -0700 Subject: [PATCH 2/5] Ci. --- crates/bevy_render/src/render_resource/bind_group.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index 4b7478dc7db70..07831af590d68 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -293,11 +293,11 @@ pub trait AsBindGroup { } /// Creates a bind group for `self` matching the layout defined in [`AsBindGroup::bind_group_layout`]. - fn as_bind_group<'w>( + fn as_bind_group( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - param: &mut SystemParamItem<'w, '_, Self::Param>, + param: &mut SystemParamItem<'_, '_, Self::Param>, ) -> Result, AsBindGroupError> { let UnpreparedBindGroup { bindings, data } = Self::unprepared_bind_group(self, layout, render_device, param)?; @@ -323,11 +323,11 @@ pub trait AsBindGroup { /// 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. - fn unprepared_bind_group<'w>( + fn unprepared_bind_group( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - param: &mut SystemParamItem<'w, '_, Self::Param>, + param: &mut SystemParamItem<'_, '_, Self::Param>, ) -> Result, AsBindGroupError>; /// Creates the bind group layout matching all bind groups returned by [`AsBindGroup::as_bind_group`] From d6c2f9e7ae5c2998b575b72a0ca23a72f5571c5e Mon Sep 17 00:00:00 2001 From: Charlotte McElwain Date: Sat, 24 Aug 2024 14:07:09 -0700 Subject: [PATCH 3/5] Ci. --- crates/bevy_pbr/src/extended_material.rs | 4 ++-- crates/bevy_render/macros/src/as_bind_group.rs | 4 ++-- examples/shader/texture_binding_array.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_pbr/src/extended_material.rs b/crates/bevy_pbr/src/extended_material.rs index f54f8d0361012..81b3c6ebbbfd7 100644 --- a/crates/bevy_pbr/src/extended_material.rs +++ b/crates/bevy_pbr/src/extended_material.rs @@ -148,11 +148,11 @@ impl AsBindGroup for ExtendedMaterial { type Data = (::Data, ::Data); type Param = (::Param, ::Param); - fn unprepared_bind_group<'w>( + fn unprepared_bind_group( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - (base_param, extended_param): &mut SystemParamItem<'w, '_, Self::Param>, + (base_param, extended_param): &mut SystemParamItem<'_, '_, Self::Param>, ) -> Result, AsBindGroupError> { // add together the bindings of the base material and the user material let UnpreparedBindGroup { diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 91d5c4bfbaeea..81fb920f3c444 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -533,11 +533,11 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { Some(#struct_name_literal) } - fn unprepared_bind_group<'w>( + fn unprepared_bind_group( &self, layout: &#render_path::render_resource::BindGroupLayout, render_device: &#render_path::renderer::RenderDevice, - (images, fallback_image): &mut #ecs_path::system::SystemParamItem<'w, '_, Self::Param>, + (images, fallback_image): &mut #ecs_path::system::SystemParamItem<'_, '_, Self::Param>, ) -> Result<#render_path::render_resource::UnpreparedBindGroup, #render_path::render_resource::AsBindGroupError> { let bindings = vec![#(#binding_impls,)*]; diff --git a/examples/shader/texture_binding_array.rs b/examples/shader/texture_binding_array.rs index 178ec5dbbc8be..26763c9105b2e 100644 --- a/examples/shader/texture_binding_array.rs +++ b/examples/shader/texture_binding_array.rs @@ -141,11 +141,11 @@ impl AsBindGroup for BindlessMaterial { }) } - fn unprepared_bind_group<'w>( + fn unprepared_bind_group( &self, _layout: &BindGroupLayout, _render_device: &RenderDevice, - _param: &mut SystemParamItem<'w, '_, Self::Param>, + _param: &mut SystemParamItem<'_, '_, Self::Param>, ) -> Result, AsBindGroupError> { // we implement as_bind_group directly because panic!("bindless texture arrays can't be owned") From ae7b336d3953634e8ada2a8c270896c23e9df338 Mon Sep 17 00:00:00 2001 From: Charlotte McElwain Date: Sat, 24 Aug 2024 14:12:10 -0700 Subject: [PATCH 4/5] Ci. --- examples/shader/texture_binding_array.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/shader/texture_binding_array.rs b/examples/shader/texture_binding_array.rs index 26763c9105b2e..53e774df0485e 100644 --- a/examples/shader/texture_binding_array.rs +++ b/examples/shader/texture_binding_array.rs @@ -101,11 +101,11 @@ impl AsBindGroup for BindlessMaterial { type Param = (SRes>, SRes); - fn as_bind_group<'w>( + fn as_bind_group( &self, layout: &BindGroupLayout, render_device: &RenderDevice, - (image_assets, fallback_image): &mut SystemParamItem<'w, '_, Self::Param>, + (image_assets, fallback_image): &mut SystemParamItem<'_, '_, Self::Param>, ) -> Result, AsBindGroupError> { // retrieve the render resources from handles let mut images = vec![]; From a1a8d7f3f83ccd9b1a142ed6dad80b8dd7e37f23 Mon Sep 17 00:00:00 2001 From: Charlotte McElwain Date: Sat, 24 Aug 2024 14:26:15 -0700 Subject: [PATCH 5/5] Ci. --- crates/bevy_render/src/render_resource/bind_group.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index 07831af590d68..4380890e17c27 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -58,7 +58,7 @@ impl Deref for BindGroup { /// /// This is an opinionated trait that is intended to make it easy to generically /// convert a type into a [`BindGroup`]. It provides access to specific render resources, -/// such as [`RenderAssets`] and [`FallbackImage`]. If a type has a [`Handle`](bevy_asset::Handle), +/// such as [`RenderAssets`] and [`crate::texture::FallbackImage`]. If a type has a [`Handle`](bevy_asset::Handle), /// these can be used to retrieve the corresponding [`Texture`](crate::render_resource::Texture) resource. /// /// [`AsBindGroup::as_bind_group`] is intended to be called once, then the result cached somewhere. It is generally @@ -116,7 +116,7 @@ impl Deref for BindGroup { /// * This field's [`Handle`](bevy_asset::Handle) will be used to look up the matching [`Texture`](crate::render_resource::Texture) /// GPU resource, which will be bound as a texture in shaders. The field will be assumed to implement [`Into>>`]. In practice, /// most fields should be a [`Handle`](bevy_asset::Handle) or [`Option>`]. If the value of an [`Option>`] is -/// [`None`], the [`FallbackImage`] resource will be used instead. This attribute can be used in conjunction with a `sampler` binding attribute +/// [`None`], the [`crate::texture::FallbackImage`] resource will be used instead. This attribute can be used in conjunction with a `sampler` binding attribute /// (with a different binding index) if a binding of the sampler for the [`Image`](crate::texture::Image) is also required. /// /// | Arguments | Values | Default | @@ -131,7 +131,7 @@ impl Deref for BindGroup { /// * This field's [`Handle`](bevy_asset::Handle) will be used to look up the matching [`Texture`](crate::render_resource::Texture) /// GPU resource, which will be bound as a storage texture in shaders. The field will be assumed to implement [`Into>>`]. In practice, /// most fields should be a [`Handle`](bevy_asset::Handle) or [`Option>`]. If the value of an [`Option>`] is -/// [`None`], the [`FallbackImage`] resource will be used instead. +/// [`None`], the [`crate::texture::FallbackImage`] resource will be used instead. /// /// | Arguments | Values | Default | /// |------------------------|--------------------------------------------------------------------------------------------|---------------| @@ -144,7 +144,7 @@ impl Deref for BindGroup { /// * This field's [`Handle`](bevy_asset::Handle) will be used to look up the matching [`Sampler`] GPU /// resource, which will be bound as a sampler in shaders. The field will be assumed to implement [`Into>>`]. In practice, /// most fields should be a [`Handle`](bevy_asset::Handle) or [`Option>`]. If the value of an [`Option>`] is -/// [`None`], the [`FallbackImage`] resource will be used instead. This attribute can be used in conjunction with a `texture` binding attribute +/// [`None`], the [`crate::texture::FallbackImage`] resource will be used instead. This attribute can be used in conjunction with a `texture` binding attribute /// (with a different binding index) if a binding of the texture for the [`Image`](crate::texture::Image) is also required. /// /// | Arguments | Values | Default | @@ -188,7 +188,7 @@ impl Deref for BindGroup { /// color_texture: Option>, /// } /// ``` -/// This is useful if you want a texture to be optional. When the value is [`None`], the [`FallbackImage`] will be used for the binding instead, which defaults +/// This is useful if you want a texture to be optional. When the value is [`None`], the [`crate::texture::FallbackImage`] will be used for the binding instead, which defaults /// to "pure white". /// /// Field uniforms with the same index will be combined into a single binding: