Fix rounding accumulation bug (#521)
(cherry picked from commit 25ffaee23176000364b17e1b5b29f361a764890a)
b2a38dafcff66e44a3146eb7a33d192015f626ac · by Nico Burns · (3 years ago) · parent cd0c4403a · browse the tree at this commit
diff --git a/src/compute/mod.rs b/src/compute/mod.rs
index 6fd974350..48ad57ea1 100644
--- a/src/compute/mod.rs
+++ b/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);
}
}
diff --git a/src/data.rs b/src/data.rs
index 03a06c673..729a9da9c 100644
--- a/src/data.rs
+++ b/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
diff --git a/src/node.rs b/src/node.rs
index 500f9d9eb..932cd068a 100644
--- a/src/node.rs
+++ b/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
diff --git a/tests/relayout.rs b/tests/relayout.rs
index 2575141ec..6a771602c 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();
+
+ // <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);
+ }
+}