-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Move feathers propagation into PostUpdate #20446
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
Conversation
31d9904 to
5ea2dd7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The propagation systems for TextFont need to run in UiSystems::Propagate so the fonts are up-to-date for measure_text_system and detect_text_needs_rerender in UiSystems::Content.
TextColor it doesn't matter though, the colors are only needed during extraction.
|
Yeah, what @ickshonpe said. Testing: is pretty easy if you look fast. Run the feathers example without this fix - right at the start, you'll see all of the text in a different font, and then one frame later (one long frame - the app is still starting up so things are a bit slow) the text will "pop" into the correct font. Apply the fix, and this problem goes away. The problem is even more evident in the popup menu example - but that only runs in cart's BSN branch. |
|
Awesome, thank you both. I'll get to that tomorrow. |
|
I've applied the fix, and cannot reproduce the bug on this branch. I also couldn't reproduce it on main, but this is definitely a better plan regardless so I'm not too upset about that. |
viridia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, well on main the bug was non-deterministic.
Objective
HierarchyPropagatePluginscheduling configurable #20433 (comment), these probably belong inPostUpdateto ensure that spawned objects inUpdatehave the correct values immediately without the need for ordering.PostUpdate#20356.Solution
UpdatetoPostUpdateTesting