Smaller extracted sprite struct #3556
Conversation
Reduces struct size from 176 to 160.
Reduces struct size from 160 to 144. Moves expensive matrix computation from Extract stage to Prepare stage.
inodentry
added
A-Rendering
| @@ -487,6 +487,15 @@ impl Color { | |||
| } => [hue, saturation, lightness, alpha], | |||
| } | |||
| } | |||
|
|
|||
| pub fn as_linear_abgr_u32(self) -> u32 { | |||
There was a problem hiding this comment.
Is this going to cause a loss of fidelity in our colors due to discretization?
There was a problem hiding this comment.
No change from before. We have already been doing this. It was just done in prepare_sprites. The color passed to the shader was already encoded like this.
I just moved that computation earlier to the Extract stage, so that the color can be stored the same way in the ExtractedSprite struct, too, making it smaller.
| * Mat4::from_translation( | ||
| alignment_offset * scale_factor + text_glyph.position.extend(0.), | ||
| ); | ||
| let mut text_transform = transform.clone(); |
There was a problem hiding this comment.
Does this change break text rotation? The information no longer seems to exist.
There was a problem hiding this comment.
No... it's cloning the original transform, which preserves everything.
Also, I ran the example (after the commit fixing it), and it works. ;)
| @@ -487,6 +487,15 @@ impl Color { | |||
| } => [hue, saturation, lightness, alpha], | |||
| } | |||
| } | |||
|
|
|||
| pub fn as_linear_abgr_u32(self) -> u32 { | |||
There was a problem hiding this comment.
Only if as_linear_rgba_f32 and friends, are also made const fn. Not my call to make.
It should probably have an inline annotation, though. Added that.
| @@ -337,7 +338,7 @@ pub fn prepare_sprites( | |||
| let mut current_batch_colored = false; | |||
| let mut last_z = 0.0; | |||
| for extracted_sprite in extracted_sprites.sprites.iter() { | |||
| let colored = extracted_sprite.color != Color::WHITE; | |||
| let colored = extracted_sprite.color != 0xFFFFFFFF; | |||
There was a problem hiding this comment.
Losing the nice name here is unfortunate. Ideally we could store at least this value as a named constant (although it is very recognizable).
There was a problem hiding this comment.
I could add a line just before:
const WHITE: u32 = 0xFFFFFFFF;But I don't think that would add anything to help clarity 😆 the constant is pretty obvious as is.
On my laptop, if I remove the improvements from #3554, this PR makes |
|
OK, on further investigation, my compositor was obscuring the results and I wasn't seeing the performance changes very well. I just tested it in Windows, and this paints a different picture. This PR does indeed actually make things slower. I am very confused. I also tried just the first change with the color encoding, which is quite trivial, and it also reduced perf. The struct is smaller, the CPU computations should be the same. ... What/Why? I guess I will close this PR. |
Optimize the
ExtractedSpritestruct to have smaller size.With a large number of sprites (like in bevymark/many_sprites) this saves memory and slightly improves perf.
The struct size has been reduced from 176 bytes to 144 bytes.
This is accomplished by:
u32instead ofColor, in the format that is later passed to the shader. The (cheap) conversion is moved to the Extract stage.GlobalTransforminstead ofMat4. This moves the expensive matrix computation into the Prepare stage, instead of the Extract stage.Due to touching the sorting function in
prepare_sprites, this PR will have a merge conflict with #3554. One of us will have to rebase, depending on who makes it in first. I usedunstable_sortandpartial_cmphere, as that PR does (we already know it makes an improvement).