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

apply the inverse scale factor using the transform in UI text extraction #9457

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Aug 16, 2023

Objective

bevy_ui::extract_text_uinodes multiplies the position and atlas coordinates of each glyph by the inverse scale factor. It would be more efficient to apply the scaling directly to the text node's transform.

Solution

Scale the transform by the inverse scale factor instead.

cargo run --profile stress-test --features trace_tracy --example many_glyphs
text-transform-improvement

Changelog

bevy_ui::extract_text_uinodes:
The text node's transform is scaled by the inverse scale factor, instead of multiplying the positions and atlas coordinates of each individual glyph.

Scale the transform by the inverse scale factor,
Instead of applying it to the position and atlas coordinates of each individual glyph.
@ickshonpe ickshonpe changed the title text extraction performance improvement apply the inverse scale factor using the transform in UI text extraction Aug 16, 2023
@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times A-UI Graphical user interfaces, styles, layouts, and widgets labels Aug 16, 2023
@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Aug 16, 2023
@rparrett
Copy link
Contributor

Is this effectively reverting #8197? It seems like it reintroduces the bug that was fixing.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Aug 16, 2023

Oh you are right, this doesn't work the text is still distorted when clipped. I must have been on the wrong branch or something when I tested it earlier, I was thinking that the other changes since #8197 must have fixed the clipping.

Is this effectively reverting #8197? It seems like it reintroduces the bug that was fixing.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Aug 16, 2023

Avoids any problems now by only applying the scaling to the transform when text is unclipped.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@ickshonpe
Copy link
Contributor Author

An alternative is that we could scale the texture rects and atlas size stored in the texture atlas. It would simplify extraction but seems like a hack though.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@alice-i-cecile
Copy link
Member

Avoids any problems now by only applying the scaling to the transform when text is unclipped.

Can you run a new set of benchmarks quick? It sounds like we've introduced a branch since that last one.

@alice-i-cecile alice-i-cecile removed this from the 0.12 milestone Sep 30, 2023
@TimJentzsch TimJentzsch added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Performance A change motivated by improving speed, memory usage or compile times S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants