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

Fix issue where % width would be wrong if physical and relative padding defined on parent (#1662)

Browse Source 
Summary:
X-link: https://github.com/facebook/react-native/pull/44791

Pull Request resolved: https://github.com/facebook/yoga/pull/1662

This should fix https://github.com/facebook/yoga/issues/1657. Rather insidious bug but we had code like

```
  // The total padding/border for a given axis does not depend on the direction
  // so hardcoding LTR here to avoid piping direction to this function
  return node->style().computeInlineStartPaddingAndBorder(
             axis, Direction::LTR, widthSize) +
      node->style().computeInlineEndPaddingAndBorder(
          axis, Direction::LTR, widthSize);
```

That comment is NOT true if someone sets both the physical edge and relative edge. So like paddingLeft and paddingEnd for RTL. This diff simply pipes the direction to that spot to use instead of hardcoding LTR. Every file changed is just to pipe `direction`.

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D58169843

fbshipit-source-id: 5b4854dddc019285076bd06955557edf73ef7ec5
This commit is contained in:
Joe Vilches
2024-06-10 18:25:19 -07:00
committed by Facebook GitHub Bot
parent fb53cb7443
commit 72b7e5b5cf
7 changed files with 212 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
gentest/fixtures/YGPaddingTest.html
View File

@@ -13,10 +13,17 @@
<div style="height: 10px;"></div>
</div>
<div id="padding_center_child" style="width: 100px; height: 100px; padding-start: 10px; padding-top: 10; padding-end: 20px; padding-bottom: 20px; align-items: center; justify-content: center;">
<div id="padding_center_child"
style="width: 100px; height: 100px; padding-start: 10px; padding-top: 10; padding-end: 20px; padding-bottom: 20px; align-items: center; justify-content: center;">
<div style="height: 10px; width: 10px;"></div>
</div>
<div id="child_with_padding_align_end" style="width: 200px; height: 200px; justify-content: flex-end; align-items: flex-end;">
<div id="child_with_padding_align_end"
style="width: 200px; height: 200px; justify-content: flex-end; align-items: flex-end;">
<div style="width: 100px; height: 100px; padding: 20px;"></div>
</div>
<div id="physical_and_relative_edge_defined"
style="width: 200px; height: 200px; padding-left: 20px; padding-end: 50px;">
<div style="width: 100%; height: 50px;"></div>
</div>

44
java/tests/com/facebook/yoga/YGPaddingTest.java
View File

@@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<176b6a98b0b08ef3f3fd20289e8fd089>>
* @generated SignedSource<<e7b2dc11dc748322103e0c8d1196dc64>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGPaddingTest.html
*/
@@ -273,6 +273,48 @@ public class YGPaddingTest {
assertEquals(100f, root_child0.getLayoutHeight(), 0.0f);
}
@Test
public void test_physical_and_relative_edge_defined() {
YogaConfig config = YogaConfigFactory.create();
final YogaNode root = createNode(config);
root.setPositionType(YogaPositionType.ABSOLUTE);
root.setPadding(YogaEdge.LEFT, 20);
root.setPadding(YogaEdge.END, 50);
root.setWidth(200f);
root.setHeight(200f);
final YogaNode root_child0 = createNode(config);
root_child0.setWidthPercent(100f);
root_child0.setHeight(50f);
root.addChildAt(root_child0, 0);
root.setDirection(YogaDirection.LTR);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(200f, root.getLayoutWidth(), 0.0f);
assertEquals(200f, root.getLayoutHeight(), 0.0f);
assertEquals(20f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(130f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(50f, root_child0.getLayoutHeight(), 0.0f);
root.setDirection(YogaDirection.RTL);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(200f, root.getLayoutWidth(), 0.0f);
assertEquals(200f, root.getLayoutHeight(), 0.0f);
assertEquals(50f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(150f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(50f, root_child0.getLayoutHeight(), 0.0f);
}
private YogaNode createNode(YogaConfig config) {
return mNodeFactory.create(config);
}

49
javascript/tests/generated/YGPaddingTest.test.ts
View File

@@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<a14ac3da5d4dc41e829f7820349c288a>>
* @generated SignedSource<<688d750852e836fc6403042d89d5ab51>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGPaddingTest.html
*/
@@ -303,3 +303,50 @@ test('child_with_padding_align_end', () => {
config.free();
}
});
test('physical_and_relative_edge_defined', () => {
const config = Yoga.Config.create();
let root;
try {
root = Yoga.Node.create(config);
root.setPositionType(PositionType.Absolute);
root.setPadding(Edge.Left, 20);
root.setPadding(Edge.End, 50);
root.setWidth(200);
root.setHeight(200);
const root_child0 = Yoga.Node.create(config);
root_child0.setWidth("100%");
root_child0.setHeight(50);
root.insertChild(root_child0, 0);
root.calculateLayout(undefined, undefined, Direction.LTR);
expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(200);
expect(root.getComputedHeight()).toBe(200);
expect(root_child0.getComputedLeft()).toBe(20);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(130);
expect(root_child0.getComputedHeight()).toBe(50);
root.calculateLayout(undefined, undefined, Direction.RTL);
expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(200);
expect(root.getComputedHeight()).toBe(200);
expect(root_child0.getComputedLeft()).toBe(50);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(150);
expect(root_child0.getComputedHeight()).toBe(50);
} finally {
if (typeof root !== 'undefined') {
root.freeRecursive();
}
config.free();
}
});

45
tests/generated/YGPaddingTest.cpp
View File

@@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*
* clang-format off
* @generated SignedSource<<32fe835176961bebfe905dfd47874f5c>>
* @generated SignedSource<<f1bb0b5643b1db2ff78e5c327f45f0f3>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGPaddingTest.html
*/
@@ -264,3 +264,46 @@ TEST(YogaTest, child_with_padding_align_end) {
YGConfigFree(config);
}
TEST(YogaTest, physical_and_relative_edge_defined) {
const YGConfigRef config = YGConfigNew();
const YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);
YGNodeStyleSetPadding(root, YGEdgeLeft, 20);
YGNodeStyleSetPadding(root, YGEdgeEnd, 50);
YGNodeStyleSetWidth(root, 200);
YGNodeStyleSetHeight(root, 200);
const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetWidthPercent(root_child0, 100);
YGNodeStyleSetHeight(root_child0, 50);
YGNodeInsertChild(root, root_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(200, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(130, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0));
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(200, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(150, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0));
YGNodeFreeRecursive(root);
YGConfigFree(config);
}

2
yoga/algorithm/AbsoluteLayout.cpp
View File

@@ -342,6 +342,7 @@ void layoutAbsoluteChild(
childWidth = boundAxis(
child,
FlexDirection::Row,
direction,
childWidth,
containingBlockWidth,
containingBlockWidth);
@@ -373,6 +374,7 @@ void layoutAbsoluteChild(
childHeight = boundAxis(
child,
FlexDirection::Column,
direction,
childHeight,
containingBlockHeight,
containingBlockWidth);

10
yoga/algorithm/BoundAxis.h
View File

@@ -19,13 +19,12 @@ namespace facebook::yoga {
inline float paddingAndBorderForAxis(
const yoga::Node* const node,
const FlexDirection axis,
const Direction direction,
const float widthSize) {
// The total padding/border for a given axis does not depend on the direction
// so hardcoding LTR here to avoid piping direction to this function
return node->style().computeInlineStartPaddingAndBorder(
axis, Direction::LTR, widthSize) +
axis, direction, widthSize) +
node->style().computeInlineEndPaddingAndBorder(
axis, Direction::LTR, widthSize);
axis, direction, widthSize);
}
inline FloatOptional boundAxisWithinMinAndMax(
@@ -60,13 +59,14 @@ inline FloatOptional boundAxisWithinMinAndMax(
inline float boundAxis(
const yoga::Node* const node,
const FlexDirection axis,
const Direction direction,
const float value,
const float axisSize,
const float widthSize) {
return yoga::maxOrDefined(
boundAxisWithinMinAndMax(node, axis, FloatOptional{value}, axisSize)
.unwrap(),
paddingAndBorderForAxis(node, axis, widthSize));
paddingAndBorderForAxis(node, axis, direction, widthSize));
}
} // namespace facebook::yoga

76
yoga/algorithm/CalculateLayout.cpp
View File

@@ -97,23 +97,25 @@ static void computeFlexBasisForChild(
(child->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::WebFlexBasis) &&
child->getLayout().computedFlexBasisGeneration != generationCount)) {
const FloatOptional paddingAndBorder =
FloatOptional(paddingAndBorderForAxis(child, mainAxis, ownerWidth));
const FloatOptional paddingAndBorder = FloatOptional(
paddingAndBorderForAxis(child, mainAxis, direction, ownerWidth));
child->setLayoutComputedFlexBasis(
yoga::maxOrDefined(resolvedFlexBasis, paddingAndBorder));
}
} else if (isMainAxisRow && isRowStyleDimDefined) {
// The width is definite, so use that as the flex basis.
const FloatOptional paddingAndBorder = FloatOptional(
paddingAndBorderForAxis(child, FlexDirection::Row, ownerWidth));
const FloatOptional paddingAndBorder =
FloatOptional(paddingAndBorderForAxis(
child, FlexDirection::Row, direction, ownerWidth));
child->setLayoutComputedFlexBasis(yoga::maxOrDefined(
child->getResolvedDimension(Dimension::Width).resolve(ownerWidth),
paddingAndBorder));
} else if (!isMainAxisRow && isColumnStyleDimDefined) {
// The height is definite, so use that as the flex basis.
const FloatOptional paddingAndBorder = FloatOptional(
paddingAndBorderForAxis(child, FlexDirection::Column, ownerWidth));
const FloatOptional paddingAndBorder =
FloatOptional(paddingAndBorderForAxis(
child, FlexDirection::Column, direction, ownerWidth));
child->setLayoutComputedFlexBasis(yoga::maxOrDefined(
child->getResolvedDimension(Dimension::Height).resolve(ownerHeight),
paddingAndBorder));
@@ -244,13 +246,14 @@ static void computeFlexBasisForChild(
child->setLayoutComputedFlexBasis(FloatOptional(yoga::maxOrDefined(
child->getLayout().measuredDimension(dimension(mainAxis)),
paddingAndBorderForAxis(child, mainAxis, ownerWidth))));
paddingAndBorderForAxis(child, mainAxis, direction, ownerWidth))));
}
child->setLayoutComputedFlexBasisGeneration(generationCount);
}
static void measureNodeWithMeasureFunc(
yoga::Node* const node,
const Direction direction,
float availableWidth,
float availableHeight,
const SizingMode widthSizingMode,
@@ -292,12 +295,18 @@ static void measureNodeWithMeasureFunc(
// Don't bother sizing the text if both dimensions are already defined.
node->setLayoutMeasuredDimension(
boundAxis(
node, FlexDirection::Row, availableWidth, ownerWidth, ownerWidth),
node,
FlexDirection::Row,
direction,
availableWidth,
ownerWidth,
ownerWidth),
Dimension::Width);
node->setLayoutMeasuredDimension(
boundAxis(
node,
FlexDirection::Column,
direction,
availableHeight,
ownerHeight,
ownerWidth),
@@ -330,6 +339,7 @@ static void measureNodeWithMeasureFunc(
boundAxis(
node,
FlexDirection::Row,
direction,
(widthSizingMode == SizingMode::MaxContent ||
widthSizingMode == SizingMode::FitContent)
? measuredSize.width + paddingAndBorderAxisRow
@@ -342,6 +352,7 @@ static void measureNodeWithMeasureFunc(
boundAxis(
node,
FlexDirection::Column,
direction,
(heightSizingMode == SizingMode::MaxContent ||
heightSizingMode == SizingMode::FitContent)
? measuredSize.height + paddingAndBorderAxisColumn
@@ -356,6 +367,7 @@ static void measureNodeWithMeasureFunc(
// or the minimum size as indicated by the padding and border sizes.
static void measureNodeWithoutChildren(
yoga::Node* const node,
const Direction direction,
const float availableWidth,
const float availableHeight,
const SizingMode widthSizingMode,
@@ -372,7 +384,8 @@ static void measureNodeWithoutChildren(
layout.border(PhysicalEdge::Left) + layout.border(PhysicalEdge::Right);
}
node->setLayoutMeasuredDimension(
boundAxis(node, FlexDirection::Row, width, ownerWidth, ownerWidth),
boundAxis(
node, FlexDirection::Row, direction, width, ownerWidth, ownerWidth),
Dimension::Width);
float height = availableHeight;
@@ -383,12 +396,19 @@ static void measureNodeWithoutChildren(
layout.border(PhysicalEdge::Top) + layout.border(PhysicalEdge::Bottom);
}
node->setLayoutMeasuredDimension(
boundAxis(node, FlexDirection::Column, height, ownerHeight, ownerWidth),
boundAxis(
node,
FlexDirection::Column,
direction,
height,
ownerHeight,
ownerWidth),
Dimension::Height);
}
static bool measureNodeWithFixedSize(
yoga::Node* const node,
const Direction direction,
const float availableWidth,
const float availableHeight,
const SizingMode widthSizingMode,
@@ -405,6 +425,7 @@ static bool measureNodeWithFixedSize(
boundAxis(
node,
FlexDirection::Row,
direction,
yoga::isUndefined(availableWidth) ||
(widthSizingMode == SizingMode::FitContent &&
availableWidth < 0.0f)
@@ -418,6 +439,7 @@ static bool measureNodeWithFixedSize(
boundAxis(
node,
FlexDirection::Column,
direction,
yoga::isUndefined(availableHeight) ||
(heightSizingMode == SizingMode::FitContent &&
availableHeight < 0.0f)
@@ -619,6 +641,7 @@ static float distributeFreeSpaceSecondPass(
updatedMainSize = boundAxis(
currentLineChild,
mainAxis,
direction,
childSize,
availableInnerMainDim,
availableInnerWidth);
@@ -633,6 +656,7 @@ static float distributeFreeSpaceSecondPass(
updatedMainSize = boundAxis(
currentLineChild,
mainAxis,
direction,
childFlexBasis +
flexLine.layout.remainingFreeSpace /
flexLine.layout.totalFlexGrowFactors * flexGrowFactor,
@@ -756,6 +780,7 @@ static float distributeFreeSpaceSecondPass(
// is removed from the remaingfreespace.
static void distributeFreeSpaceFirstPass(
FlexLine& flexLine,
const Direction direction,
const FlexDirection mainAxis,
const float mainAxisownerSize,
const float availableInnerMainDim,
@@ -788,6 +813,7 @@ static void distributeFreeSpaceFirstPass(
boundMainSize = boundAxis(
currentLineChild,
mainAxis,
direction,
baseMainSize,
availableInnerMainDim,
availableInnerWidth);
@@ -816,6 +842,7 @@ static void distributeFreeSpaceFirstPass(
boundMainSize = boundAxis(
currentLineChild,
mainAxis,
direction,
baseMainSize,
availableInnerMainDim,
availableInnerWidth);
@@ -878,6 +905,7 @@ static void resolveFlexibleLength(
// First pass: detect the flex items whose min/max constraints trigger
distributeFreeSpaceFirstPass(
flexLine,
direction,
mainAxis,
mainAxisownerSize,
availableInnerMainDim,
@@ -1261,6 +1289,7 @@ static void calculateLayoutImpl(
if (node->hasMeasureFunc()) {
measureNodeWithMeasureFunc(
node,
direction,
availableWidth - marginAxisRow,
availableHeight - marginAxisColumn,
widthSizingMode,
@@ -1276,6 +1305,7 @@ static void calculateLayoutImpl(
if (childCount == 0) {
measureNodeWithoutChildren(
node,
direction,
availableWidth - marginAxisRow,
availableHeight - marginAxisColumn,
widthSizingMode,
@@ -1290,6 +1320,7 @@ static void calculateLayoutImpl(
if (!performLayout &&
measureNodeWithFixedSize(
node,
direction,
availableWidth - marginAxisRow,
availableHeight - marginAxisColumn,
widthSizingMode,
@@ -1316,9 +1347,9 @@ static void calculateLayoutImpl(
const float crossAxisownerSize = isMainAxisRow ? ownerHeight : ownerWidth;
const float paddingAndBorderAxisMain =
paddingAndBorderForAxis(node, mainAxis, ownerWidth);
paddingAndBorderForAxis(node, mainAxis, direction, ownerWidth);
const float paddingAndBorderAxisCross =
paddingAndBorderForAxis(node, crossAxis, ownerWidth);
paddingAndBorderForAxis(node, crossAxis, direction, ownerWidth);
const float leadingPaddingAndBorderCross =
node->style().computeFlexStartPaddingAndBorder(
crossAxis, direction, ownerWidth);
@@ -1539,6 +1570,7 @@ static void calculateLayoutImpl(
boundAxis(
node,
crossAxis,
direction,
flexLine.layout.crossDim + paddingAndBorderAxisCross,
crossAxisownerSize,
ownerWidth) -
@@ -1559,6 +1591,7 @@ static void calculateLayoutImpl(
boundAxis(
node,
crossAxis,
direction,
flexLine.layout.crossDim + paddingAndBorderAxisCross,
crossAxisownerSize,
ownerWidth) -
@@ -1733,8 +1766,13 @@ static void calculateLayoutImpl(
.unwrap()
: totalLineCrossDim + paddingAndBorderAxisCross;
const float innerCrossDim =
boundAxis(node, crossAxis, unclampedCrossDim, ownerHeight, ownerWidth) -
const float innerCrossDim = boundAxis(
node,
crossAxis,
direction,
unclampedCrossDim,
ownerHeight,
ownerWidth) -
paddingAndBorderAxisCross;
const float remainingAlignContentDim = innerCrossDim - totalLineCrossDim;
@@ -1935,6 +1973,7 @@ static void calculateLayoutImpl(
boundAxis(
node,
FlexDirection::Row,
direction,
availableWidth - marginAxisRow,
ownerWidth,
ownerWidth),
@@ -1944,6 +1983,7 @@ static void calculateLayoutImpl(
boundAxis(
node,
FlexDirection::Column,
direction,
availableHeight - marginAxisColumn,
ownerHeight,
ownerWidth),
@@ -1958,7 +1998,12 @@ static void calculateLayoutImpl(
// doesn't go below the padding and border amount.
node->setLayoutMeasuredDimension(
boundAxis(
node, mainAxis, maxLineMainDim, mainAxisownerSize, ownerWidth),
node,
mainAxis,
direction,
maxLineMainDim,
mainAxisownerSize,
ownerWidth),
dimension(mainAxis));
} else if (
@@ -1987,6 +2032,7 @@ static void calculateLayoutImpl(
boundAxis(
node,
crossAxis,
direction,
totalLineCrossDim + paddingAndBorderAxisCross,
crossAxisownerSize,
ownerWidth),