Incorrect cached child size is used because YGNodeCanUseCachedMeasurement() does not respect min/max dimensions #1003

Open
opened 2020-05-04 06:02:48 -07:00 by nickolas-pohilets · 2 comments
nickolas-pohilets commented 2020-05-04 06:02:48 -07:00 (Migrated from github.com)

Report

Issues and Steps to Reproduce

See the attached example.

Child node has height = 10, minHeight = 40%, maxHeight = 60%.

When parent is measured against height = NaN, child height is computed to be 10, which is correct.

But then parent is laid out against height = 30.
Now child height should be max(40% * 30, min(10, 60% * 30)) = max(12, min(10, 18)) = 12.
But YGNodeCanUseCachedMeasurement() returns true and cached height of 10 is used instead.

Expected Behavior

In all the cases, final layout should have child height of 12.

Actual Behavior

In some of the scenarios incorrect cached height is used.

Link to Code

https://pastebin.com/x5e6ZL2b

# Report - [x] I have searched [existing issues](https://github.com/facebook/yoga/issues) and this is not a duplicate # Issues and Steps to Reproduce See the attached example. Child node has height = 10, minHeight = 40%, maxHeight = 60%. When parent is measured against height = NaN, child height is computed to be 10, which is correct. But then parent is laid out against height = 30. Now child height should be `max(40% * 30, min(10, 60% * 30)) = max(12, min(10, 18)) = 12`. But `YGNodeCanUseCachedMeasurement()` returns `true` and cached height of 10 is used instead. # Expected Behavior In all the cases, final layout should have child height of 12. # Actual Behavior In some of the scenarios incorrect cached height is used. # Link to Code https://pastebin.com/x5e6ZL2b
woehrl01 commented 2020-06-11 12:27:15 -07:00 (Migrated from github.com)

Hi @nickolas-pohilets,

thanks for pointing that out. I just pushed PR for a fix.

Hi @nickolas-pohilets, thanks for pointing that out. I just pushed PR for a fix.
woehrl01 commented 2020-06-11 12:38:50 -07:00 (Migrated from github.com)

I'm not sure if the PR will be merged, as it is a quite edge case here, which leads to increasing the binary size for each node.

In that case I suggest, adding another root node, where you set height and width directly and pass YGUndefined to YGNodeCalculateLayout for width and height.

Changing these values on that new root node lead to proper invalidation of the tree.

I'm not sure if the PR will be merged, as it is a quite edge case here, which leads to increasing the binary size for each node. In that case I suggest, adding another root node, where you set height and width directly and pass `YGUndefined` to `YGNodeCalculateLayout` for width and height. Changing these values on that new root node lead to proper invalidation of the tree.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#1003
No description provided.