Fix 1px wide gaps between adjacent UI nodes (#374)

* Fractional pixel investigations

* Enable rounding test

* Reinstate old rounding function

* Add option to disable rounding of layout output

* Match Taffy's rounding in gentests

Disable rounding for the  "percent moderate" tests which fail due to
Taffy's lack of precision (only using f32) causing it round in the
opposite direction for those two tests.

* Simplify and document round_layout function

scripts/gentest/src/main.rs

@@ -121,8 +121,11 @@ async fn test_root_element(client: Client, name: String, fixture_path: impl AsRe
 fn generate_test(name: impl AsRef<str>, description: &Value) -> TokenStream {
     let name = name.as_ref();
     let name = Ident::new(name, Span::call_site());
+    let use_rounding = description["useRounding"].as_bool().unwrap();
+    let assertions = generate_assertions("node", description, use_rounding);
     let node_description = generate_node("node", description);
-    let assertions = generate_assertions("node", description);
+
+    let set_rounding_mode = if use_rounding { quote!() } else { quote!(taffy.disable_rounding();) };
 
     quote!(
         #[test]
@@ -131,6 +134,7 @@ fn generate_test(name: impl AsRef<str>, description: &Value) -> TokenStream {
             use taffy::{layout::Layout, prelude::*};
             use slotmap::Key;
             let mut taffy = taffy::Taffy::new();
+            #set_rounding_mode
             #node_description
             taffy.compute_layout(node, taffy::geometry::Size::MAX_CONTENT).unwrap();
 
@@ -143,8 +147,8 @@ fn generate_test(name: impl AsRef<str>, description: &Value) -> TokenStream {
     )
 }
 
-fn generate_assertions(ident: &str, node: &Value) -> TokenStream {
-    let layout = &node["layout"];
+fn generate_assertions(ident: &str, node: &Value, use_rounding: bool) -> TokenStream {
+    let layout = if use_rounding { &node["smartRoundedLayout"] } else { &node["unroundedLayout"] };
 
     let read_f32 = |s: &str| layout[s].as_f64().unwrap() as f32;
     let width = read_f32("width");
@@ -156,23 +160,35 @@ fn generate_assertions(ident: &str, node: &Value) -> TokenStream {
         let mut c = Vec::new();
         if let Value::Array(ref value) = node["children"] {
             for (idx, child) in value.iter().enumerate() {
-                c.push(generate_assertions(&format!("{ident}{idx}"), child));
+                c.push(generate_assertions(&format!("{ident}{idx}"), child, use_rounding));
             }
         };
         c.into_iter().fold(quote!(), |a, b| quote!(#a #b))
     };
 
     let ident = Ident::new(ident, Span::call_site());
 
-    quote!(
-        let Layout { size, location, .. } = taffy.layout(#ident).unwrap();
-        assert_eq!(size.width, #width, "width of node {:?}. Expected {}. Actual {}", #ident.data(),  #width, size.width);
-        assert_eq!(size.height, #height, "height of node {:?}. Expected {}. Actual {}", #ident.data(),  #height, size.height);
-        assert_eq!(location.x, #x, "x of node {:?}. Expected {}. Actual {}", #ident.data(),  #x, location.x);
-        assert_eq!(location.y, #y, "y of node {:?}. Expected {}. Actual {}", #ident.data(),  #y, location.y);
+    if use_rounding {
+        quote!(
+            let Layout { size, location, .. } = taffy.layout(#ident).unwrap();
+            assert_eq!(size.width, #width, "width of node {:?}. Expected {}. Actual {}", #ident.data(),  #width, size.width);
+            assert_eq!(size.height, #height, "height of node {:?}. Expected {}. Actual {}", #ident.data(),  #height, size.height);
+            assert_eq!(location.x, #x, "x of node {:?}. Expected {}. Actual {}", #ident.data(),  #x, location.x);
+            assert_eq!(location.y, #y, "y of node {:?}. Expected {}. Actual {}", #ident.data(),  #y, location.y);
 
-        #children
-    )
+            #children
+        )
+    } else {
+        quote!(
+            let Layout { size, location, .. } = taffy.layout(#ident).unwrap();
+            assert!(size.width - #width < 0.1, "width of node {:?}. Expected {}. Actual {}", #ident.data(),  #width, size.width);
+            assert!(size.height - #height < 0.1, "height of node {:?}. Expected {}. Actual {}", #ident.data(),  #height, size.height);
+            assert!(location.x - #x < 0.1, "x of node {:?}. Expected {}. Actual {}", #ident.data(),  #x, location.x);
+            assert!(location.y - #y < 0.1, "y of node {:?}. Expected {}. Actual {}", #ident.data(),  #y, location.y);
+
+            #children
+        )
+    }
 }
 
 macro_rules! dim_quoted_renamed {

scripts/gentest/test_helper.js

@@ -173,6 +173,10 @@ function parseGridPosition(input) {
 
 function describeElement(e) {
 
+  // Get precise, unrounded dimensions for the current element and it's parent
+  let boundingRect = e.getBoundingClientRect();
+  let parentBoundingRect = e.parentNode.getBoundingClientRect();
+
   return {
     style: {
       display: parseEnum(e.style.display),
@@ -249,13 +253,36 @@ function describeElement(e) {
     // So we're only interested in the text content of leaf nodes
     textContent: e.childElementCount === 0 && e.textContent.length && e.textContent !== "\n" ? e.textContent : undefined,
 
-    layout: {
+    // The layout of the node in full precision (floating-point)
+    unroundedLayout: {
+      width: boundingRect.width,
+      height: boundingRect.height,
+      x: boundingRect.x - parentBoundingRect.x,
+      y: boundingRect.y - parentBoundingRect.y,
+    },
+
+    // The naively rounded layout of the node. This is equivalent to calling Math.round() on
+    // each value in the unrounded layout individually
+    naivelyRoundedLayout: {
       width: e.offsetWidth,
       height: e.offsetHeight,
       x: e.offsetLeft + e.parentNode.clientLeft,
       y: e.offsetTop + e.parentNode.clientTop,
     },
 
+    // The naive rounding can result in 1px gaps in the layout, so Taffy uses a smarter algorithm to avoid this.
+    // Chrome also uses a smarter algorithm, but it doesn't expose the output of that rounding.
+    // So we just emulate Taffy's computation here.
+    smartRoundedLayout: {
+      width: Math.round(boundingRect.right) - Math.round(boundingRect.left),
+      height: Math.round(boundingRect.bottom) - Math.round(boundingRect.top),
+      x: Math.round(boundingRect.x - parentBoundingRect.x),
+      y: Math.round(boundingRect.y - parentBoundingRect.y),
+    },
+
+    // Whether the test should enable rounding
+    useRounding: e.getAttribute("data-test-rounding") !== "false",
+
     children: Array.from(e.children).map(c => describeElement(c)),
   }
 }

src/compute/mod.rs

@@ -11,7 +11,7 @@ use crate::data::CACHE_SIZE;
 use crate::error::TaffyError;
 use crate::geometry::{Point, Size};
 use crate::layout::{Cache, Layout, RunMode, SizeAndBaselines, SizingMode};
-use crate::node::Node;
+use crate::node::{Node, Taffy};
 use crate::style::{AvailableSpace, Display};
 use crate::sys::round;
 use crate::tree::LayoutTree;
@@ -25,14 +25,10 @@ use self::leaf::LeafAlgorithm;
 use crate::debug::NODE_LOGGER;
 
 /// Updates the stored layout of the provided `node` and its children
-pub fn compute_layout(
-    tree: &mut impl LayoutTree,
-    root: Node,
-    available_space: Size<AvailableSpace>,
-) -> Result<(), TaffyError> {
+pub fn compute_layout(taffy: &mut Taffy, root: Node, available_space: Size<AvailableSpace>) -> Result<(), TaffyError> {
     // Recursively compute node layout
     let size_and_baselines = GenericAlgorithm::perform_layout(
-        tree,
+        taffy,
         root,
         Size::NONE,
         available_space.into_options(),
@@ -41,10 +37,12 @@ pub fn compute_layout(
     );
 
     let layout = Layout { order: 0, size: size_and_baselines.size, location: Point::ZERO };
-    *tree.layout_mut(root) = layout;
+    *taffy.layout_mut(root) = layout;
 
-    // Recursively round the layout's of this node and all children
-    round_layout(tree, root);
+    // If rounding is enabled, recursively round the layout's of this node and all children
+    if taffy.config.use_rounding {
+        round_layout(taffy, root, 0.0, 0.0);
+    }
 
     Ok(())
 }
@@ -390,20 +388,27 @@ fn perform_hidden_layout(tree: &mut impl LayoutTree, node: Node) {
     }
 }
 
-/// Rounds the calculated [`NodeData`] according to the spec
-fn round_layout(tree: &mut impl LayoutTree, root: Node) {
-    let layout = tree.layout_mut(root);
+/// 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
+///   - 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);
 
-    layout.size.width = round(layout.size.width);
-    layout.size.height = round(layout.size.height);
-
-    // Satisfy the borrow checker here by re-indexing to shorten the lifetime to the loop scope
-    for x in 0..tree.child_count(root) {
-        let child = tree.child(root, x);
-        round_layout(tree, child);
+    let child_count = tree.child_count(node);
+    for index in 0..child_count {
+        let child = tree.child(node, index);
+        round_layout(tree, child, abs_x, abs_y);
     }
 }
 

src/node.rs

@@ -32,6 +32,18 @@ pub enum MeasureFunc {
     Boxed(Box<dyn Measurable>),
 }
 
+/// Global configuration values for a Taffy instance
+pub(crate) struct TaffyConfig {
+    /// Whether to round layout values
+    pub(crate) use_rounding: bool,
+}
+
+impl Default for TaffyConfig {
+    fn default() -> Self {
+        Self { use_rounding: true }
+    }
+}
+
 /// A tree of UI [`Nodes`](`Node`), suitable for UI layout
 pub struct Taffy {
     /// The [`NodeData`] for each node stored in this tree
@@ -49,6 +61,9 @@ pub struct Taffy {
     ///
     /// The indexes in the outer vector correspond to the position of the child [`NodeData`]
     pub(crate) parents: SlotMap<Node, Option<Node>>,
+
+    /// Layout mode configuration
+    pub(crate) config: TaffyConfig,
 }
 
 impl Default for Taffy {
@@ -140,9 +155,20 @@ impl Taffy {
             children: SlotMap::with_capacity(capacity),
             parents: SlotMap::with_capacity(capacity),
             measure_funcs: SparseSecondaryMap::with_capacity(capacity),
+            config: TaffyConfig::default(),
         }
     }
 
+    /// Enable rounding of layout values. Rounding is enabled by default.
+    pub fn enable_rounding(&mut self) {
+        self.config.use_rounding = true;
+    }
+
+    /// Disable rounding of layout values. Rounding is enabled by default.
+    pub fn disable_rounding(&mut self) {
+        self.config.use_rounding = false;
+    }
+
     /// Creates and adds a new unattached leaf node to the tree, and returns the [`Node`] of the new node
     pub fn new_leaf(&mut self, layout: Style) -> TaffyResult<Node> {
         let id = self.nodes.insert(NodeData::new(layout));

test_fixtures/grid_percent_items_nested_moderate.html

@@ -9,7 +9,7 @@
 <head/>
 <body>
 
-<div id="test-root" style="display: grid;width: 200px;padding: 3px;">
+<div id="test-root" data-test-rounding="false" style="display: grid;width: 200px;padding: 3px;">
   <div style="display: grid;width: 50%; margin: 5px; padding: 3%;">
     <div style="width: 45%; margin: 5%; padding: 3px;"></div>
   </div>

test_fixtures/percentage_moderate_complexity.html

@@ -9,7 +9,7 @@
 <head/>
 <body>
 
-<div id="test-root" style="display: flex;flex-direction:column;width: 200px;padding: 3px;">
+<div id="test-root" data-test-rounding="false" style="display: flex;flex-direction:column;width: 200px;padding: 3px;">
   <div style="display: flex;flex-direction:column;width: 50%; margin: 5px; padding: 3%;">
     <div style="width: 45%; margin: 5%; padding: 3px;"></div>
   </div>

test_fixtures/rounding_fractial_input_5.html

@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <script src="../scripts/gentest/test_helper.js"></script>
+  <link rel="stylesheet" type="text/css" href="../scripts/gentest/test_base_style.css">
+  <title>
+    Test description
+  </title>
+<head/>
+<body>
+
+<div id="test-root" style="height: 100px;width:963.3333px;justify-content: center;">
+  <div style="width:100.3px;height:100.3px"></div>
+  <div style="width:100.3px;height:100.3px"></div>
+</div>
+
+</body>
+</html>

test_fixtures/rounding_fractial_input_6.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <script src="../scripts/gentest/test_helper.js"></script>
+  <link rel="stylesheet" type="text/css" href="../scripts/gentest/test_base_style.css">
+  <title>
+    Test description
+  </title>
+<head/>
+<body>
+
+<div id="test-root" style="width:7px;">
+  <div style="width:50%;flex-wrap: wrap;">
+    <div style="width: 2px;height: 10px;background-color: red"></div>
+    <div style="width: 2px;height: 10px;background-color: green"></div>
+  </div>
+  <div style="width:50%;flex-wrap: wrap;">
+    <div style="width: 2px;height: 10px;background-color: yellow"></div>
+    <div style="width: 2px;height: 10px;background-color: blue"></div>
+  </div>
+</div>
+
+</body>
+</html>

test_fixtures/rounding_fractial_input_7.html

@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <script src="../scripts/gentest/test_helper.js"></script>
+  <link rel="stylesheet" type="text/css" href="../scripts/gentest/test_base_style.css">
+  <title>
+    Test description
+  </title>
+<head/>
+<body>
+
+<div id="test-root" style="width:7px;">
+  <div style="width:25%;flex-wrap: wrap;">
+    <div style="width: 1px;height: 10px;background-color: red"></div>
+    <div style="width: 1px;height: 10px;background-color: green"></div>
+  </div>
+  <div style="width:25%;flex-wrap: wrap;">
+    <div style="width: 1px;height: 10px;background-color: yellow"></div>
+    <div style="width: 1px;height: 10px;background-color: blue"></div>
+  </div>
+  <div style="width:25%;flex-wrap: wrap;">
+    <div style="width: 1px;height: 10px;background-color: red"></div>
+    <div style="width: 1px;height: 10px;background-color: green"></div>
+  </div>
+    <div style="width:25%;flex-wrap: wrap;">
+    <div style="width: 1px;height: 10px;background-color: yellow"></div>
+    <div style="width: 1px;height: 10px;background-color: blue"></div>
+  </div>
+</div>
+
+</body>
+</html>

test_fixtures/xrounding_inner_node_controversy_combined.html → test_fixtures/rounding_inner_node_controversy_combined.html

@@ -14,8 +14,7 @@
   <div style="height: 100%; flex-grow:1; flex-direction: column;">
     <div style="width: 100%; flex-grow:1;"></div>
     <div style="flex-direction: column;width: 100%; flex-grow:1;">
-      <div style="flex-grow:1; width: 100%;">
-      </div>
+      <div style="flex-grow:1; width: 100%;"></div>
     </div>
     <div style="width: 100%; flex-grow:1;"></div>
   </div>