From 25ffaee23176000364b17e1b5b29f361a764890a Mon Sep 17 00:00:00 2001 From: Nico Burns Date: Sun, 23 Jul 2023 12:44:31 +0100 Subject: [PATCH] Fix rounding accumulation bug (#521) * WIP * When rounding is enabled, also store unrounded layout values. Fixes https://github.com/DioxusLabs/taffy/issues/501 * Remove get_raw method from Cache struct * Update rounding relayout test to actually assert * cargo fmt * Fix doc links * Rename abs_x/abs_y to cumulative_x/cumulative_y * Document when final_layout is rounded vs unrounded --- src/compute/taffy_tree.rs | 38 +++++++++++++------- src/tree/node.rs | 18 ++++++++-- src/tree/taffy_tree/tree.rs | 20 +++++++++-- tests/relayout.rs | 72 +++++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 19 deletions(-) diff --git a/src/compute/taffy_tree.rs b/src/compute/taffy_tree.rs index 3cac30f7e..3d2efae5e 100644 --- a/src/compute/taffy_tree.rs +++ b/src/compute/taffy_tree.rs @@ -39,6 +39,8 @@ pub(crate) fn compute_layout( root: NodeId, available_space: Size, ) -> Result<(), TaffyError> { + taffy.is_layouting = true; + // Recursively compute node layout let size_and_baselines = perform_node_layout( taffy, @@ -58,6 +60,8 @@ pub(crate) fn compute_layout( round_layout(taffy, root, 0.0, 0.0); } + taffy.is_layouting = false; + Ok(()) } @@ -137,6 +141,7 @@ fn compute_node_layout( debug_log_node(known_dimensions, parent_size, available_space, run_mode, sizing_mode); #[cfg(any(feature = "debug", feature = "profile"))] NODE_LOGGER.pop_node(); + return cached_size_and_baselines; } @@ -273,25 +278,32 @@ fn perform_taffy_tree_hidden_layout(tree: &mut Taffy, node: NodeId) { } /// Rounds the calculated [`Layout`] to exact pixel values +/// /// In order to ensure that no gaps in the layout are introduced we: -/// - Always round based on the absolute coordinates rather than parent-relative coordinates +/// - Always round based on the cumulative x/y coordinates (relative to the viewport) rather than +/// parent-relative coordinates /// - Compute width/height by first rounding the top/bottom/left/right and then computing the difference /// rather than rounding the width/height directly -/// /// See for more context -fn round_layout(tree: &mut impl LayoutTree, node: NodeId, abs_x: f32, abs_y: f32) { - let layout = tree.layout_mut(node); - let abs_x = abs_x + layout.location.x; - let abs_y = abs_y + layout.location.y; +/// +/// In order to prevent innacuracies caused by rounding already-rounded values, we read from `unrounded_layout` +/// and write to `final_layout`. +fn round_layout(tree: &mut Taffy, node_id: NodeId, cumulative_x: f32, cumulative_y: f32) { + let node = &mut tree.nodes[node_id.into()]; + let unrounded_layout = node.unrounded_layout; + let layout = &mut node.final_layout; + + let cumulative_x = cumulative_x + unrounded_layout.location.x; + let cumulative_y = cumulative_y + unrounded_layout.location.y; - layout.location.x = round(layout.location.x); - layout.location.y = round(layout.location.y); - layout.size.width = round(abs_x + layout.size.width) - round(abs_x); - layout.size.height = round(abs_y + layout.size.height) - round(abs_y); + layout.location.x = round(unrounded_layout.location.x); + layout.location.y = round(unrounded_layout.location.y); + layout.size.width = round(cumulative_x + unrounded_layout.size.width) - round(cumulative_x); + layout.size.height = round(cumulative_y + unrounded_layout.size.height) - round(cumulative_y); - let child_count = tree.child_count(node); + let child_count = tree.child_count(node_id).unwrap(); for index in 0..child_count { - let child = tree.child(node, index); - round_layout(tree, child, abs_x, abs_y); + let child = tree.child(node_id, index); + round_layout(tree, child, cumulative_x, cumulative_y); } } diff --git a/src/tree/node.rs b/src/tree/node.rs index db1bc84b5..99bf09fd3 100644 --- a/src/tree/node.rs +++ b/src/tree/node.rs @@ -68,8 +68,14 @@ impl From for DefaultKey { pub(crate) struct NodeData { /// The layout strategy used by this node pub(crate) style: Style, - /// The results of the layout computation - pub(crate) layout: Layout, + + /// The always unrounded results of the layout computation. We must store this separately from the rounded + /// layout to avoid errors from rounding already-rounded values. See . + pub(crate) unrounded_layout: Layout, + + /// The final results of the layout computation. + /// These may be rounded or unrounded depending on what the `use_rounding` config setting is set to. + pub(crate) final_layout: Layout, /// Should we try and measure this node? pub(crate) needs_measure: bool, @@ -82,7 +88,13 @@ impl NodeData { /// Create the data for a new node #[must_use] pub const fn new(style: Style) -> Self { - Self { style, cache: Cache::new(), layout: Layout::new(), needs_measure: false } + Self { + style, + cache: Cache::new(), + unrounded_layout: Layout::new(), + final_layout: Layout::new(), + needs_measure: false, + } } /// Marks a node and all of its parents (recursively) as dirty diff --git a/src/tree/taffy_tree/tree.rs b/src/tree/taffy_tree/tree.rs index a409c9e3c..4576987aa 100644 --- a/src/tree/taffy_tree/tree.rs +++ b/src/tree/taffy_tree/tree.rs @@ -44,6 +44,11 @@ pub struct Taffy { /// Layout mode configuration pub(crate) config: TaffyConfig, + + /// Hack to allow the `LayoutTree::layout_mut` function to expose the `NodeData.unrounded_layout` of a node to + /// the layout algorithms during layout, while exposing the `NodeData.final_layout` when called by external users. + /// This allows us to fix without breaking backwards compatibility + pub(crate) is_layouting: bool, } impl Default for Taffy { @@ -82,12 +87,20 @@ impl LayoutTree for Taffy { #[inline(always)] fn layout(&self, node: NodeId) -> &Layout { - &self.nodes[node.into()].layout + if self.is_layouting && self.config.use_rounding { + &self.nodes[node.into()].unrounded_layout + } else { + &self.nodes[node.into()].final_layout + } } #[inline(always)] fn layout_mut(&mut self, node: NodeId) -> &mut Layout { - &mut self.nodes[node.into()].layout + if self.is_layouting && self.config.use_rounding { + &mut self.nodes[node.into()].unrounded_layout + } else { + &mut self.nodes[node.into()].final_layout + } } #[inline(always)] @@ -159,6 +172,7 @@ impl Taffy { parents: SlotMap::with_capacity(capacity), measure_funcs: SparseSecondaryMap::with_capacity(capacity), config: TaffyConfig::default(), + is_layouting: false, } } @@ -394,7 +408,7 @@ impl Taffy { /// Return this node layout relative to its parent pub fn layout(&self, node: NodeId) -> TaffyResult<&Layout> { - Ok(&self.nodes[node.into()].layout) + Ok(&self.nodes[node.into()].final_layout) } /// Marks the layout computation of this node and its children as outdated diff --git a/tests/relayout.rs b/tests/relayout.rs index 4e58245a2..8f5fa9e24 100644 --- a/tests/relayout.rs +++ b/tests/relayout.rs @@ -319,3 +319,75 @@ fn toggle_grid_container_display_none() { assert_eq!(layout.size.width, 0.0); assert_eq!(layout.size.height, 0.0); } + +#[test] +fn relayout_is_stable_with_rounding() { + let mut taffy = Taffy::new(); + taffy.enable_rounding(); + + //
+ //
+ //
+ //
+ //
+ //
+ //
+ + let inner = + taffy.new_leaf(Style { min_size: Size { width: length(300.), height: auto() }, ..Default::default() }).unwrap(); + let wrapper = taffy + .new_with_children( + Style { + size: Size { width: length(150.), height: auto() }, + justify_content: Some(JustifyContent::End), + ..Default::default() + }, + &[inner], + ) + .unwrap(); + let outer = taffy + .new_with_children( + Style { + size: Size { width: percent(1.), height: auto() }, + inset: Rect { left: length(1.5), right: auto(), top: auto(), bottom: auto() }, + ..Default::default() + }, + &[wrapper], + ) + .unwrap(); + let root = taffy + .new_with_children( + Style { size: Size { width: length(1920.), height: length(1080.) }, ..Default::default() }, + &[outer], + ) + .unwrap(); + for _ in 0..5 { + taffy.mark_dirty(root).ok(); + taffy.compute_layout(root, Size::MAX_CONTENT).ok(); + taffy::util::print_tree(&taffy, root); + + let root_layout = taffy.layout(root).unwrap(); + assert_eq!(root_layout.location.x, 0.0); + assert_eq!(root_layout.location.y, 0.0); + assert_eq!(root_layout.size.width, 1920.0); + assert_eq!(root_layout.size.height, 1080.0); + + let outer_layout = taffy.layout(outer).unwrap(); + assert_eq!(outer_layout.location.x, 2.0); + assert_eq!(outer_layout.location.y, 0.0); + assert_eq!(outer_layout.size.width, 1920.0); + assert_eq!(outer_layout.size.height, 1080.0); + + let wrapper_layout = taffy.layout(wrapper).unwrap(); + assert_eq!(wrapper_layout.location.x, 0.0); + assert_eq!(wrapper_layout.location.y, 0.0); + assert_eq!(wrapper_layout.size.width, 150.0); + assert_eq!(wrapper_layout.size.height, 1080.0); + + let inner_layout = taffy.layout(inner).unwrap(); + assert_eq!(inner_layout.location.x, -150.0); + assert_eq!(inner_layout.location.y, 0.0); + assert_eq!(inner_layout.size.width, 301.0); + assert_eq!(inner_layout.size.height, 1080.0); + } +}