-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
bevy_pbr2: added vertex color support #3187
Conversation
With this PR, colors could also be enabled in the gltf2 loader bevy/pipelined/bevy_gltf2/src/loader.rs Lines 152 to 157 in 0bf90bb
|
See also #3120 |
I'm very new to Bevy and hadn't seen your PR yet. Would make it definitely cleaner to add in the color support! Once that PR is merged, I'll update the code here. |
}; | ||
|
||
[[stage(fragment)]] | ||
fn fragment(in: FragmentInput) -> [[location(0)]] vec4<f32> { | ||
var output_color: vec4<f32> = material.base_color; | ||
|
||
#ifdef VERTEX_COLORS | ||
output_color = in.world_color; |
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.
Doesn't it make more sense for the vertex color to be multiplied with the base color from the material, rather than overriding it?
It seems a lot more useful to me (though I admit I don't know what's the typical/standard behavior for this case / what other game engines do).
That way, the shader combines the color from the texture (if any), the color from the material (if any), and the color from the vertex (if any). This gives the most flexibility, as the user could use all of these variables to create cool graphics effects. It seems strictly more useful than what you are doing right now.
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.
Yep totally makes sense. I've actually moved on to another method and personally no longer use the color vertices. I set the material base color to white and then create a custom texture with the colors I need. Instead of using vertex colors I then simply use correct uv coords for the texture. Once that other pr is merged, I'll update this PR and change it to multiply instead. Great suggestion
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.
Which other PR?
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 think he’s referring to #3120
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 thought so but didn't see any direct reference. :) I agree it makes sense to get the flexible vertex attributes merged first and then this. I have added both of these to the rendering project board so they get prioritised as these are common problems that people hit.
Now that #3959 is merged, I think this can be reworked on top of that. It should hopefully be simpler to do too! :) |
Closing as vertex colours are now supported via a different PR. |
Objective
There's already an attribute (
Mesh::ATTRIBUTE_COLOR
) to set custom vertex colors, but they are not used in the pbr2 pipeline.Solution
This PR adds support for vertex colors (
Mesh::ATTRIBUTE_COLOR
) in the pbr2 pipeline by initially setting the output_color to the custom vertex color, instead of the base color of the material.I've also added an example based on the custom_attributre example, that renders a cube with custom vertex colors using the pbr2 pipeline.