From e9a961d7e3005d30e7487fdc556ce4ecd7cfd753 Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Mon, 18 Jul 2022 01:20:42 -0400 Subject: [PATCH 1/9] better errors, support more options via name-value pairs in texture and sampler attributes. --- crates/bevy_macro_utils/src/attrs.rs | 14 + .../bevy_render/macros/src/as_bind_group.rs | 383 ++++++++++++++++-- crates/bevy_render/macros/src/lib.rs | 7 +- 3 files changed, 361 insertions(+), 43 deletions(-) diff --git a/crates/bevy_macro_utils/src/attrs.rs b/crates/bevy_macro_utils/src/attrs.rs index 196c4868452dd..4f2ed491e7e0d 100644 --- a/crates/bevy_macro_utils/src/attrs.rs +++ b/crates/bevy_macro_utils/src/attrs.rs @@ -31,3 +31,17 @@ pub fn get_lit_str(attr_name: Symbol, lit: &syn::Lit) -> syn::Result<&syn::LitSt )) } } + +pub fn get_lit_bool(attr_name: Symbol, lit: &syn::Lit) -> syn::Result { + if let syn::Lit::Bool(lit) = lit { + Ok(lit.value()) + } else { + Err(syn::Error::new_spanned( + lit, + format!( + "expected {} attribute to be a bool value, `true` or `false`: `{} = ...`", + attr_name, attr_name + ), + )) + } +} diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index f0c196dfd308c..678b3804780fa 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -1,17 +1,17 @@ -use bevy_macro_utils::BevyManifest; +use bevy_macro_utils::{BevyManifest, Symbol, get_lit_str, get_lit_bool}; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; use quote::quote; use syn::{ - parse::ParseStream, parse_macro_input, token::Comma, Data, DataStruct, DeriveInput, Field, - Fields, LitInt, + Error, parse::ParseStream, Result, Data, DataStruct, Fields, + Meta, NestedMeta, Lit, LitStr }; -const BINDING_ATTRIBUTE_NAME: &str = "binding"; -const UNIFORM_ATTRIBUTE_NAME: &str = "uniform"; -const TEXTURE_ATTRIBUTE_NAME: &str = "texture"; -const SAMPLER_ATTRIBUTE_NAME: &str = "sampler"; -const BIND_GROUP_DATA_ATTRIBUTE_NAME: &str = "bind_group_data"; +const BINDING_ATTRIBUTE_NAME: Symbol = Symbol("binding"); +const UNIFORM_ATTRIBUTE_NAME: Symbol = Symbol("uniform"); +const TEXTURE_ATTRIBUTE_NAME: Symbol = Symbol("texture"); +const SAMPLER_ATTRIBUTE_NAME: Symbol = Symbol("sampler"); +const BIND_GROUP_DATA_ATTRIBUTE_NAME: Symbol = Symbol("bind_group_data"); #[derive(Copy, Clone, Debug)] enum BindingType { @@ -29,13 +29,48 @@ enum BindingState<'a> { }, OccupiedConvertedUniform, OccupiedMergableUniform { - uniform_fields: Vec<&'a Field>, + uniform_fields: Vec<&'a syn::Field>, }, } -pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { - let ast = parse_macro_input!(input as DeriveInput); +fn get_binding_nested_meta(attr: &syn::Attribute) -> Result<(u32, Vec)> { + match attr.parse_meta() { + // Parse #[foo(0, ...)] + Ok(Meta::List(meta)) => { + let mut nested_iter = meta.nested.into_iter(); + let binding_meta = nested_iter.next().ok_or_else(|| Error::new_spanned( + attr, + "expected #[foo(u32, ...)]" + ))?; + + let lit_int = if let NestedMeta::Lit(Lit::Int(lit_int)) = binding_meta { + lit_int + } else { + return Err(Error::new_spanned( + attr, + "expected #[foo(u32, ...)]" + )); + }; + + Ok(( + lit_int.base10_parse()?, + nested_iter.collect() + )) + }, + Ok(other) => { + Err(Error::new_spanned( + other, + "expected #[foo(...)]" + )) + } + Err(err) => { + Err(err) + } + } +} + +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"); @@ -59,15 +94,18 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { let (binding_index, converted_shader_type) = attr .parse_args_with(|input: ParseStream| { let binding_index = input - .parse::() + .parse::() .and_then(|i| i.base10_parse::())?; - input.parse::()?; + input.parse::()?; let converted_shader_type = input.parse::()?; Ok((binding_index, converted_shader_type)) }) - .unwrap_or_else(|_| { - panic!("struct-level uniform bindings must be in the format: uniform(BINDING_INDEX, ConvertedShaderType)"); - }); + .map_err(|_| { + Error::new_spanned( + attr, + "struct-level uniform bindings must be in the format: uniform(BINDING_INDEX, ConvertedShaderType)" + ) + })?; binding_impls.push(quote! {{ use #render_path::render_resource::AsBindGroupShaderType; @@ -118,7 +156,12 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { fields: Fields::Named(fields), .. }) => &fields.named, - _ => panic!("Expected a struct with named fields"), + _ => { + return Err(Error::new_spanned( + ast, + "Expected a struct with named fields" + )); + }, }; // Read field-level attributes @@ -127,9 +170,12 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { let attr_ident = if let Some(ident) = attr.path.get_ident() { ident } else { - continue; + return Err(Error::new_spanned( + attr, + "Expected identifier" + )); }; - + let binding_type = if attr_ident == UNIFORM_ATTRIBUTE_NAME { BindingType::Uniform } else if attr_ident == TEXTURE_ATTRIBUTE_NAME { @@ -140,17 +186,7 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { continue; }; - let binding_index = attr - .parse_args_with(|input: ParseStream| { - let binding_index = input - .parse::() - .and_then(|i| i.base10_parse::()) - .expect("binding index was not a valid u32"); - Ok(binding_index) - }) - .unwrap_or_else(|_| { - panic!("Invalid `{}` attribute format", BINDING_ATTRIBUTE_NAME) - }); + let (binding_index, nested_meta_items) = get_binding_nested_meta(attr)?; let field_name = field.ident.as_ref().unwrap(); let required_len = binding_index as usize + 1; @@ -180,18 +216,29 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { }}, } }, - BindingState::Occupied { binding_type, ident: occupied_ident} => panic!( - "The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by the field '{occupied_ident}' of type {binding_type:?}." - ), - BindingState::OccupiedConvertedUniform => panic!( - "The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by a struct-level uniform binding at the same index." - ), + BindingState::Occupied { binding_type, ident: occupied_ident} => { + return Err(Error::new_spanned( + attr, + format!("The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by the field '{occupied_ident}' of type {binding_type:?}.") + )); + }, + BindingState::OccupiedConvertedUniform => { + return Err(Error::new_spanned( + attr, + format!("The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by a struct-level uniform binding at the same index.") + )); + }, BindingState::OccupiedMergableUniform { uniform_fields } => { match binding_type { BindingType::Uniform => { uniform_fields.push(field); }, - _ => {panic!("The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by a {:?}.", BindingType::Uniform)}, + _ => { + return Err(Error::new_spanned( + attr, + format!("The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by a {:?}.", BindingType::Uniform) + )); + }, } }, } @@ -200,6 +247,26 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { BindingType::Uniform => { /* uniform codegen is deferred to account for combined uniform bindings */ } BindingType::Texture => { + let texture_attrs = get_texture_attrs(nested_meta_items)?; + + let sample_type = match texture_attrs.sample_type { + BindingTextureSampleType::Float { filterable } => quote! { Float { filterable: #filterable } }, + BindingTextureSampleType::Depth => quote! { Depth }, + BindingTextureSampleType::Sint => quote! { Sint }, + BindingTextureSampleType::Uint => quote! { Uint }, + }; + + let dimension = match texture_attrs.dimension { + BindingTextureDimension::D1 => quote! { D1 }, + BindingTextureDimension::D2 => quote! { D2 }, + BindingTextureDimension::D2Array => quote! { D2Array }, + BindingTextureDimension::Cube => quote! { Cube }, + BindingTextureDimension::CubeArray => quote! { CubeArray }, + BindingTextureDimension::D3 => quote! { D3 }, + }; + + let multisampled = texture_attrs.multisampled; + binding_impls.push(quote! { #render_path::render_resource::OwnedBindingResource::TextureView({ let handle: Option<&#asset_path::Handle<#render_path::texture::Image>> = (&self.#field_name).into(); @@ -216,15 +283,23 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { binding: #binding_index, visibility: #render_path::render_resource::ShaderStages::all(), ty: #render_path::render_resource::BindingType::Texture { - multisampled: false, - sample_type: #render_path::render_resource::TextureSampleType::Float { filterable: true }, - view_dimension: #render_path::render_resource::TextureViewDimension::D2, + multisampled: #multisampled, + sample_type: #render_path::render_resource::TextureSampleType::#sample_type, + view_dimension: #render_path::render_resource::TextureViewDimension::#dimension, }, count: None, } }); } BindingType::Sampler => { + let sampler_attrs = get_sampler_attrs(nested_meta_items)?; + + let sampler_binding_type = match sampler_attrs.sampler_binding_type { + SamplerBindingType::Filtering => quote! { Filtering }, + SamplerBindingType::NonFiltering => quote! { NonFiltering }, + SamplerBindingType::Comparison => quote! { Comparison }, + }; + binding_impls.push(quote! { #render_path::render_resource::OwnedBindingResource::Sampler({ let handle: Option<&#asset_path::Handle<#render_path::texture::Image>> = (&self.#field_name).into(); @@ -240,7 +315,7 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { #render_path::render_resource::BindGroupLayoutEntry { binding: #binding_index, visibility: #render_path::render_resource::ShaderStages::all(), - ty: #render_path::render_resource::BindingType::Sampler(#render_path::render_resource::SamplerBindingType::Filtering), + ty: #render_path::render_resource::BindingType::Sampler(#render_path::render_resource::SamplerBindingType::#sampler_binding_type), count: None, } }); @@ -297,6 +372,7 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { &format!("_{struct_name}AsBindGroupUniformStructBindGroup{binding_index}"), Span::call_site(), ); + let field_name = uniform_fields.iter().map(|f| f.ident.as_ref().unwrap()); let field_type = uniform_fields.iter().map(|f| &f.ty); field_struct_impls.push(quote! { @@ -348,7 +424,7 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { (prepared_data.clone(), prepared_data) }; - TokenStream::from(quote! { + Ok(TokenStream::from(quote! { #(#field_struct_impls)* impl #impl_generics #render_path::render_resource::AsBindGroup for #struct_name #ty_generics #where_clause { @@ -385,5 +461,228 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { }) } } + })) +} + +#[derive(Default)] +enum BindingTextureDimension { + D1, + #[default] + D2, + D2Array, + Cube, + CubeArray, + D3, +} + +enum BindingTextureSampleType { + Float { + filterable: bool, + }, + Depth, + Sint, + Uint, +} + +struct TextureAttrs { + dimension: BindingTextureDimension, + sample_type: BindingTextureSampleType, + multisampled: bool, +} + +impl Default for BindingTextureSampleType { + fn default() -> Self { + BindingTextureSampleType::Float { + filterable: true + } + } +} + +impl Default for TextureAttrs { + fn default() -> Self { + Self { + dimension: Default::default(), + sample_type: Default::default(), + multisampled: true + } + } +} + +const DIMENSION: Symbol = Symbol("dimension"); +const SAMPLE_TYPE: Symbol = Symbol("sample_type"); +const FILTERABLE: Symbol = Symbol("filterable"); +const MULTISAMPLED: Symbol = Symbol("multisampled"); + +// Values for `dimension` attribute. +const DIM_1D: &str = "1d"; +const DIM_2D: &str = "2d"; +const DIM_3D: &str = "3d"; +const DIM_2D_ARRAY: &str = "2d_array"; +const DIM_CUBE: &str = "cube"; +const DIM_CUBE_ARRAY: &str = "cube_array"; + +// Values for sample `type` attribute. +const FLOAT: &str = "float"; +const DEPTH: &str = "depth"; +const S_INT: &str = "s_int"; +const U_INT: &str = "u_int"; + +fn get_texture_attrs(metas: Vec) -> Result { + let mut dimension = Default::default(); + let mut sample_type = Default::default(); + let mut multisampled = Default::default(); + let mut filterable = None; + let mut filterable_ident = None; + + for meta in metas { + use syn::{ + Meta::NameValue, + NestedMeta::Meta, + }; + match meta { + Meta(NameValue(m)) if m.path == DIMENSION => { + let value = get_lit_str(DIMENSION, &m.lit)?; + dimension = get_texture_dimension_value(&value)?; + }, + Meta(NameValue(m)) if m.path == SAMPLE_TYPE => { + let value = get_lit_str(SAMPLE_TYPE, &m.lit)?; + sample_type = get_texture_sample_type_value(&value)?; + }, + Meta(NameValue(m)) if m.path == MULTISAMPLED => { + multisampled = get_lit_bool(MULTISAMPLED, &m.lit)?; + }, + Meta(NameValue(m)) if m.path == FILTERABLE => { + filterable = get_lit_bool(FILTERABLE, &m.lit)?.into(); + filterable_ident = m.path.into(); + }, + Meta(NameValue(m)) => { + return Err(Error::new_spanned( + m.path, + "Not a valid name. Available attributes: `dimension`, `sample_type`, `multisampled`, or `filterable`." + )); + }, + _ => { + return Err(Error::new_spanned( + meta, + "Not a name value pair: `foo = \"...\"`" + )); + } + } + } + + // Resolve `filterable` since the float + // sample type is the one that contains the value. + if let Some(filterable) = filterable { + let path = filterable_ident.unwrap(); + match sample_type { + BindingTextureSampleType::Float { filterable: _ } => sample_type = BindingTextureSampleType::Float { filterable }, + _ => { + return Err(Error::new_spanned( + path.clone(), + "Type must be `float` to use the `filterable` attribute." + )); + } + }; + } + + Ok(TextureAttrs { + dimension, + sample_type, + multisampled, + }) +} + +fn get_texture_dimension_value(lit_str: &LitStr) -> Result { + match lit_str.value().as_str() { + DIM_1D => Ok(BindingTextureDimension::D1), + DIM_2D => Ok(BindingTextureDimension::D2), + DIM_2D_ARRAY => Ok(BindingTextureDimension::D2Array), + DIM_3D => Ok(BindingTextureDimension::D3), + DIM_CUBE => Ok(BindingTextureDimension::Cube), + DIM_CUBE_ARRAY => Ok(BindingTextureDimension::CubeArray), + + _ => Err(Error::new_spanned( + lit_str, + "Not a valid dimension. Must be `1d`, `2d`, `2d_array`, `3d`, `cube` or `cube_array`." + )), + } +} + +fn get_texture_sample_type_value(lit_str: &LitStr) -> Result { + match lit_str.value().as_str() { + FLOAT => Ok(BindingTextureSampleType::Float { filterable: true }), + DEPTH => Ok(BindingTextureSampleType::Depth), + S_INT => Ok(BindingTextureSampleType::Sint), + U_INT => Ok(BindingTextureSampleType::Uint), + + _ => Err(Error::new_spanned( + lit_str, + "Not a valid sample type. Must be `float`, `depth`, `s_int` or `u_int`." + )), + } +} + +#[derive(Default)] +struct SamplerAttrs { + sampler_binding_type: SamplerBindingType +} + +#[derive(Default)] +enum SamplerBindingType { + #[default] + Filtering, + NonFiltering, + Comparison, +} + +const SAMPLER_TYPE: Symbol = Symbol("sampler_type"); + +const FILTERING: &str = "filtering"; +const NON_FILTERING: &str = "non_filtering"; +const COMPARISON: &str = "comparison"; + +fn get_sampler_attrs(metas: Vec) -> Result { + let mut sampler_binding_type = Default::default(); + + for meta in metas { + use syn::{ + Meta::NameValue, + NestedMeta::Meta, + }; + match meta { + Meta(NameValue(m)) if m.path == SAMPLER_TYPE => { + let value = get_lit_str(DIMENSION, &m.lit)?; + sampler_binding_type = get_sampler_binding_type_value(&value)?; + }, + Meta(NameValue(m)) => { + return Err(Error::new_spanned( + m.path, + "Not a valid name. Available attributes: `sampler_type`." + )); + }, + _ => { + return Err(Error::new_spanned( + meta, + "Not a name value pair: `foo = \"...\"`" + )); + } + } + } + + Ok(SamplerAttrs { + sampler_binding_type, }) } + +fn get_sampler_binding_type_value(lit_str: &LitStr) -> Result { + match lit_str.value().as_str() { + FILTERING => Ok(SamplerBindingType::Filtering), + NON_FILTERING => Ok(SamplerBindingType::NonFiltering), + COMPARISON => Ok(SamplerBindingType::Comparison), + + _ => Err(Error::new_spanned( + lit_str, + "Not a valid dimension. Must be `filtering`, `non_filtering`, or `comparison`." + )), + } +} \ No newline at end of file diff --git a/crates/bevy_render/macros/src/lib.rs b/crates/bevy_render/macros/src/lib.rs index 5e0851a69a3dc..332412f8870f9 100644 --- a/crates/bevy_render/macros/src/lib.rs +++ b/crates/bevy_render/macros/src/lib.rs @@ -3,6 +3,8 @@ mod extract_resource; use bevy_macro_utils::BevyManifest; use proc_macro::TokenStream; +use syn::{parse_macro_input, DeriveInput}; +use quote::quote; pub(crate) fn bevy_render_path() -> syn::Path { BevyManifest::default() @@ -18,5 +20,8 @@ pub fn derive_extract_resource(input: TokenStream) -> TokenStream { #[proc_macro_derive(AsBindGroup, attributes(uniform, texture, sampler, bind_group_data))] pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + as_bind_group::derive_as_bind_group(input) -} + .unwrap_or_else(|err| err.to_compile_error().into() ) +} \ No newline at end of file From 38fdaf9eb027a8cd50c991661a8892eabe453aa2 Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Mon, 18 Jul 2022 01:21:35 -0400 Subject: [PATCH 2/9] update array_texture example to use AsBindGroup derive macro --- examples/shader/array_texture.rs | 83 ++------------------------------ 1 file changed, 5 insertions(+), 78 deletions(-) diff --git a/examples/shader/array_texture.rs b/examples/shader/array_texture.rs index 59873724765bd..dce73b83fb120 100644 --- a/examples/shader/array_texture.rs +++ b/examples/shader/array_texture.rs @@ -2,17 +2,7 @@ use bevy::{ asset::LoadState, prelude::*, reflect::TypeUuid, - render::{ - render_asset::RenderAssets, - render_resource::{ - AsBindGroup, AsBindGroupError, BindGroupDescriptor, BindGroupEntry, BindGroupLayout, - BindGroupLayoutDescriptor, BindGroupLayoutEntry, BindingResource, BindingType, - OwnedBindingResource, PreparedBindGroup, SamplerBindingType, ShaderRef, ShaderStages, - TextureSampleType, TextureViewDimension, - }, - renderer::RenderDevice, - texture::FallbackImage, - }, + render::render_resource::{AsBindGroup, ShaderRef}, }; /// This example illustrates how to create a texture for use with a `texture_2d_array` shader @@ -98,9 +88,11 @@ fn create_array_texture( } } -#[derive(Debug, Clone, TypeUuid)] +#[derive(AsBindGroup, Debug, Clone, TypeUuid)] #[uuid = "9c5a0ddf-1eaf-41b4-9832-ed736fd26af3"] struct ArrayTextureMaterial { + #[texture(0, dimension = "2d_array")] + #[sampler(1)] array_texture: Handle, } @@ -108,69 +100,4 @@ impl Material for ArrayTextureMaterial { fn fragment_shader() -> ShaderRef { "shaders/array_texture.wgsl".into() } -} - -impl AsBindGroup for ArrayTextureMaterial { - type Data = (); - - fn as_bind_group( - &self, - layout: &BindGroupLayout, - render_device: &RenderDevice, - images: &RenderAssets, - _fallback_image: &FallbackImage, - ) -> Result, AsBindGroupError> { - let image = images - .get(&self.array_texture) - .ok_or(AsBindGroupError::RetryNextUpdate)?; - let bind_group = render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(&image.texture_view), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&image.sampler), - }, - ], - label: Some("array_texture_material_bind_group"), - layout, - }); - - Ok(PreparedBindGroup { - bind_group, - bindings: vec![ - OwnedBindingResource::TextureView(image.texture_view.clone()), - OwnedBindingResource::Sampler(image.sampler.clone()), - ], - data: (), - }) - } - - fn bind_group_layout(render_device: &RenderDevice) -> BindGroupLayout { - render_device.create_bind_group_layout(&BindGroupLayoutDescriptor { - entries: &[ - // Array Texture - BindGroupLayoutEntry { - binding: 0, - visibility: ShaderStages::FRAGMENT, - ty: BindingType::Texture { - multisampled: false, - sample_type: TextureSampleType::Float { filterable: true }, - view_dimension: TextureViewDimension::D2Array, - }, - count: None, - }, - // Array Texture Sampler - BindGroupLayoutEntry { - binding: 1, - visibility: ShaderStages::FRAGMENT, - ty: BindingType::Sampler(SamplerBindingType::Filtering), - count: None, - }, - ], - label: None, - }) - } -} +} \ No newline at end of file From 5ef96c5a5e82108ce7d37466fe85c4643f1e651e Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Mon, 18 Jul 2022 14:39:01 -0400 Subject: [PATCH 3/9] run cargo fmt --- .../bevy_render/macros/src/as_bind_group.rs | 162 ++++++++---------- crates/bevy_render/macros/src/lib.rs | 7 +- examples/shader/array_texture.rs | 2 +- 3 files changed, 74 insertions(+), 97 deletions(-) diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 678b3804780fa..9df5697445dbc 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -1,10 +1,9 @@ -use bevy_macro_utils::{BevyManifest, Symbol, get_lit_str, get_lit_bool}; +use bevy_macro_utils::{get_lit_bool, get_lit_str, BevyManifest, Symbol}; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; use quote::quote; use syn::{ - Error, parse::ParseStream, Result, Data, DataStruct, Fields, - Meta, NestedMeta, Lit, LitStr + parse::ParseStream, Data, DataStruct, Error, Fields, Lit, LitStr, Meta, NestedMeta, Result, }; const BINDING_ATTRIBUTE_NAME: Symbol = Symbol("binding"); @@ -39,34 +38,20 @@ fn get_binding_nested_meta(attr: &syn::Attribute) -> Result<(u32, Vec { let mut nested_iter = meta.nested.into_iter(); - let binding_meta = nested_iter.next().ok_or_else(|| Error::new_spanned( - attr, - "expected #[foo(u32, ...)]" - ))?; + let binding_meta = nested_iter + .next() + .ok_or_else(|| Error::new_spanned(attr, "expected #[foo(u32, ...)]"))?; let lit_int = if let NestedMeta::Lit(Lit::Int(lit_int)) = binding_meta { lit_int } else { - return Err(Error::new_spanned( - attr, - "expected #[foo(u32, ...)]" - )); + return Err(Error::new_spanned(attr, "expected #[foo(u32, ...)]")); }; - Ok(( - lit_int.base10_parse()?, - nested_iter.collect() - )) - }, - Ok(other) => { - Err(Error::new_spanned( - other, - "expected #[foo(...)]" - )) - } - Err(err) => { - Err(err) + Ok((lit_int.base10_parse()?, nested_iter.collect())) } + Ok(other) => Err(Error::new_spanned(other, "expected #[foo(...)]")), + Err(err) => Err(err), } } @@ -158,10 +143,10 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { }) => &fields.named, _ => { return Err(Error::new_spanned( - ast, - "Expected a struct with named fields" + ast, + "Expected a struct with named fields", )); - }, + } }; // Read field-level attributes @@ -170,12 +155,9 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { let attr_ident = if let Some(ident) = attr.path.get_ident() { ident } else { - return Err(Error::new_spanned( - attr, - "Expected identifier" - )); + return Err(Error::new_spanned(attr, "Expected identifier")); }; - + let binding_type = if attr_ident == UNIFORM_ATTRIBUTE_NAME { BindingType::Uniform } else if attr_ident == TEXTURE_ATTRIBUTE_NAME { @@ -211,34 +193,36 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { } }); BindingState::Occupied { - binding_type, - ident: field_name, - }}, + binding_type, + ident: field_name, + } + } } - }, - BindingState::Occupied { binding_type, ident: occupied_ident} => { + } + BindingState::Occupied { + binding_type, + ident: occupied_ident, + } => { return Err(Error::new_spanned( - attr, + attr, format!("The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by the field '{occupied_ident}' of type {binding_type:?}.") )); - }, + } BindingState::OccupiedConvertedUniform => { return Err(Error::new_spanned( - attr, + attr, format!("The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by a struct-level uniform binding at the same index.") )); - }, - BindingState::OccupiedMergableUniform { uniform_fields } => { - match binding_type { - BindingType::Uniform => { - uniform_fields.push(field); - }, - _ => { - return Err(Error::new_spanned( - attr, + } + BindingState::OccupiedMergableUniform { uniform_fields } => match binding_type { + BindingType::Uniform => { + uniform_fields.push(field); + } + _ => { + return Err(Error::new_spanned( + attr, format!("The '{field_name}' field cannot be assigned to binding {binding_index} because it is already occupied by a {:?}.", BindingType::Uniform) )); - }, } }, } @@ -250,7 +234,9 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { let texture_attrs = get_texture_attrs(nested_meta_items)?; let sample_type = match texture_attrs.sample_type { - BindingTextureSampleType::Float { filterable } => quote! { Float { filterable: #filterable } }, + BindingTextureSampleType::Float { filterable } => { + quote! { Float { filterable: #filterable } } + } BindingTextureSampleType::Depth => quote! { Depth }, BindingTextureSampleType::Sint => quote! { Sint }, BindingTextureSampleType::Uint => quote! { Uint }, @@ -372,7 +358,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { &format!("_{struct_name}AsBindGroupUniformStructBindGroup{binding_index}"), Span::call_site(), ); - + let field_name = uniform_fields.iter().map(|f| f.ident.as_ref().unwrap()); let field_type = uniform_fields.iter().map(|f| &f.ty); field_struct_impls.push(quote! { @@ -476,9 +462,7 @@ enum BindingTextureDimension { } enum BindingTextureSampleType { - Float { - filterable: bool, - }, + Float { filterable: bool }, Depth, Sint, Uint, @@ -492,9 +476,7 @@ struct TextureAttrs { impl Default for BindingTextureSampleType { fn default() -> Self { - BindingTextureSampleType::Float { - filterable: true - } + BindingTextureSampleType::Float { filterable: true } } } @@ -503,7 +485,7 @@ impl Default for TextureAttrs { Self { dimension: Default::default(), sample_type: Default::default(), - multisampled: true + multisampled: true, } } } @@ -535,36 +517,33 @@ fn get_texture_attrs(metas: Vec) -> Result { let mut filterable_ident = None; for meta in metas { - use syn::{ - Meta::NameValue, - NestedMeta::Meta, - }; + use syn::{Meta::NameValue, NestedMeta::Meta}; match meta { Meta(NameValue(m)) if m.path == DIMENSION => { let value = get_lit_str(DIMENSION, &m.lit)?; dimension = get_texture_dimension_value(&value)?; - }, + } Meta(NameValue(m)) if m.path == SAMPLE_TYPE => { let value = get_lit_str(SAMPLE_TYPE, &m.lit)?; sample_type = get_texture_sample_type_value(&value)?; - }, + } Meta(NameValue(m)) if m.path == MULTISAMPLED => { multisampled = get_lit_bool(MULTISAMPLED, &m.lit)?; - }, + } Meta(NameValue(m)) if m.path == FILTERABLE => { filterable = get_lit_bool(FILTERABLE, &m.lit)?.into(); filterable_ident = m.path.into(); - }, + } Meta(NameValue(m)) => { return Err(Error::new_spanned( - m.path, + m.path, "Not a valid name. Available attributes: `dimension`, `sample_type`, `multisampled`, or `filterable`." )); - }, + } _ => { return Err(Error::new_spanned( - meta, - "Not a name value pair: `foo = \"...\"`" + meta, + "Not a name value pair: `foo = \"...\"`", )); } } @@ -575,11 +554,13 @@ fn get_texture_attrs(metas: Vec) -> Result { if let Some(filterable) = filterable { let path = filterable_ident.unwrap(); match sample_type { - BindingTextureSampleType::Float { filterable: _ } => sample_type = BindingTextureSampleType::Float { filterable }, + BindingTextureSampleType::Float { filterable: _ } => { + sample_type = BindingTextureSampleType::Float { filterable } + } _ => { return Err(Error::new_spanned( - path.clone(), - "Type must be `float` to use the `filterable` attribute." + path.clone(), + "Type must be `float` to use the `filterable` attribute.", )); } }; @@ -602,8 +583,8 @@ fn get_texture_dimension_value(lit_str: &LitStr) -> Result Ok(BindingTextureDimension::CubeArray), _ => Err(Error::new_spanned( - lit_str, - "Not a valid dimension. Must be `1d`, `2d`, `2d_array`, `3d`, `cube` or `cube_array`." + lit_str, + "Not a valid dimension. Must be `1d`, `2d`, `2d_array`, `3d`, `cube` or `cube_array`.", )), } } @@ -616,15 +597,15 @@ fn get_texture_sample_type_value(lit_str: &LitStr) -> Result Ok(BindingTextureSampleType::Uint), _ => Err(Error::new_spanned( - lit_str, - "Not a valid sample type. Must be `float`, `depth`, `s_int` or `u_int`." + lit_str, + "Not a valid sample type. Must be `float`, `depth`, `s_int` or `u_int`.", )), } } #[derive(Default)] struct SamplerAttrs { - sampler_binding_type: SamplerBindingType + sampler_binding_type: SamplerBindingType, } #[derive(Default)] @@ -645,25 +626,22 @@ fn get_sampler_attrs(metas: Vec) -> Result { let mut sampler_binding_type = Default::default(); for meta in metas { - use syn::{ - Meta::NameValue, - NestedMeta::Meta, - }; + use syn::{Meta::NameValue, NestedMeta::Meta}; match meta { Meta(NameValue(m)) if m.path == SAMPLER_TYPE => { let value = get_lit_str(DIMENSION, &m.lit)?; sampler_binding_type = get_sampler_binding_type_value(&value)?; - }, + } Meta(NameValue(m)) => { return Err(Error::new_spanned( - m.path, - "Not a valid name. Available attributes: `sampler_type`." + m.path, + "Not a valid name. Available attributes: `sampler_type`.", )); - }, + } _ => { return Err(Error::new_spanned( - meta, - "Not a name value pair: `foo = \"...\"`" + meta, + "Not a name value pair: `foo = \"...\"`", )); } } @@ -681,8 +659,8 @@ fn get_sampler_binding_type_value(lit_str: &LitStr) -> Result Ok(SamplerBindingType::Comparison), _ => Err(Error::new_spanned( - lit_str, - "Not a valid dimension. Must be `filtering`, `non_filtering`, or `comparison`." + lit_str, + "Not a valid dimension. Must be `filtering`, `non_filtering`, or `comparison`.", )), } -} \ No newline at end of file +} diff --git a/crates/bevy_render/macros/src/lib.rs b/crates/bevy_render/macros/src/lib.rs index 332412f8870f9..1a258d61b32b3 100644 --- a/crates/bevy_render/macros/src/lib.rs +++ b/crates/bevy_render/macros/src/lib.rs @@ -3,8 +3,8 @@ mod extract_resource; use bevy_macro_utils::BevyManifest; use proc_macro::TokenStream; -use syn::{parse_macro_input, DeriveInput}; use quote::quote; +use syn::{parse_macro_input, DeriveInput}; pub(crate) fn bevy_render_path() -> syn::Path { BevyManifest::default() @@ -22,6 +22,5 @@ pub fn derive_extract_resource(input: TokenStream) -> TokenStream { pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); - as_bind_group::derive_as_bind_group(input) - .unwrap_or_else(|err| err.to_compile_error().into() ) -} \ No newline at end of file + as_bind_group::derive_as_bind_group(input).unwrap_or_else(|err| err.to_compile_error().into()) +} diff --git a/examples/shader/array_texture.rs b/examples/shader/array_texture.rs index dce73b83fb120..2c288625d8fb9 100644 --- a/examples/shader/array_texture.rs +++ b/examples/shader/array_texture.rs @@ -100,4 +100,4 @@ impl Material for ArrayTextureMaterial { fn fragment_shader() -> ShaderRef { "shaders/array_texture.wgsl".into() } -} \ No newline at end of file +} From 57c0e6a64f0a72f6beca82f987fa8ed1778f6d21 Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Mon, 18 Jul 2022 14:44:08 -0400 Subject: [PATCH 4/9] remove unused `binding` constant and imports --- crates/bevy_render/macros/src/as_bind_group.rs | 1 - crates/bevy_render/macros/src/lib.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 9df5697445dbc..5d18fe1267fa4 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -6,7 +6,6 @@ use syn::{ parse::ParseStream, Data, DataStruct, Error, Fields, Lit, LitStr, Meta, NestedMeta, Result, }; -const BINDING_ATTRIBUTE_NAME: Symbol = Symbol("binding"); const UNIFORM_ATTRIBUTE_NAME: Symbol = Symbol("uniform"); const TEXTURE_ATTRIBUTE_NAME: Symbol = Symbol("texture"); const SAMPLER_ATTRIBUTE_NAME: Symbol = Symbol("sampler"); diff --git a/crates/bevy_render/macros/src/lib.rs b/crates/bevy_render/macros/src/lib.rs index 1a258d61b32b3..863b48baf5fa0 100644 --- a/crates/bevy_render/macros/src/lib.rs +++ b/crates/bevy_render/macros/src/lib.rs @@ -3,7 +3,6 @@ mod extract_resource; use bevy_macro_utils::BevyManifest; use proc_macro::TokenStream; -use quote::quote; use syn::{parse_macro_input, DeriveInput}; pub(crate) fn bevy_render_path() -> syn::Path { From 72d6cb4305ed2eb4e473020678383dcca2f07672 Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Mon, 18 Jul 2022 17:04:30 -0400 Subject: [PATCH 5/9] organize code, split out parsing, use Parse trait --- .../bevy_render/macros/src/as_bind_group.rs | 133 ++++++++++++------ 1 file changed, 89 insertions(+), 44 deletions(-) diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 5d18fe1267fa4..2353311833810 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -3,7 +3,9 @@ use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; use quote::quote; use syn::{ - parse::ParseStream, Data, DataStruct, Error, Fields, Lit, LitStr, Meta, NestedMeta, Result, + parse::{Parse, ParseStream}, + punctuated::Punctuated, + Data, DataStruct, Error, Fields, LitInt, LitStr, NestedMeta, Result, Token, }; const UNIFORM_ATTRIBUTE_NAME: Symbol = Symbol("uniform"); @@ -31,29 +33,6 @@ enum BindingState<'a> { }, } -fn get_binding_nested_meta(attr: &syn::Attribute) -> Result<(u32, Vec)> { - match attr.parse_meta() { - // Parse #[foo(0, ...)] - Ok(Meta::List(meta)) => { - let mut nested_iter = meta.nested.into_iter(); - - let binding_meta = nested_iter - .next() - .ok_or_else(|| Error::new_spanned(attr, "expected #[foo(u32, ...)]"))?; - - let lit_int = if let NestedMeta::Lit(Lit::Int(lit_int)) = binding_meta { - lit_int - } else { - return Err(Error::new_spanned(attr, "expected #[foo(u32, ...)]")); - }; - - Ok((lit_int.base10_parse()?, nested_iter.collect())) - } - Ok(other) => Err(Error::new_spanned(other, "expected #[foo(...)]")), - Err(err) => Err(err), - } -} - pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { let manifest = BevyManifest::default(); let render_path = manifest.get_path("bevy_render"); @@ -75,21 +54,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { attr_prepared_data_ident = Some(prepared_data_ident); } } else if attr_ident == UNIFORM_ATTRIBUTE_NAME { - let (binding_index, converted_shader_type) = attr - .parse_args_with(|input: ParseStream| { - let binding_index = input - .parse::() - .and_then(|i| i.base10_parse::())?; - input.parse::()?; - let converted_shader_type = input.parse::()?; - Ok((binding_index, converted_shader_type)) - }) - .map_err(|_| { - Error::new_spanned( - attr, - "struct-level uniform bindings must be in the format: uniform(BINDING_INDEX, ConvertedShaderType)" - ) - })?; + let (binding_index, converted_shader_type) = get_uniform_binding_attr(attr)?; binding_impls.push(quote! {{ use #render_path::render_resource::AsBindGroupShaderType; @@ -167,7 +132,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { continue; }; - let (binding_index, nested_meta_items) = get_binding_nested_meta(attr)?; + let (binding_index, nested_meta_items) = get_binding_nested_attr(attr)?; let field_name = field.ident.as_ref().unwrap(); let required_len = binding_index as usize + 1; @@ -449,6 +414,86 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { })) } +/// Represents the arguments for the `uniform` binding attribute. +/// +/// If parsed, represents an attribute +/// like `#[uniform(LitInt, Ident)]` +struct UniformBindingMeta { + lit_int: LitInt, + _comma: Token![,], + ident: Ident, +} + +/// Represents the arguments for any general binding attribute. +/// +/// If parsed, represents an attribute +/// like `#[foo(LitInt, ...)]` where the rest is optional `NestedMeta`. +enum BindingMeta { + IndexOnly(LitInt), + IndexWithOptions(BindingIndexOptions), +} + +/// Represents the arguments for an attribute with a list of arguments. +/// +/// This represents, for example, `#[texture(0, dimension = "2d_array")]`. +struct BindingIndexOptions { + lit_int: LitInt, + _comma: Token![,], + meta_list: Punctuated, +} + +impl Parse for BindingMeta { + fn parse(input: ParseStream) -> Result { + if input.peek2(Token![,]) { + input.parse().map(Self::IndexWithOptions) + } else { + input.parse().map(Self::IndexOnly) + } + } +} + +impl Parse for BindingIndexOptions { + fn parse(input: ParseStream) -> Result { + Ok(Self { + lit_int: input.parse()?, + _comma: input.parse()?, + meta_list: input.parse_terminated(NestedMeta::parse)?, + }) + } +} + +impl Parse for UniformBindingMeta { + fn parse(input: ParseStream) -> Result { + Ok(Self { + lit_int: input.parse()?, + _comma: input.parse()?, + ident: input.parse()?, + }) + } +} + +fn get_uniform_binding_attr(attr: &syn::Attribute) -> Result<(u32, Ident)> { + let uniform_binding_meta = attr.parse_args_with(UniformBindingMeta::parse)?; + + let binding_index = uniform_binding_meta.lit_int.base10_parse()?; + let ident = uniform_binding_meta.ident; + + Ok((binding_index, ident)) +} + +fn get_binding_nested_attr(attr: &syn::Attribute) -> Result<(u32, Vec)> { + let binding_meta = attr.parse_args_with(BindingMeta::parse)?; + + match binding_meta { + BindingMeta::IndexOnly(lit_int) => Ok((lit_int.base10_parse()?, Vec::new())), + BindingMeta::IndexWithOptions(BindingIndexOptions { + lit_int, + _comma: _, + meta_list, + }) => Ok((lit_int.base10_parse()?, meta_list.into_iter().collect())), + } +} + #[derive(Default)] enum BindingTextureDimension { D1, @@ -520,11 +565,11 @@ fn get_texture_attrs(metas: Vec) -> Result { match meta { Meta(NameValue(m)) if m.path == DIMENSION => { let value = get_lit_str(DIMENSION, &m.lit)?; - dimension = get_texture_dimension_value(&value)?; + dimension = get_texture_dimension_value(value)?; } Meta(NameValue(m)) if m.path == SAMPLE_TYPE => { let value = get_lit_str(SAMPLE_TYPE, &m.lit)?; - sample_type = get_texture_sample_type_value(&value)?; + sample_type = get_texture_sample_type_value(value)?; } Meta(NameValue(m)) if m.path == MULTISAMPLED => { multisampled = get_lit_bool(MULTISAMPLED, &m.lit)?; @@ -558,7 +603,7 @@ fn get_texture_attrs(metas: Vec) -> Result { } _ => { return Err(Error::new_spanned( - path.clone(), + path, "Type must be `float` to use the `filterable` attribute.", )); } @@ -629,7 +674,7 @@ fn get_sampler_attrs(metas: Vec) -> Result { match meta { Meta(NameValue(m)) if m.path == SAMPLER_TYPE => { let value = get_lit_str(DIMENSION, &m.lit)?; - sampler_binding_type = get_sampler_binding_type_value(&value)?; + sampler_binding_type = get_sampler_binding_type_value(value)?; } Meta(NameValue(m)) => { return Err(Error::new_spanned( From 850123005eb7e632323d1cc18e2ac82e6e41b020 Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Mon, 18 Jul 2022 18:20:09 -0400 Subject: [PATCH 6/9] support visibility, use ToTokens for attr args instead of quoting in the main derive --- .../bevy_render/macros/src/as_bind_group.rs | 207 ++++++++++++++---- 1 file changed, 170 insertions(+), 37 deletions(-) diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 2353311833810..3546cb23b7ab8 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -1,7 +1,7 @@ use bevy_macro_utils::{get_lit_bool, get_lit_str, BevyManifest, Symbol}; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; -use quote::quote; +use quote::{quote, ToTokens}; use syn::{ parse::{Parse, ParseStream}, punctuated::Punctuated, @@ -195,27 +195,12 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { BindingType::Uniform => { /* uniform codegen is deferred to account for combined uniform bindings */ } BindingType::Texture => { - let texture_attrs = get_texture_attrs(nested_meta_items)?; - - let sample_type = match texture_attrs.sample_type { - BindingTextureSampleType::Float { filterable } => { - quote! { Float { filterable: #filterable } } - } - BindingTextureSampleType::Depth => quote! { Depth }, - BindingTextureSampleType::Sint => quote! { Sint }, - BindingTextureSampleType::Uint => quote! { Uint }, - }; - - let dimension = match texture_attrs.dimension { - BindingTextureDimension::D1 => quote! { D1 }, - BindingTextureDimension::D2 => quote! { D2 }, - BindingTextureDimension::D2Array => quote! { D2Array }, - BindingTextureDimension::Cube => quote! { Cube }, - BindingTextureDimension::CubeArray => quote! { CubeArray }, - BindingTextureDimension::D3 => quote! { D3 }, - }; - - let multisampled = texture_attrs.multisampled; + let TextureAttrs { + dimension, + sample_type, + multisampled, + visibility, + } = get_texture_attrs(nested_meta_items)?; binding_impls.push(quote! { #render_path::render_resource::OwnedBindingResource::TextureView({ @@ -228,27 +213,24 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { }) }); - binding_layouts.push(quote!{ + binding_layouts.push(quote! { #render_path::render_resource::BindGroupLayoutEntry { binding: #binding_index, - visibility: #render_path::render_resource::ShaderStages::all(), + visibility: #render_path::render_resource::#visibility, ty: #render_path::render_resource::BindingType::Texture { multisampled: #multisampled, - sample_type: #render_path::render_resource::TextureSampleType::#sample_type, - view_dimension: #render_path::render_resource::TextureViewDimension::#dimension, + sample_type: #render_path::render_resource::#sample_type, + view_dimension: #render_path::render_resource::#dimension, }, count: None, } }); } BindingType::Sampler => { - let sampler_attrs = get_sampler_attrs(nested_meta_items)?; - - let sampler_binding_type = match sampler_attrs.sampler_binding_type { - SamplerBindingType::Filtering => quote! { Filtering }, - SamplerBindingType::NonFiltering => quote! { NonFiltering }, - SamplerBindingType::Comparison => quote! { Comparison }, - }; + let SamplerAttrs { + sampler_binding_type, + visibility, + } = get_sampler_attrs(nested_meta_items)?; binding_impls.push(quote! { #render_path::render_resource::OwnedBindingResource::Sampler({ @@ -264,8 +246,8 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { binding_layouts.push(quote!{ #render_path::render_resource::BindGroupLayoutEntry { binding: #binding_index, - visibility: #render_path::render_resource::ShaderStages::all(), - ty: #render_path::render_resource::BindingType::Sampler(#render_path::render_resource::SamplerBindingType::#sampler_binding_type), + visibility: #render_path::render_resource::#visibility, + ty: #render_path::render_resource::BindingType::Sampler(#render_path::render_resource::#sampler_binding_type), count: None, } }); @@ -494,6 +476,94 @@ fn get_binding_nested_attr(attr: &syn::Attribute) -> Result<(u32, Vec quote! { ShaderStages::all() }, + ShaderStageVisibility::None => quote! { ShaderStages::NONE }, + ShaderStageVisibility::Flags(flags) => { + let mut quoted = Vec::new(); + + if flags.vertex { + quoted.push(quote! { ShaderStages::VERTEX }); + } + if flags.fragment { + quoted.push(quote! { ShaderStages::FRAGMENT }); + } + if flags.compute { + quoted.push(quote! { ShaderStages::COMPUTE }); + } + + quote! { #(#quoted)|* } + } + }); + } +} + +const VISIBILITY: Symbol = Symbol("VISIBILITY"); +const VISIBILITY_VERTEX: Symbol = Symbol("VERTEX"); +const VISIBILITY_FRAGMENT: Symbol = Symbol("FRAGMENT"); +const VISIBILITY_COMPUTE: Symbol = Symbol("COMPUTE"); +const VISIBILITY_ALL: Symbol = Symbol("ALL"); +const VISIBILITY_NONE: Symbol = Symbol("NONE"); + +fn get_visibility_flag_value( + nested_metas: &Punctuated, +) -> Result { + let mut visibility = VisibilityFlags::default(); + + for meta in nested_metas { + use syn::{Meta::Path, NestedMeta::Meta}; + match meta { + // Parse `visibility(ALL)]`. + Meta(Path(path)) if path == VISIBILITY_ALL => { + return Ok(ShaderStageVisibility::All) + } + // Parse `visibility(NONE)]`. + Meta(Path(path)) if path == VISIBILITY_NONE => { + return Ok(ShaderStageVisibility::None) + } + // Parse `visibility(VERTEX, ...)]`. + Meta(Path(path)) if path == VISIBILITY_VERTEX => { + visibility.vertex = true; + } + // Parse `visibility(FRAGMENT, ...)]`. + Meta(Path(path)) if path == VISIBILITY_FRAGMENT => { + visibility.fragment = true; + } + // Parse `visibility(COMPUTE, ...)]`. + Meta(Path(path)) if path == VISIBILITY_COMPUTE => { + visibility.compute = true; + } + Meta(Path(path)) => return Err(Error::new_spanned( + path, + "Not a valid visibility flag. Must be `ALL`, `NONE`, `VERTEX`, `FRAGMENT` or `COMPUTE`." + )), + _ => return Err(Error::new_spanned( + meta, + "Invalid visibility format: `visibility(...)`.", + )), + } + } + + Ok(ShaderStageVisibility::Flags(visibility)) +} + #[derive(Default)] enum BindingTextureDimension { D1, @@ -512,10 +582,37 @@ enum BindingTextureSampleType { Uint, } +impl ToTokens for BindingTextureDimension { + fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { + tokens.extend(match self { + BindingTextureDimension::D1 => quote! { TextureViewDimension::D1 }, + BindingTextureDimension::D2 => quote! { TextureViewDimension::D2 }, + BindingTextureDimension::D2Array => quote! { TextureViewDimension::D2Array }, + BindingTextureDimension::Cube => quote! { TextureViewDimension::Cube }, + BindingTextureDimension::CubeArray => quote! { TextureViewDimension::CubeArray }, + BindingTextureDimension::D3 => quote! { TextureViewDimension::D3 }, + }); + } +} + +impl ToTokens for BindingTextureSampleType { + fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { + tokens.extend(match self { + BindingTextureSampleType::Float { filterable } => { + quote! { TextureSampleType::Float { filterable: #filterable } } + } + BindingTextureSampleType::Depth => quote! { TextureSampleType::Depth }, + BindingTextureSampleType::Sint => quote! { TextureSampleType::Sint }, + BindingTextureSampleType::Uint => quote! { TextureSampleType::Uint }, + }); + } +} + struct TextureAttrs { dimension: BindingTextureDimension, sample_type: BindingTextureSampleType, multisampled: bool, + visibility: ShaderStageVisibility, } impl Default for BindingTextureSampleType { @@ -530,6 +627,7 @@ impl Default for TextureAttrs { dimension: Default::default(), sample_type: Default::default(), multisampled: true, + visibility: Default::default(), } } } @@ -560,24 +658,37 @@ fn get_texture_attrs(metas: Vec) -> Result { let mut filterable = None; let mut filterable_ident = None; + let mut visibility = Default::default(); + for meta in metas { - use syn::{Meta::NameValue, NestedMeta::Meta}; + use syn::{ + Meta::{List, NameValue}, + NestedMeta::Meta, + }; match meta { + // Parse #[texture(0, dimension = "...")]. Meta(NameValue(m)) if m.path == DIMENSION => { let value = get_lit_str(DIMENSION, &m.lit)?; dimension = get_texture_dimension_value(value)?; } + // Parse #[texture(0, sample_type = "...")]. Meta(NameValue(m)) if m.path == SAMPLE_TYPE => { let value = get_lit_str(SAMPLE_TYPE, &m.lit)?; sample_type = get_texture_sample_type_value(value)?; } + // Parse #[texture(0, multisampled = "...")]. Meta(NameValue(m)) if m.path == MULTISAMPLED => { multisampled = get_lit_bool(MULTISAMPLED, &m.lit)?; } + // Parse #[texture(0, filterable = "...")]. Meta(NameValue(m)) if m.path == FILTERABLE => { filterable = get_lit_bool(FILTERABLE, &m.lit)?.into(); filterable_ident = m.path.into(); } + // Parse #[texture(0, visibility(...))]. + Meta(List(m)) if m.path == VISIBILITY => { + visibility = get_visibility_flag_value(&m.nested)?; + } Meta(NameValue(m)) => { return Err(Error::new_spanned( m.path, @@ -614,6 +725,7 @@ fn get_texture_attrs(metas: Vec) -> Result { dimension, sample_type, multisampled, + visibility, }) } @@ -650,6 +762,7 @@ fn get_texture_sample_type_value(lit_str: &LitStr) -> Result quote! { SamplerBindingType::Filtering }, + SamplerBindingType::NonFiltering => quote! { SamplerBindingType::NonFiltering }, + SamplerBindingType::Comparison => quote! { SamplerBindingType::Comparison }, + }); + } +} + const SAMPLER_TYPE: Symbol = Symbol("sampler_type"); const FILTERING: &str = "filtering"; @@ -668,14 +791,23 @@ const COMPARISON: &str = "comparison"; fn get_sampler_attrs(metas: Vec) -> Result { let mut sampler_binding_type = Default::default(); + let mut visibility = Default::default(); for meta in metas { - use syn::{Meta::NameValue, NestedMeta::Meta}; + use syn::{ + Meta::{List, NameValue}, + NestedMeta::Meta, + }; match meta { + // Parse #[sampler(0, sampler_type = "..."))]. Meta(NameValue(m)) if m.path == SAMPLER_TYPE => { let value = get_lit_str(DIMENSION, &m.lit)?; sampler_binding_type = get_sampler_binding_type_value(value)?; } + // Parse #[sampler(0, visibility(...))]. + Meta(List(m)) if m.path == VISIBILITY => { + visibility = get_visibility_flag_value(&m.nested)?; + } Meta(NameValue(m)) => { return Err(Error::new_spanned( m.path, @@ -693,6 +825,7 @@ fn get_sampler_attrs(metas: Vec) -> Result { Ok(SamplerAttrs { sampler_binding_type, + visibility, }) } From 042077b2f09022edad9a6429f5c06854f8049593 Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Mon, 18 Jul 2022 18:22:41 -0400 Subject: [PATCH 7/9] fix wrong symbol name for visibility --- crates/bevy_render/macros/src/as_bind_group.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 3546cb23b7ab8..6524b9cf405e8 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -515,7 +515,7 @@ impl ToTokens for ShaderStageVisibility { } } -const VISIBILITY: Symbol = Symbol("VISIBILITY"); +const VISIBILITY: Symbol = Symbol("visibility"); const VISIBILITY_VERTEX: Symbol = Symbol("VERTEX"); const VISIBILITY_FRAGMENT: Symbol = Symbol("FRAGMENT"); const VISIBILITY_COMPUTE: Symbol = Symbol("COMPUTE"); From f65f88db860aeef2d52a9f3fa981b5baf5a53e14 Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Tue, 19 Jul 2022 08:46:09 -0400 Subject: [PATCH 8/9] proper hygiene for visibility flag, lowercased flags --- .../bevy_render/macros/src/as_bind_group.rs | 72 ++++++++++++------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 6524b9cf405e8..7c3c4d0380a6a 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -202,6 +202,9 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { visibility, } = get_texture_attrs(nested_meta_items)?; + let visibility = + visibility.hygenic_quote("e! { #render_path::render_resource }); + binding_impls.push(quote! { #render_path::render_resource::OwnedBindingResource::TextureView({ let handle: Option<&#asset_path::Handle<#render_path::texture::Image>> = (&self.#field_name).into(); @@ -216,7 +219,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { binding_layouts.push(quote! { #render_path::render_resource::BindGroupLayoutEntry { binding: #binding_index, - visibility: #render_path::render_resource::#visibility, + visibility: #visibility, ty: #render_path::render_resource::BindingType::Texture { multisampled: #multisampled, sample_type: #render_path::render_resource::#sample_type, @@ -232,6 +235,9 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { visibility, } = get_sampler_attrs(nested_meta_items)?; + let visibility = + visibility.hygenic_quote("e! { #render_path::render_resource }); + binding_impls.push(quote! { #render_path::render_resource::OwnedBindingResource::Sampler({ let handle: Option<&#asset_path::Handle<#render_path::texture::Image>> = (&self.#field_name).into(); @@ -246,7 +252,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { binding_layouts.push(quote!{ #render_path::render_resource::BindGroupLayoutEntry { binding: #binding_index, - visibility: #render_path::render_resource::#visibility, + visibility: #visibility, ty: #render_path::render_resource::BindingType::Sampler(#render_path::render_resource::#sampler_binding_type), count: None, } @@ -491,68 +497,84 @@ struct VisibilityFlags { compute: bool, } -impl ToTokens for ShaderStageVisibility { - fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { - tokens.extend(match self { - ShaderStageVisibility::All => quote! { ShaderStages::all() }, - ShaderStageVisibility::None => quote! { ShaderStages::NONE }, +impl ShaderStageVisibility { + fn vertex_fragment() -> Self { + Self::Flags(VisibilityFlags::vertex_fragment()) + } +} + +impl VisibilityFlags { + fn vertex_fragment() -> Self { + Self { + vertex: true, + fragment: true, + ..Default::default() + } + } +} + +impl ShaderStageVisibility { + fn hygenic_quote(&self, path: &proc_macro2::TokenStream) -> proc_macro2::TokenStream { + match self { + ShaderStageVisibility::All => quote! { #path::ShaderStages::all() }, + ShaderStageVisibility::None => quote! { #path::ShaderStages::NONE }, ShaderStageVisibility::Flags(flags) => { let mut quoted = Vec::new(); if flags.vertex { - quoted.push(quote! { ShaderStages::VERTEX }); + quoted.push(quote! { #path::ShaderStages::VERTEX }); } if flags.fragment { - quoted.push(quote! { ShaderStages::FRAGMENT }); + quoted.push(quote! { #path::ShaderStages::FRAGMENT }); } if flags.compute { - quoted.push(quote! { ShaderStages::COMPUTE }); + quoted.push(quote! { #path::ShaderStages::COMPUTE }); } quote! { #(#quoted)|* } } - }); + } } } const VISIBILITY: Symbol = Symbol("visibility"); -const VISIBILITY_VERTEX: Symbol = Symbol("VERTEX"); -const VISIBILITY_FRAGMENT: Symbol = Symbol("FRAGMENT"); -const VISIBILITY_COMPUTE: Symbol = Symbol("COMPUTE"); -const VISIBILITY_ALL: Symbol = Symbol("ALL"); -const VISIBILITY_NONE: Symbol = Symbol("NONE"); +const VISIBILITY_VERTEX: Symbol = Symbol("vertex"); +const VISIBILITY_FRAGMENT: Symbol = Symbol("fragment"); +const VISIBILITY_COMPUTE: Symbol = Symbol("compute"); +const VISIBILITY_ALL: Symbol = Symbol("all"); +const VISIBILITY_NONE: Symbol = Symbol("none"); fn get_visibility_flag_value( nested_metas: &Punctuated, ) -> Result { - let mut visibility = VisibilityFlags::default(); + let mut visibility = VisibilityFlags::vertex_fragment(); for meta in nested_metas { use syn::{Meta::Path, NestedMeta::Meta}; match meta { - // Parse `visibility(ALL)]`. + // Parse `visibility(all)]`. Meta(Path(path)) if path == VISIBILITY_ALL => { return Ok(ShaderStageVisibility::All) } - // Parse `visibility(NONE)]`. + // Parse `visibility(none)]`. Meta(Path(path)) if path == VISIBILITY_NONE => { return Ok(ShaderStageVisibility::None) } - // Parse `visibility(VERTEX, ...)]`. + // Parse `visibility(vertex, ...)]`. Meta(Path(path)) if path == VISIBILITY_VERTEX => { visibility.vertex = true; } - // Parse `visibility(FRAGMENT, ...)]`. + // Parse `visibility(fragment, ...)]`. Meta(Path(path)) if path == VISIBILITY_FRAGMENT => { visibility.fragment = true; } - // Parse `visibility(COMPUTE, ...)]`. + // Parse `visibility(compute, ...)]`. Meta(Path(path)) if path == VISIBILITY_COMPUTE => { visibility.compute = true; } Meta(Path(path)) => return Err(Error::new_spanned( path, - "Not a valid visibility flag. Must be `ALL`, `NONE`, `VERTEX`, `FRAGMENT` or `COMPUTE`." + "Not a valid visibility flag. Must be `all`, `none`, or a list-combination of `vertex`, `fragment` and/or `compute`." )), _ => return Err(Error::new_spanned( meta, @@ -658,7 +680,7 @@ fn get_texture_attrs(metas: Vec) -> Result { let mut filterable = None; let mut filterable_ident = None; - let mut visibility = Default::default(); + let mut visibility = ShaderStageVisibility::vertex_fragment(); for meta in metas { use syn::{ @@ -791,7 +813,7 @@ const COMPARISON: &str = "comparison"; fn get_sampler_attrs(metas: Vec) -> Result { let mut sampler_binding_type = Default::default(); - let mut visibility = Default::default(); + let mut visibility = ShaderStageVisibility::vertex_fragment(); for meta in metas { use syn::{ From 2fadaaf1b62bd55a516d24d550a888a0bf95fd8e Mon Sep 17 00:00:00 2001 From: Wrapperup Date: Tue, 19 Jul 2022 17:30:11 -0400 Subject: [PATCH 9/9] don't error on non-ident attribute paths --- crates/bevy_render/macros/src/as_bind_group.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 7c3c4d0380a6a..30888a5227055 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -119,7 +119,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { let attr_ident = if let Some(ident) = attr.path.get_ident() { ident } else { - return Err(Error::new_spanned(attr, "Expected identifier")); + continue; }; let binding_type = if attr_ident == UNIFORM_ATTRIBUTE_NAME {