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

text: Set autosize bounds lazily #18870

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kjarosh
Copy link
Member

@kjarosh kjarosh commented Dec 5, 2024

Auto size does not set the bounds immediately, but lazily after some selected operations, independently of relayout.

This patch removes the requested_width/height and replaces it with proper lazily set autosize bounds.

Some tests are added which prove that relayout and lazy autosize bounds are independent of each other, and prove when autosize bounds are lazily updated.

The pixel bounds returned by Transform behave sometimes
differently compared to regular bounds.
This simplifies setting bounds, as it doesn't require a gc context.
This is especially helpful when setting the bounds lazily after autosize.
Auto size does not set the bounds immediately, but lazily after
some selected operations, independently of relayout.

This patch removes the `requested_width/height` and replaces it
with proper lazily set autosize bounds.
This test verifies when lazy autosize bounds are applied taking into
consideration various interactions with other objects and classes.
This test verifies lazy autozise bounds behavior
in relation to various events.
@kjarosh kjarosh added text Issues relating to text rendering/input A-core Area: Core player, where no other category fits T-fix Type: Bug fix (in something that's supposed to work already) labels Dec 5, 2024
@adrian17
Copy link
Collaborator

adrian17 commented Dec 9, 2024

Can you also please test #11632 on this?

@kjarosh
Copy link
Member Author

kjarosh commented Dec 9, 2024

Can you also please test #11632 on this?

It seems that the text boxes there already work on master (probably due to #18494, but just guessing)

@@ -1,12 +1,12 @@
150
103
Copy link
Collaborator

Choose a reason for hiding this comment

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

was the old test output created wrongly?
It the new one equal to FP?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's correct. Somehow the output.txt wasn't from FP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core player, where no other category fits T-fix Type: Bug fix (in something that's supposed to work already) text Issues relating to text rendering/input
Projects
None yet
2 participants