Skip to content

Commit

Permalink
[Cleanup] Remove RenderBox::hasOverridingLogicalHeight
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=273885

Reviewed by Simon Fraser.

This patch eliminates a redundant hash lookup by
1. removing RenderBox::hasOverridingLogicalHeight and
2. changing the return type of RenderBox::overridingLogicalHeight from LayoutUnit to std::optional<LayoutUnit>

so that the following (highly common) pattern
  if (hasOverridingLogicalHeight())
    ... + overridingLogicalHeight() + ...

can be turned into
  if (auto overridingLogicalHeight = this->overridingLogicalHeight())
    ... + *overridingLogicalHeight + ...

* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::availableLogicalHeightForPercentageComputation const):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::overridingLogicalHeight const):
(WebCore::RenderBox::perpendicularContainingBlockLogicalHeight const):
(WebCore::RenderBox::cacheIntrinsicContentLogicalHeightForFlexItem const):
(WebCore::RenderBox::computeLogicalHeight const):
(WebCore::RenderBox::computePercentageLogicalHeight const):
(WebCore::RenderBox::computeReplacedLogicalHeightUsing const):
(WebCore::RenderBox::availableLogicalHeightUsing const):
(WebCore::RenderBox::shouldComputeLogicalWidthFromAspectRatio const):
(WebCore::RenderBox::hasOverridingLogicalHeight const): Deleted.
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::heightForChild):
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::OverridingSizesScope::OverridingSizesScope):
(WebCore::RenderFlexibleBox::usedChildOverridingCrossSizeForPercentageResolution):
(WebCore::RenderFlexibleBox::usedChildOverridingMainSizeForPercentageResolution):
(WebCore::RenderFlexibleBox::applyStretchAlignmentToChild):
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutGrid):
(WebCore::RenderGrid::layoutMasonry):
* Source/WebCore/rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::paint):
* Source/WebCore/rendering/RenderTable.cpp:
(WebCore::RenderTable::layout):
* Source/WebCore/rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::calcRowLogicalHeight):

Canonical link: https://commits.webkit.org/278557@main
  • Loading branch information
@alanbaradlay
alanbaradlay committed May 9, 2024
1 parent d44032f commit 6330fd9
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 99 deletions.
89 changes: 47 additions & 42 deletions Source/WebCore/rendering/RenderBlock.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3196,52 +3196,57 @@ bool RenderBlock::hasDefiniteLogicalHeight() const

std::optional<LayoutUnit> RenderBlock::availableLogicalHeightForPercentageComputation() const
{
std::optional<LayoutUnit> availableHeight;

// For anonymous blocks that are skipped during percentage height calculation,
// we consider them to have an indefinite height.
if (skipContainingBlockForPercentHeightCalculation(*this, false))
return availableHeight;

const auto& styleToUse = style();

// A positioned element that specified both top/bottom or that specifies
// height should be treated as though it has a height explicitly specified
// that can be used for any percentage computations.
bool isOutOfFlowPositionedWithSpecifiedHeight = isOutOfFlowPositioned() && (!styleToUse.logicalHeight().isAuto() || (!styleToUse.logicalTop().isAuto() && !styleToUse.logicalBottom().isAuto()));

if (auto usedChildOverridingLogicalHeightForPercentageResolutionForFlex = (isFlexItem() ? downcast<RenderFlexibleBox>(parent())->usedChildOverridingLogicalHeightForPercentageResolution(*this) : std::nullopt))
availableHeight = overridingContentLogicalHeight(*usedChildOverridingLogicalHeightForPercentageResolutionForFlex);
else if (isGridItem() && hasOverridingLogicalHeight())
availableHeight = overridingContentLogicalHeight(overridingLogicalHeight());
else if (styleToUse.logicalHeight().isFixed()) {
LayoutUnit contentBoxHeight = adjustContentBoxLogicalHeightForBoxSizing((LayoutUnit)styleToUse.logicalHeight().value());
availableHeight = std::max(0_lu, constrainContentBoxLogicalHeightByMinMax(contentBoxHeight - scrollbarLogicalHeight(), std::nullopt));
} else if (shouldComputeLogicalHeightFromAspectRatio()) {
// Only grid is expected to be in a state where it is calculating pref width and having unknown logical width.
if (isRenderGrid() && preferredLogicalWidthsDirty() && !style().logicalWidth().isSpecified())
return availableHeight;
availableHeight = blockSizeFromAspectRatio(horizontalBorderAndPaddingExtent(), verticalBorderAndPaddingExtent(), LayoutUnit(style().logicalAspectRatio()), style().boxSizingForAspectRatio(), logicalWidth(), style().aspectRatioType(), isRenderReplaced());
} else if (isOutOfFlowPositionedWithSpecifiedHeight) {
// Don't allow this to affect the block' size() member variable, since this
// can get called while the block is still laying out its kids.
LogicalExtentComputedValues computedValues = computeLogicalHeight(logicalHeight(), 0_lu);
availableHeight = std::max(0_lu, computedValues.m_extent - borderAndPaddingLogicalHeight() - scrollbarLogicalHeight());
} else if (styleToUse.logicalHeight().isPercentOrCalculated()) {
std::optional<LayoutUnit> heightWithScrollbar = computePercentageLogicalHeight(styleToUse.logicalHeight());
if (heightWithScrollbar) {
LayoutUnit contentBoxHeightWithScrollbar = adjustContentBoxLogicalHeightForBoxSizing(heightWithScrollbar.value());
// We need to adjust for min/max height because this method does not
// handle the min/max of the current block, its caller does. So the
// return value from the recursive call will not have been adjusted
// yet.
LayoutUnit contentBoxHeight = constrainContentBoxLogicalHeightByMinMax(contentBoxHeightWithScrollbar - scrollbarLogicalHeight(), std::nullopt);
availableHeight = std::max(0_lu, contentBoxHeight);
return { };

auto availableHeight = [&]() -> std::optional<LayoutUnit> {
if (auto overridingLogicalHeightForFlex = (isFlexItem() ? downcast<RenderFlexibleBox>(parent())->usedChildOverridingLogicalHeightForPercentageResolution(*this) : std::nullopt))
return overridingContentLogicalHeight(*overridingLogicalHeightForFlex);

if (auto overridingLogicalHeightForGrid = (isGridItem() ? overridingLogicalHeight() : std::nullopt))
return overridingContentLogicalHeight(*overridingLogicalHeightForGrid);

auto& style = this->style();
if (style.logicalHeight().isFixed()) {
auto contentBoxHeight = adjustContentBoxLogicalHeightForBoxSizing(LayoutUnit { style.logicalHeight().value() });
return std::max(0_lu, constrainContentBoxLogicalHeightByMinMax(contentBoxHeight - scrollbarLogicalHeight(), { }));
}
} else if (isRenderView())
availableHeight = view().pageOrViewLogicalHeight();

return availableHeight;

if (shouldComputeLogicalHeightFromAspectRatio()) {
// Only grid is expected to be in a state where it is calculating pref width and having unknown logical width.
if (isRenderGrid() && preferredLogicalWidthsDirty() && !style.logicalWidth().isSpecified())
return { };
return blockSizeFromAspectRatio(horizontalBorderAndPaddingExtent(), verticalBorderAndPaddingExtent(), LayoutUnit { style.logicalAspectRatio() }, style.boxSizingForAspectRatio(), logicalWidth(), style.aspectRatioType(), isRenderReplaced());
}

// A positioned element that specified both top/bottom or that specifies
// height should be treated as though it has a height explicitly specified
// that can be used for any percentage computations.
auto isOutOfFlowPositionedWithSpecifiedHeight = isOutOfFlowPositioned() && (!style.logicalHeight().isAuto() || (!style.logicalTop().isAuto() && !style.logicalBottom().isAuto()));
if (isOutOfFlowPositionedWithSpecifiedHeight) {
// Don't allow this to affect the block' size() member variable, since this
// can get called while the block is still laying out its kids.
return std::max(0_lu, computeLogicalHeight(logicalHeight(), 0_lu).m_extent - borderAndPaddingLogicalHeight() - scrollbarLogicalHeight());
}

if (style.logicalHeight().isPercentOrCalculated()) {
if (auto heightWithScrollbar = computePercentageLogicalHeight(style.logicalHeight())) {
auto contentBoxHeightWithScrollbar = adjustContentBoxLogicalHeightForBoxSizing(heightWithScrollbar.value());
// We need to adjust for min/max height because this method does not handle the min/max of the current block, its caller does.
// So the return value from the recursive call will not have been adjusted yet.
return std::max(0_lu, constrainContentBoxLogicalHeightByMinMax(contentBoxHeightWithScrollbar - scrollbarLogicalHeight(), { }));
}
return { };
}

if (isRenderView())
return view().pageOrViewLogicalHeight();

return { };
};
return availableHeight();
}

void RenderBlock::layoutExcludedChildren(bool relayoutChildren)
Expand Down
50 changes: 23 additions & 27 deletions Source/WebCore/rendering/RenderBox.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1271,11 +1271,6 @@ LayoutUnit RenderBox::maxPreferredLogicalWidth() const
return m_maxPreferredLogicalWidth;
}

bool RenderBox::hasOverridingLogicalHeight() const
{
return gOverridingLogicalHeightMap && gOverridingLogicalHeightMap->contains(*this);
}

bool RenderBox::hasOverridingLogicalWidth() const
{
return gOverridingLogicalWidthMap && gOverridingLogicalWidthMap->contains(*this);
Expand Down Expand Up @@ -1319,10 +1314,11 @@ LayoutUnit RenderBox::overridingLogicalWidth() const
return gOverridingLogicalWidthMap->get(*this);
}

LayoutUnit RenderBox::overridingLogicalHeight() const
std::optional<LayoutUnit> RenderBox::overridingLogicalHeight() const
{
ASSERT(hasOverridingLogicalHeight());
return gOverridingLogicalHeightMap->get(*this);
if (!gOverridingLogicalHeightMap)
return { };
return gOverridingLogicalHeightMap->getOptional(*this);
}

std::optional<LayoutUnit> RenderBox::overridingContainingBlockContentWidth() const
Expand Down Expand Up @@ -2368,24 +2364,24 @@ LayoutUnit RenderBox::perpendicularContainingBlockLogicalHeight() const
return height.value();
}

RenderBlock* cb = containingBlock();
if (cb->hasOverridingLogicalHeight())
return cb->overridingContentLogicalHeight(cb->overridingLogicalHeight());
auto* containingBlock = this->containingBlock();
if (auto overridingLogicalHeight = containingBlock->overridingLogicalHeight())
return containingBlock->overridingContentLogicalHeight(*overridingLogicalHeight);

const RenderStyle& containingBlockStyle = cb->style();
const RenderStyle& containingBlockStyle = containingBlock->style();
Length logicalHeightLength = containingBlockStyle.logicalHeight();

// FIXME: For now just support fixed heights. Eventually should support percentage heights as well.
if (!logicalHeightLength.isFixed()) {
LayoutUnit fillFallbackExtent = containingBlockStyle.isHorizontalWritingMode() ? view().frameView().layoutSize().height() : view().frameView().layoutSize().width();
LayoutUnit fillAvailableExtent = containingBlock()->availableLogicalHeight(ExcludeMarginBorderPadding);
LayoutUnit fillAvailableExtent = containingBlock->availableLogicalHeight(ExcludeMarginBorderPadding);
view().addPercentHeightDescendant(const_cast<RenderBox&>(*this));
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=158286 We also need to perform the same percentHeightDescendant treatment to the element which dictates the return value for containingBlock()->availableLogicalHeight() above.
return std::min(fillAvailableExtent, fillFallbackExtent);
}

// Use the content box logical height as specified by the style.
return cb->adjustContentBoxLogicalHeightForBoxSizing(LayoutUnit(logicalHeightLength.value()));
return containingBlock->adjustContentBoxLogicalHeightForBoxSizing(LayoutUnit(logicalHeightLength.value()));
}

void RenderBox::mapLocalToContainer(const RenderLayerModelObject* ancestorContainer, TransformState& transformState, OptionSet<MapCoordinatesMode> mode, bool* wasFixed) const
Expand Down Expand Up @@ -3134,7 +3130,7 @@ void RenderBox::cacheIntrinsicContentLogicalHeightForFlexItem(LayoutUnit height)
CheckedPtr flexibleBox = dynamicDowncast<RenderFlexibleBox>(parent());
if (!flexibleBox)
return;
if (hasOverridingLogicalHeight() || shouldComputeLogicalHeightFromAspectRatio())
if (overridingLogicalHeight() || shouldComputeLogicalHeightFromAspectRatio())
return;
flexibleBox->setCachedChildIntrinsicContentLogicalHeight(*this, height);
}
Expand Down Expand Up @@ -3220,9 +3216,9 @@ RenderBox::LogicalExtentComputedValues RenderBox::computeLogicalHeight(LayoutUni
// grab our cached flexible height.
// FIXME: Account for block-flow in flexible boxes.
// https://bugs.webkit.org/show_bug.cgi?id=46418
if (hasOverridingLogicalHeight() && (parent()->isFlexibleBoxIncludingDeprecated() || parent()->isRenderGrid())) {
h = Length(overridingLogicalHeight(), LengthType::Fixed);
} else if (treatAsReplaced)
if (auto overridingLogicalHeightForFlexOrGrid = (parent()->isFlexibleBoxIncludingDeprecated() || parent()->isRenderGrid() ? overridingLogicalHeight() : std::nullopt))
h = Length(*overridingLogicalHeightForFlexOrGrid, LengthType::Fixed);
else if (treatAsReplaced)
h = Length(computeReplacedLogicalHeight() + borderAndPaddingLogicalHeight(), LengthType::Fixed);
else {
h = logicalHeightInUse;
Expand Down Expand Up @@ -3452,10 +3448,10 @@ std::optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length
// Table cells violate what the CSS spec says to do with heights. Basically we
// don't care if the cell specified a height or not. We just always make ourselves
// be a percentage of the cell's current content height.
if (!cb->hasOverridingLogicalHeight())
auto overridingLogicalHeight = cb->overridingLogicalHeight();
if (!overridingLogicalHeight)
return tableCellShouldHaveZeroInitialSize(*cb, *this, scrollsOverflowY()) ? std::optional<LayoutUnit>(0) : std::nullopt;

availableHeight = cb->overridingLogicalHeight() - cb->computedCSSPaddingBefore() - cb->computedCSSPaddingAfter() - cb->borderBefore() - cb->borderAfter() - cb->scrollbarLogicalHeight();
availableHeight = *overridingLogicalHeight - cb->computedCSSPaddingBefore() - cb->computedCSSPaddingAfter() - cb->borderBefore() - cb->borderAfter() - cb->scrollbarLogicalHeight();
}
} else
availableHeight = cb->availableLogicalHeightForPercentageComputation();
Expand All @@ -3471,7 +3467,7 @@ std::optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length
// then we must subtract the border and padding from the cell's
// |availableHeight| (given by |overridingLogicalHeight|) to arrive
// at the child's computed height.
bool subtractBorderAndPadding = isRenderTable() || (is<RenderTableCell>(*cb) && !skippedAutoHeightContainingBlock && cb->hasOverridingLogicalHeight() && style().boxSizing() == BoxSizing::ContentBox);
bool subtractBorderAndPadding = isRenderTable() || (is<RenderTableCell>(*cb) && !skippedAutoHeightContainingBlock && cb->overridingLogicalHeight() && style().boxSizing() == BoxSizing::ContentBox);
if (subtractBorderAndPadding) {
result -= borderAndPaddingLogicalHeight();
return std::max(0_lu, result);
Expand Down Expand Up @@ -3660,8 +3656,8 @@ LayoutUnit RenderBox::computeReplacedLogicalHeightUsing(SizeType heightType, Len
block->addPercentHeightDescendant(*const_cast<RenderBox*>(this));
if (auto usedChildOverridingLogicalHeightForPercentageResolutionForFlex = (block->isFlexItem() ? downcast<RenderFlexibleBox>(block->parent())->usedChildOverridingLogicalHeightForPercentageResolution(*block) : std::nullopt))
stretchedHeight = block->overridingContentLogicalHeight(*usedChildOverridingLogicalHeightForPercentageResolutionForFlex);
else if (block->isGridItem() && block->hasOverridingLogicalHeight() && !hasPerpendicularContainingBlock)
stretchedHeight = block->overridingContentLogicalHeight(block->overridingLogicalHeight());
else if (auto usedChildOverridingLogicalHeightForGrid = (block->isGridItem() && !hasPerpendicularContainingBlock ? block->overridingLogicalHeight() : std::nullopt))
stretchedHeight = block->overridingContentLogicalHeight(*usedChildOverridingLogicalHeightForGrid);
}

// FIXME: This calculation is not patched for block-flow yet.
Expand Down Expand Up @@ -3722,8 +3718,8 @@ LayoutUnit RenderBox::availableLogicalHeightUsing(const Length& h, AvailableLogi
// artificially. We're going to rely on this cell getting expanded to some new
// height, and then when we lay out again we'll use the calculation below.
if (isRenderTableCell() && (h.isAuto() || h.isPercentOrCalculated())) {
if (hasOverridingLogicalHeight())
return overridingLogicalHeight() - computedCSSPaddingBefore() - computedCSSPaddingAfter() - borderBefore() - borderAfter() - scrollbarLogicalHeight();
if (auto overridingLogicalHeight = this->overridingLogicalHeight())
return *overridingLogicalHeight - computedCSSPaddingBefore() - computedCSSPaddingAfter() - borderBefore() - borderAfter() - scrollbarLogicalHeight();
return logicalHeight() - borderAndPaddingLogicalHeight();
}

Expand Down Expand Up @@ -5600,7 +5596,7 @@ bool RenderBox::shouldComputeLogicalWidthFromAspectRatio() const
auto isResolvablePercentageHeight = [&] {
return style().logicalHeight().isPercentOrCalculated() && (isOutOfFlowPositioned() || percentageLogicalHeightIsResolvable());
};
return hasOverridingLogicalHeight() || shouldComputeLogicalWidthFromAspectRatioAndInsets(*this) || style().logicalHeight().isFixed() || isResolvablePercentageHeight();
return overridingLogicalHeight() || shouldComputeLogicalWidthFromAspectRatioAndInsets(*this) || style().logicalHeight().isFixed() || isResolvablePercentageHeight();
}

LayoutUnit RenderBox::computeLogicalWidthFromAspectRatioInternal() const
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/rendering/RenderBox.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,7 @@ class RenderBox : public RenderBoxModelObject {
LayoutUnit maxPreferredLogicalWidth() const override;

LayoutUnit overridingLogicalWidth() const;
LayoutUnit overridingLogicalHeight() const;
bool hasOverridingLogicalHeight() const;
std::optional<LayoutUnit> overridingLogicalHeight() const;
bool hasOverridingLogicalWidth() const;
void setOverridingLogicalHeight(LayoutUnit);
void setOverridingLogicalWidth(LayoutUnit);
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ static LayoutUnit widthForChild(RenderBox* child)

static LayoutUnit heightForChild(RenderBox* child)
{
if (child->hasOverridingLogicalHeight())
return child->overridingLogicalHeight();
if (auto overridingLogicalHeight = child->overridingLogicalHeight())
return *overridingLogicalHeight;
return child->logicalHeight();
}

Expand Down
21 changes: 6 additions & 15 deletions Source/WebCore/rendering/RenderFlexibleBox.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,7 @@ class OverridingSizesScope {
SET_OR_CLEAR_OVERRIDING_SIZE(m_box, Width, size);
}
if (axis == Axis::Both || axis == Axis::Block) {
if (box.hasOverridingLogicalHeight())
m_overridingHeight = box.overridingLogicalHeight();
m_overridingHeight = box.overridingLogicalHeight();
SET_OR_CLEAR_OVERRIDING_SIZE(m_box, Height, size);
}
}
Expand Down Expand Up @@ -1619,10 +1618,7 @@ std::optional<LayoutUnit> RenderFlexibleBox::usedChildOverridingCrossSizeForPerc
ASSERT(mainAxisIsChildInlineAxis(child));
if (alignmentForChild(child) != ItemPosition::Stretch)
return { };

if (child.hasOverridingLogicalHeight())
return child.overridingLogicalHeight();
return { };
return child.overridingLogicalHeight();
}

// This method is only called whenever a descendant of a flex item wants to resolve a percentage in its
Expand All @@ -1636,21 +1632,16 @@ std::optional<LayoutUnit> RenderFlexibleBox::usedChildOverridingMainSizeForPerce
ASSERT(!mainAxisIsChildInlineAxis(child));

// The main size of a fully inflexible item with a definite flex basis is, by definition, definite.
if (child.style().flexGrow() == 0.0 && child.style().flexShrink() == 0.0 && childMainSizeIsDefinite(child, flexBasisForChild(child))) {
if (child.hasOverridingLogicalHeight())
return child.overridingLogicalHeight();
return { };
}
if (child.style().flexGrow() == 0.0 && child.style().flexShrink() == 0.0 && childMainSizeIsDefinite(child, flexBasisForChild(child)))
return child.overridingLogicalHeight();

// This function implements section 9.8. Definite and Indefinite Sizes, case 2) of the flexbox spec.
// If the flex container has a definite main size the flex item post-flexing main size is also treated
// as definite. We make up a percentage to check whether we have a definite size.
if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), UpdatePercentageHeightDescendants::Yes))
return { };

if (child.hasOverridingLogicalHeight())
return child.overridingLogicalHeight();
return { };
return child.overridingLogicalHeight();
}

std::optional<LayoutUnit> RenderFlexibleBox::usedChildOverridingLogicalHeightForPercentageResolution(const RenderBox& child)
Expand Down Expand Up @@ -2533,7 +2524,7 @@ void RenderFlexibleBox::applyStretchAlignmentToChild(RenderBox& child, LayoutUni
// So, redo it here.
childNeedsRelayout = true;
}
if (childNeedsRelayout || !child.hasOverridingLogicalHeight())
if (childNeedsRelayout || !child.overridingLogicalHeight())
child.setOverridingLogicalHeight(desiredLogicalHeight);
if (childNeedsRelayout) {
SetForScope resetChildLogicalHeight(m_shouldResetChildLogicalHeightBeforeLayout, true);
Expand Down
Loading

0 comments on commit 6330fd9

Please sign in to comment.