facebook / yoga

Yoga is an embeddable layout engine targeting web standards.
https://yogalayout.dev/
MIT License
17.25k stars 1.42k forks source link

absolute percent position of root element is not correct #1658

Closed robbert-ef closed 3 months ago

robbert-ef commented 4 months ago

Report

Issues and Steps to Reproduce

YGNodeRef root = YGNodeNew();
YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);
YGNodeStyleSetPositionPercent(root, YGEdgeLeft, 10);
YGNodeStyleSetPositionPercent(root, YGEdgeTop, 10);
YGNodeStyleSetWidthPercent(root, 80);
YGNodeStyleSetHeightPercent(root, 80);

YGNodeCalculateLayout(root, 300, 200, YGDirectionLTR);
float left = YGNodeLayoutGetLeft(root);
float top = YGNodeLayoutGetTop(root);
float width = YGNodeLayoutGetWidth(root);
float height = YGNodeLayoutGetHeight(root);

Expected Behavior

left = 30 top = 20 width = 240 height = 160

Actual Behavior

left = 20 (not ok, 10% of 300 is 30) top = 30 (not ok, 10% of 200 is 20) width = 240 (ok) height = 160 (ok)

When using the following similar code in the playground, the result is correct:

<Layout>
  <Node style={{width: 300, height: 200 }}>
    <Node style={{ position: "absolute", width: "80%", height: "80%", left: "10%", top: "10%" }} />
  </Node>
</Layout>
NickGerleman commented 4 months ago

The playground example is giving a root node with explicit dimensions, then calculating layout with infinite free space available, so it makes sense it could give a different answer if this bug is specific to behavior on root node.

Interesting that the result is as if width and height were swapped when calculating inset percentage reference. We’ve had bugs before with getting confused on flex direction, which could maybe cause this.

@joevilches going to assign your way, since you worked on the absolute positioning code recently.

joevilches commented 4 months ago

@robbert-ef thanks for the report! I was able to verify the problem and it seems to be a bit more general than you described. We get this issue when we have the following:

Neither the position type nor the fact that this is the root matters (actually, when we are not working on the root, absolute position will be correct). Your second example works because the nodes have the same flex direction. The "flex direction" of the root's owner is undefined, but the relevant code seems to assume "row" while the default flex-direction is column. Ill put up a fix here soon

joevilches commented 3 months ago

The attached commit should fix the issue