Skip to content
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

[Merged by Bors] - Make vertex colors work without textures in bevy_sprite #5685

Closed

Conversation

jwagner
Copy link
Contributor

@jwagner jwagner commented Aug 14, 2022

Objective

This PR changes it possible to use vertex colors without a texture using the bevy_sprite ColorMaterial.

Fixes #5679

Solution

  • Made multiplication of the output color independent of the COLOR_MATERIAL_FLAGS_TEXTURE_BIT bit
  • Extended mesh2d_vertex_color_texture example to show off both vertex colors and tinting

Not sure if extending the existing example was the right call but it seems to be reasonable to me.

I couldn't find any tests for the shaders and I think adding shader testing would be beyond the scope of this PR. So no tests in this PR. 😬

@jwagner jwagner changed the title Sprite independent vertex colors Make vertex colors work without textures in bevy_sprite ColorMaterial Aug 14, 2022
@jwagner jwagner changed the title Make vertex colors work without textures in bevy_sprite ColorMaterial Make vertex colors work without textures in bevy_sprite Aug 14, 2022
@Weibye Weibye added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Aug 14, 2022
@jwagner
Copy link
Contributor Author

jwagner commented Aug 14, 2022

Improved the example a bit. Before I was cloning the entire mesh to use it in two entities, now I'm only cloning the handle.

Copy link
Contributor

@BorisBoutillier BorisBoutillier left a comment

Choose a reason for hiding this comment

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

I think updating the example is a good call.

Comment on lines 28 to 35
var output_color: vec4<f32> = material.color;
#ifdef VERTEX_COLORS
output_color = output_color * in.color;
#endif
if ((material.flags & COLOR_MATERIAL_FLAGS_TEXTURE_BIT) != 0u) {
#ifdef VERTEX_COLORS
output_color = output_color * textureSample(texture, texture_sampler, in.uv) * in.color;
#else
output_color = output_color * textureSample(texture, texture_sampler, in.uv);
#endif
}
return output_color;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should keep the #ifdef and #endif start of line for consistency with all other shader code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, pushed an update.

// Spawn the quad with vertex colors
commands.spawn_bundle(MaterialMesh2dBundle {
mesh: mesh_handle.clone(),
transform: Transform::default()
Copy link
Contributor

Choose a reason for hiding this comment

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

This could use from_translation.
Same for below :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the hint. Changed it.

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 15, 2022
@cart
Copy link
Member

cart commented Aug 16, 2022

bors r+

bors bot pushed a commit that referenced this pull request Aug 16, 2022
# Objective

This PR changes it possible to use vertex colors without a texture using the bevy_sprite ColorMaterial.

Fixes #5679 

## Solution

- Made multiplication of the output color independent of the COLOR_MATERIAL_FLAGS_TEXTURE_BIT bit
- Extended mesh2d_vertex_color_texture example to show off both vertex colors and tinting

Not sure if extending the existing example was the right call but it seems to be reasonable to me.

I couldn't find any tests for the shaders and I think adding shader testing would be beyond the scope of this PR. So no tests in this PR. 😬 

Co-authored-by: Jonas Wagner <jonas@29a.ch>
@bors bors bot changed the title Make vertex colors work without textures in bevy_sprite [Merged by Bors] - Make vertex colors work without textures in bevy_sprite Aug 16, 2022
@bors bors bot closed this Aug 16, 2022
maccesch pushed a commit to Synphonyte/bevy that referenced this pull request Sep 28, 2022
)

# Objective

This PR changes it possible to use vertex colors without a texture using the bevy_sprite ColorMaterial.

Fixes bevyengine#5679 

## Solution

- Made multiplication of the output color independent of the COLOR_MATERIAL_FLAGS_TEXTURE_BIT bit
- Extended mesh2d_vertex_color_texture example to show off both vertex colors and tinting

Not sure if extending the existing example was the right call but it seems to be reasonable to me.

I couldn't find any tests for the shaders and I think adding shader testing would be beyond the scope of this PR. So no tests in this PR. 😬 

Co-authored-by: Jonas Wagner <jonas@29a.ch>
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
)

# Objective

This PR changes it possible to use vertex colors without a texture using the bevy_sprite ColorMaterial.

Fixes bevyengine#5679 

## Solution

- Made multiplication of the output color independent of the COLOR_MATERIAL_FLAGS_TEXTURE_BIT bit
- Extended mesh2d_vertex_color_texture example to show off both vertex colors and tinting

Not sure if extending the existing example was the right call but it seems to be reasonable to me.

I couldn't find any tests for the shaders and I think adding shader testing would be beyond the scope of this PR. So no tests in this PR. 😬 

Co-authored-by: Jonas Wagner <jonas@29a.ch>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
)

# Objective

This PR changes it possible to use vertex colors without a texture using the bevy_sprite ColorMaterial.

Fixes bevyengine#5679 

## Solution

- Made multiplication of the output color independent of the COLOR_MATERIAL_FLAGS_TEXTURE_BIT bit
- Extended mesh2d_vertex_color_texture example to show off both vertex colors and tinting

Not sure if extending the existing example was the right call but it seems to be reasonable to me.

I couldn't find any tests for the shaders and I think adding shader testing would be beyond the scope of this PR. So no tests in this PR. 😬 

Co-authored-by: Jonas Wagner <jonas@29a.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vertex Colors without a texture in 2D ColorMaterial
7 participants