[Merged by Bors] - Sprites - keep color as 4 f32

[mockersf]

Objective

• Fix ClearColor and Sprite same value yields different color #4356

Solution

• Do not reduce the color of sprites to 4 u8

[alice-i-cecile]

I think this is a better solution than rounding. Color consistency and accuracy is better than the very small performance win.

[cart]

Yeah I think this is the move. I'll hold off on merging just to give some space for dissenting opinions.

[alice-i-cecile]

Ping @bevyengine/rendering-team for quick opinions on this :)

[bytemuck]

This is the way to go IMO.

[rparrett]

Should mesh2d_manual get the same treatment?

If so, are as_rgba_u32 and as_linear_rgba_u32 still useful to keep around?

[HackerFoo]

This would probably be a good place to use FP16 in the future: gpuweb/gpuweb#658

[mockersf]

Should mesh2d_manual get the same treatment?

This examples needs to use a u32 as the attribute for mesh color is marked as Uint32:

bevy/crates/bevy_render/src/mesh/mesh/mod.rs

Lines 74 to 76 in 63fee25

	/// Per vertex coloring. Use in conjunction with [`Mesh::insert_attribute`]
	pub const ATTRIBUTE_COLOR: MeshVertexAttribute =
	MeshVertexAttribute::new("Vertex_Color", 4, VertexFormat::Uint32);

It could change there too, but I don't know if there was another reason there for it to be u32

[CptPotato]

I prefer this fix over #4357 👍

An alternative would be passing the color using 4 x u8 (as before) but encoded using SRGB color and then doing the transform back to linear in the shader. But I think f32/f16 is the better option as this also works with higher bit depths and unclipped colors, which allows brightening sprites, HDR, etc..

Regarding the vertex color, it makes sense to me to keep them compressed as u32 in my opinion, since switching to 4 x f32 would increase the memory required per vertex quite a bit.
I haven't tested the vertex colors, but I think they probably have the same precision issue. Maybe doing the SRGB encoding I mentioned above makes sense for the vertex color in the future?

[inodentry]

I was originally one of the proponents of this idea to "compress" the color into a u32. In retrospect I think I am now convinced that it was over-optimizing, trading off correctness and support for some valid use cases, for a drop of performance. I agree with this PR that we should not do that anymore. :)

The current sprites renderer was designed with WebGL2 limitations in mind. If we really want performance gains for sprites rendering, we should look into having a fast path (using storage buffers and vertex pulling, or other such render techniqes) for platforms that aren't so limited, and leave the current implementation as a fallback for platforms that don't support it. That would be much more beneficial, than trying to "micro-optimize" the individual struct fields.

[cart]

Sounds like we have consensus!

[cart]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-android
• build (nightly, ubuntu-latest)
• build (stable, macos-latest)
• build (stable, ubuntu-latest)
• build (stable, windows-latest)
• build-wasm (nightly, ubuntu-latest)
• build-wasm (stable, ubuntu-latest)
• check-benches
• check-doc
• check-markdown-links
• check-missing-examples-in-docs
• check-unused-dependencies
• ci
• markdownlint
• miri
• run-examples