Fixed rounding issues with YGRoundValueToPixelGrid and negative floats #702

2018-01-24T00:45:31-08:00

Edensan commented

2018-01-24 00:45:31 -08:00

(Migrated from github.com)

While implementing Yoga into a custom framework I found out that sizes of objects that had been layout with Yoga were sometimes 1px smaller/bigger than expected, causing all sorts of unexpected logic to trigger.

Turns out the culprit was YGRoundValueToPixelGrid.

It was computing values regardless of sign and thus negative inputs were outputting unexpected results (eg. if scaledValue == -0.6f closest rounding should be -1.0f but would be 0.0f instead).

I have updated the code to handle positive and negative values symmetrically in order to ensure that two values with a same offset between them will conserve the offset during animations regardless of the rounding type.

Please check the test cases to confirm the logic.

While implementing Yoga into a custom framework I found out that sizes of objects that had been layout with Yoga were sometimes 1px smaller/bigger than expected, causing all sorts of unexpected logic to trigger. Turns out the culprit was YGRoundValueToPixelGrid. It was computing values regardless of sign and thus negative inputs were outputting unexpected results (eg. if scaledValue == -0.6f closest rounding should be -1.0f but would be 0.0f instead). I have updated the code to handle positive and negative values symmetrically in order to ensure that two values with a same offset between them will conserve the offset during animations regardless of the rounding type. Please check the test cases to confirm the logic.

matthargett (Migrated from github.com) approved these changes 2018-01-24 11:30:11 -08:00

matthargett (Migrated from github.com) left a comment

LGTM. The tests you added were really helpful in understanding the issue!

lunarraid commented

2018-03-18 14:21:10 -07:00

(Migrated from github.com)

This looks like it's covering the same issue as https://github.com/facebook/yoga/pull/688. I'd really like to see one of these merged in as I'm being forced to use a fork of this project due to this issue. Does anyone have an update on this?

This pull request has changes conflicting with the target branch.

tests/YGRoundingFunctionTest.cpp
yoga/Yoga.cpp

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin Edensan/edensan:Edensan/edensan

git checkout Edensan/edensan

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#702