Skip to content

Improve spec_v2 patterns #20114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 13 additions & 109 deletions crates/bevy_render/macros/src/specializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ const SPECIALIZE_ALL_IDENT: &str = "all";
const KEY_ATTR_IDENT: &str = "key";
const KEY_DEFAULT_IDENT: &str = "default";

const BASE_DESCRIPTOR_ATTR_IDENT: &str = "base_descriptor";

enum SpecializeImplTargets {
All,
Specific(Vec<Path>),
Expand Down Expand Up @@ -87,7 +85,6 @@ struct FieldInfo {
ty: Type,
member: Member,
key: Key,
use_base_descriptor: bool,
}

impl FieldInfo {
Expand Down Expand Up @@ -117,15 +114,6 @@ impl FieldInfo {
parse_quote!(#ty: #specialize_path::Specializer<#target_path>)
}
}

fn get_base_descriptor_predicate(
&self,
specialize_path: &Path,
target_path: &Path,
) -> WherePredicate {
let ty = &self.ty;
parse_quote!(#ty: #specialize_path::GetBaseDescriptor<#target_path>)
}
}

fn get_field_info(
Expand All @@ -151,12 +139,8 @@ fn get_field_info(

let mut use_key_field = true;
let mut key = Key::Index(key_index);
let mut use_base_descriptor = false;
for attr in &field.attrs {
match &attr.meta {
Meta::Path(path) if path.is_ident(&BASE_DESCRIPTOR_ATTR_IDENT) => {
use_base_descriptor = true;
}
Meta::List(MetaList { path, tokens, .. }) if path.is_ident(&KEY_ATTR_IDENT) => {
let owned_tokens = tokens.clone().into();
let Ok(parsed_key) = syn::parse::<Key>(owned_tokens) else {
Expand Down Expand Up @@ -190,7 +174,6 @@ fn get_field_info(
ty: field_ty,
member: field_member,
key,
use_base_descriptor,
});
}

Expand Down Expand Up @@ -261,41 +244,18 @@ pub fn impl_specializer(input: TokenStream) -> TokenStream {
})
.collect();

let base_descriptor_fields = field_info
.iter()
.filter(|field| field.use_base_descriptor)
.collect::<Vec<_>>();

if base_descriptor_fields.len() > 1 {
return syn::Error::new(
Span::call_site(),
"Too many #[base_descriptor] attributes found. It must be present on exactly one field",
)
.into_compile_error()
.into();
}

let base_descriptor_field = base_descriptor_fields.first().copied();

match targets {
SpecializeImplTargets::All => {
let specialize_impl = impl_specialize_all(
&specialize_path,
&ecs_path,
&ast,
&field_info,
&key_patterns,
&key_tuple_idents,
);
let get_base_descriptor_impl = base_descriptor_field
.map(|field_info| impl_get_base_descriptor_all(&specialize_path, &ast, field_info))
.unwrap_or_default();
[specialize_impl, get_base_descriptor_impl]
.into_iter()
.collect()
}
SpecializeImplTargets::Specific(targets) => {
let specialize_impls = targets.iter().map(|target| {
SpecializeImplTargets::All => impl_specialize_all(
&specialize_path,
&ecs_path,
&ast,
&field_info,
&key_patterns,
&key_tuple_idents,
),
SpecializeImplTargets::Specific(targets) => targets
.iter()
.map(|target| {
impl_specialize_specific(
&specialize_path,
&ecs_path,
Expand All @@ -305,14 +265,8 @@ pub fn impl_specializer(input: TokenStream) -> TokenStream {
&key_patterns,
&key_tuple_idents,
)
});
let get_base_descriptor_impls = targets.iter().filter_map(|target| {
base_descriptor_field.map(|field_info| {
impl_get_base_descriptor_specific(&specialize_path, &ast, field_info, target)
})
});
specialize_impls.chain(get_base_descriptor_impls).collect()
}
})
.collect(),
}
}

Expand Down Expand Up @@ -406,56 +360,6 @@ fn impl_specialize_specific(
})
}

fn impl_get_base_descriptor_specific(
specialize_path: &Path,
ast: &DeriveInput,
base_descriptor_field_info: &FieldInfo,
target_path: &Path,
) -> TokenStream {
let struct_name = &ast.ident;
let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();
let field_ty = &base_descriptor_field_info.ty;
let field_member = &base_descriptor_field_info.member;
TokenStream::from(quote!(
impl #impl_generics #specialize_path::GetBaseDescriptor<#target_path> for #struct_name #type_generics #where_clause {
fn get_base_descriptor(&self) -> <#target_path as #specialize_path::Specializable>::Descriptor {
<#field_ty as #specialize_path::GetBaseDescriptor<#target_path>>::get_base_descriptor(&self.#field_member)
}
}
))
}

fn impl_get_base_descriptor_all(
specialize_path: &Path,
ast: &DeriveInput,
base_descriptor_field_info: &FieldInfo,
) -> TokenStream {
let target_path = Path::from(format_ident!("T"));
let struct_name = &ast.ident;
let mut generics = ast.generics.clone();
generics.params.insert(
0,
parse_quote!(#target_path: #specialize_path::Specializable),
);

let where_clause = generics.make_where_clause();
where_clause.predicates.push(
base_descriptor_field_info.get_base_descriptor_predicate(specialize_path, &target_path),
);

let (_, type_generics, _) = ast.generics.split_for_impl();
let (impl_generics, _, where_clause) = &generics.split_for_impl();
let field_ty = &base_descriptor_field_info.ty;
let field_member = &base_descriptor_field_info.member;
TokenStream::from(quote! {
impl #impl_generics #specialize_path::GetBaseDescriptor<#target_path> for #struct_name #type_generics #where_clause {
fn get_base_descriptor(&self) -> <#target_path as #specialize_path::Specializable>::Descriptor {
<#field_ty as #specialize_path::GetBaseDescriptor<#target_path>>::get_base_descriptor(&self.#field_member)
}
}
})
}

pub fn impl_specializer_key(input: TokenStream) -> TokenStream {
let bevy_render_path: Path = crate::bevy_render_path();
let specialize_path = {
Expand Down
101 changes: 1 addition & 100 deletions crates/bevy_render/src/render_resource/specializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ use super::{
CachedComputePipelineId, CachedRenderPipelineId, ComputePipeline, ComputePipelineDescriptor,
PipelineCache, RenderPipeline, RenderPipelineDescriptor,
};
use bevy_ecs::{
error::BevyError,
resource::Resource,
world::{FromWorld, World},
};
use bevy_ecs::error::BevyError;
use bevy_platform::{
collections::{
hash_map::{Entry, VacantEntry},
Expand Down Expand Up @@ -260,91 +256,12 @@ macro_rules! impl_specialization_key_tuple {
// TODO: How to we fake_variadics this?
all_tuples!(impl_specialization_key_tuple, 0, 12, T);

/// Defines a specializer that can also provide a "base descriptor".
///
/// In order to be composable, [`Specializer`] implementers don't create full
/// descriptors, only transform them. However, [`SpecializedCache`]s need a
/// "base descriptor" at creation time in order to have something for the
/// [`Specializer`] to work off of. This trait allows [`SpecializedCache`]
/// to impl [`FromWorld`] for [`Specializer`]s that also satisfy [`FromWorld`]
/// and [`GetBaseDescriptor`].
///
/// This trait can be also derived with `#[derive(Specializer)]`, by marking
/// a field with `#[base_descriptor]` to use its [`GetBaseDescriptor`] implementation.
///
/// Example:
/// ```rust
/// # use bevy_ecs::error::BevyError;
/// # use bevy_render::render_resource::Specializer;
/// # use bevy_render::render_resource::GetBaseDescriptor;
/// # use bevy_render::render_resource::SpecializerKey;
/// # use bevy_render::render_resource::RenderPipeline;
/// # use bevy_render::render_resource::RenderPipelineDescriptor;
/// struct A;
/// struct B;
///
/// impl Specializer<RenderPipeline> for A {
/// # type Key = ();
/// #
/// # fn specialize(
/// # &self,
/// # key: (),
/// # _descriptor: &mut RenderPipelineDescriptor
/// # ) -> Result<(), BevyError> {
/// # Ok(key)
/// # }
/// // ...
/// }
///
/// impl Specializer<RenderPipeline> for B {
/// # type Key = ();
/// #
/// # fn specialize(
/// # &self,
/// # key: (),
/// # _descriptor: &mut RenderPipelineDescriptor
/// # ) -> Result<(), BevyError> {
/// # Ok(key)
/// # }
/// // ...
/// }
///
/// impl GetBaseDescriptor<RenderPipeline> for B {
/// fn get_base_descriptor(&self) -> RenderPipelineDescriptor {
/// # todo!()
/// // ...
/// }
/// }
///
///
/// #[derive(Specializer)]
/// #[specialize(RenderPipeline)]
/// struct C {
/// a: A,
/// #[base_descriptor]
/// b: B,
/// }
///
/// /*
/// The generated implementation:
/// impl GetBaseDescriptor for C {
/// fn get_base_descriptor(&self) -> RenderPipelineDescriptor {
/// self.b.base_descriptor()
/// }
/// }
/// */
/// ```
pub trait GetBaseDescriptor<T: Specializable>: Specializer<T> {
fn get_base_descriptor(&self) -> T::Descriptor;
}

pub type SpecializerFn<T, S> =
fn(<S as Specializer<T>>::Key, &mut <T as Specializable>::Descriptor) -> Result<(), BevyError>;

/// A cache for specializable resources. For a given key type the resulting
/// resource will only be created if it is missing, retrieving it from the
/// cache otherwise.
#[derive(Resource)]
pub struct SpecializedCache<T: Specializable, S: Specializer<T>> {
specializer: S,
user_specializer: Option<SpecializerFn<T, S>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the thought on how this should work now? Should we copy the specialization fn pointer into whatever specializer component we create?

Copy link
Contributor Author

@ecoskey ecoskey Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a simple wrapper too:

struct WithUserSpecializer<T: Specializable, S: Specializer<T>> {
  specializer: S,
  user_specializer: SpecializerFn<T, S>
}

I just think it's a bit niche to put in the collection itself. Also (marginally) easier to iterate on if we want to turn user_specializer into a Box<dyn DynSpecialize> or something later

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine to pull it out here and figure out what the requirements actually are when I tackle bevy_material in 0.18.

Expand Down Expand Up @@ -447,19 +364,3 @@ impl<T: Specializable, S: Specializer<T>> SpecializedCache<T, S> {
Ok(id)
}
}

/// [`SpecializedCache`] implements [`FromWorld`] for [`Specializer`]s
/// that also satisfy [`FromWorld`] and [`GetBaseDescriptor`]. This will
/// create a [`SpecializedCache`] with no user specializer, and the base
/// descriptor take from the specializer's [`GetBaseDescriptor`] implementation.
impl<T, S> FromWorld for SpecializedCache<T, S>
where
T: Specializable,
S: FromWorld + Specializer<T> + GetBaseDescriptor<T>,
{
fn from_world(world: &mut World) -> Self {
let specializer = S::from_world(world);
let base_descriptor = specializer.get_base_descriptor();
Self::new(specializer, None, base_descriptor)
}
}
Loading
Loading