Changeset 269840 in webkit


Ignore:
Timestamp:
Nov 16, 2020 1:50:48 AM (3 years ago)
Author:
svillar@igalia.com
Message:

[css-flex] Better naming from some methods
​https://bugs.webkit.org/show_bug.cgi?id=218745

Reviewed by Javier Fernandez.

This patch improves the naming of several confusing methods.

  • hasOrthogonalFlow(): was too generic. It was basically checking that the flexbox container main size was not the child's inline size. So we renamed

it to mainAxisIsChildInlineAxis() reverting the output. For most of the cases it was enough to negate the output of the method to have the same
check, but in other cases we decided to swap the if-else blocks as it was more convenient.

  • {main|cross}AxisLengthIsDefinite(): the name was misleading because you don't know whether the method is evaluating the child's length

or the container's (the flexbox). Replaced by child{Main|Cross}SizeIsDefinite().

  • setOverrideMainAxisContentSizeForChild(): this had 2 problems. First it should use overriding instead of override. Second the size which is

overriden is not the content size, it used to be but not anymore.

No new tests as there is no change in functionality.

  • rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::firstLineBaseline const):
(WebCore::RenderFlexibleBox::mainAxisIsChildInlineAxis const): Renamed from hasOrthogonalFlow(). Also the behaviour
is now reversed from the original function.
(WebCore::RenderFlexibleBox::childIntrinsicLogicalHeight const):
(WebCore::RenderFlexibleBox::childIntrinsicLogicalWidth const):
(WebCore::RenderFlexibleBox::crossAxisIntrinsicExtentForChild const):
(WebCore::RenderFlexibleBox::mainAxisContentExtent): Early return instead of if-else block.
(WebCore::RenderFlexibleBox::useChildAspectRatio const):
(WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const): Use a single line for division operators.
(WebCore::RenderFlexibleBox::childMainSizeIsDefinite const): Renamed from mainAxisLengthIsDefinite.
(WebCore::RenderFlexibleBox::childCrossSizeIsDefinite const): Renamed from crossAxisLengthIsDefinite.
(WebCore::RenderFlexibleBox::cacheChildMainSize):
(WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild):
(WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax):
(WebCore::RenderFlexibleBox::crossSizeForPercentageResolution):
(WebCore::RenderFlexibleBox::mainSizeForPercentageResolution):
(WebCore::RenderFlexibleBox::childLogicalHeightForPercentageResolution):
(WebCore::RenderFlexibleBox::adjustChildSizeForAspectRatioCrossAxisMinAndMax):
(WebCore::RenderFlexibleBox::setOverridingMainSizeForChild): Renamed from setOverrideMainAxisContentSizeForChild.
(WebCore::RenderFlexibleBox::alignmentForChild const):
(WebCore::RenderFlexibleBox::childHasIntrinsicMainAxisSize const):
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
(WebCore::RenderFlexibleBox::applyStretchAlignmentToChild):
(WebCore::RenderFlexibleBox::hasOrthogonalFlow const): Deleted.
(WebCore::RenderFlexibleBox::mainAxisLengthIsDefinite const): Deleted.
(WebCore::RenderFlexibleBox::crossAxisLengthIsDefinite const): Deleted.
(WebCore::RenderFlexibleBox::setOverrideMainAxisContentSizeForChild): Deleted.

  • rendering/RenderFlexibleBox.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r269839 r269840  
     12020-11-10  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-flex] Better naming from some methods
     4        https://bugs.webkit.org/show_bug.cgi?id=218745
     5
     6        Reviewed by Javier Fernandez.
     7
     8        This patch improves the naming of several confusing methods.
     9        * hasOrthogonalFlow(): was too generic. It was basically checking that the flexbox container main size was not the child's inline size. So we renamed
     10        it to mainAxisIsChildInlineAxis() reverting the output. For most of the cases it was enough to negate the output of the method to have the same
     11        check, but in other cases we decided to swap the if-else blocks as it was more convenient.
     12        * {main|cross}AxisLengthIsDefinite(): the name was misleading because you don't know whether the method is evaluating the child's length
     13        or the container's (the flexbox). Replaced by child{Main|Cross}SizeIsDefinite().
     14        * setOverrideMainAxisContentSizeForChild(): this had 2 problems. First it should use overriding instead of override. Second the size which is
     15        overriden is not the content size, it used to be but not anymore.
     16
     17        No new tests as there is no change in functionality.
     18
     19        * rendering/RenderFlexibleBox.cpp:
     20        (WebCore::RenderFlexibleBox::firstLineBaseline const):
     21        (WebCore::RenderFlexibleBox::mainAxisIsChildInlineAxis const): Renamed from hasOrthogonalFlow(). Also the behaviour
     22        is now reversed from the original function.
     23        (WebCore::RenderFlexibleBox::childIntrinsicLogicalHeight const):
     24        (WebCore::RenderFlexibleBox::childIntrinsicLogicalWidth const):
     25        (WebCore::RenderFlexibleBox::crossAxisIntrinsicExtentForChild const):
     26        (WebCore::RenderFlexibleBox::mainAxisContentExtent): Early return instead of if-else block.
     27        (WebCore::RenderFlexibleBox::useChildAspectRatio const):
     28        (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const): Use a single line for division operators.
     29        (WebCore::RenderFlexibleBox::childMainSizeIsDefinite const): Renamed from mainAxisLengthIsDefinite.
     30        (WebCore::RenderFlexibleBox::childCrossSizeIsDefinite const): Renamed from crossAxisLengthIsDefinite.
     31        (WebCore::RenderFlexibleBox::cacheChildMainSize):
     32        (WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild):
     33        (WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax):
     34        (WebCore::RenderFlexibleBox::crossSizeForPercentageResolution):
     35        (WebCore::RenderFlexibleBox::mainSizeForPercentageResolution):
     36        (WebCore::RenderFlexibleBox::childLogicalHeightForPercentageResolution):
     37        (WebCore::RenderFlexibleBox::adjustChildSizeForAspectRatioCrossAxisMinAndMax):
     38        (WebCore::RenderFlexibleBox::setOverridingMainSizeForChild): Renamed from setOverrideMainAxisContentSizeForChild.
     39        (WebCore::RenderFlexibleBox::alignmentForChild const):
     40        (WebCore::RenderFlexibleBox::childHasIntrinsicMainAxisSize const):
     41        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
     42        (WebCore::RenderFlexibleBox::applyStretchAlignmentToChild):
     43        (WebCore::RenderFlexibleBox::hasOrthogonalFlow const): Deleted.
     44        (WebCore::RenderFlexibleBox::mainAxisLengthIsDefinite const): Deleted.
     45        (WebCore::RenderFlexibleBox::crossAxisLengthIsDefinite const): Deleted.
     46        (WebCore::RenderFlexibleBox::setOverrideMainAxisContentSizeForChild): Deleted.
     47        * rendering/RenderFlexibleBox.h:
     48
    1492020-11-16  Philippe Normand  <pnormand@igalia.com>
    250

  • trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

    r268919 r269840  
    183183        return Optional<int>();
    184184
    185     if (!isColumnFlow() && hasOrthogonalFlow(*baselineChild))
     185    if (!isColumnFlow() && !mainAxisIsChildInlineAxis(*baselineChild))
    186186        return Optional<int>(crossAxisExtentForChild(*baselineChild) + baselineChild->logicalTop());
    187     if (isColumnFlow() && !hasOrthogonalFlow(*baselineChild))
     187    if (isColumnFlow() && mainAxisIsChildInlineAxis(*baselineChild))
    188188        return Optional<int>(mainAxisExtentForChild(*baselineChild) + baselineChild->logicalTop());
    189189
    … …  
    387387}
    388388
    389 bool RenderFlexibleBox::hasOrthogonalFlow(const RenderBox& child) const
    390 {
    391     return isHorizontalFlow() != child.isHorizontalWritingMode();
     389bool RenderFlexibleBox::mainAxisIsChildInlineAxis(const RenderBox& child) const
     390{
     391    return isHorizontalFlow() == child.isHorizontalWritingMode();
    392392}
    393393
    … …  
    464464{
    465465    // This should only be called if the logical height is the cross size
    466     ASSERT(!hasOrthogonalFlow(child));
     466    ASSERT(mainAxisIsChildInlineAxis(child));
    467467    if (needToStretchChildLogicalHeight(child)) {
    468468        LayoutUnit childContentHeight = cachedChildIntrinsicContentLogicalHeight(child);
    … …  
    476476{
    477477    // This should only be called if the logical width is the cross size
    478     ASSERT(hasOrthogonalFlow(child));
    479     if (crossAxisLengthIsDefinite(child, child.style().logicalWidth()))
     478    ASSERT(!mainAxisIsChildInlineAxis(child));
     479    if (childCrossSizeIsDefinite(child, child.style().logicalWidth()))
    480480        return child.logicalWidth();
    481481
    … …  
    494494LayoutUnit RenderFlexibleBox::crossAxisIntrinsicExtentForChild(const RenderBox& child) const
    495495{
    496     return hasOrthogonalFlow(child) ? childIntrinsicLogicalWidth(child) : childIntrinsicLogicalHeight(child);
     496    return mainAxisIsChildInlineAxis(child) ? childIntrinsicLogicalHeight(child) : childIntrinsicLogicalWidth(child);
    497497}
    498498
    … …  
    524524LayoutUnit RenderFlexibleBox::mainAxisContentExtent(LayoutUnit contentLogicalHeight)
    525525{
    526     if (isColumnFlow()) {
    527         LayoutUnit borderPaddingAndScrollbar = borderAndPaddingLogicalHeight() + scrollbarLogicalHeight();
    528         LayoutUnit borderBoxLogicalHeight = contentLogicalHeight + borderPaddingAndScrollbar;
    529         auto computedValues = computeLogicalHeight(borderBoxLogicalHeight, logicalTop());
    530         if (computedValues.m_extent == LayoutUnit::max())
    531             return computedValues.m_extent;
    532         return std::max(0_lu, computedValues.m_extent - borderPaddingAndScrollbar);
    533     }
    534     return contentLogicalWidth();
     526    if (!isColumnFlow())
     527        return contentLogicalWidth();
     528
     529    LayoutUnit borderPaddingAndScrollbar = borderAndPaddingLogicalHeight() + scrollbarLogicalHeight();
     530    LayoutUnit borderBoxLogicalHeight = contentLogicalHeight + borderPaddingAndScrollbar;
     531    auto computedValues = computeLogicalHeight(borderBoxLogicalHeight, logicalTop());
     532    if (computedValues.m_extent == LayoutUnit::max())
     533        return computedValues.m_extent;
     534    return std::max(0_lu, computedValues.m_extent - borderPaddingAndScrollbar);
    535535}
    536536
    … …  
    742742    else
    743743        crossSize = child.style().width();
    744     return crossAxisLengthIsDefinite(child, crossSize);
     744    return childCrossSizeIsDefinite(child, crossSize);
    745745}
    746746
    … …  
    756756    else {
    757757        ASSERT(crossSizeLength.isPercentOrCalculated());
    758         crossSize = hasOrthogonalFlow(child) ? adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth())) : child.computePercentageLogicalHeight(crossSizeLength);
     758        crossSize = mainAxisIsChildInlineAxis(child) ? child.computePercentageLogicalHeight(crossSizeLength) : adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth()));
    759759        if (!crossSize)
    760760            return 0_lu;
    … …  
    762762   
    763763    const LayoutSize& childIntrinsicSize = child.intrinsicSize();
    764     double ratio = childIntrinsicSize.width().toFloat() /
    765     childIntrinsicSize.height().toFloat();
     764    double ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
    766765    if (isHorizontalFlow())
    767766        return LayoutUnit(crossSize.value() * ratio);
    … …  
    777776}
    778777   
    779 bool RenderFlexibleBox::mainAxisLengthIsDefinite(const RenderBox& child, const Length& flexBasis) const
     778bool RenderFlexibleBox::childMainSizeIsDefinite(const RenderBox& child, const Length& flexBasis) const
    780779{
    781780    if (flexBasis.isAuto())
    … …  
    803802}
    804803
    805 bool RenderFlexibleBox::crossAxisLengthIsDefinite(const RenderBox& child, const Length& length) const
     804bool RenderFlexibleBox::childCrossSizeIsDefinite(const RenderBox& child, const Length& length) const
    806805{
    807806    if (length.isAuto())
    808807        return false;
    809808    if (length.isPercentOrCalculated()) {
    810         if (hasOrthogonalFlow(child) || m_hasDefiniteHeight == SizeDefiniteness::Definite)
     809        if (!mainAxisIsChildInlineAxis(child) || m_hasDefiniteHeight == SizeDefiniteness::Definite)
    811810            return true;
    812811        if (m_hasDefiniteHeight == SizeDefiniteness::Indefinite)
    … …  
    825824    ASSERT(!child.needsLayout());
    826825    LayoutUnit mainSize;
    827     if (hasOrthogonalFlow(child)) {
     826    if (mainAxisIsChildInlineAxis(child))
     827        mainSize = child.maxPreferredLogicalWidth();
     828    else {
    828829        auto flexBasis = flexBasisForChild(child);
    829         if (flexBasis.isPercentOrCalculated() && !mainAxisLengthIsDefinite(child, flexBasis))
     830        if (flexBasis.isPercentOrCalculated() && !childMainSizeIsDefinite(child, flexBasis))
    830831            mainSize = cachedChildIntrinsicContentLogicalHeight(child) + child.borderAndPaddingLogicalHeight() + child.scrollbarLogicalHeight();
    831832        else
    832833            mainSize = child.logicalHeight();
    833     } else
    834         mainSize = child.maxPreferredLogicalWidth();
     834    }
     835 
    835836    m_intrinsicSizeAlongMainAxis.set(&child, mainSize);
    836837    m_relaidOutChildren.add(&child);
    … …  
    848849   
    849850    Length flexBasis = flexBasisForChild(child);
    850     if (mainAxisLengthIsDefinite(child, flexBasis))
     851    if (childMainSizeIsDefinite(child, flexBasis))
    851852        return std::max(0_lu, computeMainAxisExtentForChild(child, MainOrPreferredSize, flexBasis).value());
    852853
    … …  
    861862    // width; for the height we need to lay out the child.
    862863    LayoutUnit mainAxisExtent;
    863     if (hasOrthogonalFlow(child)) {
     864    if (!mainAxisIsChildInlineAxis(child)) {
    864865        updateBlockChildDirtyBitsBeforeLayout(relayoutChildren, child);
    865866        if (child.needsLayout() || relayoutChildren || !m_intrinsicSizeAlongMainAxis.contains(&child)) {
    … …  
    11411142       
    11421143        Length mainSize = isHorizontalFlow() ? child.style().width() : child.style().height();
    1143         if (mainAxisLengthIsDefinite(child, mainSize)) {
     1144        if (childMainSizeIsDefinite(child, mainSize)) {
    11441145            LayoutUnit resolvedMainSize = computeMainAxisExtentForChild(child, MainOrPreferredSize, mainSize).valueOr(0);
    11451146            ASSERT(resolvedMainSize >= 0);
    … …  
    11651166Optional<LayoutUnit> RenderFlexibleBox::crossSizeForPercentageResolution(const RenderBox& child)
    11661167{
    1167     ASSERT(!hasOrthogonalFlow(child));
     1168    ASSERT(mainAxisIsChildInlineAxis(child));
    11681169    if (alignmentForChild(child) != ItemPosition::Stretch)
    11691170        return WTF::nullopt;
    … …  
    11841185Optional<LayoutUnit> RenderFlexibleBox::mainSizeForPercentageResolution(const RenderBox& child)
    11851186{
    1186     ASSERT(hasOrthogonalFlow(child));
     1187    ASSERT(!mainAxisIsChildInlineAxis(child));
    11871188    // This function implements section 9.8. Definite and Indefinite Sizes, case 2) of the flexbox spec.
    11881189    // If the flex container has a definite main size the flex item post-flexing main size is also treated
    11891190    // as definite. We make up a percentage to check whether we have a definite size.
    1190     if (!mainAxisLengthIsDefinite(child, Length(0, Percent)))
     1191    if (!childMainSizeIsDefinite(child, Length(0, Percent)))
    11911192        return WTF::nullopt;
    11921193
    … …  
    11961197Optional<LayoutUnit> RenderFlexibleBox::childLogicalHeightForPercentageResolution(const RenderBox& child)
    11971198{
    1198     if (!hasOrthogonalFlow(child))
     1199    if (mainAxisIsChildInlineAxis(child))
    11991200        return crossSizeForPercentageResolution(child);
    12001201    return mainSizeForPercentageResolution(child);
    … …  
    12061207    Length crossMax = isHorizontalFlow() ? child.style().maxHeight() : child.style().maxWidth();
    12071208   
    1208     if (crossAxisLengthIsDefinite(child, crossMax)) {
     1209    if (childCrossSizeIsDefinite(child, crossMax)) {
    12091210        LayoutUnit maxValue = computeMainSizeFromAspectRatioUsing(child, crossMax);
    12101211        childSize = std::min(maxValue, childSize);
    12111212    }
    12121213   
    1213     if (crossAxisLengthIsDefinite(child, crossMin)) {
     1214    if (childCrossSizeIsDefinite(child, crossMin)) {
    12141215        LayoutUnit minValue = computeMainSizeFromAspectRatioUsing(child, crossMin);
    12151216        childSize = std::max(minValue, childSize);
    … …  
    14221423}
    14231424
    1424 void RenderFlexibleBox::setOverrideMainAxisContentSizeForChild(RenderBox& child, LayoutUnit childPreferredSize)
    1425 {
    1426     if (hasOrthogonalFlow(child))
     1425void RenderFlexibleBox::setOverridingMainSizeForChild(RenderBox& child, LayoutUnit childPreferredSize)
     1426{
     1427    if (mainAxisIsChildInlineAxis(child))
     1428        child.setOverridingLogicalWidth(childPreferredSize + child.borderAndPaddingLogicalWidth());
     1429    else
    14271430        child.setOverridingLogicalHeight(childPreferredSize + child.borderAndPaddingLogicalHeight());
    1428     else
    1429         child.setOverridingLogicalWidth(childPreferredSize + child.borderAndPaddingLogicalWidth());
    14301431}
    14311432
    … …  
    15041505    ASSERT(align != ItemPosition::Auto && align != ItemPosition::Normal);
    15051506
    1506     if (align == ItemPosition::Baseline && hasOrthogonalFlow(child))
     1507    if (align == ItemPosition::Baseline && !mainAxisIsChildInlineAxis(child))
    15071508        align = ItemPosition::FlexStart;
    15081509
    … …  
    15611562    Length childMinSize = isHorizontalFlow() ? child.style().minWidth() : child.style().minHeight();
    15621563    Length childMaxSize = isHorizontalFlow() ? child.style().maxWidth() : child.style().maxHeight();
    1563     // FIXME: we must run mainAxisLengthIsDefinite() because it might end up calling computePercentageLogicalHeight()
     1564    // FIXME: we must run childMainSizeIsDefinite() because it might end up calling computePercentageLogicalHeight()
    15641565    // which has some side effects like calling addPercentHeightDescendant() for example so it is not possible to skip
    15651566    // the call for example by moving it to the end of the conditional expression. This is error-prone and we should
    15661567    // refactor computePercentageLogicalHeight() at some point so that it only computes stuff without those side effects.
    1567     if (!mainAxisLengthIsDefinite(child, childFlexBasis) || childMinSize.isIntrinsic() || childMaxSize.isIntrinsic())
     1568    if (!childMainSizeIsDefinite(child, childFlexBasis) || childMinSize.isIntrinsic() || childMaxSize.isIntrinsic())
    15681569        return true;
    15691570
    … …  
    16371638        ASSERT(!flexItem.box.isOutOfFlowPositioned());
    16381639
    1639         setOverrideMainAxisContentSizeForChild(child, flexItem.flexedContentSize);
     1640        setOverridingMainSizeForChild(child, flexItem.flexedContentSize);
    16401641        // The flexed content size and the override size include the scrollbar
    16411642        // width, so we need to compare to the size including the scrollbar.
    … …  
    18691870void RenderFlexibleBox::applyStretchAlignmentToChild(RenderBox& child, LayoutUnit lineCrossAxisExtent)
    18701871{
    1871     if (!hasOrthogonalFlow(child) && child.style().logicalHeight().isAuto()) {
     1872    if (mainAxisIsChildInlineAxis(child) && child.style().logicalHeight().isAuto()) {
    18721873        LayoutUnit stretchedLogicalHeight = std::max(child.borderAndPaddingLogicalHeight(),
    18731874        lineCrossAxisExtent - crossAxisMarginExtentForChild(child));
    … …  
    19011902            setCachedChildIntrinsicContentLogicalHeight(child, childIntrinsicContentLogicalHeight);
    19021903        }
    1903     } else if (hasOrthogonalFlow(child) && child.style().logicalWidth().isAuto()) {
     1904    } else if (!mainAxisIsChildInlineAxis(child) && child.style().logicalWidth().isAuto()) {
    19041905        LayoutUnit childWidth = std::max(0_lu, lineCrossAxisExtent - crossAxisMarginExtentForChild(child));
    19051906        childWidth = child.constrainLogicalWidthInFragmentByMinMax(childWidth, crossAxisContentExtent(), *this, nullptr);

  • trunk/Source/WebCore/rendering/RenderFlexibleBox.h

    r267829 r269840  
    108108    struct LineContext;
    109109   
    110     bool hasOrthogonalFlow(const RenderBox& child) const;
     110    bool mainAxisIsChildInlineAxis(const RenderBox&) const;
    111111    bool isColumnFlow() const;
    112112    bool isLeftToRightFlow() const;
    … …  
    147147    void adjustAlignmentForChild(RenderBox& child, LayoutUnit);
    148148    ItemPosition alignmentForChild(const RenderBox& child) const;
    149     bool mainAxisLengthIsDefinite(const RenderBox& child, const Length& flexBasis) const;
    150     bool crossAxisLengthIsDefinite(const RenderBox& child, const Length& flexBasis) const;
     149    bool childMainSizeIsDefinite(const RenderBox&, const Length& flexBasis) const;
     150    bool childCrossSizeIsDefinite(const RenderBox&, const Length& flexBasis) const;
    151151    bool needToStretchChildLogicalHeight(const RenderBox& child) const;
    152152    bool childHasIntrinsicMainAxisSize(const RenderBox& child) const;
    … …  
    179179   
    180180    void resetAutoMarginsAndLogicalTopInCrossAxis(RenderBox& child);
    181     void setOverrideMainAxisContentSizeForChild(RenderBox& child, LayoutUnit childPreferredSize);
     181    void setOverridingMainSizeForChild(RenderBox&, LayoutUnit);
    182182    void prepareChildForPositionedLayout(RenderBox& child);
    183183    void layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, Vector<FlexItem>&, LayoutUnit availableFreeSpace, bool relayoutChildren, Vector<LineContext>&, LayoutUnit gapBetweenItems);
Note: See TracChangeset for help on using the changeset viewer.