Fix rounding accumulation bug (#521)

(cherry picked from commit 25ffaee)

src/compute/mod.rs

@@ -26,6 +26,8 @@ use crate::debug::NODE_LOGGER;
 
 /// Updates the stored layout of the provided `node` and its children
 pub fn compute_layout(taffy: &mut Taffy, root: Node, available_space: Size<AvailableSpace>) -> Result<(), TaffyError> {
+ taffy.is_layouting = true;
+
  // Recursively compute node layout
  let size_and_baselines = GenericAlgorithm::perform_layout(
  taffy,
@@ -44,6 +46,8 @@ pub fn compute_layout(taffy: &mut Taffy, root: Node, available_space: Size<Avail
  round_layout(taffy, root, 0.0, 0.0);
  }
 
+ taffy.is_layouting = false;
+
  Ok(())
 }
 
@@ -385,26 +389,33 @@ fn perform_hidden_layout(tree: &mut impl LayoutTree, node: Node) {
 }
 
 /// 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 <https://github.com/facebook/yoga/commit/aa5b296ac78f7a22e1aeaf4891243c6bb76488e2> for more context
-fn round_layout(tree: &mut impl LayoutTree, node: Node, 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;
-
- 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);
-
- let child_count = tree.child_count(node);
+///
+/// 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: Node, cumulative_x: f32, cumulative_y: f32) {
+ let node = &mut tree.nodes[node_id];
+ 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(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_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);
  }
 }
 

src/data.rs

@@ -14,8 +14,14 @@ pub(crate) const CACHE_SIZE: usize = 7;
 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 <https://github.com/DioxusLabs/taffy/issues/501>.
+ 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,
@@ -28,7 +34,13 @@ impl NodeData {
  /// Create the data for a new node
  #[must_use]
  pub const fn new(style: Style) -> Self {
- Self { style, size_cache: [None; CACHE_SIZE], layout: Layout::new(), needs_measure: false }
+ Self {
+ style,
+ size_cache: [None; CACHE_SIZE],
+ unrounded_layout: Layout::new(),
+ final_layout: Layout::new(),
+ needs_measure: false,
+ }
  }
 
  /// Marks a node and all of its parents (recursively) as dirty

src/node.rs

@@ -64,6 +64,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 <https://github.com/DioxusLabs/taffy/issues/501> without breaking backwards compatibility
+ pub(crate) is_layouting: bool,
 }
 
 impl Default for Taffy {
@@ -95,12 +100,22 @@ impl LayoutTree for Taffy {
  &self.nodes[node].style
  }
 
+ #[inline(always)]
  fn layout(&self, node: Node) -> &Layout {
- &self.nodes[node].layout
+ if self.is_layouting && self.config.use_rounding {
+ &self.nodes[node].unrounded_layout
+ } else {
+ &self.nodes[node].final_layout
+ }
  }
 
+ #[inline(always)]
  fn layout_mut(&mut self, node: Node) -> &mut Layout {
- &mut self.nodes[node].layout
+ if self.is_layouting && self.config.use_rounding {
+ &mut self.nodes[node].unrounded_layout
+ } else {
+ &mut self.nodes[node].final_layout
+ }
  }
 
  #[inline(always)]
@@ -156,6 +171,7 @@ impl Taffy {
  parents: SlotMap::with_capacity(capacity),
  measure_funcs: SparseSecondaryMap::with_capacity(capacity),
  config: TaffyConfig::default(),
+ is_layouting: false,
  }
  }
 
@@ -352,7 +368,7 @@ impl Taffy {
 
  /// Return this node layout relative to its parent
  pub fn layout(&self, node: Node) -> TaffyResult<&Layout> {
- Ok(&self.nodes[node].layout)
+ Ok(&self.nodes[node].final_layout)
  }
 
  /// Marks the layout computation of this node and its children as outdated

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();
+
+ // <div style="width: 1920px; height: 1080px">
+ // <div style="width: 100%; left: 1.5px">
+ // <div style="width: 150px; justify-content: end">
+ // <div style="min-width: 300px" />
+ // </div>
+ // </div>
+ // </div>
+
+ let inner =
+ taffy.new_leaf(Style { min_size: Size { width: points(300.), height: auto() }, ..Default::default() }).unwrap();
+ let wrapper = taffy
+ .new_with_children(
+ Style {
+ size: Size { width: points(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: points(1.5), right: auto(), top: auto(), bottom: auto() },
+ ..Default::default()
+ },
+ &[wrapper],
+ )
+ .unwrap();
+ let root = taffy
+ .new_with_children(
+ Style { size: Size { width: points(1920.), height: points(1080.) }, ..Default::default() },
+ &[outer],
+ )
+ .unwrap();
+ for _ in 0..5 {
+ taffy.mark_dirty(root).ok();
+ taffy.compute_layout(root, Size::MAX_CONTENT).ok();
+ taffy::debug::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);
+ }
+}