Skip to content

Commit

Permalink
Fix issue where padding for box sizing is resolved against wrong refe…
Browse files Browse the repository at this point in the history
…rence length (#46799)

Summary:
X-link: facebook/yoga#1715


Content box impl had a bug where we resolved padding % against the same reference length as the dimensions. Padding should always be against containing block's width. This is also true for width, but not for height, which should be against containing block's height.

This just pipes the width into our box sizing functions.

Changelog: [Internal}

Differential Revision: D63787577
  • Loading branch information
joevilches authored and facebook-github-bot committed Oct 3, 2024
1 parent 211534b commit f00b8b3
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,10 @@ void layoutAbsoluteChild(
if (child->hasDefiniteLength(Dimension::Width, containingBlockWidth)) {
childWidth = child
->getResolvedDimension(
direction, Dimension::Width, containingBlockWidth)
direction,
Dimension::Width,
containingBlockWidth,
containingBlockWidth)
.unwrap() +
marginRow;
} else {
Expand Down Expand Up @@ -362,7 +365,10 @@ void layoutAbsoluteChild(
if (child->hasDefiniteLength(Dimension::Height, containingBlockHeight)) {
childHeight = child
->getResolvedDimension(
direction, Dimension::Height, containingBlockHeight)
direction,
Dimension::Height,
containingBlockHeight,
containingBlockWidth)
.unwrap() +
marginColumn;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,21 @@ inline FloatOptional boundAxisWithinMinAndMax(
const Direction direction,
const FlexDirection axis,
const FloatOptional value,
const float axisSize) {
const float axisSize,
const float widthSize) {
FloatOptional min;
FloatOptional max;

if (isColumn(axis)) {
min = node->style().resolvedMinDimension(
direction, Dimension::Height, axisSize);
direction, Dimension::Height, axisSize, widthSize);
max = node->style().resolvedMaxDimension(
direction, Dimension::Height, axisSize);
direction, Dimension::Height, axisSize, widthSize);
} else if (isRow(axis)) {
min = node->style().resolvedMinDimension(
direction, Dimension::Width, axisSize);
direction, Dimension::Width, axisSize, widthSize);
max = node->style().resolvedMaxDimension(
direction, Dimension::Width, axisSize);
direction, Dimension::Width, axisSize, widthSize);
}

if (max >= FloatOptional{0} && value > max) {
Expand All @@ -70,7 +71,7 @@ inline float boundAxis(
const float widthSize) {
return yoga::maxOrDefined(
boundAxisWithinMinAndMax(
node, direction, axis, FloatOptional{value}, axisSize)
node, direction, axis, FloatOptional{value}, axisSize, widthSize)
.unwrap(),
paddingAndBorderForAxis(node, axis, direction, widthSize));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ static void constrainMaxSizeForMode(
float ownerWidth,
/*in_out*/ SizingMode* mode,
/*in_out*/ float* size) {
const FloatOptional maxSize = node->style().resolvedMaxDimension(
direction, dimension(axis), ownerAxisSize) +
const FloatOptional maxSize =
node->style().resolvedMaxDimension(
direction, dimension(axis), ownerAxisSize, ownerWidth) +
FloatOptional(node->style().computeMarginForAxis(axis, ownerWidth));
switch (*mode) {
case SizingMode::StretchFit:
Expand Down Expand Up @@ -87,8 +88,8 @@ static void computeFlexBasisForChild(
SizingMode childWidthSizingMode;
SizingMode childHeightSizingMode;

const FloatOptional resolvedFlexBasis =
child->resolveFlexBasis(direction, mainAxis, mainAxisOwnerSize);
const FloatOptional resolvedFlexBasis = child->resolveFlexBasis(
direction, mainAxis, mainAxisOwnerSize, ownerWidth);
const bool isRowStyleDimDefined =
child->hasDefiniteLength(Dimension::Width, ownerWidth);
const bool isColumnStyleDimDefined =
Expand All @@ -111,15 +112,17 @@ static void computeFlexBasisForChild(
child, FlexDirection::Row, direction, ownerWidth));

child->setLayoutComputedFlexBasis(yoga::maxOrDefined(
child->getResolvedDimension(direction, Dimension::Width, ownerWidth),
child->getResolvedDimension(
direction, Dimension::Width, ownerWidth, 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, direction, ownerWidth));
child->setLayoutComputedFlexBasis(yoga::maxOrDefined(
child->getResolvedDimension(direction, Dimension::Height, ownerHeight),
child->getResolvedDimension(
direction, Dimension::Height, ownerHeight, ownerWidth),
paddingAndBorder));
} else {
// Compute the flex basis and hypothetical main size (i.e. the clamped flex
Expand All @@ -133,15 +136,18 @@ static void computeFlexBasisForChild(
child->style().computeMarginForAxis(FlexDirection::Column, ownerWidth);

if (isRowStyleDimDefined) {
childWidth =
child->getResolvedDimension(direction, Dimension::Width, ownerWidth)
.unwrap() +
childWidth = child
->getResolvedDimension(
direction, Dimension::Width, ownerWidth, ownerWidth)
.unwrap() +
marginRow;
childWidthSizingMode = SizingMode::StretchFit;
}
if (isColumnStyleDimDefined) {
childHeight =
child->getResolvedDimension(direction, Dimension::Height, ownerHeight)
child
->getResolvedDimension(
direction, Dimension::Height, ownerHeight, ownerWidth)
.unwrap() +
marginColumn;
childHeightSizingMode = SizingMode::StretchFit;
Expand Down Expand Up @@ -476,21 +482,24 @@ static float calculateAvailableInnerDimension(
const Dimension dimension,
const float availableDim,
const float paddingAndBorder,
const float ownerDim) {
const float ownerDim,
const float ownerWidth) {
float availableInnerDim = availableDim - paddingAndBorder;
// Max dimension overrides predefined dimension value; Min dimension in turn
// overrides both of the above
if (yoga::isDefined(availableInnerDim)) {
// We want to make sure our available height does not violate min and max
// constraints
const FloatOptional minDimensionOptional =
node->style().resolvedMinDimension(direction, dimension, ownerDim);
node->style().resolvedMinDimension(
direction, dimension, ownerDim, ownerWidth);
const float minInnerDim = minDimensionOptional.isUndefined()
? 0.0f
: minDimensionOptional.unwrap() - paddingAndBorder;

const FloatOptional maxDimensionOptional =
node->style().resolvedMaxDimension(direction, dimension, ownerDim);
node->style().resolvedMaxDimension(
direction, dimension, ownerDim, ownerWidth);

const float maxInnerDim = maxDimensionOptional.isUndefined()
? FLT_MAX
Expand Down Expand Up @@ -594,6 +603,7 @@ static float distributeFreeSpaceSecondPass(
const FlexDirection mainAxis,
const FlexDirection crossAxis,
const Direction direction,
const float ownerWidth,
const float mainAxisOwnerSize,
const float availableInnerMainDim,
const float availableInnerCrossDim,
Expand All @@ -618,7 +628,8 @@ static float distributeFreeSpaceSecondPass(
direction,
mainAxis,
currentLineChild->getLayout().computedFlexBasis,
mainAxisOwnerSize)
mainAxisOwnerSize,
ownerWidth)
.unwrap();
float updatedMainSize = childFlexBasis;

Expand Down Expand Up @@ -713,11 +724,13 @@ static float distributeFreeSpaceSecondPass(
? SizingMode::MaxContent
: SizingMode::FitContent;
} else {
childCrossSize =
currentLineChild
->getResolvedDimension(
direction, dimension(crossAxis), availableInnerCrossDim)
.unwrap() +
childCrossSize = currentLineChild
->getResolvedDimension(
direction,
dimension(crossAxis),
availableInnerCrossDim,
availableInnerWidth)
.unwrap() +
marginCross;
const bool isLoosePercentageMeasurement =
currentLineChild->getProcessedDimension(dimension(crossAxis))
Expand Down Expand Up @@ -808,6 +821,7 @@ static void distributeFreeSpaceFirstPass(
FlexLine& flexLine,
const Direction direction,
const FlexDirection mainAxis,
const float ownerWidth,
const float mainAxisOwnerSize,
const float availableInnerMainDim,
const float availableInnerWidth) {
Expand All @@ -823,7 +837,8 @@ static void distributeFreeSpaceFirstPass(
direction,
mainAxis,
currentLineChild->getLayout().computedFlexBasis,
mainAxisOwnerSize)
mainAxisOwnerSize,
ownerWidth)
.unwrap();

if (flexLine.layout.remainingFreeSpace < 0) {
Expand Down Expand Up @@ -917,6 +932,7 @@ static void resolveFlexibleLength(
const FlexDirection mainAxis,
const FlexDirection crossAxis,
const Direction direction,
const float ownerWidth,
const float mainAxisOwnerSize,
const float availableInnerMainDim,
const float availableInnerCrossDim,
Expand All @@ -934,6 +950,7 @@ static void resolveFlexibleLength(
flexLine,
direction,
mainAxis,
ownerWidth,
mainAxisOwnerSize,
availableInnerMainDim,
availableInnerWidth);
Expand All @@ -945,6 +962,7 @@ static void resolveFlexibleLength(
mainAxis,
crossAxis,
direction,
ownerWidth,
mainAxisOwnerSize,
availableInnerMainDim,
availableInnerCrossDim,
Expand Down Expand Up @@ -993,7 +1011,7 @@ static void justifyMainAxis(
if (style.minDimension(dimension(mainAxis)).isDefined() &&
style
.resolvedMinDimension(
direction, dimension(mainAxis), mainAxisOwnerSize)
direction, dimension(mainAxis), mainAxisOwnerSize, ownerWidth)
.isDefined()) {
// This condition makes sure that if the size of main dimension(after
// considering child nodes main dim, leading and trailing padding etc)
Expand All @@ -1005,7 +1023,7 @@ static void justifyMainAxis(
const float minAvailableMainDim =
style
.resolvedMinDimension(
direction, dimension(mainAxis), mainAxisOwnerSize)
direction, dimension(mainAxis), mainAxisOwnerSize, ownerWidth)
.unwrap() -
leadingPaddingAndBorderMain - trailingPaddingAndBorderMain;
const float occupiedSpaceByChildNodes =
Expand Down Expand Up @@ -1403,14 +1421,16 @@ static void calculateLayoutImpl(
Dimension::Width,
availableWidth - marginAxisRow,
paddingAndBorderAxisRow,
ownerWidth,
ownerWidth);
float availableInnerHeight = calculateAvailableInnerDimension(
node,
direction,
Dimension::Height,
availableHeight - marginAxisColumn,
paddingAndBorderAxisColumn,
ownerHeight);
ownerHeight,
ownerWidth);

float availableInnerMainDim =
isMainAxisRow ? availableInnerWidth : availableInnerHeight;
Expand Down Expand Up @@ -1470,6 +1490,7 @@ static void calculateLayoutImpl(
auto flexLine = calculateFlexLine(
node,
ownerDirection,
ownerWidth,
mainAxisOwnerSize,
availableInnerWidth,
availableInnerMainDim,
Expand All @@ -1494,19 +1515,27 @@ static void calculateLayoutImpl(
if (sizingModeMainDim != SizingMode::StretchFit) {
const auto& style = node->style();
const float minInnerWidth =
style.resolvedMinDimension(direction, Dimension::Width, ownerWidth)
style
.resolvedMinDimension(
direction, Dimension::Width, ownerWidth, ownerWidth)
.unwrap() -
paddingAndBorderAxisRow;
const float maxInnerWidth =
style.resolvedMaxDimension(direction, Dimension::Width, ownerWidth)
style
.resolvedMaxDimension(
direction, Dimension::Width, ownerWidth, ownerWidth)
.unwrap() -
paddingAndBorderAxisRow;
const float minInnerHeight =
style.resolvedMinDimension(direction, Dimension::Height, ownerHeight)
style
.resolvedMinDimension(
direction, Dimension::Height, ownerHeight, ownerWidth)
.unwrap() -
paddingAndBorderAxisColumn;
const float maxInnerHeight =
style.resolvedMaxDimension(direction, Dimension::Height, ownerHeight)
style
.resolvedMaxDimension(
direction, Dimension::Height, ownerHeight, ownerWidth)
.unwrap() -
paddingAndBorderAxisColumn;

Expand Down Expand Up @@ -1559,6 +1588,7 @@ static void calculateLayoutImpl(
mainAxis,
crossAxis,
direction,
ownerWidth,
mainAxisOwnerSize,
availableInnerMainDim,
availableInnerCrossDim,
Expand Down Expand Up @@ -1802,7 +1832,10 @@ static void calculateLayoutImpl(
? availableInnerCrossDim + paddingAndBorderAxisCross
: node->hasDefiniteLength(dimension(crossAxis), crossAxisOwnerSize)
? node->getResolvedDimension(
direction, dimension(crossAxis), crossAxisOwnerSize)
direction,
dimension(crossAxis),
crossAxisOwnerSize,
ownerWidth)
.unwrap()
: totalLineCrossDim + paddingAndBorderAxisCross;

Expand Down Expand Up @@ -2058,7 +2091,8 @@ static void calculateLayoutImpl(
direction,
mainAxis,
FloatOptional{maxLineMainDim},
mainAxisOwnerSize)
mainAxisOwnerSize,
ownerWidth)
.unwrap()),
paddingAndBorderAxisMain),
dimension(mainAxis));
Expand Down Expand Up @@ -2092,7 +2126,8 @@ static void calculateLayoutImpl(
crossAxis,
FloatOptional{
totalLineCrossDim + paddingAndBorderAxisCross},
crossAxisOwnerSize)
crossAxisOwnerSize,
ownerWidth)
.unwrap()),
paddingAndBorderAxisCross),
dimension(crossAxis));
Expand Down Expand Up @@ -2384,13 +2419,20 @@ void calculateLayout(
if (node->hasDefiniteLength(Dimension::Width, ownerWidth)) {
width =
(node->getResolvedDimension(
direction, dimension(FlexDirection::Row), ownerWidth)
direction,
dimension(FlexDirection::Row),
ownerWidth,
ownerWidth)
.unwrap() +
node->style().computeMarginForAxis(FlexDirection::Row, ownerWidth));
widthSizingMode = SizingMode::StretchFit;
} else if (style.resolvedMaxDimension(direction, Dimension::Width, ownerWidth)
} else if (style
.resolvedMaxDimension(
direction, Dimension::Width, ownerWidth, ownerWidth)
.isDefined()) {
width = style.resolvedMaxDimension(direction, Dimension::Width, ownerWidth)
width = style
.resolvedMaxDimension(
direction, Dimension::Width, ownerWidth, ownerWidth)
.unwrap();
widthSizingMode = SizingMode::FitContent;
} else {
Expand All @@ -2404,17 +2446,21 @@ void calculateLayout(
if (node->hasDefiniteLength(Dimension::Height, ownerHeight)) {
height =
(node->getResolvedDimension(
direction, dimension(FlexDirection::Column), ownerHeight)
direction,
dimension(FlexDirection::Column),
ownerHeight,
ownerWidth)
.unwrap() +
node->style().computeMarginForAxis(FlexDirection::Column, ownerWidth));
heightSizingMode = SizingMode::StretchFit;
} else if (style
.resolvedMaxDimension(
direction, Dimension::Height, ownerHeight)
direction, Dimension::Height, ownerHeight, ownerWidth)
.isDefined()) {
height =
style.resolvedMaxDimension(direction, Dimension::Height, ownerHeight)
.unwrap();
height = style
.resolvedMaxDimension(
direction, Dimension::Height, ownerHeight, ownerWidth)
.unwrap();
heightSizingMode = SizingMode::FitContent;
} else {
height = ownerHeight;
Expand Down
Loading

0 comments on commit f00b8b3

Please sign in to comment.