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
25ffaee23176000364b17e1b5b29f361a764890a · ruwaq Nico Burns · 2023-07-23T11:44:31Z (3 wata ñawpaq) · tayta 2d78dce1a · kay commitpi sach'ata qhaway
Semántico sayk'uy · 10
src/compute/taffy_tree.rs
src/tree/node.rs
silq'uyninpi t'ikrakurqan def NodeData ukhunpi t'ikrakurqan fn new pub
src/tree/taffy_tree/tree.rs
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<AvailableSpace>,
) -> 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 <https://github.com/facebook/yoga/commit/aa5b296ac78f7a22e1aeaf4891243c6bb76488e2> 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<NodeId> 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 <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,
@@ -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 <https://github.com/DioxusLabs/taffy/issues/501> 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();
+
+ // <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: 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);
+ }
+}