Fix negative value rounding issue for nodes across an axis #688
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
can you add the new test to the PR please? |
|
There's already a test above that reproduce the issue. |
|
What is the status of this? This bug is pretty heavily impacting my project and forcing me to run a branch with this fix. It would be nice to get this merged in. |
There was a problem hiding this comment.
@davidaurelio has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@davidaurelio has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@davidaurelio has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@davidaurelio has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@davidaurelio has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@rozele has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@rozele has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: This fix issue facebook/yoga#683 the rounding calculation is incorrect if a node is crossing an axis and it will shrink it's width/height on layout calculation. The following test reproduce the issue : ``` TEST(YogaTest, node_shrink_on_axis) { const YGConfigRef config = YGConfigNew(); const YGNodeRef root = YGNodeNewWithConfig(config); const YGNodeRef child = YGNodeNewWithConfig(config); YGNodeInsertChild(root, child, 0); YGNodeStyleSetWidth(child, 10); YGNodeStyleSetHeight(child, 10); YGNodeStyleSetPosition(root, YGEdgeLeft, -0.75f); YGNodeStyleSetPosition(root, YGEdgeTop, -0.75f); YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); ASSERT_FLOAT_EQ(YGNodeLayoutGetWidth(child), 10); ASSERT_FLOAT_EQ(YGNodeLayoutGetHeight(child), 10); YGNodeFreeRecursive(root); YGConfigFree(config); } ``` X-link: facebook/yoga#688 Reviewed By: NickGerleman Differential Revision: D13866122 Pulled By: rozele fbshipit-source-id: 4faf8a9efc86723c303f600d730660a2e13d8a73
Summary: This fix issue facebook/yoga#683 the rounding calculation is incorrect if a node is crossing an axis and it will shrink it's width/height on layout calculation. The following test reproduce the issue : ``` TEST(YogaTest, node_shrink_on_axis) { const YGConfigRef config = YGConfigNew(); const YGNodeRef root = YGNodeNewWithConfig(config); const YGNodeRef child = YGNodeNewWithConfig(config); YGNodeInsertChild(root, child, 0); YGNodeStyleSetWidth(child, 10); YGNodeStyleSetHeight(child, 10); YGNodeStyleSetPosition(root, YGEdgeLeft, -0.75f); YGNodeStyleSetPosition(root, YGEdgeTop, -0.75f); YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); ASSERT_FLOAT_EQ(YGNodeLayoutGetWidth(child), 10); ASSERT_FLOAT_EQ(YGNodeLayoutGetHeight(child), 10); YGNodeFreeRecursive(root); YGConfigFree(config); } ``` X-link: facebook/yoga#688 Reviewed By: NickGerleman Differential Revision: D13866122 Pulled By: rozele fbshipit-source-id: 4faf8a9efc86723c303f600d730660a2e13d8a73
|
This pull request has been reverted by 996267d. |
|
As a heads up, it looks like this change ended up making subtle shifts to existing layouts in many of our screenshot tests, so this change was reverted to investigate. |
) Summary: This fix issue facebook/yoga#683 the rounding calculation is incorrect if a node is crossing an axis and it will shrink it's width/height on layout calculation. The following test reproduce the issue : ``` TEST(YogaTest, node_shrink_on_axis) { const YGConfigRef config = YGConfigNew(); const YGNodeRef root = YGNodeNewWithConfig(config); const YGNodeRef child = YGNodeNewWithConfig(config); YGNodeInsertChild(root, child, 0); YGNodeStyleSetWidth(child, 10); YGNodeStyleSetHeight(child, 10); YGNodeStyleSetPosition(root, YGEdgeLeft, -0.75f); YGNodeStyleSetPosition(root, YGEdgeTop, -0.75f); YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); ASSERT_FLOAT_EQ(YGNodeLayoutGetWidth(child), 10); ASSERT_FLOAT_EQ(YGNodeLayoutGetHeight(child), 10); YGNodeFreeRecursive(root); YGConfigFree(config); } ``` X-link: facebook/yoga#688 Reviewed By: NickGerleman Differential Revision: D13866122 Pulled By: rozele fbshipit-source-id: 4faf8a9efc86723c303f600d730660a2e13d8a73
This fix issue #683 the rounding calculation is incorrect if a node is crossing an axis and it will shrink it's width/height on layout calculation.
The following test reproduce the issue :