Skip to content

Commit

Permalink
[IFC] InlineFormattingContext should not cache any state
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260919

Reviewed by Antti Koivisto.

FormattingContext classes are essentially just a collection of static-like functions with no state whatsoever.
This patch moves m_maximumIntrinsicWidthResultForSingleLine from InlineFormattingContext to InlineFormattingState.

(It is used to cache some state between preferred width computation and the subsequent line layout.)

* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/layout/formattingContexts/inline/AbstractLineBuilder.h:
(): Deleted.
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::layout):
(WebCore::Layout::InlineFormattingContext::computedIntrinsicSizes):
(WebCore::Layout::InlineFormattingContext::createDisplayContentForLineFromCachedContent):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.h:
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingState.h:
(WebCore::Layout::InlineFormattingState::setMaximumIntrinsicWidthLayoutResult):
(WebCore::Layout::InlineFormattingState::clearMaximumIntrinsicWidthLayoutResult):
(WebCore::Layout::InlineFormattingState::maximumIntrinsicWidthLayoutResult):
* Source/WebCore/layout/formattingContexts/inline/IntrinsicWidthHandler.h:
(WebCore::Layout::IntrinsicWidthHandler::maximumIntrinsicWidthResult):
(WebCore::Layout::IntrinsicWidthHandler::maximumIntrinsicWidthResult const): Deleted.
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:
(WebCore::LayoutIntegration::LineLayout::LineLayout):
(WebCore::LayoutIntegration::LineLayout::computeIntrinsicWidthConstraints):
(WebCore::LayoutIntegration::LineLayout::layout):
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.h:

Canonical link: https://commits.webkit.org/267472@main
  • Loading branch information
alanbaradlay committed Aug 30, 2023
1 parent 2f546b2 commit a58f000
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 81 deletions.
4 changes: 4 additions & 0 deletions Source/WebCore/WebCore.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -5273,6 +5273,7 @@
DDA3D5CF2A7D844D000ECEE1 /* TextOnlySimpleLineBuilder.h in Headers */ = {isa = PBXBuildFile; fileRef = DDA3D5CE2A7D844D000ECEE1 /* TextOnlySimpleLineBuilder.h */; settings = {ATTRIBUTES = (Private, ); }; };
DDB04F3A278E5539008D3678 /* libWTF.a in Product Dependencies */ = {isa = PBXBuildFile; fileRef = DDB04F39278E5531008D3678 /* libWTF.a */; };
DDC8E9562A3DE60400FF8ECF /* LogicalFlexItem.h in Headers */ = {isa = PBXBuildFile; fileRef = DDC8E9552A3DE60400FF8ECF /* LogicalFlexItem.h */; settings = {ATTRIBUTES = (Private, ); }; };
DDD517EC2A9FC5440069AF81 /* LineLayoutResult.h in Headers */ = {isa = PBXBuildFile; fileRef = DDD517EB2A9FC5440069AF81 /* LineLayoutResult.h */; settings = {ATTRIBUTES = (Private, ); }; };
DDDFE83E2846ECE2006F1EE5 /* cocoa in Copy PDF.js Extras */ = {isa = PBXBuildFile; fileRef = DDDFE83B2846ECD3006F1EE5 /* cocoa */; };
DDDFE83F2846ECE5006F1EE5 /* content-script.js in Copy PDF.js Extras */ = {isa = PBXBuildFile; fileRef = DDDFE83C2846ECD4006F1EE5 /* content-script.js */; };
DDE9931E278D0E4B00F60D26 /* libWebKitAdditions.a in Product Dependencies */ = {isa = PBXBuildFile; fileRef = DDE99308278D080B00F60D26 /* libWebKitAdditions.a */; };
Expand Down Expand Up @@ -18295,6 +18296,7 @@
DDA3D5D02A7D847E000ECEE1 /* TextOnlySimpleLineBuilder.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = TextOnlySimpleLineBuilder.cpp; sourceTree = "<group>"; };
DDB04F39278E5531008D3678 /* libWTF.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; path = libWTF.a; sourceTree = BUILT_PRODUCTS_DIR; };
DDC8E9552A3DE60400FF8ECF /* LogicalFlexItem.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LogicalFlexItem.h; sourceTree = "<group>"; };
DDD517EB2A9FC5440069AF81 /* LineLayoutResult.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LineLayoutResult.h; sourceTree = "<group>"; };
DDDFE83A2846ECD3006F1EE5 /* adwaita */ = {isa = PBXFileReference; lastKnownFileType = folder; path = adwaita; sourceTree = "<group>"; };
DDDFE83B2846ECD3006F1EE5 /* cocoa */ = {isa = PBXFileReference; lastKnownFileType = folder; path = cocoa; sourceTree = "<group>"; };
DDDFE83C2846ECD4006F1EE5 /* content-script.js */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.javascript; path = "content-script.js"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -21311,6 +21313,7 @@
6F1CC1DD225F8B4200720AD2 /* InlineTextItem.h */,
DD9467D52A92D3A300458979 /* IntrinsicWidthHandler.cpp */,
DD9467D62A92D3B800458979 /* IntrinsicWidthHandler.h */,
DDD517EB2A9FC5440069AF81 /* LineLayoutResult.h */,
DDA3D5D02A7D847E000ECEE1 /* TextOnlySimpleLineBuilder.cpp */,
DDA3D5CE2A7D844D000ECEE1 /* TextOnlySimpleLineBuilder.h */,
);
Expand Down Expand Up @@ -39746,6 +39749,7 @@
84730D911248F0B300D3A9C9 /* LightSource.h in Headers */,
B22279650D00BF220071B782 /* LinearGradientAttributes.h in Headers */,
AB31C91E10AE1B8E000C7B92 /* LineClampValue.h in Headers */,
DDD517EC2A9FC5440069AF81 /* LineLayoutResult.h in Headers */,
FFEFAB2A18380DA000514534 /* LineLayoutState.h in Headers */,
6F1B101427E2D524007178E6 /* LineSelection.h in Headers */,
FFDBC047183D27B700407109 /* LineWidth.h in Headers */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@

#pragma once

#include "FloatingState.h"
#include "InlineLine.h"
#include "InlineLineTypes.h"
#include "InlineRect.h"
#include "LayoutUnits.h"
#include "LineLayoutResult.h"

namespace WebCore {
namespace Layout {
Expand All @@ -39,64 +35,6 @@ struct LineInput {
InlineRect initialLogicalRect;
};

struct LineLayoutResult {
using PlacedFloatList = FloatingState::FloatList;
using SuspendedFloatList = Vector<const Box*>;

InlineItemRange inlineItemRange;
Line::RunList inlineContent;

struct FloatContent {
PlacedFloatList placedFloats;
SuspendedFloatList suspendedFloats;
bool hasIntrusiveFloat { false };
};
FloatContent floatContent { };

struct ContentGeometry {
InlineLayoutUnit logicalLeft { 0.f };
InlineLayoutUnit logicalWidth { 0.f };
InlineLayoutUnit logicalRightIncludingNegativeMargin { 0.f }; // Note that with negative horizontal margin value, contentLogicalLeft + contentLogicalWidth is not necessarily contentLogicalRight.
std::optional<InlineLayoutUnit> trailingOverflowingContentWidth { };
};
ContentGeometry contentGeometry { };

struct LineGeometry {
InlineLayoutPoint logicalTopLeft;
InlineLayoutUnit logicalWidth { 0.f };
InlineLayoutUnit initialLogicalLeftIncludingIntrusiveFloats { 0.f };
std::optional<InlineLayoutUnit> initialLetterClearGap { };
};
LineGeometry lineGeometry { };

struct HangingContent {
bool shouldContributeToScrollableOverflow { false };
InlineLayoutUnit logicalWidth { 0.f };
};
HangingContent hangingContent { };

struct Directionality {
Vector<int32_t> visualOrderList;
TextDirection inlineBaseDirection { TextDirection::LTR };
};
Directionality directionality { };

struct IsFirstLast {
enum class FirstFormattedLine : uint8_t {
No,
WithinIFC,
WithinBFC
};
FirstFormattedLine isFirstFormattedLine { FirstFormattedLine::WithinIFC };
bool isLastLineWithInlineContent { true };
};
IsFirstLast isFirstLast { };
// Misc
size_t nonSpanningInlineLevelBoxCount { 0 };
InlineLayoutUnit trimmedTrailingWhitespaceWidth { 0.f }; // only used for line-break: after-white-space currently
std::optional<InlineLayoutUnit> hintForNextLineTopToAvoidIntrusiveFloat { }; // This is only used for cases when intrusive floats prevent any content placement at current vertical position.
};

class AbstractLineBuilder {
public:
virtual LineLayoutResult layoutInlineContent(const LineInput&, const std::optional<PreviousLine>&) = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ InlineLayoutResult InlineFormattingContext::layout(const ConstraintsForInlineCon
auto needsLayoutStartPosition = !lineDamage || !lineDamage->start() ? InlineItemPosition() : lineDamage->start()->inlineItemPosition;
auto needsInlineItemsUpdate = inlineFormattingState.inlineItems().isEmpty() || lineDamage;
if (needsInlineItemsUpdate) {
// FIXME: This shoul go to invalidation.
m_maximumIntrinsicWidthResultForSingleLine = { };
// FIXME: This should go to invalidation.
inlineFormattingState.clearMaximumIntrinsicWidthLayoutResult();
InlineItemsBuilder { root(), inlineFormattingState }.build(needsLayoutStartPosition);
}

Expand Down Expand Up @@ -130,9 +130,9 @@ IntrinsicWidthConstraints InlineFormattingContext::computedIntrinsicSizes(const

auto intrinsicWidthHandler = IntrinsicWidthHandler { *this };
auto intrinsicSizes = intrinsicWidthHandler.computedIntrinsicSizes();
m_maximumIntrinsicWidthResultForSingleLine = intrinsicWidthHandler.maximumIntrinsicWidthResult();

formattingState().setIntrinsicWidthConstraints(intrinsicSizes);
inlineFormattingState.setIntrinsicWidthConstraints(intrinsicSizes);
if (intrinsicWidthHandler.maximumIntrinsicWidthResult())
inlineFormattingState.setMaximumIntrinsicWidthLayoutResult(WTFMove(*intrinsicWidthHandler.maximumIntrinsicWidthResult()));
return intrinsicSizes;
}

Expand Down Expand Up @@ -306,27 +306,31 @@ void InlineFormattingContext::resetGeometryForClampedContent(const InlineItemRan

bool InlineFormattingContext::createDisplayContentForLineFromCachedContent(const ConstraintsForInlineContent& constraints, const InlineLayoutState& inlineLayoutState, InlineLayoutResult& layoutResult)
{
if (!m_maximumIntrinsicWidthResultForSingleLine)
auto& inlineFormattingState = formattingState();

if (!inlineFormattingState.maximumIntrinsicWidthLayoutResult())
return false;

auto& maximumIntrinsicWidthResultForSingleLine = inlineFormattingState.maximumIntrinsicWidthLayoutResult();
auto horizontalAvailableSpace = constraints.horizontal().logicalWidth;
if (m_maximumIntrinsicWidthResultForSingleLine->constraint > horizontalAvailableSpace) {
m_maximumIntrinsicWidthResultForSingleLine = { };
if (maximumIntrinsicWidthResultForSingleLine->constraint > horizontalAvailableSpace) {
inlineFormattingState.clearMaximumIntrinsicWidthLayoutResult();
return false;
}
if (!inlineLayoutState.parentBlockLayoutState().floatingState().isEmpty()) {
m_maximumIntrinsicWidthResultForSingleLine = { };
inlineFormattingState.clearMaximumIntrinsicWidthLayoutResult();
return false;
}

auto& lineBreakingResult = m_maximumIntrinsicWidthResultForSingleLine->result;
auto& lineBreakingResult = maximumIntrinsicWidthResultForSingleLine->result;
auto restoreTrimmedTrailingWhitespaceIfApplicable = [&] {
if (root().style().lineBreak() != LineBreak::AfterWhiteSpace || !lineBreakingResult.trimmedTrailingWhitespaceWidth)
return;
if (ceiledLayoutUnit(lineBreakingResult.contentGeometry.logicalWidth) + LayoutUnit::epsilon() <= horizontalAvailableSpace)
return;
if (!Line::restoreTrimmedTrailingWhitespace(lineBreakingResult.trimmedTrailingWhitespaceWidth, lineBreakingResult.inlineContent)) {
ASSERT_NOT_REACHED();
m_maximumIntrinsicWidthResultForSingleLine = { };
inlineFormattingState.clearMaximumIntrinsicWidthLayoutResult();
return;
}
lineBreakingResult.contentGeometry.logicalWidth += lineBreakingResult.trimmedTrailingWhitespaceWidth;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class InlineFormattingContext final : public FormattingContext {

const InlineFormattingGeometry m_inlineFormattingGeometry;
const InlineFormattingQuirks m_inlineFormattingQuirks;
std::optional<IntrinsicWidthHandler::LineBreakingResult> m_maximumIntrinsicWidthResultForSingleLine { };
};

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "FormattingState.h"
#include "InlineDisplayContent.h"
#include "InlineItem.h"
#include "IntrinsicWidthHandler.h"
#include <wtf/IsoMalloc.h>

namespace WebCore {
Expand All @@ -53,8 +54,13 @@ class InlineFormattingState : public FormattingState {
void clearInlineItems() { m_inlineItems.clear(); }
void shrinkToFit() { m_inlineItems.shrinkToFit(); }

void setMaximumIntrinsicWidthLayoutResult(IntrinsicWidthHandler::LineBreakingResult&& layoutResult) { m_maximumIntrinsicWidthLayoutResult = WTFMove(layoutResult); }
void clearMaximumIntrinsicWidthLayoutResult() { m_maximumIntrinsicWidthLayoutResult = { }; }
std::optional<IntrinsicWidthHandler::LineBreakingResult>& maximumIntrinsicWidthLayoutResult() { return m_maximumIntrinsicWidthLayoutResult; }

private:
InlineItems m_inlineItems;
std::optional<IntrinsicWidthHandler::LineBreakingResult> m_maximumIntrinsicWidthLayoutResult { };
bool m_isNonBidiTextAndForcedLineBreakOnlyContent { false };
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@

#pragma once

#include "AbstractLineBuilder.h"
#include "LineLayoutResult.h"

namespace WebCore {
namespace Layout {

class AbstractLineBuilder;
class ElementBox;
class InlineFormattingContext;
class InlineFormattingState;
Expand All @@ -47,7 +48,7 @@ class IntrinsicWidthHandler {
InlineLayoutUnit constraint;
LineLayoutResult result;
};
std::optional<LineBreakingResult> maximumIntrinsicWidthResult() const { return m_maximumIntrinsicWidthResultForSingleLine; }
std::optional<LineBreakingResult>& maximumIntrinsicWidthResult() { return m_maximumIntrinsicWidthResultForSingleLine; }

private:
enum class MayCacheLayoutResult : bool { No, Yes };
Expand Down
94 changes: 94 additions & 0 deletions Source/WebCore/layout/formattingContexts/inline/LineLayoutResult.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright (C) 2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
* THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
* BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
* THE POSSIBILITY OF SUCH DAMAGE.
*/

#pragma once

#include "FloatingState.h"
#include "InlineLineTypes.h"
#include "LayoutUnits.h"

namespace WebCore {
namespace Layout {

struct LineLayoutResult {
using PlacedFloatList = FloatingState::FloatList;
using SuspendedFloatList = Vector<const Box*>;

InlineItemRange inlineItemRange;
Line::RunList inlineContent;

struct FloatContent {
PlacedFloatList placedFloats;
SuspendedFloatList suspendedFloats;
bool hasIntrusiveFloat { false };
};
FloatContent floatContent { };

struct ContentGeometry {
InlineLayoutUnit logicalLeft { 0.f };
InlineLayoutUnit logicalWidth { 0.f };
InlineLayoutUnit logicalRightIncludingNegativeMargin { 0.f }; // Note that with negative horizontal margin value, contentLogicalLeft + contentLogicalWidth is not necessarily contentLogicalRight.
std::optional<InlineLayoutUnit> trailingOverflowingContentWidth { };
};
ContentGeometry contentGeometry { };

struct LineGeometry {
InlineLayoutPoint logicalTopLeft;
InlineLayoutUnit logicalWidth { 0.f };
InlineLayoutUnit initialLogicalLeftIncludingIntrusiveFloats { 0.f };
std::optional<InlineLayoutUnit> initialLetterClearGap { };
};
LineGeometry lineGeometry { };

struct HangingContent {
bool shouldContributeToScrollableOverflow { false };
InlineLayoutUnit logicalWidth { 0.f };
};
HangingContent hangingContent { };

struct Directionality {
Vector<int32_t> visualOrderList;
TextDirection inlineBaseDirection { TextDirection::LTR };
};
Directionality directionality { };

struct IsFirstLast {
enum class FirstFormattedLine : uint8_t {
No,
WithinIFC,
WithinBFC
};
FirstFormattedLine isFirstFormattedLine { FirstFormattedLine::WithinIFC };
bool isLastLineWithInlineContent { true };
};
IsFirstLast isFirstLast { };
// Misc
size_t nonSpanningInlineLevelBoxCount { 0 };
InlineLayoutUnit trimmedTrailingWhitespaceWidth { 0.f }; // only used for line-break: after-white-space currently
std::optional<InlineLayoutUnit> hintForNextLineTopToAvoidIntrusiveFloat { }; // This is only used for cases when intrusive floats prevent any content placement at current vertical position.
};

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ LineLayout::LineLayout(RenderBlockFlow& flow)
, m_layoutState(flow.view().layoutState())
, m_blockFormattingState(layoutState().ensureBlockFormattingState(rootLayoutBox()))
, m_inlineFormattingState(layoutState().ensureInlineFormattingState(rootLayoutBox()))
, m_inlineFormattingContext(rootLayoutBox(), m_inlineFormattingState)
{
}

Expand Down Expand Up @@ -528,7 +527,7 @@ void LineLayout::updateOverflow()

std::pair<LayoutUnit, LayoutUnit> LineLayout::computeIntrinsicWidthConstraints()
{
auto constraints = m_inlineFormattingContext.computedIntrinsicSizes(m_lineDamage.get());
auto constraints = Layout::InlineFormattingContext { rootLayoutBox(), m_inlineFormattingState }.computedIntrinsicSizes(m_lineDamage.get());
return { constraints.minimum, constraints.maximum };
}

Expand Down Expand Up @@ -588,7 +587,7 @@ std::optional<LayoutRect> LineLayout::layout()
}();
auto parentBlockLayoutState = Layout::BlockLayoutState { m_blockFormattingState.floatingState(), lineClamp(flow()), textBoxTrim(flow()), intrusiveInitialLetterBottom() };
auto inlineLayoutState = Layout::InlineLayoutState { parentBlockLayoutState, WTFMove(m_nestedListMarkerOffsets) };
auto layoutResult = m_inlineFormattingContext.layout(inlineContentConstraints, inlineLayoutState, m_lineDamage.get());
auto layoutResult = Layout::InlineFormattingContext { rootLayoutBox(), m_inlineFormattingState }.layout(inlineContentConstraints, inlineLayoutState, m_lineDamage.get());

auto repaintRect = LayoutRect { constructContent(inlineLayoutState, WTFMove(layoutResult)) };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ class LineLayout : public CanMakeCheckedPtr {
Layout::BlockFormattingState& m_blockFormattingState;
Layout::InlineFormattingState& m_inlineFormattingState;
std::optional<Layout::ConstraintsForInlineContent> m_inlineContentConstraints;
Layout::InlineFormattingContext m_inlineFormattingContext;
// FIXME: This should be part of LayoutState.
std::unique_ptr<Layout::InlineDamage> m_lineDamage;
std::unique_ptr<InlineContent> m_inlineContent;
Expand Down

0 comments on commit a58f000

Please sign in to comment.