From 7a45fb39bff870f35967d5e0ae69212587f35020 Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Tue, 21 Aug 2018 17:23:25 -0700 Subject: [PATCH] Solve the bug related to baseline height Summary: This diff fixes the height calculation logic for the nodes with baseline. Before height calculation for baseline was done at wrong place. The task was created due to the regression caused by D9219678. Reviewed By: IanChilds Differential Revision: D9421551 fbshipit-source-id: 3fbb738314130b346c4186ec45d00c9ea63bc9f4 --- tests/YGAlignBaselineTest.cpp | 70 +++++++++++++++++++++++++++++++++++ yoga/Yoga.cpp | 52 ++++++++++++++++---------- 2 files changed, 103 insertions(+), 19 deletions(-) diff --git a/tests/YGAlignBaselineTest.cpp b/tests/YGAlignBaselineTest.cpp index 477ecb48..e5981ed4 100644 --- a/tests/YGAlignBaselineTest.cpp +++ b/tests/YGAlignBaselineTest.cpp @@ -14,6 +14,76 @@ _baselineFunc(YGNodeRef node, const float width, const float height) { return height / 2; } +static YGSize _measure1( + YGNodeRef node, + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { + return YGSize{ + .width = 42, + .height = 50, + }; +} + +static YGSize _measure2( + YGNodeRef node, + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { + return YGSize{ + .width = 279, + .height = 126, + }; +} + +// Test case for bug in T32999822 +TEST(YogaTest, align_baseline_parent_ht_not_specified) { + YGConfigRef config = YGConfigNew(); + + const YGNodeRef root = YGNodeNewWithConfig(config); + YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow); + YGNodeStyleSetAlignContent(root, YGAlignStretch); + YGNodeStyleSetAlignItems(root, YGAlignBaseline); + YGNodeStyleSetWidth(root, 340); + YGNodeStyleSetMaxHeight(root, 170); + YGNodeStyleSetMinHeight(root, 0); + + const YGNodeRef root_child0 = YGNodeNewWithConfig(config); + YGNodeStyleSetFlexGrow(root_child0, 0); + YGNodeStyleSetFlexShrink(root_child0, 1); + YGNodeSetMeasureFunc(root_child0, _measure1); + YGNodeInsertChild(root, root_child0, 0); + + const YGNodeRef root_child1 = YGNodeNewWithConfig(config); + YGNodeStyleSetFlexGrow(root_child1, 0); + YGNodeStyleSetFlexShrink(root_child1, 1); + YGNodeSetMeasureFunc(root_child1, _measure2); + YGNodeInsertChild(root, root_child1, 1); + + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root)); + ASSERT_FLOAT_EQ(340, YGNodeLayoutGetWidth(root)); + ASSERT_FLOAT_EQ(126, YGNodeLayoutGetHeight(root)); + + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0)); + ASSERT_FLOAT_EQ(42, YGNodeLayoutGetWidth(root_child0)); + ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0)); + ASSERT_FLOAT_EQ(76, YGNodeLayoutGetTop(root_child0)); + + ASSERT_FLOAT_EQ(42, YGNodeLayoutGetLeft(root_child1)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1)); + ASSERT_FLOAT_EQ(279, YGNodeLayoutGetWidth(root_child1)); + ASSERT_FLOAT_EQ(126, YGNodeLayoutGetHeight(root_child1)); + + YGNodeFreeRecursive(root); + + YGConfigFree(config); +} + TEST(YogaTest, align_baseline_with_no_parent_ht) { YGConfigRef config = YGConfigNew(); diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index 621564f4..c6d38bb4 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -2511,6 +2511,9 @@ static void YGJustifyMainAxis( leadingPaddingAndBorderMain + leadingMainDim; collectedFlexItemsValues.crossDim = 0; + float maxAscentForCurrentLine = 0; + float maxDescentForCurrentLine = 0; + bool isNodeBaselineLayout = YGIsBaselineLayout(node); for (uint32_t i = startOfLineIndex; i < collectedFlexItemsValues.endOfLineIndex; i++) { @@ -2575,11 +2578,30 @@ static void YGJustifyMainAxis( collectedFlexItemsValues.mainDim += betweenMainDim + YGNodeDimWithMargin(child, mainAxis, availableInnerWidth); - // The cross dimension is the max of the elements dimension since - // there can only be one element in that cross dimension. - collectedFlexItemsValues.crossDim = YGFloatMax( - collectedFlexItemsValues.crossDim, - YGNodeDimWithMargin(child, crossAxis, availableInnerWidth)); + if (isNodeBaselineLayout) { + // If the child is baseline aligned then the cross dimension is + // calculated by adding maxAscent and maxDescent from the baseline. + const float ascent = YGBaseline(child) + + YGUnwrapFloatOptional(child->getLeadingMargin( + YGFlexDirectionColumn, availableInnerWidth)); + const float descent = + child->getLayout().measuredDimensions[YGDimensionHeight] + + YGUnwrapFloatOptional(child->getMarginForAxis( + YGFlexDirectionColumn, availableInnerWidth)) - + ascent; + + maxAscentForCurrentLine = + YGFloatMax(maxAscentForCurrentLine, ascent); + maxDescentForCurrentLine = + YGFloatMax(maxDescentForCurrentLine, descent); + } else { + // The cross dimension is the max of the elements dimension since + // there can only be one element in that cross dimension in the case + // when the items are not baseline aligned + collectedFlexItemsValues.crossDim = YGFloatMax( + collectedFlexItemsValues.crossDim, + YGNodeDimWithMargin(child, crossAxis, availableInnerWidth)); + } } } else if (performLayout) { child->setLayoutPosition( @@ -2590,6 +2612,11 @@ static void YGJustifyMainAxis( } } collectedFlexItemsValues.mainDim += trailingPaddingAndBorderMain; + + if (isNodeBaselineLayout) { + collectedFlexItemsValues.crossDim = + maxAscentForCurrentLine + maxDescentForCurrentLine; + } } // @@ -3187,9 +3214,9 @@ static void YGNodelayoutImpl( // STEP 8: MULTI-LINE CONTENT ALIGNMENT // currentLead stores the size of the cross dim - float currentLead = leadingPaddingAndBorderCross; if (performLayout && (lineCount > 1 || YGIsBaselineLayout(node))) { float crossDimLead = 0; + float currentLead = leadingPaddingAndBorderCross; if (!YGFloatIsUndefined(availableInnerCrossDim)) { const float remainingAlignContentDim = availableInnerCrossDim - totalLineCrossDim; @@ -3372,7 +3399,6 @@ static void YGNodelayoutImpl( } } } - currentLead += lineHeight; } } @@ -3452,18 +3478,6 @@ static void YGNodelayoutImpl( dim[crossAxis]); } - if (performLayout && - node->getStyle().dimensions[dim[crossAxis]].unit == YGUnitAuto && - node->getStyle().alignItems == YGAlignBaseline) { - node->setLayoutMeasuredDimension( - YGNodeBoundAxis( - node, - crossAxis, - currentLead + paddingAndBorderAxisRow, - crossAxisownerSize, - ownerWidth), - dim[crossAxis]); - } // As we only wrapped in normal direction yet, we need to reverse the // positions on wrap-reverse. if (performLayout && node->getStyle().flexWrap == YGWrapWrapReverse) {