Skip to content

Commit

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

Reviewed by Simon Fraser.

This patch removes redundant hash lookups on override size (see webkit.org/b/273880 273885 273887).
(This one is a slightly more verbose as HashMap contains an std:optional<LayoutUnit>. It is supposed to differentiate no-override/indefinite-override/definite-override states.
We should instead have an indicator for such override types e.g. struct vs. single LayoutUnit)

* Source/WebCore/rendering/GridLayoutFunctions.cpp:
(WebCore::GridLayoutFunctions::overridingContainingBlockContentSizeForChild):
(WebCore::GridLayoutFunctions::hasOverridingContainingBlockContentSizeForChild): Deleted.
* Source/WebCore/rendering/GridLayoutFunctions.h:
* Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithmStrategy::logicalHeightForChild const):
(WebCore::GridTrackSizingAlgorithmStrategy::updateOverridingContainingBlockContentSizeForChild const):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::overridingContainingBlockContentWidth const):
(WebCore::RenderBox::overridingContainingBlockContentHeight const):
(WebCore::RenderBox::overridingContainingBlockContentLogicalWidth const):
(WebCore::RenderBox::overridingContainingBlockContentLogicalHeight const):
(WebCore::RenderBox::setOverridingContainingBlockContentLogicalWidth):
(WebCore::RenderBox::setOverridingContainingBlockContentLogicalHeight):
(WebCore::RenderBox::containingBlockLogicalWidthForContent const):
(WebCore::RenderBox::containingBlockLogicalHeightForContent const):
(WebCore::RenderBox::perpendicularContainingBlockLogicalHeight const):
(WebCore::RenderBox::computePercentageLogicalHeight const):
(WebCore::RenderBox::replacedMinMaxLogicalHeightComputesAsNone const):
(WebCore::RenderBox::containingBlockLogicalWidthForPositioned const):
(WebCore::RenderBox::containingBlockLogicalHeightForPositioned const):
(WebCore::RenderBox::overridingLogicalHeight const): Deleted.
(WebCore::RenderBox::hasOverridingContainingBlockContentWidth const): Deleted.
(WebCore::RenderBox::hasOverridingContainingBlockContentHeight const): Deleted.
(WebCore::RenderBox::hasOverridingContainingBlockContentLogicalWidth const): Deleted.
(WebCore::RenderBox::hasOverridingContainingBlockContentLogicalHeight const): Deleted.
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::hasAutoHeightOrContainingBlockWithAutoHeight const):
(WebCore::RenderBoxModelObject::relativePositionOffset const):
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::placeItemsOnGrid):
(WebCore::overrideSizeChanged):
(WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
(WebCore::RenderGrid::applySubgridStretchAlignmentToChildIfNeeded):
(WebCore::RenderGrid::updateAutoMarginsInRowAxisIfNeeded):
(WebCore::RenderGrid::updateAutoMarginsInColumnAxisIfNeeded):
(WebCore::RenderGrid::gridAreaPositionForOutOfFlowChild const):

Canonical link: https://commits.webkit.org/278616@main
  • Loading branch information
alanbaradlay committed May 10, 2024
1 parent 852eb0c commit cdde289
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 142 deletions.
7 changes: 1 addition & 6 deletions Source/WebCore/rendering/GridLayoutFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,7 @@ GridTrackSizingDirection flowAwareDirectionForParent(const RenderGrid& grid, con
return isOrthogonalParent(grid, parent) ? (direction == GridTrackSizingDirection::ForColumns ? GridTrackSizingDirection::ForRows : GridTrackSizingDirection::ForColumns) : direction;
}

bool hasOverridingContainingBlockContentSizeForChild(const RenderBox& child, GridTrackSizingDirection direction)
{
return direction == GridTrackSizingDirection::ForColumns ? child.hasOverridingContainingBlockContentLogicalWidth() : child.hasOverridingContainingBlockContentLogicalHeight();
}

std::optional<LayoutUnit> overridingContainingBlockContentSizeForChild(const RenderBox& child, GridTrackSizingDirection direction)
std::optional<RenderBox::ContainingBlockOverrideValue> overridingContainingBlockContentSizeForChild(const RenderBox& child, GridTrackSizingDirection direction)
{
return direction == GridTrackSizingDirection::ForColumns ? child.overridingContainingBlockContentLogicalWidth() : child.overridingContainingBlockContentLogicalHeight();
}
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/rendering/GridLayoutFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ bool isOrthogonalParent(const RenderGrid&, const RenderElement& parent);
bool isAspectRatioBlockSizeDependentChild(const RenderBox&);
GridTrackSizingDirection flowAwareDirectionForChild(const RenderGrid&, const RenderBox&, GridTrackSizingDirection);
GridTrackSizingDirection flowAwareDirectionForParent(const RenderGrid&, const RenderElement& parent, GridTrackSizingDirection);
bool hasOverridingContainingBlockContentSizeForChild(const RenderBox&, GridTrackSizingDirection);
std::optional<LayoutUnit> overridingContainingBlockContentSizeForChild(const RenderBox&, GridTrackSizingDirection);
std::optional<RenderBox::ContainingBlockOverrideValue> overridingContainingBlockContentSizeForChild(const RenderBox&, GridTrackSizingDirection);
bool hasRelativeOrIntrinsicSizeForChild(const RenderBox& child, GridTrackSizingDirection);

bool isFlippedDirection(const RenderGrid&, GridTrackSizingDirection);
Expand Down
9 changes: 7 additions & 2 deletions Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,12 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::logicalHeightForChild(RenderBox& ch
GridTrackSizingDirection childBlockDirection = GridLayoutFunctions::flowAwareDirectionForChild(*renderGrid(), child, GridTrackSizingDirection::ForRows);
// If |child| has a relative logical height, we shouldn't let it override its intrinsic height, which is
// what we are interested in here. Thus we need to set the block-axis override size to nullopt (no possible resolution).
if (GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, GridTrackSizingDirection::ForRows) && shouldClearOverridingContainingBlockContentSizeForChild(child, GridTrackSizingDirection::ForRows)) {
auto hasOverridingContainingBlockContentSizeForChild = [&] {
if (auto overridingContainingBlockContentSizeForChild = GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, GridTrackSizingDirection::ForRows); overridingContainingBlockContentSizeForChild && *overridingContainingBlockContentSizeForChild)
return true;
return false;
};
if (hasOverridingContainingBlockContentSizeForChild() && shouldClearOverridingContainingBlockContentSizeForChild(child, GridTrackSizingDirection::ForRows)) {
setOverridingContainingBlockContentSizeForChild(*renderGrid(), child, childBlockDirection, std::nullopt);
child.setNeedsLayout(MarkOnlyThis);
}
Expand Down Expand Up @@ -1056,7 +1061,7 @@ bool GridTrackSizingAlgorithmStrategy::updateOverridingContainingBlockContentSiz
}
}

if (GridLayoutFunctions::hasOverridingContainingBlockContentSizeForChild(child, direction) && GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, direction) == overrideSize)
if (auto overridingContainingBlockContentSizeForChild = GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, direction); overridingContainingBlockContentSizeForChild && *overridingContainingBlockContentSizeForChild == overrideSize)
return false;

setOverridingContainingBlockContentSizeForChild(*renderGrid(), child, direction, overrideSize);
Expand Down
161 changes: 62 additions & 99 deletions Source/WebCore/rendering/RenderBox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ static OverridingLengthMap* gOverridingLogicalHeightLengthMap = nullptr;
static OverridingLengthMap* gOverridingLogicalWidthLengthMap = nullptr;

// FIXME: We should store these based on physical direction.
using OverrideOptionalSizeMap = HashMap<SingleThreadWeakRef<const RenderBox>, std::optional<LayoutUnit>>;
using OverrideOptionalSizeMap = HashMap<SingleThreadWeakRef<const RenderBox>, RenderBox::ContainingBlockOverrideValue>;
static OverrideOptionalSizeMap* gOverridingContainingBlockContentLogicalHeightMap = nullptr;
static OverrideOptionalSizeMap* gOverridingContainingBlockContentLogicalWidthMap = nullptr;

Expand Down Expand Up @@ -1317,74 +1317,42 @@ std::optional<LayoutUnit> RenderBox::overridingLogicalHeight() const
return gOverridingLogicalHeightMap->getOptional(*this);
}

std::optional<LayoutUnit> RenderBox::overridingContainingBlockContentWidth() const
std::optional<RenderBox::ContainingBlockOverrideValue> RenderBox::overridingContainingBlockContentWidth(WritingMode writingMode) const
{
ASSERT(hasOverridingContainingBlockContentWidth());
return containingBlock()->style().isHorizontalWritingMode()
? gOverridingContainingBlockContentLogicalWidthMap->get(*this)
: gOverridingContainingBlockContentLogicalHeightMap->get(*this);
if (WebCore::isHorizontalWritingMode(writingMode))
return gOverridingContainingBlockContentLogicalWidthMap ? gOverridingContainingBlockContentLogicalWidthMap->getOptional(*this) : std::nullopt;
return gOverridingContainingBlockContentLogicalHeightMap ? gOverridingContainingBlockContentLogicalHeightMap->getOptional(*this) : std::nullopt;
}

std::optional<LayoutUnit> RenderBox::overridingContainingBlockContentHeight() const
std::optional<RenderBox::ContainingBlockOverrideValue> RenderBox::overridingContainingBlockContentHeight(WritingMode writingMode) const
{
ASSERT(hasOverridingContainingBlockContentHeight());
return containingBlock()->style().isHorizontalWritingMode()
? gOverridingContainingBlockContentLogicalHeightMap->get(*this)
: gOverridingContainingBlockContentLogicalWidthMap->get(*this);
if (WebCore::isHorizontalWritingMode(writingMode))
return gOverridingContainingBlockContentLogicalHeightMap ? gOverridingContainingBlockContentLogicalHeightMap->getOptional(*this) : std::nullopt;
return gOverridingContainingBlockContentLogicalWidthMap ? gOverridingContainingBlockContentLogicalWidthMap->getOptional(*this) : std::nullopt;
}

bool RenderBox::hasOverridingContainingBlockContentWidth() const
std::optional<RenderBox::ContainingBlockOverrideValue> RenderBox::overridingContainingBlockContentLogicalWidth() const
{
RenderBlock* cb = containingBlock();
if (!cb)
return false;

return cb->style().isHorizontalWritingMode()
? gOverridingContainingBlockContentLogicalWidthMap && gOverridingContainingBlockContentLogicalWidthMap->contains(*this)
: gOverridingContainingBlockContentLogicalHeightMap && gOverridingContainingBlockContentLogicalHeightMap->contains(*this);
}

bool RenderBox::hasOverridingContainingBlockContentHeight() const
{
RenderBlock* cb = containingBlock();
if (!cb)
return false;

return cb->style().isHorizontalWritingMode()
? gOverridingContainingBlockContentLogicalHeightMap && gOverridingContainingBlockContentLogicalHeightMap->contains(*this)
: gOverridingContainingBlockContentLogicalWidthMap && gOverridingContainingBlockContentLogicalWidthMap->contains(*this);
}

std::optional<LayoutUnit> RenderBox::overridingContainingBlockContentLogicalWidth() const
{
ASSERT(hasOverridingContainingBlockContentLogicalWidth());
return gOverridingContainingBlockContentLogicalWidthMap->get(*this);
}

std::optional<LayoutUnit> RenderBox::overridingContainingBlockContentLogicalHeight() const
{
ASSERT(hasOverridingContainingBlockContentLogicalHeight());
return gOverridingContainingBlockContentLogicalHeightMap->get(*this);
}

bool RenderBox::hasOverridingContainingBlockContentLogicalWidth() const
{
return gOverridingContainingBlockContentLogicalWidthMap && gOverridingContainingBlockContentLogicalWidthMap->contains(*this);
if (!gOverridingContainingBlockContentLogicalWidthMap)
return { };
return gOverridingContainingBlockContentLogicalWidthMap->getOptional(*this);
}

bool RenderBox::hasOverridingContainingBlockContentLogicalHeight() const
std::optional<RenderBox::ContainingBlockOverrideValue> RenderBox::overridingContainingBlockContentLogicalHeight() const
{
return gOverridingContainingBlockContentLogicalHeightMap && gOverridingContainingBlockContentLogicalHeightMap->contains(*this);
if (!gOverridingContainingBlockContentLogicalHeightMap)
return { };
return gOverridingContainingBlockContentLogicalHeightMap->getOptional(*this);
}

void RenderBox::setOverridingContainingBlockContentLogicalWidth(std::optional<LayoutUnit> logicalWidth)
void RenderBox::setOverridingContainingBlockContentLogicalWidth(ContainingBlockOverrideValue logicalWidth)
{
if (!gOverridingContainingBlockContentLogicalWidthMap)
gOverridingContainingBlockContentLogicalWidthMap = new OverrideOptionalSizeMap;
gOverridingContainingBlockContentLogicalWidthMap->set(*this, logicalWidth);
}

void RenderBox::setOverridingContainingBlockContentLogicalHeight(std::optional<LayoutUnit> logicalHeight)
void RenderBox::setOverridingContainingBlockContentLogicalHeight(ContainingBlockOverrideValue logicalHeight)
{
if (!gOverridingContainingBlockContentLogicalHeightMap)
gOverridingContainingBlockContentLogicalHeightMap = new OverrideOptionalSizeMap;
Expand Down Expand Up @@ -2291,28 +2259,28 @@ LayoutUnit RenderBox::shrinkLogicalWidthToAvoidFloats(LayoutUnit childMarginStar

LayoutUnit RenderBox::containingBlockLogicalWidthForContent() const
{
if (hasOverridingContainingBlockContentLogicalWidth())
return overridingContainingBlockContentLogicalWidth().value_or(0_lu);
if (auto overridingContainingBlockContentLogicalWidth = this->overridingContainingBlockContentLogicalWidth())
return overridingContainingBlockContentLogicalWidth->value_or(0_lu);

if (RenderBlock* cb = containingBlock()) {
if (isOutOfFlowPositioned())
return cb->clientLogicalWidth();
return cb->availableLogicalWidth();
}
if (auto* containingBlock = this->containingBlock())
return isOutOfFlowPositioned() ? containingBlock->clientLogicalWidth() : containingBlock->availableLogicalWidth();

ASSERT_NOT_REACHED();
return 0_lu;
}

LayoutUnit RenderBox::containingBlockLogicalHeightForContent(AvailableLogicalHeightType heightType) const
{
if (hasOverridingContainingBlockContentLogicalHeight()) {
if (auto overridingContainingBlockContentLogicalHeight = this->overridingContainingBlockContentLogicalHeight(); overridingContainingBlockContentLogicalHeight && *overridingContainingBlockContentLogicalHeight) {
// FIXME: Containing block for a grid item is the grid area it's located in. We need to return whatever
// height value we get from overridingContainingBlockContentLogicalHeight() here, including std::nullopt.
if (auto height = overridingContainingBlockContentLogicalHeight())
return height.value();
return overridingContainingBlockContentLogicalHeight->value();
}

if (RenderBlock* cb = containingBlock())
return cb->availableLogicalHeight(heightType);
if (auto* containingBlock = this->containingBlock())
return containingBlock->availableLogicalHeight(heightType);

ASSERT_NOT_REACHED();
return 0_lu;
}

Expand Down Expand Up @@ -2347,10 +2315,8 @@ LayoutUnit RenderBox::containingBlockAvailableLineWidthInFragment(RenderFragment

LayoutUnit RenderBox::perpendicularContainingBlockLogicalHeight() const
{
if (hasOverridingContainingBlockContentLogicalHeight()) {
if (auto height = overridingContainingBlockContentLogicalHeight())
return height.value();
}
if (auto overridingContainingBlockContentLogicalHeight = this->overridingContainingBlockContentLogicalHeight(); overridingContainingBlockContentLogicalHeight && *overridingContainingBlockContentLogicalHeight)
return overridingContainingBlockContentLogicalHeight->value();

auto* containingBlock = this->containingBlock();
if (auto overridingLogicalHeight = containingBlock->overridingLogicalHeight())
Expand Down Expand Up @@ -3411,8 +3377,6 @@ static bool tableCellShouldHaveZeroInitialSize(const RenderBlock& block, const R

std::optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length& height, UpdatePercentageHeightDescendants updateDescendants) const
{
std::optional<LayoutUnit> availableHeight;

bool skippedAutoHeightContainingBlock = false;
RenderBlock* cb = containingBlock();
const RenderBox* containingBlockChild = this;
Expand All @@ -3428,26 +3392,27 @@ std::optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length
if (updateDescendants == UpdatePercentageHeightDescendants::Yes)
cb->addPercentHeightDescendant(const_cast<RenderBox&>(*this));

bool isOrthogonal = isHorizontal != cb->isHorizontalWritingMode();
if (hasOverridingContainingBlockContentLogicalWidth() && isOrthogonal)
availableHeight = overridingContainingBlockContentLogicalWidth();
else if (hasOverridingContainingBlockContentLogicalHeight() && !isOrthogonal)
availableHeight = overridingContainingBlockContentLogicalHeight();
else if (isOrthogonal)
availableHeight = containingBlockChild->containingBlockLogicalWidthForContent();
else if (is<RenderTableCell>(*cb)) {
if (!skippedAutoHeightContainingBlock) {
// 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.
auto overridingLogicalHeight = cb->overridingLogicalHeight();
if (!overridingLogicalHeight)
return tableCellShouldHaveZeroInitialSize(*cb, *this, scrollsOverflowY()) ? std::optional<LayoutUnit>(0) : std::nullopt;
availableHeight = *overridingLogicalHeight - cb->computedCSSPaddingBefore() - cb->computedCSSPaddingAfter() - cb->borderBefore() - cb->borderAfter() - cb->scrollbarLogicalHeight();
}
} else
availableHeight = cb->availableLogicalHeightForPercentageComputation();

auto availableHeight = std::optional<LayoutUnit> { };
auto isOrthogonal = isHorizontal != cb->isHorizontalWritingMode();
if (auto overridingAvailableHeight = isOrthogonal ? overridingContainingBlockContentLogicalWidth() : overridingContainingBlockContentLogicalHeight())
availableHeight = *overridingAvailableHeight;
else {
if (isOrthogonal)
availableHeight = containingBlockChild->containingBlockLogicalWidthForContent();
else if (is<RenderTableCell>(*cb)) {
if (!skippedAutoHeightContainingBlock) {
// 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.
auto overridingLogicalHeight = cb->overridingLogicalHeight();
if (!overridingLogicalHeight)
return tableCellShouldHaveZeroInitialSize(*cb, *this, scrollsOverflowY()) ? std::optional<LayoutUnit>(0) : std::nullopt;
availableHeight = *overridingLogicalHeight - cb->computedCSSPaddingBefore() - cb->computedCSSPaddingAfter() - cb->borderBefore() - cb->borderAfter() - cb->scrollbarLogicalHeight();
}
} else
availableHeight = cb->availableLogicalHeightForPercentageComputation();
}

if (!availableHeight)
return availableHeight;

Expand Down Expand Up @@ -3593,8 +3558,10 @@ bool RenderBox::replacedMinMaxLogicalHeightComputesAsNone(SizeType sizeType) con
if (logicalHeight == initialLogicalHeight)
return true;

if (logicalHeight.isPercentOrCalculated() && hasOverridingContainingBlockContentLogicalHeight())
return overridingContainingBlockContentLogicalHeight() == std::nullopt;
if (logicalHeight.isPercentOrCalculated()) {
if (auto overridingContainingBlockContentLogicalHeight = this->overridingContainingBlockContentLogicalHeight())
return !*overridingContainingBlockContentLogicalHeight;
}

// Make sure % min-height and % max-height resolve to none if the containing block has auto height.
// Note that the "height" case for replaced elements was handled by hasReplacedLogicalHeight, which is why
Expand Down Expand Up @@ -3799,10 +3766,8 @@ LayoutUnit RenderBox::containingBlockLogicalWidthForPositioned(const RenderBoxMo
if (checkForPerpendicularWritingMode && containingBlock.isHorizontalWritingMode() != isHorizontalWritingMode())
return containingBlockLogicalHeightForPositioned(containingBlock, false);

if (hasOverridingContainingBlockContentLogicalWidth()) {
if (auto width = overridingContainingBlockContentLogicalWidth())
return width.value();
}
if (auto overridingContainingBlockContentLogicalWidth = this->overridingContainingBlockContentLogicalWidth(); overridingContainingBlockContentLogicalWidth && *overridingContainingBlockContentLogicalWidth)
return overridingContainingBlockContentLogicalWidth->value();

if (auto* box = dynamicDowncast<RenderBox>(containingBlock)) {
bool isFixedPosition = isFixedPositioned();
Expand Down Expand Up @@ -3851,10 +3816,8 @@ LayoutUnit RenderBox::containingBlockLogicalHeightForPositioned(const RenderBoxM
if (checkForPerpendicularWritingMode && containingBlock.isHorizontalWritingMode() != isHorizontalWritingMode())
return containingBlockLogicalWidthForPositioned(containingBlock, nullptr, false);

if (hasOverridingContainingBlockContentLogicalHeight()) {
if (auto height = overridingContainingBlockContentLogicalHeight())
return height.value();
}
if (auto overridingContainingBlockContentLogicalHeight = this->overridingContainingBlockContentLogicalHeight(); overridingContainingBlockContentLogicalHeight && *overridingContainingBlockContentLogicalHeight)
return overridingContainingBlockContentLogicalHeight->value();

if (auto* box = dynamicDowncast<RenderBox>(containingBlock)) {
bool isFixedPosition = isFixedPositioned();
Expand Down
Loading

0 comments on commit cdde289

Please sign in to comment.