-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat: vertex attribute validation for Material #6334
base: main
Are you sure you want to change the base?
feat: vertex attribute validation for Material #6334
Conversation
@dataphract Cool idea! Do you think one of the examples (maybe |
Definitely a good idea, just want to get consensus on how the interface should look before updating examples. Those could be in one or more follow-up PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the overall idea. Left a couple of high-level comments and marking request changes for now.
This adds a new trait method, `Material::required_vertex_attributes`, which allows implementors to specify a list of vertex attributes that are required for the `Material` to function. A new system has been added to `MaterialPlugin` which emits an error message when an entity is found to have a `Mesh` that lacks required vertex attributes for its `Material`.
c9f0598
to
165d551
Compare
This ended up being a surprisingly difficult system to write due to the distinction @superdump pointed out -- it needs to detect changes to the mesh assets themselves and not just the handles. This requires a lot of bookkeeping (did this entity's mesh change? did it change its mesh? has this mesh already been validated?) but ultimately it still runs linearly in the number of entities and doesn't require new allocations on every invocation. The new error message looks like this:
|
Objective
Fixes #6128. (Original suggestion from #6127 (review)).
Solution
This PR adds a new trait method,
Material::required_vertex_attributes
, which allows implementors to specify a list of vertex attributes that are required for theMaterial
to function.A new system has been added to
MaterialPlugin
which emits an error message when an entity is found to have aMesh
that lacks required vertex attributes for itsMaterial
.Changelog
Added
Material::required_vertex_attributes
trait method, allowing implementors to specify the vertex attributes required for aMaterial
.verify_material_mesh_attributes()
system toMaterialPlugin
. Errors are now emitted if aMaterial
is applied to aMesh
missing required attributes.name()
andid()
methods onMeshVertexAttribute
to retrieve the name and unique ID.