From 00943f4f08e62a745952e5ad9fa717aa24de840c Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Sun, 9 Jul 2023 08:33:22 +0100 Subject: [PATCH] Growing UI nodes Fix (#8931) # Objective fixes #8911, #7712 ## Solution Rounding was added to Taffy which fixed issue #7712. The implementation uses the f32 `round` method which rounds ties (fractional part is a half) away from zero. Issue #8911 occurs when a node's min and max bounds on either axis are "ties" and zero is between them. Then the bounds are rounded away from each other, and the node grows by a pixel. This alone shouldn't cause the node to expand continuously, but I think there is some interaction with the way Taffy recomputes a layout from its cached data that I didn't identify. This PR fixes #8911 by first disabling Taffy's internal rounding and using an alternative rounding function that rounds ties up. Then, instead of rounding the values of the internal layout tree as Taffy's built-in rounding does, we leave those values unmodified and only the values stored in the components are rounded. This requires walking the tree for the UI node geometry update rather than iterating through a query. Because the component values are regenerated each update, that should mean that UI updates are idempotent (ish) now and make the growing node behaviour seen in issue #8911 impossible. I expected a performance regression, but it's an improvement on main: ``` cargo run --profile stress-test --features trace_tracy --example many_buttons ``` ui-rounding-fix-compare I guess it makes sense to do the rounding together with the node size and position updates. --- ## Changelog `bevy_ui::layout`: * Taffy's built-in rounding is disabled and rounding is now performed by `ui_layout_system`. * Instead of rounding the values of the internal layout tree as Taffy's built-in rounding does, we leave those values unmodified and only the values stored in the components are rounded. This requires walking the tree for the UI node geometry update rather than iterating through a query. Because the component values are regenerated each update, that should mean that UI updates are idempotent now and make the growing node behaviour seen in issue #8911 impossible. * Added two helper functions `round_ties_up` and `round_layout_coordinates`. --------- Co-authored-by: Carter Anderson --- crates/bevy_ui/src/layout/mod.rs | 112 +++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 27 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index cc4cc9301db91..8063eaac76a2a 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -64,10 +64,12 @@ impl fmt::Debug for UiSurface { impl Default for UiSurface { fn default() -> Self { + let mut taffy = Taffy::new(); + taffy.disable_rounding(); Self { entity_to_taffy: Default::default(), window_nodes: Default::default(), - taffy: Taffy::new(), + taffy, } } } @@ -223,9 +225,10 @@ pub fn ui_layout_system( style_query: Query<(Entity, Ref