Fix rounding of negative numbers (#825)

Summary:
`YGRoundValueToPixelGrid` currently rounds negative numbers incorrectly. For example:

```
YGRoundValueToPixelGrid(-2.2, 1.0, /* ceil */ false, /* floor */ true) = -2.0
```

However, that operation is supposed to take the floor of the number so the result should acutally be `-3.0`.

There's a detailed comment in `YGRoundValueToPixelGrid` about the fix and why it works.

A symptom that manifested because of this bug is that text nodes could get smaller and smaller on each layout pass. For details see #824.

Fixes #824

Adam Comella
Microsoft Corp.
Pull Request resolved: #825

Reviewed By: priteshrnandgaonkar

Differential Revision: D10282064

Pulled By: shergin

fbshipit-source-id: 16ca966e6cb0cfc88b1dbf4ba31e7b1dbe1f2049

tests/YGRoundingFunctionTest.cpp

@@ -1,10 +1,10 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
+/*
+ *  Copyright (c) 2018-present, Facebook, Inc.
+ *
+ *  This source code is licensed under the MIT license found in the LICENSE
+ *  file in the root directory of this source tree.
  *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
  */
-
 #include <gtest/gtest.h>
 #include <yoga/Yoga.h>
 #include <yoga/Yoga-internal.h>
@@ -17,6 +17,13 @@ TEST(YogaTest, rounding_value) {
   ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(5.999999, 2.0, false, false));
   ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(5.999999, 2.0, true, false));
   ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(5.999999, 2.0, false, true));
+  // Same tests for negative numbers
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.000001, 2.0, false, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.000001, 2.0, true, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.000001, 2.0, false, true));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-5.999999, 2.0, false, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-5.999999, 2.0, true, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-5.999999, 2.0, false, true));
 
   // Test that numbers with fraction are rounded correctly accounting for ceil/floor flags
   ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(6.01, 2.0, false, false));
@@ -25,4 +32,50 @@ TEST(YogaTest, rounding_value) {
   ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(5.99, 2.0, false, false));
   ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(5.99, 2.0, true, false));
   ASSERT_FLOAT_EQ(5.5, YGRoundValueToPixelGrid(5.99, 2.0, false, true));
+  // Same tests for negative numbers
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.01, 2.0, false, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.01, 2.0, true, false));
+  ASSERT_FLOAT_EQ(-6.5, YGRoundValueToPixelGrid(-6.01, 2.0, false, true));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-5.99, 2.0, false, false));
+  ASSERT_FLOAT_EQ(-5.5, YGRoundValueToPixelGrid(-5.99, 2.0, true, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-5.99, 2.0, false, true));
+}
+
+static YGSize measureText(
+    YGNodeRef node,
+    float width,
+    YGMeasureMode widthMode,
+    float height,
+    YGMeasureMode heightMode) {
+  return (YGSize){.width = 10, .height = 10};
+}
+
+// Regression test for https://github.com/facebook/yoga/issues/824
+TEST(YogaTest, consistent_rounding_during_repeated_layouts) {
+  const YGConfigRef config = YGConfigNew();
+  YGConfigSetPointScaleFactor(config, 2);
+
+  const YGNodeRef root = YGNodeNewWithConfig(config);
+  YGNodeStyleSetMargin(root, YGEdgeTop, -1.49);
+  YGNodeStyleSetWidth(root, 500);
+  YGNodeStyleSetHeight(root, 500);
+
+  const YGNodeRef node0 = YGNodeNewWithConfig(config);
+  YGNodeInsertChild(root, node0, 0);
+
+  const YGNodeRef node1 = YGNodeNewWithConfig(config);
+  YGNodeSetMeasureFunc(node1, measureText);
+  YGNodeInsertChild(node0, node1, 0);
+
+  for (int i = 0; i < 5; i++) {
+    // Dirty the tree so YGRoundToPixelGrid runs again
+    YGNodeStyleSetMargin(root, YGEdgeLeft, i + 1);
+
+    YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
+    ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(node1));
+  }
+
+  YGNodeFreeRecursive(root);
+
+  YGConfigFree(config);
 }

yoga/Yoga.cpp

@@ -1,11 +1,10 @@
 /*
- *  Copyright (c) Facebook, Inc. and its affiliates.
+ *  Copyright (c) 2018-present, Facebook, Inc.
  *
  *  This source code is licensed under the MIT license found in the LICENSE
  *  file in the root directory of this source tree.
  *
  */
-
 #include "Yoga.h"
 #include <float.h>
 #include <string.h>
@@ -3608,7 +3607,27 @@ float YGRoundValueToPixelGrid(
     const bool forceCeil,
     const bool forceFloor) {
   float scaledValue = value * pointScaleFactor;
+  // We want to calculate `fractial` such that `floor(scaledValue) = scaledValue
+  // - fractial`.
   float fractial = fmodf(scaledValue, 1.0f);
+  if (fractial < 0) {
+    // This branch is for handling negative numbers for `value`.
+    //
+    // Regarding `floor` and `ceil`. Note that for a number x, `floor(x) <= x <=
+    // ceil(x)` even for negative numbers. Here are a couple of examples:
+    //   - x =  2.2: floor( 2.2) =  2, ceil( 2.2) =  3
+    //   - x = -2.2: floor(-2.2) = -3, ceil(-2.2) = -2
+    //
+    // Regarding `fmodf`. For fractional negative numbers, `fmodf` returns a
+    // negative number. For example, `fmodf(-2.2) = -0.2`. However, we want
+    // `fractial` to be the number such that subtracting it from `value` will
+    // give us `floor(value)`. In the case of negative numbers, adding 1 to
+    // `fmodf(value)` gives us this. Let's continue the example from above:
+    //   - fractial = fmodf(-2.2) = -0.2
+    //   - Add 1 to the fraction: fractial2 = fractial + 1 = -0.2 + 1 = 0.8
+    //   - Finding the `floor`: -2.2 - fractial2 = -2.2 - 0.8 = -3
+    ++fractial;
+  }
   if (YGFloatsEqual(fractial, 0)) {
     // First we check if the value is already rounded
     scaledValue = scaledValue - fractial;