Subtract rounded edges in pixel space when rounding layout results

tests/YGRoundingMeasureFuncTest.cpp

@@ -8,6 +8,9 @@
 #include <gtest/gtest.h>
 #include <yoga/Yoga.h>
 
+#include <iomanip>
+#include <limits>
+
 static YGSize _measureFloor(
     YGNodeConstRef /*node*/,
     float width,
@@ -137,3 +140,68 @@ TEST(
 
   YGConfigFree(config);
 }
+static YGSize _measureExactMultipleOfLineHeight(
+    YGNodeConstRef /*node*/,
+    float /*width*/,
+    YGMeasureMode /*widthMode*/,
+    float /*height*/,
+    YGMeasureMode /*heightMode*/) {
+  // 12 lines of 24pt: a height that is exactly representable and exactly on the pixel grid at any
+  // scale factor, so any shortfall in the committed height comes from rounding, not from the input.
+  return YGSize{
+      300.0f,
+      288.0f,
+  };
+}
+
+// A node with a measure function must never be committed a size smaller than it measured — the
+// rounding code calls this out explicitly ("we never want to round down its size as this could lead
+// to unwanted text truncation"), and forces ceil/floor on the node's edges to guarantee it.
+//
+// That guarantee used to be defeated by how the dimension was derived. Rounding each edge back to
+// points narrows it to float, and the dimension is the difference of two such edges, so each
+// operand's representation error leaked into the result. It only surfaced when pointScaleFactor made
+// the conversion inexact (n/2 is dyadic and always exact, n/3 almost never is) and when the absolute
+// coordinates were large enough for one float ULP to matter. A node measured at exactly 288.0 could
+// then be committed 287.999755859375 — enough for a platform text engine applying a strict "does this
+// line still fit" test to silently drop an entire trailing line.
+//
+// Note the exact comparison: ASSERT_FLOAT_EQ tolerates 4 ULPs and would not catch a 1-ULP shortfall.
+TEST(YogaTest, rounding_measured_size_is_never_rounded_down_at_large_offsets) {
+  const float pointScaleFactor = 3.0f;
+  const float measuredHeight = 288.0f;
+
+  // Sweep pixel-grid-aligned offsets through a range where one float ULP is significant.
+  for (int scaledOffset = 11000; scaledOffset <= 12000; scaledOffset++) {
+    const float offset = static_cast<float>(scaledOffset) / pointScaleFactor;
+
+    YGConfigRef config = YGConfigNew();
+    YGConfigSetPointScaleFactor(config, pointScaleFactor);
+
+    YGNodeRef root = YGNodeNewWithConfig(config);
+    YGNodeStyleSetWidth(root, 400.0f);
+    YGNodeStyleSetHeight(root, offset + 1000.0f);
+
+    YGNodeRef root_child0 = YGNodeNewWithConfig(config);
+    YGNodeStyleSetWidth(root_child0, 400.0f);
+    YGNodeStyleSetHeight(root_child0, offset);
+    YGNodeInsertChild(root, root_child0, 0);
+
+    YGNodeRef root_child1 = YGNodeNewWithConfig(config);
+    YGNodeSetMeasureFunc(root_child1, _measureExactMultipleOfLineHeight);
+    YGNodeInsertChild(root, root_child1, 1);
+
+    YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
+
+    // Printed at full float precision: the shortfall is a single ULP, so the default formatting
+    // would render both values as "288" and hide the difference.
+    const float committedHeight = YGNodeLayoutGetHeight(root_child1);
+    ASSERT_EQ(measuredHeight, committedHeight)
+        << std::setprecision(std::numeric_limits<float>::max_digits10)
+        << "measured height " << measuredHeight << " was committed as " << committedHeight
+        << " at offset " << offset;
+
+    YGNodeFreeRecursive(root);
+    YGConfigFree(config);
+  }
+}

yoga/algorithm/PixelGrid.cpp

@@ -12,7 +12,18 @@
 
 namespace facebook::yoga {
 
-float roundValueToPixelGrid(
+// Rounds `value` to the pixel grid and returns the result *in pixel space* (i.e. still multiplied by
+// `pointScaleFactor`), where a grid-aligned value is always an exact integer.
+//
+// Callers that need a difference of two rounded values must subtract in this space rather than convert each
+// operand back to points first: `scaledValue / pointScaleFactor` is generally not representable (for a 3x
+// screen it almost never is), and narrowing each operand to `float` before subtracting leaks that
+// representation error into the result. The error grows with the magnitude of the operands, so for a node far
+// down a long scrolling list it becomes large enough to matter — a height of exactly 288.0 points can come
+// back as 287.999755859375, which is enough for a text node to lose an entire trailing line when the platform
+// text engine checks whether the last line still fits. Subtracting two exact integers first, and narrowing
+// once at the end, keeps the returned dimension exactly grid-aligned.
+double roundValueToPixelGridScaled(
     const double value,
     const double pointScaleFactor,
     const bool forceCeil,
@@ -57,6 +68,25 @@ float roundValueToPixelGrid(
              ? 1.0
              : 0.0);
   }
+  return scaledValue;
+}
+
+float roundValueToPixelGrid(
+    const double value,
+    const double pointScaleFactor,
+    const bool forceCeil,
+    const bool forceFloor) {
+  const double scaledValue =
+      roundValueToPixelGridScaled(value, pointScaleFactor, forceCeil, forceFloor);
+  return (std::isnan(scaledValue) || std::isnan(pointScaleFactor))
+      ? YGUndefined
+      : (float)(scaledValue / pointScaleFactor);
+}
+
+// Converts a pixel-space value produced by `roundValueToPixelGridScaled()` back to points.
+static float pixelGridValueToPoints(
+    const double scaledValue,
+    const double pointScaleFactor) {
   return (std::isnan(scaledValue) || std::isnan(pointScaleFactor))
       ? YGUndefined
       : (float)(scaledValue / pointScaleFactor);
@@ -86,13 +116,15 @@ void roundLayoutResultsToPixelGrid(
     // size as this could lead to unwanted text truncation.
     const bool textRounding = node->getNodeType() == NodeType::Text;
 
-    node->setLayoutPosition(
-        roundValueToPixelGrid(nodeLeft, pointScaleFactor, false, textRounding),
-        PhysicalEdge::Left);
+    const double scaledLeft =
+        roundValueToPixelGridScaled(nodeLeft, pointScaleFactor, false, textRounding);
+    const double scaledTop =
+        roundValueToPixelGridScaled(nodeTop, pointScaleFactor, false, textRounding);
 
     node->setLayoutPosition(
-        roundValueToPixelGrid(nodeTop, pointScaleFactor, false, textRounding),
-        PhysicalEdge::Top);
+        pixelGridValueToPoints(scaledLeft, pointScaleFactor), PhysicalEdge::Left);
+    node->setLayoutPosition(
+        pixelGridValueToPoints(scaledTop, pointScaleFactor), PhysicalEdge::Top);
 
     // We multiply dimension by scale factor and if the result is close to the
     // whole number, we don't have any fraction To verify if the result is close
@@ -106,25 +138,32 @@ void roundLayoutResultsToPixelGrid(
     const bool hasFractionalHeight =
         !yoga::inexactEquals(round(scaledNodeHeight), scaledNodeHeight);
 
+    // The dimensions are derived as the difference of two rounded absolute edges. Both operands are exact
+    // integers in pixel space, so subtracting there and narrowing once yields an exactly grid-aligned
+    // dimension; converting each edge back to points first and subtracting in `float` would not.
+    const double scaledAbsoluteLeft =
+        roundValueToPixelGridScaled(absoluteNodeLeft, pointScaleFactor, false, textRounding);
+    const double scaledAbsoluteRight = roundValueToPixelGridScaled(
+        absoluteNodeRight,
+        pointScaleFactor,
+        (textRounding && hasFractionalWidth),
+        (textRounding && !hasFractionalWidth));
+
+    const double scaledAbsoluteTop =
+        roundValueToPixelGridScaled(absoluteNodeTop, pointScaleFactor, false, textRounding);
+    const double scaledAbsoluteBottom = roundValueToPixelGridScaled(
+        absoluteNodeBottom,
+        pointScaleFactor,
+        (textRounding && hasFractionalHeight),
+        (textRounding && !hasFractionalHeight));
+
     node->getLayout().setDimension(
         Dimension::Width,
-        roundValueToPixelGrid(
-            absoluteNodeRight,
-            pointScaleFactor,
-            (textRounding && hasFractionalWidth),
-            (textRounding && !hasFractionalWidth)) -
-            roundValueToPixelGrid(
-                absoluteNodeLeft, pointScaleFactor, false, textRounding));
+        pixelGridValueToPoints(scaledAbsoluteRight - scaledAbsoluteLeft, pointScaleFactor));
 
     node->getLayout().setDimension(
         Dimension::Height,
-        roundValueToPixelGrid(
-            absoluteNodeBottom,
-            pointScaleFactor,
-            (textRounding && hasFractionalHeight),
-            (textRounding && !hasFractionalHeight)) -
-            roundValueToPixelGrid(
-                absoluteNodeTop, pointScaleFactor, false, textRounding));
+        pixelGridValueToPoints(scaledAbsoluteBottom - scaledAbsoluteTop, pointScaleFactor));
   }
 
   for (yoga::Node* child : node->getChildren()) {

yoga/algorithm/PixelGrid.h

@@ -12,6 +12,17 @@
 
 namespace facebook::yoga {
 
+// Round a point value to the nearest physical pixel based on DPI
+// (pointScaleFactor), returning the result in pixel space (still scaled by
+// pointScaleFactor), where a grid-aligned value is an exact integer. Use this,
+// rather than subtracting two `roundValueToPixelGrid()` results, whenever a
+// difference of two rounded values is needed.
+double roundValueToPixelGridScaled(
+    double value,
+    double pointScaleFactor,
+    bool forceCeil,
+    bool forceFloor);
+
 // Round a point value to the nearest physical pixel based on DPI
 // (pointScaleFactor)
 float roundValueToPixelGrid(