DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Fix rounding of negative numbers (#825)

Browse Source 
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 https://github.com/facebook/yoga/issues/824.

Fixes #824

Adam Comella
Microsoft Corp.
Pull Request resolved: https://github.com/facebook/yoga/pull/825

Reviewed By: priteshrnandgaonkar

Differential Revision: D10282064

Pulled By: shergin

fbshipit-source-id: 16ca966e6cb0cfc88b1dbf4ba31e7b1dbe1f2049
This commit is contained in:
Adam Comella
2018-10-11 18:17:14 -07:00
committed by Facebook Github Bot
parent b747286c1d
commit 786ccddd7b
2 changed files with 79 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
tests/YGRoundingFunctionTest.cpp
View File

@@ -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);
}

23
yoga/Yoga.cpp
View File

@@ -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;