Conversation
TODO: migration guide
|
moved the blocking items to this pr, so it should be merged before the others actually |
tychedelia
left a comment
There was a problem hiding this comment.
This approach seems like a nice cleanup, just a few questions.
| } | ||
| } | ||
|
|
||
| static EMPTY_BIND_GROUP_LAYOUT: OnceLock<BindGroupLayout> = OnceLock::new(); |
There was a problem hiding this comment.
Hmm, so we now use an empty layout in a few places post wgpu 25, but I'm a little wary about this. I guess the set_at api is nicer than just doing a push like we do in some places, but I'm not sure I understand why it should ever be necessary to implicitly fill in the holes in a layout?
There was a problem hiding this comment.
why it should ever be necessary to implicitly fill in the holes in a layout?
Only with the assumption that it'll be properly filled in by other logic, like by composing specializers.
There's probably a better solution than filling_set_at, but since specializers can't really rely on their input having a certain shape (like what the current length of the vec is) both push and direct indexing cause problems without bounds checks or padding.
| #[derive(Resource)] | ||
| pub struct SpecializedCache<T: Specializable, S: Specializer<T>> { | ||
| specializer: S, | ||
| user_specializer: Option<SpecializerFn<T, S>>, |
There was a problem hiding this comment.
What's the thought on how this should work now? Should we copy the specialization fn pointer into whatever specializer component we create?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I'm fine to pull it out here and figure out what the requirements actually are when I tackle bevy_material in 0.18.
|
github is bad at tracking renames, new pr is here: #20348 |
Objective
set_targetandset_layoutwill be the most used I think.pushing to each vec, the methods pad the length of the vec if necessary and set the value directly.Specializers,GetBaseDescriptor&SpecializedCache: Resourceboth seem like anti-patterns, especially with dynamic materials on the horizonuser_specializers. If anyone needs that functionality they can easily make a wrapper for it.Solution