- Revision
- 254661
- Author
- za...@apple.com
- Date
- 2020-01-15 18:23:39 -0800 (Wed, 15 Jan 2020)
Log Message
[LFC][IFC] LineBreaker::shouldWrapInlineContent should take the candidate content width
https://bugs.webkit.org/show_bug.cgi?id=206305
<rdar://problem/58613977>
Reviewed by Antti Koivisto.
We already have the width information of the candidate runs. Let's not loop through the runs just to re-collect the logical width.
~3% progression on PerformanceTests/Layout/line-layout-simple.html.
* layout/inlineformatting/InlineLineBreaker.cpp:
(WebCore::Layout::LineBreaker::shouldWrapInlineContent):
(WebCore::Layout::LineBreaker::tryWrappingInlineContent const):
(WebCore::Layout::ContinuousContent::ContinuousContent):
* layout/inlineformatting/InlineLineBreaker.h:
* layout/inlineformatting/LineLayoutContext.cpp:
(WebCore::Layout::LineCandidateContent::inlineContentLogicalWidth const):
(WebCore::Layout::LineCandidateContent::append):
(WebCore::Layout::LineCandidateContent::reset):
(WebCore::Layout::LineLayoutContext::tryAddingInlineItems):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (254660 => 254661)
--- trunk/Source/WebCore/ChangeLog 2020-01-16 02:09:56 UTC (rev 254660)
+++ trunk/Source/WebCore/ChangeLog 2020-01-16 02:23:39 UTC (rev 254661)
@@ -1,3 +1,25 @@
+2020-01-15 Zalan Bujtas <za...@apple.com>
+
+ [LFC][IFC] LineBreaker::shouldWrapInlineContent should take the candidate content width
+ https://bugs.webkit.org/show_bug.cgi?id=206305
+ <rdar://problem/58613977>
+
+ Reviewed by Antti Koivisto.
+
+ We already have the width information of the candidate runs. Let's not loop through the runs just to re-collect the logical width.
+ ~3% progression on PerformanceTests/Layout/line-layout-simple.html.
+
+ * layout/inlineformatting/InlineLineBreaker.cpp:
+ (WebCore::Layout::LineBreaker::shouldWrapInlineContent):
+ (WebCore::Layout::LineBreaker::tryWrappingInlineContent const):
+ (WebCore::Layout::ContinuousContent::ContinuousContent):
+ * layout/inlineformatting/InlineLineBreaker.h:
+ * layout/inlineformatting/LineLayoutContext.cpp:
+ (WebCore::Layout::LineCandidateContent::inlineContentLogicalWidth const):
+ (WebCore::Layout::LineCandidateContent::append):
+ (WebCore::Layout::LineCandidateContent::reset):
+ (WebCore::Layout::LineLayoutContext::tryAddingInlineItems):
+
2020-01-15 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r254565.
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLineBreaker.cpp (254660 => 254661)
--- trunk/Source/WebCore/layout/inlineformatting/InlineLineBreaker.cpp 2020-01-16 02:09:56 UTC (rev 254660)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLineBreaker.cpp 2020-01-16 02:23:39 UTC (rev 254661)
@@ -50,8 +50,20 @@
return whitespace == WhiteSpace::Pre || whitespace == WhiteSpace::PreWrap || whitespace == WhiteSpace::BreakSpaces;
}
+static inline Optional<size_t> lastWrapOpportunityIndex(const LineBreaker::RunList& runList)
+{
+ // <span style="white-space: pre">no_wrap</span><span>yes wrap</span><span style="white-space: pre">no_wrap</span>.
+ // [container start][no_wrap][container end][container start][yes] <- continuous content
+ // [ ] <- continuous content
+ // [wrap][container end][container start][no_wrap][container end] <- continuous content
+ // Return #0 as the index where the second continuous content can wrap at.
+ ASSERT(!runList.isEmpty());
+ auto lastItemIndex = runList.size() - 1;
+ return isWrappingAllowed(runList[lastItemIndex].inlineItem.style()) ? makeOptional(lastItemIndex) : WTF::nullopt;
+}
+
struct ContinuousContent {
- ContinuousContent(const LineBreaker::RunList&);
+ ContinuousContent(const LineBreaker::RunList&, InlineLayoutUnit contentLogicalWidth);
const LineBreaker::RunList& runs() const { return m_runs; }
bool isEmpty() const { return m_runs.isEmpty(); }
@@ -64,7 +76,6 @@
bool hasTrailingCollapsibleContent() const { return !!m_trailingCollapsibleContent.width; }
bool isTrailingContentFullyCollapsible() const { return m_trailingCollapsibleContent.isFullyCollapsible; }
- Optional<size_t> lastWrapOpportunityIndex() const;
Optional<size_t> firstTextRunIndex() const;
Optional<size_t> lastContentRunIndex() const;
@@ -104,40 +115,44 @@
return whitespace == WhiteSpace::Normal || whitespace == WhiteSpace::NoWrap || whitespace == WhiteSpace::PreWrap || whitespace == WhiteSpace::PreLine;
}
-LineBreaker::Result LineBreaker::shouldWrapInlineContent(const RunList& candidateRuns, const LineStatus& lineStatus)
+LineBreaker::Result LineBreaker::shouldWrapInlineContent(const RunList& candidateRuns, InlineLayoutUnit candidateContentLogicalWidth, const LineStatus& lineStatus)
{
- auto candidateContent = ContinuousContent { candidateRuns };
- ASSERT(!candidateContent.isEmpty());
- auto result = tryWrappingInlineContent(candidateContent, lineStatus);
- // If this is not the end of the line, hold on to the last eligible line wrap opportunity so that we could revert back
- // to this position if no other line breaking opportunity exists in this content.
- if (result.isEndOfLine == IsEndOfLine::Yes)
- return result;
- if (auto lastLineWrapOpportunityIndex = candidateContent.lastWrapOpportunityIndex()) {
- auto isEligibleLineWrapOpportunity = [&] (auto& candidateItem) {
- // Just check for leading collapsible whitespace for now.
- if (!lineStatus.lineIsEmpty || !candidateItem.isText() || !downcast<InlineTextItem>(candidateItem).isWhitespace())
- return true;
- return shouldKeepBeginningOfLineWhitespace(candidateItem.style());
- };
- auto& inlineItem = candidateContent.runs()[*lastLineWrapOpportunityIndex].inlineItem;
- if (isEligibleLineWrapOpportunity(inlineItem))
- m_lastWrapOpportunity = &inlineItem;
+ auto inlineContentWrapping = [&] {
+ if (candidateContentLogicalWidth <= lineStatus.availableWidth)
+ return Result { Result::Action::Keep };
+#if USE_FLOAT_AS_INLINE_LAYOUT_UNIT
+ // Preferred width computation sums up floats while line breaker substracts them. This can lead to epsilon-scale differences.
+ if (WTF::areEssentiallyEqual(candidateContentLogicalWidth, lineStatus.availableWidth))
+ return Result { Result::Action::Keep };
+#endif
+ return tryWrappingInlineContent(candidateRuns, candidateContentLogicalWidth, lineStatus);
+ };
+
+ auto result = inlineContentWrapping();
+ if (result.action == Result::Action::Keep) {
+ // If this is not the end of the line, hold on to the last eligible line wrap opportunity so that we could revert back
+ // to this position if no other line breaking opportunity exists in this content.
+ if (auto lastLineWrapOpportunityIndex = lastWrapOpportunityIndex(candidateRuns)) {
+ auto isEligibleLineWrapOpportunity = [&] (auto& candidateItem) {
+ // Just check for leading collapsible whitespace for now.
+ if (!lineStatus.lineIsEmpty || !candidateItem.isText() || !downcast<InlineTextItem>(candidateItem).isWhitespace())
+ return true;
+ return shouldKeepBeginningOfLineWhitespace(candidateItem.style());
+ };
+ auto& inlineItem = candidateRuns[*lastLineWrapOpportunityIndex].inlineItem;
+ if (isEligibleLineWrapOpportunity(inlineItem))
+ m_lastWrapOpportunity = &inlineItem;
+ }
}
return result;
}
-LineBreaker::Result LineBreaker::tryWrappingInlineContent(const ContinuousContent& candidateContent, const LineStatus& lineStatus) const
+LineBreaker::Result LineBreaker::tryWrappingInlineContent(const RunList& candidateRuns, InlineLayoutUnit candidateContentLogicalWidth, const LineStatus& lineStatus) const
{
- if (candidateContent.width() <= lineStatus.availableWidth)
- return { Result::Action::Keep };
+ auto candidateContent = ContinuousContent { candidateRuns, candidateContentLogicalWidth };
+ ASSERT(!candidateContent.isEmpty());
-#if USE_FLOAT_AS_INLINE_LAYOUT_UNIT
- // Preferred width computation sums up floats while line breaker substracts them. This can lead to epsilon-scale differences.
- if (WTF::areEssentiallyEqual(candidateContent.width(), lineStatus.availableWidth))
- return { Result::Action::Keep };
-#endif
-
+ ASSERT(candidateContent.width() > lineStatus.availableWidth);
if (candidateContent.hasTrailingCollapsibleContent()) {
ASSERT(candidateContent.hasTextContentOnly());
auto IsEndOfLine = isContentWrappingAllowed(candidateContent) ? IsEndOfLine::Yes : IsEndOfLine::No;
@@ -340,8 +355,9 @@
return { };
}
-ContinuousContent::ContinuousContent(const LineBreaker::RunList& runs)
+ContinuousContent::ContinuousContent(const LineBreaker::RunList& runs, InlineLayoutUnit contentLogicalWidth)
: m_runs(runs)
+ , m_width(contentLogicalWidth)
{
// Figure out the trailing collapsible state.
for (auto& run : WTF::makeReversedRange(m_runs)) {
@@ -372,13 +388,6 @@
break;
}
}
- // The trailing whitespace loop above is mostly about inspecting the last entry, so while it
- // looks like we are looping through the m_runs twice, it's really just one full loop in addition to checking the last run.
- for (auto& run : m_runs) {
- // Line break is not considered an inline content.
- ASSERT(!run.inlineItem.isLineBreak());
- m_width += run.logicalWidth;
- }
}
bool ContinuousContent::hasTextContentOnly() const
@@ -439,17 +448,6 @@
return true;
}
-Optional<size_t> ContinuousContent::lastWrapOpportunityIndex() const
-{
- // <span style="white-space: pre">no_wrap</span><span>yes wrap</span><span style="white-space: pre">no_wrap</span>.
- // [container start][no_wrap][container end][container start][yes] <- continuous content
- // [ ] <- continuous content
- // [wrap][container end][container start][no_wrap][container end] <- continuous content
- // Return #0 as the index where the second continuous content can wrap at.
- auto lastItemIndex = m_runs.size() - 1;
- return isWrappingAllowed(m_runs[lastItemIndex].inlineItem.style()) ? makeOptional(lastItemIndex) : WTF::nullopt;
-}
-
void ContinuousContent::TrailingCollapsibleContent::reset()
{
isFullyCollapsible = false;
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLineBreaker.h (254660 => 254661)
--- trunk/Source/WebCore/layout/inlineformatting/InlineLineBreaker.h 2020-01-16 02:09:56 UTC (rev 254660)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLineBreaker.h 2020-01-16 02:23:39 UTC (rev 254661)
@@ -79,7 +79,7 @@
bool lineHasFullyCollapsibleTrailingRun { false };
bool lineIsEmpty { true };
};
- Result shouldWrapInlineContent(const RunList& candidateRuns, const LineStatus&);
+ Result shouldWrapInlineContent(const RunList& candidateRuns, InlineLayoutUnit candidateContentLogicalWidth, const LineStatus&);
bool shouldWrapFloatBox(InlineLayoutUnit floatLogicalWidth, InlineLayoutUnit availableWidth, bool lineIsEmpty);
void setHyphenationDisabled() { n_hyphenationIsDisabled = true; }
@@ -95,7 +95,7 @@
// [container start][span1][container end][between][container start][span2][container end]
// see https://drafts.csswg.org/css-text-3/#line-break-details
Optional<WrappedTextContent> wrapTextContent(const RunList&, const LineStatus&) const;
- Result tryWrappingInlineContent(const ContinuousContent&, const LineStatus&) const;
+ Result tryWrappingInlineContent(const RunList&, InlineLayoutUnit candidateContentLogicalWidth, const LineStatus&) const;
Optional<PartialRun> tryBreakingTextRun(const Run& overflowRun, InlineLayoutUnit availableWidth) const;
enum class WordBreakRule {
Modified: trunk/Source/WebCore/layout/inlineformatting/LineLayoutContext.cpp (254660 => 254661)
--- trunk/Source/WebCore/layout/inlineformatting/LineLayoutContext.cpp 2020-01-16 02:09:56 UTC (rev 254660)
+++ trunk/Source/WebCore/layout/inlineformatting/LineLayoutContext.cpp 2020-01-16 02:23:39 UTC (rev 254661)
@@ -181,6 +181,7 @@
bool hasIntrusiveFloats() const { return !m_floats.isEmpty(); }
const LineBreaker::RunList& inlineRuns() const { return m_inlineRuns; }
+ InlineLayoutUnit inlineContentLogicalWidth() const { return m_inlineContentLogicalWidth; }
const LineLayoutContext::FloatList& floats() const { return m_floats; }
const InlineItem* trailingLineBreak() const { return m_trailingLineBreak; }
@@ -190,6 +191,7 @@
private:
void setTrailingLineBreak(const InlineItem& lineBreakItem) { m_trailingLineBreak = &lineBreakItem; }
+ InlineLayoutUnit m_inlineContentLogicalWidth { 0 };
LineBreaker::RunList m_inlineRuns;
LineLayoutContext::FloatList m_floats;
const InlineItem* m_trailingLineBreak { nullptr };
@@ -202,11 +204,13 @@
return setTrailingLineBreak(inlineItem);
if (inlineItem.isFloat())
return m_floats.append(makeWeakPtr(inlineItem));
+ m_inlineContentLogicalWidth += *logicalWidth;
m_inlineRuns.append({ inlineItem, *logicalWidth });
}
void LineCandidateContent::reset()
{
+ m_inlineContentLogicalWidth = 0;
m_inlineRuns.clear();
m_floats.clear();
m_trailingLineBreak = nullptr;
@@ -410,7 +414,7 @@
lineBreaker.setHyphenationDisabled();
auto& candidateRuns = candidateContent.inlineRuns();
- auto result = lineBreaker.shouldWrapInlineContent(candidateRuns, lineStatus);
+ auto result = lineBreaker.shouldWrapInlineContent(candidateRuns, candidateContent.inlineContentLogicalWidth(), lineStatus);
if (result.action == LineBreaker::Result::Action::Keep) {
// This continuous content can be fully placed on the current line.
commitContent(line, candidateRuns, { });
