Fixed rounding issues with YGRoundValueToPixelGrid and negative floats

tests/YGRoundingFunctionTest.cpp

@@ -20,6 +20,14 @@ TEST(YogaTest, rounding_value) {
   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 with 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));
   ASSERT_FLOAT_EQ(6.5, YGRoundValueToPixelGrid(6.01, 2.0, true, false));
@@ -27,4 +35,45 @@ 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));
+  ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(6.49, 1.0, false, false));
+  ASSERT_FLOAT_EQ(7.0, YGRoundValueToPixelGrid(6.50, 1.0, false, false));
+  ASSERT_FLOAT_EQ(7.0, YGRoundValueToPixelGrid(6.51, 1.0, false, false));
+  ASSERT_FLOAT_EQ(7.0, YGRoundValueToPixelGrid(6.50, 1.0, true, false));
+  ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(6.50, 1.0, false, true));
+
+  // Test with 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));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.49, 1.0, false, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.50, 1.0, false, false));
+  ASSERT_FLOAT_EQ(-7.0, YGRoundValueToPixelGrid(-6.51, 1.0, false, false));
+  ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-6.50, 1.0, true, false));
+  ASSERT_FLOAT_EQ(-7.0, YGRoundValueToPixelGrid(-6.50, 1.0, false, true));
+
+  // Do a simple translation test to ensure that a distance between 2 values
+  // stays consistent during an animation, even after rounding them.
+  const int LAPS = 3;
+  const int LAP_CLOSEST = 0;
+  const int LAP_CEIL = 1;
+  const int LAP_FLOOR = 2;
+  for (int currentLap = LAP_CLOSEST; currentLap < LAPS; ++currentLap)
+  {
+    float left = -2.0f;
+    float right = 2.0f;
+    const float distance = right-left;
+    float totalDistance = 1.1f;
+    const float step = 0.01f;
+    while (totalDistance >= 0.0f) {
+      left += step;
+      right += step;
+      const float snappedLeft = YGRoundValueToPixelGrid(left, 1.0, currentLap==LAP_CEIL, currentLap==LAP_FLOOR);
+      const float snappedRight = YGRoundValueToPixelGrid(right, 1.0, currentLap==LAP_CEIL, currentLap==LAP_FLOOR);
+      ASSERT_FLOAT_EQ(distance, (snappedRight-snappedLeft));
+      totalDistance -= step;
+    }
+  }
 }

yoga/Yoga.cpp

@@ -2928,21 +2928,41 @@ float YGRoundValueToPixelGrid(const float value,
                               const bool forceCeil,
                               const bool forceFloor) {
   float scaledValue = value * pointScaleFactor;
-  float fractial = fmodf(scaledValue, 1.0);
+  const float fractial = fmodf(scaledValue, 1.0f);
-  if (YGFloatsEqual(fractial, 0)) {
+  const float absoluteFractial = fabs(fractial);
-    // First we check if the value is already rounded
+
-    scaledValue = scaledValue - fractial;
+  // 1. Remove any remainder from the scaledValue
-  } else if (YGFloatsEqual(fractial, 1.0)) {
+  scaledValue = scaledValue - fractial;
-    scaledValue = scaledValue - fractial + 1.0;
+
+  // 2. Figure out rounding
+  // Note: It is important that the following rounding algorithm
+  //       ensures that both positive and negative values are treated exactly the same.
+  if (YGFloatsEqual(absoluteFractial, 0.0f)) {
+    // Already whole (or close enough), skip rounding
+  } else if (YGFloatsEqual(absoluteFractial, 1.0f)) {
+    // Already whole (or close enough), skip rounding
+    scaledValue += (fractial < 0.0f) ? -1.0f : 1.0f;
   } else if (forceCeil) {
-    // Next we check if we need to use forced rounding
+    // Force round to upper whole value
-    scaledValue = scaledValue - fractial + 1.0f;
+    if (fractial > 0.0f) {
+      scaledValue += 1.0f;
+    }
   } else if (forceFloor) {
-    scaledValue = scaledValue - fractial;
+    // Force round to lower whole value
+    if (fractial < 0.0f) {
+      scaledValue -= 1.0f;
+    }
   } else {
-    // Finally we just round the value
+    // Round to closest whole value
-    scaledValue = scaledValue - fractial +
+    if (fractial > 0.0f) {
-        (fractial > 0.5f || YGFloatsEqual(fractial, 0.5f) ? 1.0f : 0.0f);
+      if (absoluteFractial > 0.5f || YGFloatsEqual(absoluteFractial, 0.5f)) {
+        scaledValue += 1.0f;
+      }
+    } else {
+      if (absoluteFractial > 0.5f && !YGFloatsEqual(absoluteFractial, 0.5f)) {
+        scaledValue -= 1.0f;
+      }
+    }
   }
   return scaledValue / pointScaleFactor;
 }