Improved text batching #14848
Improved text batching #14848
Conversation
…` and `ExtractedGlpyh` to hold the extracted geometry.
`extract_uinode_text` extracts glyphs grouped into sections.
ickshonpe
added
A-UI
alice-i-cecile
added
M-Needs-Release-Note
| section_index, | ||
| .. | ||
| }, | ||
| ) in text_layout_info.glyphs.iter().enumerate() |
There was a problem hiding this comment.
nit: this is a bit hard to read, moving the PositionedGlyph destructuring into the for loop body could make it easier
There was a problem hiding this comment.
I don't know. I prefer the more concise deconstruction,it uses less vertical space and the inner code block isn't that large . It's not that important though if consensus favours the other way I don't mind changing it.
TrialDragon
added
the
D-Modest
Made `atlas_scaling` non-optional for text sections.
| let size = glyph.rect.size(); | ||
|
|
||
| let rect_size = glyph_rect.size().extend(1.0); | ||
|
|
||
| // Specify the corners of the glyph | ||
| let positions = QUAD_VERTEX_POSITIONS.map(|pos| { | ||
| (glyph.transform * (pos * rect_size).extend(1.)).xyz() | ||
| }); | ||
|
|
||
| let positions_diff = if let Some(clip) = extracted_uinode.clip { | ||
| [ | ||
| Vec2::new( | ||
| f32::max(clip.min.x - positions[0].x, 0.), | ||
| f32::max(clip.min.y - positions[0].y, 0.), | ||
| ), | ||
| Vec2::new( | ||
| f32::min(clip.max.x - positions[1].x, 0.), | ||
| f32::max(clip.min.y - positions[1].y, 0.), | ||
| ), | ||
| Vec2::new( | ||
| f32::min(clip.max.x - positions[2].x, 0.), | ||
| f32::min(clip.max.y - positions[2].y, 0.), | ||
| ), | ||
| Vec2::new( | ||
| f32::max(clip.min.x - positions[3].x, 0.), | ||
| f32::min(clip.max.y - positions[3].y, 0.), | ||
| ), | ||
| ] | ||
| } else { | ||
| [Vec2::ZERO; 4] | ||
| }; | ||
|
|
||
| let positions_clipped = [ | ||
| positions[0] + positions_diff[0].extend(0.), | ||
| positions[1] + positions_diff[1].extend(0.), | ||
| positions[2] + positions_diff[2].extend(0.), | ||
| positions[3] + positions_diff[3].extend(0.), | ||
| ]; | ||
|
|
||
| // cull nodes that are completely clipped | ||
| let transformed_rect_size = | ||
| glyph.transform.transform_vector3(rect_size); | ||
| if positions_diff[0].x - positions_diff[1].x | ||
| >= transformed_rect_size.x | ||
| || positions_diff[1].y - positions_diff[2].y | ||
| >= transformed_rect_size.y | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| let uvs = [ | ||
| Vec2::new( | ||
| glyph.rect.min.x + positions_diff[0].x, | ||
| glyph.rect.min.y + positions_diff[0].y, | ||
| ), | ||
| Vec2::new( | ||
| glyph.rect.max.x + positions_diff[1].x, | ||
| glyph.rect.min.y + positions_diff[1].y, | ||
| ), | ||
| Vec2::new( | ||
| glyph.rect.max.x + positions_diff[2].x, | ||
| glyph.rect.max.y + positions_diff[2].y, | ||
| ), | ||
| Vec2::new( | ||
| glyph.rect.min.x + positions_diff[3].x, | ||
| glyph.rect.max.y + positions_diff[3].y, | ||
| ), | ||
| ] | ||
| .map(|pos| pos / atlas_extent); |
There was a problem hiding this comment.
Some of these calculations are duplicated in the Node / Glyph cases. It's not a big issue but maybe they could be moved to a small helper function or closure? This would make the code less verbose and ensure that they are equivalent if they change later.
There was a problem hiding this comment.
This is definitely ugly I agree, I don't want to tackle it here though as it's very fragile and there are some bugs with clipping that need to be fixed.
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
alice-i-cecile
added
S-Ready-For-Final-Review
|
@ickshonpe I'm comfortable merging this: let me know when CI is green and merge conflicts are resolved? |
|
Running into some conflicts with the retained render world changes I think, not sure where the problem is. |
Okay panic over, somehow I managed to accidently replace all the continues in the prepare function with returns 😂, should be ready to merge now |
Objective
The UI text rendering is really slow because it extracts each glyph as a separate ui node even though all the glyphs in a text section have the same texture, color and clipping rects.
Solution
Store the glyphs in a seperate contiguous array, queue one transparent ui item per text section which has indices into the glyph array.
Testing
cargo run --example many_glyphs --releaseRuns at about 22fps on main and 95fps with this PR on my computer.
I'll do some proper comparisons once I work out why tracy 11 is refusing to run.