Modified: trunk/Source/WebCore/ChangeLog (271350 => 271351)
--- trunk/Source/WebCore/ChangeLog 2021-01-10 18:09:09 UTC (rev 271350)
+++ trunk/Source/WebCore/ChangeLog 2021-01-10 19:13:01 UTC (rev 271351)
@@ -1,3 +1,30 @@
+2021-01-10 Zalan Bujtas <[email protected]>
+
+ [LFC][IFC] Make "inline level box affects line box geometry" logic more explicit
+ https://bugs.webkit.org/show_bug.cgi?id=220232
+
+ Reviewed by Antti Koivisto.
+
+ Since InlineLevelBox::isEmpty makes little sense in case of non-inline-boxes (e.g. <br>), let's make
+ the following set of changes:
+ 1. Introduce InlineLevelBox::setHasContent for inline boxes only (<span>has content</span>, <div>root inlinebox has content</div>)
+ 2. Both atomic and line box logic is moved to InlineFormattingContext::Quirks::inlineLevelBoxAffectsLineBox
+ This is also in preparation for getting rid of the is-considered-empty concept for line/line box.
+
+ * layout/inlineformatting/InlineFormattingContext.h:
+ * layout/inlineformatting/InlineFormattingContextGeometry.cpp:
+ (WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
+ (WebCore::Layout::LineBoxBuilder::computeLineBoxHeightAndAlignInlineLevelBoxesVertically):
+ * layout/inlineformatting/InlineFormattingContextQuirks.cpp:
+ (WebCore::Layout::InlineFormattingContext::Quirks::inlineLevelBoxAffectsLineBox const):
+ (WebCore::Layout::InlineFormattingContext::Quirks::shouldInlineLevelBoxStretchLineBox const): Deleted.
+ * layout/inlineformatting/InlineLineBox.cpp:
+ (WebCore::Layout::LineBox::InlineLevelBox::setHasContent):
+ * layout/inlineformatting/InlineLineBox.h:
+ (WebCore::Layout::LineBox::InlineLevelBox::hasContent const):
+ (WebCore::Layout::LineBox::InlineLevelBox::isEmpty const): Deleted.
+ (WebCore::Layout::LineBox::InlineLevelBox::setIsNonEmpty): Deleted.
+
2021-01-10 Antti Koivisto <[email protected]>
[LFC][Integration] Factor marked text style collection code out of InlineTextBox
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h (271350 => 271351)
--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h 2021-01-10 18:09:09 UTC (rev 271350)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h 2021-01-10 19:13:01 UTC (rev 271351)
@@ -51,7 +51,7 @@
public:
InlineLayoutUnit initialLineHeight() const;
bool hasSoftWrapOpportunityAtImage() const;
- bool shouldInlineLevelBoxStretchLineBox(const LineBox&, const LineBox::InlineLevelBox&) const;
+ bool inlineLevelBoxAffectsLineBox(const LineBox::InlineLevelBox&, const LineBox&) const;
private:
friend class InlineFormattingContext;
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp (271350 => 271351)
--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp 2021-01-10 18:09:09 UTC (rev 271350)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp 2021-01-10 19:13:01 UTC (rev 271351)
@@ -221,7 +221,7 @@
auto inlineBoxHasImaginaryStrut = layoutState().inNoQuirksMode();
auto isInitiallyConsideredNonEmpty = !lineBox.isConsideredEmpty() && inlineBoxHasImaginaryStrut;
if (isInitiallyConsideredNonEmpty)
- rootInlineBox->setIsNonEmpty();
+ rootInlineBox->setHasContent();
setVerticalGeometryForInlineBox(*rootInlineBox);
simplifiedVerticalAlignment = { { } , rootInlineBox->layoutBounds().height(), rootInlineBox->layoutBounds().ascent - rootInlineBox->baseline() };
lineBox.addRootInlineBox(WTFMove(rootInlineBox));
@@ -294,10 +294,6 @@
auto atomicInlineLevelBox = LineBox::InlineLevelBox::createAtomicInlineLevelBox(layoutBox, logicalLeft, { run.logicalWidth(), marginBoxHeight });
atomicInlineLevelBox->setBaseline(ascent);
atomicInlineLevelBox->setLayoutBounds(LineBox::InlineLevelBox::LayoutBounds { ascent, marginBoxHeight - ascent });
- // While in practice when the negative vertical margin makes the layout bounds empty (e.g: height: 100px; margin-top: -100px;), and this inline
- // level box contributes 0px to the line box height, it should not be considered empty.
- if (marginBoxHeight || inlineLevelBoxGeometry.marginBefore() || inlineLevelBoxGeometry.marginAfter())
- atomicInlineLevelBox->setIsNonEmpty();
// Let's estimate the logical top so that we can avoid running the alignment on simple inline boxes.
auto alignInlineBoxIfEligible = [&] {
if (m_inlineLevelBoxesNeedVerticalAlignment)
@@ -335,7 +331,7 @@
auto inlineBoxHasImaginaryStrut = layoutState().inNoQuirksMode() && !lineBox.isConsideredEmpty();
auto isConsideredNonEmpty = inlineBoxHasImaginaryStrut || inlineBoxGeometry.horizontalPadding().valueOr(0_lu) || inlineBoxGeometry.horizontalBorder();
if (isConsideredNonEmpty)
- inlineBox->setIsNonEmpty();
+ inlineBox->setHasContent();
setVerticalGeometryForInlineBox(*inlineBox);
lineBox.addInlineLevelBox(WTFMove(inlineBox));
continue;
@@ -349,13 +345,12 @@
}
if (run.isText() || run.isSoftLineBreak()) {
// FIXME: Adjust non-empty inline box height when glyphs from the non-primary font stretch the box.
- lineBox.inlineLevelBoxForLayoutBox(layoutBox.parent()).setIsNonEmpty();
+ lineBox.inlineLevelBoxForLayoutBox(layoutBox.parent()).setHasContent();
continue;
}
if (run.isHardLineBreak()) {
auto lineBreakBox = LineBox::InlineLevelBox::createLineBreakBox(layoutBox, logicalLeft);
setVerticalGeometryForInlineBox(*lineBreakBox);
- lineBreakBox->setIsNonEmpty();
lineBox.addInlineLevelBox(WTFMove(lineBreakBox));
continue;
}
@@ -392,7 +387,7 @@
auto minimumLogicalTop = Optional<InlineLayoutUnit> { };
auto maximumLogicalBottom = Optional<InlineLayoutUnit> { };
- if (!rootInlineBox.isEmpty()) {
+ if (quirks.inlineLevelBoxAffectsLineBox(rootInlineBox, lineBox)) {
minimumLogicalTop = InlineLayoutUnit { };
maximumLogicalBottom = rootInlineBox.layoutBounds().height();
inlineLevelBoxAbsoluteTopAndBottomMap.add(&rootInlineBox, AbsoluteTopAndBottom { *minimumLogicalTop, *maximumLogicalBottom });
@@ -454,7 +449,7 @@
auto absoluteLogicalBottom = absoluteLogicalTop + inlineLevelBox->layoutBounds().height();
inlineLevelBoxAbsoluteTopAndBottomMap.add(inlineLevelBox.get(), AbsoluteTopAndBottom { absoluteLogicalTop, absoluteLogicalBottom });
// Stretch the min/max absolute values if applicable.
- if (quirks.shouldInlineLevelBoxStretchLineBox(lineBox, *inlineLevelBox)) {
+ if (quirks.inlineLevelBoxAffectsLineBox(*inlineLevelBox, lineBox)) {
minimumLogicalTop = std::min(minimumLogicalTop.valueOr(absoluteLogicalTop), absoluteLogicalTop);
maximumLogicalBottom = std::max(maximumLogicalBottom.valueOr(absoluteLogicalBottom), absoluteLogicalBottom);
}
@@ -464,7 +459,7 @@
// 2. Check if the line box relative aligned inline boxes (top, bottom etc) have enough room and stretch the line box further if needed.
auto lineBoxLogicalHeight = maximumLogicalBottom.valueOr(InlineLayoutUnit()) - minimumLogicalTop.valueOr(InlineLayoutUnit());
for (auto* lineBoxRelativeInlineLevelBox : lineBoxRelativeInlineLevelBoxes) {
- if (!quirks.shouldInlineLevelBoxStretchLineBox(lineBox, *lineBoxRelativeInlineLevelBox))
+ if (!quirks.inlineLevelBoxAffectsLineBox(*lineBoxRelativeInlineLevelBox, lineBox))
continue;
lineBoxLogicalHeight = std::max(lineBoxLogicalHeight, lineBoxRelativeInlineLevelBox->layoutBounds().height());
}
@@ -477,7 +472,7 @@
inlineLevelBoxAbsoluteBaselineOffsetMap.add(&rootInlineBox, InlineLayoutUnit { });
auto maximumTopOffsetFromRootInlineBoxBaseline = Optional<InlineLayoutUnit> { };
- if (!rootInlineBox.isEmpty())
+ if (quirks.inlineLevelBoxAffectsLineBox(rootInlineBox, lineBox))
maximumTopOffsetFromRootInlineBoxBaseline = rootInlineBox.layoutBounds().ascent;
for (auto& inlineLevelBox : lineBox.nonRootInlineLevelBoxes()) {
@@ -535,7 +530,7 @@
}
}
inlineLevelBoxAbsoluteBaselineOffsetMap.add(inlineLevelBox.get(), absoluteBaselineOffset);
- auto affectsRootInlineBoxVerticalPosition = quirks.shouldInlineLevelBoxStretchLineBox(lineBox, *inlineLevelBox);
+ auto affectsRootInlineBoxVerticalPosition = quirks.inlineLevelBoxAffectsLineBox(*inlineLevelBox, lineBox);
if (affectsRootInlineBoxVerticalPosition)
maximumTopOffsetFromRootInlineBoxBaseline = std::max(maximumTopOffsetFromRootInlineBoxBaseline.valueOr(std::numeric_limits<InlineLayoutUnit>::lowest()), absoluteBaselineOffset + inlineLevelBox->layoutBounds().ascent);
}
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp (271350 => 271351)
--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp 2021-01-10 18:09:09 UTC (rev 271350)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp 2021-01-10 19:13:01 UTC (rev 271351)
@@ -40,17 +40,24 @@
return root.style().fontMetrics().floatHeight();
}
-bool InlineFormattingContext::Quirks::shouldInlineLevelBoxStretchLineBox(const LineBox& lineBox, const LineBox::InlineLevelBox& inlineLevelBox) const
+bool InlineFormattingContext::Quirks::inlineLevelBoxAffectsLineBox(const LineBox::InlineLevelBox& inlineLevelBox, const LineBox& lineBox) const
{
- if (inlineLevelBox.isEmpty())
- return false;
- if (layoutState().inNoQuirksMode())
- return true;
- if (!inlineLevelBox.isLineBreakBox())
- return true;
- // <br> in non-standard mode stretches the line box only when the line is empty.
- // e.g. <div><span><br></span></div> will stretch but <div>this will not stretch to 200px<span style="font-size: 200px;"><br></span></div>
- return lineBox.isConsideredEmpty();
+ if (inlineLevelBox.isInlineBox())
+ return inlineLevelBox.hasContent();
+ if (inlineLevelBox.isLineBreakBox()) {
+ // <br> in non-standard mode stretches the line box only when the line is empty.
+ // e.g. <div><span><br></span></div> will stretch but <div>this will not stretch to 200px<span style="font-size: 200px;"><br></span></div>
+ return layoutState().inNoQuirksMode() ? true : lineBox.isConsideredEmpty();
+ }
+ if (inlineLevelBox.isAtomicInlineLevelBox()) {
+ if (inlineLevelBox.layoutBounds().height())
+ return true;
+ // While in practice when the negative vertical margin makes the layout bounds empty (e.g: height: 100px; margin-top: -100px;), and this inline
+ // level box contributes 0px to the line box height, it still needs to be taken into account while computing line box geometries.
+ auto& boxGeometry = formattingContext().geometryForBox(inlineLevelBox.layoutBox());
+ return boxGeometry.marginBefore() || boxGeometry.marginAfter();
+ }
+ return false;
}
bool InlineFormattingContext::Quirks::hasSoftWrapOpportunityAtImage() const
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.cpp (271350 => 271351)
--- trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.cpp 2021-01-10 18:09:09 UTC (rev 271350)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.cpp 2021-01-10 19:13:01 UTC (rev 271351)
@@ -71,6 +71,12 @@
m_logicalRect.setHeight(roundToInt(logicalHeight));
}
+void LineBox::InlineLevelBox::setHasContent()
+{
+ ASSERT(isInlineBox());
+ m_hasContent = true;
+}
+
bool LineBox::InlineLevelBox::hasLineBoxRelativeAlignment() const
{
auto verticalAlignment = layoutBox().style().verticalAlign();
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.h (271350 => 271351)
--- trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.h 2021-01-10 18:09:09 UTC (rev 271350)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLineBox.h 2021-01-10 19:13:01 UTC (rev 271351)
@@ -77,8 +77,8 @@
};
LayoutBounds layoutBounds() const { return m_layoutBounds; }
- bool isEmpty() const { return m_isEmpty; }
- void setIsNonEmpty() { m_isEmpty = false; }
+ bool hasContent() const { return m_hasContent; }
+ void setHasContent();
VerticalAlign verticalAlign() const { return layoutBox().style().verticalAlign(); }
const Box& layoutBox() const { return *m_layoutBox; }
@@ -124,7 +124,7 @@
LayoutBounds m_layoutBounds;
InlineLayoutUnit m_baseline { 0 };
Optional<InlineLayoutUnit> m_descent;
- bool m_isEmpty { true };
+ bool m_hasContent { false };
Type m_type { Type::InlineBox };
};
