Title: [289155] trunk/Source/WebCore
Revision
289155
Author
za...@apple.com
Date
2022-02-05 08:28:36 -0800 (Sat, 05 Feb 2022)

Log Message

[LFC][IFC] Move all the line box vertical alignment logic to LineBoxVerticalAligner
https://bugs.webkit.org/show_bug.cgi?id=236171

Reviewed by Antti Koivisto.

This patch is in preparation for adding ideographic baseline support (vertical writing mode).

LineBoxBuilder::constructAndAlignInlineLevelBoxes has grown large and it's time to move some
code out of this function (the "align" part). Now all the vertical alignment logic, including
the check for simplified vertical alignment is part of the LineBoxVerticalAligner class.
While in this patch we initiate an extra loop on LineBox::nonRootInlineLevelBoxes(), it may very well be a
perf win for the most common cases where the root inline box has no child inline boxes at all
(as previously we called updateCanUseSimplifiedAlignment() on every text run by passing in the parent inline box).

* layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:
(WebCore::Layout::LineBoxBuilder::build):
(WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
(WebCore::Layout::LineBoxBuilder::constructAndAlignInlineLevelBoxes): Deleted.
* layout/formattingContexts/inline/InlineLineBoxBuilder.h:
* layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp:
(WebCore::Layout::LineBoxVerticalAligner::LineBoxVerticalAligner):
(WebCore::Layout::LineBoxVerticalAligner::computeLogicalHeightAndAlign const):
(WebCore::Layout::LineBoxVerticalAligner::canUseSimplifiedAlignmentForInlineLevelBox): Deleted.
* layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h:
(WebCore::Layout::LineBoxVerticalAligner::formattingContext const):
(WebCore::Layout::LineBoxVerticalAligner::layoutState const):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (289154 => 289155)


--- trunk/Source/WebCore/ChangeLog	2022-02-05 09:25:12 UTC (rev 289154)
+++ trunk/Source/WebCore/ChangeLog	2022-02-05 16:28:36 UTC (rev 289155)
@@ -1,3 +1,32 @@
+2022-02-05  Alan Bujtas  <za...@apple.com>
+
+        [LFC][IFC] Move all the line box vertical alignment logic to LineBoxVerticalAligner
+        https://bugs.webkit.org/show_bug.cgi?id=236171
+
+        Reviewed by Antti Koivisto.
+
+        This patch is in preparation for adding ideographic baseline support (vertical writing mode).
+
+        LineBoxBuilder::constructAndAlignInlineLevelBoxes has grown large and it's time to move some
+        code out of this function (the "align" part). Now all the vertical alignment logic, including
+        the check for simplified vertical alignment is part of the LineBoxVerticalAligner class.
+        While in this patch we initiate an extra loop on LineBox::nonRootInlineLevelBoxes(), it may very well be a
+        perf win for the most common cases where the root inline box has no child inline boxes at all
+        (as previously we called updateCanUseSimplifiedAlignment() on every text run by passing in the parent inline box).
+
+        * layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:
+        (WebCore::Layout::LineBoxBuilder::build):
+        (WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
+        (WebCore::Layout::LineBoxBuilder::constructAndAlignInlineLevelBoxes): Deleted.
+        * layout/formattingContexts/inline/InlineLineBoxBuilder.h:
+        * layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp:
+        (WebCore::Layout::LineBoxVerticalAligner::LineBoxVerticalAligner):
+        (WebCore::Layout::LineBoxVerticalAligner::computeLogicalHeightAndAlign const):
+        (WebCore::Layout::LineBoxVerticalAligner::canUseSimplifiedAlignmentForInlineLevelBox): Deleted.
+        * layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h:
+        (WebCore::Layout::LineBoxVerticalAligner::formattingContext const):
+        (WebCore::Layout::LineBoxVerticalAligner::layoutState const):
+
 2022-02-05  Philippe Normand  <pnorm...@igalia.com>
 
         [Flatpak SDK] Update to FDO 21.08.10 and GStreamer 1.20 releases

Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp (289154 => 289155)


--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp	2022-02-05 09:25:12 UTC (rev 289154)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp	2022-02-05 16:28:36 UTC (rev 289155)
@@ -109,7 +109,8 @@
     auto rootInlineBoxAlignmentOffset = valueOrDefault(Layout::horizontalAlignmentOffset(rootStyle.textAlign(), lineContent, lineContent.inlineBaseDirection == TextDirection::LTR));
     // FIXME: The overflowing hanging content should be part of the ink overflow.  
     auto lineBox = LineBox { rootBox(), rootInlineBoxAlignmentOffset, lineContent.contentLogicalWidth - lineContent.hangingContentWidth, lineIndex, lineContent.nonSpanningInlineLevelBoxCount };
-    auto lineBoxLogicalHeight = constructAndAlignInlineLevelBoxes(lineBox, lineContent, lineIndex);
+    constructInlineLevelBoxes(lineBox, lineContent, lineIndex);
+    auto lineBoxLogicalHeight = LineBoxVerticalAligner { formattingContext() }.computeLogicalHeightAndAlign(lineBox);
     return { lineBox, lineBoxLogicalHeight };
 }
 
@@ -204,20 +205,11 @@
     inlineBox.setLayoutBounds({ floorf(heightAndLayoutBounds.layoutBounds.ascent), ceilf(heightAndLayoutBounds.layoutBounds.descent) });
 }
 
-InlineLayoutUnit LineBoxBuilder::constructAndAlignInlineLevelBoxes(LineBox& lineBox, const LineBuilder::LineContent& lineContent, size_t lineIndex)
+void LineBoxBuilder::constructInlineLevelBoxes(LineBox& lineBox, const LineBuilder::LineContent& lineContent, size_t lineIndex)
 {
     auto& rootInlineBox = lineBox.rootInlineBox();
     setInitialVerticalGeometryForInlineBox(rootInlineBox);
 
-    // FIXME: Add fast path support for line-height content.
-    // FIXME: We should always be able to exercise the fast path when the line has no content at all, even in non-standards mode or with line-height set.
-    auto canUseSimplifiedAlignment = layoutState().inStandardsMode() && rootInlineBox.isPreferredLineHeightFontMetricsBased();
-    auto updateCanUseSimplifiedAlignment = [&](auto& inlineLevelBox, std::optional<const BoxGeometry> boxGeometry = std::nullopt) {
-        if (!canUseSimplifiedAlignment)
-            return;
-        canUseSimplifiedAlignment = LineBoxVerticalAligner::canUseSimplifiedAlignmentForInlineLevelBox(rootInlineBox, inlineLevelBox, boxGeometry);
-    };
-
     auto styleToUse = [&] (const auto& layoutBox) -> const RenderStyle& {
         return !lineIndex ? layoutBox.firstLineStyle() : layoutBox.style();
     };
@@ -273,7 +265,6 @@
             auto atomicInlineLevelBox = InlineLevelBox::createAtomicInlineLevelBox(layoutBox, style, logicalLeft, { inlineLevelBoxGeometry.borderBoxWidth(), marginBoxHeight });
             atomicInlineLevelBox.setBaseline(ascent);
             atomicInlineLevelBox.setLayoutBounds(InlineLevelBox::LayoutBounds { ascent, marginBoxHeight - ascent });
-            updateCanUseSimplifiedAlignment(atomicInlineLevelBox, inlineLevelBoxGeometry);
             lineBox.addInlineLevelBox(WTFMove(atomicInlineLevelBox));
             continue;
         }
@@ -287,7 +278,6 @@
             auto logicalWidth = rootInlineBox.logicalWidth() - adjustedLogicalStart;
             auto inlineBox = InlineLevelBox::createInlineBox(layoutBox, style, adjustedLogicalStart, logicalWidth, InlineLevelBox::LineSpanningInlineBox::Yes);
             setInitialVerticalGeometryForInlineBox(inlineBox);
-            updateCanUseSimplifiedAlignment(inlineBox);
             lineBox.addInlineLevelBox(WTFMove(inlineBox));
             continue;
         }
@@ -303,7 +293,6 @@
             auto inlineBox = InlineLevelBox::createInlineBox(layoutBox, style, logicalLeft, initialLogicalWidth);
             inlineBox.setIsFirstBox();
             setInitialVerticalGeometryForInlineBox(inlineBox);
-            updateCanUseSimplifiedAlignment(inlineBox);
             lineBox.addInlineLevelBox(WTFMove(inlineBox));
             continue;
         }
@@ -321,7 +310,6 @@
             // make sure we don't end up with negative logical width on the inline box.
             inlineBox.setLogicalWidth(std::max(0.f, inlineBoxLogicalRight - inlineBox.logicalLeft()));
             inlineBox.setIsLastBox();
-            updateCanUseSimplifiedAlignment(inlineBox);
             continue;
         }
         if (run.isText()) {
@@ -331,7 +319,6 @@
             if (!fallbackFonts.isEmpty()) {
                 // Adjust non-empty inline box height when glyphs from the non-primary font stretch the box.
                 adjustVerticalGeometryForInlineBoxWithFallbackFonts(parentInlineBox, fallbackFonts);
-                updateCanUseSimplifiedAlignment(parentInlineBox);
             }
             continue;
         }
@@ -343,7 +330,6 @@
             auto lineBreakBox = InlineLevelBox::createLineBreakBox(layoutBox, style, logicalLeft);
             auto& parentInlineBox = lineBox.inlineLevelBoxForLayoutBox(layoutBox.parent());
             setVerticalGeometryForLineBreakBox(lineBreakBox, parentInlineBox);
-            updateCanUseSimplifiedAlignment(lineBreakBox);
             lineBox.addInlineLevelBox(WTFMove(lineBreakBox));
             continue;
         }
@@ -353,12 +339,7 @@
         }
         ASSERT_NOT_REACHED();
     }
-
     lineBox.setHasContent(lineHasContent);
-
-    auto verticalAligner = LineBoxVerticalAligner { formattingContext() };
-    canUseSimplifiedAlignment = canUseSimplifiedAlignment || !lineHasContent;
-    return verticalAligner.computeLogicalHeightAndAlign(lineBox, canUseSimplifiedAlignment);
 }
 
 }

Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.h (289154 => 289155)


--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.h	2022-02-05 09:25:12 UTC (rev 289154)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.h	2022-02-05 16:28:36 UTC (rev 289155)
@@ -52,7 +52,7 @@
     void setInitialVerticalGeometryForInlineBox(InlineLevelBox&) const;
     void setVerticalGeometryForLineBreakBox(InlineLevelBox& lineBreakBox, const InlineLevelBox& parentInlineBox) const;
     void adjustVerticalGeometryForInlineBoxWithFallbackFonts(InlineLevelBox&, const TextUtil::FallbackFontList&) const;
-    InlineLayoutUnit constructAndAlignInlineLevelBoxes(LineBox&, const LineBuilder::LineContent&, size_t lineIndex);
+    void constructInlineLevelBoxes(LineBox&, const LineBuilder::LineContent&, size_t lineIndex);
 
     const InlineFormattingContext& formattingContext() const { return m_inlineFormattingContext; }
     const Box& rootBox() const { return formattingContext().root(); }

Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp (289154 => 289155)


--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp	2022-02-05 09:25:12 UTC (rev 289154)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp	2022-02-05 16:28:36 UTC (rev 289155)
@@ -35,36 +35,45 @@
 namespace Layout {
 
 LineBoxVerticalAligner::LineBoxVerticalAligner(const InlineFormattingContext& inlineFormattingContext)
-    : m_inlineFormattingGeometry(inlineFormattingContext)
+    : m_inlineFormattingContext(inlineFormattingContext)
+    , m_inlineFormattingGeometry(inlineFormattingContext)
 {
 }
 
-bool LineBoxVerticalAligner::canUseSimplifiedAlignmentForInlineLevelBox(const InlineLevelBox& rootInlineBox, const InlineLevelBox& inlineLevelBox, std::optional<const BoxGeometry> inlineLevelBoxGeometry)
+InlineLayoutUnit LineBoxVerticalAligner::computeLogicalHeightAndAlign(LineBox& lineBox) const
 {
-    if (inlineLevelBox.isAtomicInlineLevelBox()) {
-        ASSERT(inlineLevelBoxGeometry);
-        // Baseline aligned, non-stretchy direct children are considered to be simple for now.
-        auto& layoutBox = inlineLevelBox.layoutBox();
-        return &layoutBox.parent() == &rootInlineBox.layoutBox()
-            && inlineLevelBox.verticalAlign().type == VerticalAlign::Baseline
-            && !inlineLevelBoxGeometry->marginBefore()
-            && !inlineLevelBoxGeometry->marginAfter()
-            && inlineLevelBoxGeometry->marginBoxHeight() <= rootInlineBox.baseline();
-    }
-    if (inlineLevelBox.isLineBreakBox()) {
-        // Baseline aligned, non-stretchy line breaks e.g. <div><span><br></span></div> but not <div><span style="font-size: 100px;"><br></span></div>.
-        return inlineLevelBox.verticalAlign().type == VerticalAlign::Baseline && inlineLevelBox.baseline() <= rootInlineBox.baseline();
-    }
-    if (inlineLevelBox.isInlineBox()) {
-        // Baseline aligned, non-stretchy inline boxes e.g. <div><span></span></div> but not <div><span style="font-size: 100px;"></span></div>.
-        return inlineLevelBox.verticalAlign().type == VerticalAlign::Baseline && inlineLevelBox.layoutBounds() == rootInlineBox.layoutBounds();
-    }
-    return false;
-}
+    auto canUseSimplifiedAlignment = [&] {
+        if (!lineBox.hasContent())
+            return true;
+        auto& rootInlineBox = lineBox.rootInlineBox();
+        if (!layoutState().inStandardsMode() || !rootInlineBox.isPreferredLineHeightFontMetricsBased() || rootInlineBox.verticalAlign().type != VerticalAlign::Baseline)
+            return false;
 
-InlineLayoutUnit LineBoxVerticalAligner::computeLogicalHeightAndAlign(LineBox& lineBox, bool useSimplifiedAlignment) const
-{
-    if (useSimplifiedAlignment)
+        for (auto& inlineLevelBox : lineBox.nonRootInlineLevelBoxes()) {
+            auto shouldUseSimplifiedAlignment = false;
+            if (inlineLevelBox.isAtomicInlineLevelBox()) {
+                // Baseline aligned, non-stretchy direct children are considered to be simple for now.
+                auto& layoutBox = inlineLevelBox.layoutBox();
+                if (&layoutBox.parent() != &rootInlineBox.layoutBox() || inlineLevelBox.verticalAlign().type != VerticalAlign::Baseline)
+                    shouldUseSimplifiedAlignment = false;
+                else {
+                    auto& inlineLevelBoxGeometry = formattingContext().geometryForBox(layoutBox);
+                    shouldUseSimplifiedAlignment = !inlineLevelBoxGeometry.marginBefore() && !inlineLevelBoxGeometry.marginAfter() && inlineLevelBoxGeometry.marginBoxHeight() <= rootInlineBox.baseline();
+                }
+            } else if (inlineLevelBox.isLineBreakBox()) {
+                // Baseline aligned, non-stretchy line breaks e.g. <div><span><br></span></div> but not <div><span style="font-size: 100px;"><br></span></div>.
+                shouldUseSimplifiedAlignment = inlineLevelBox.verticalAlign().type == VerticalAlign::Baseline && inlineLevelBox.baseline() <= rootInlineBox.baseline();
+            } else if (inlineLevelBox.isInlineBox()) {
+                // Baseline aligned, non-stretchy inline boxes e.g. <div><span></span></div> but not <div><span style="font-size: 100px;"></span></div>.
+                shouldUseSimplifiedAlignment = inlineLevelBox.verticalAlign().type == VerticalAlign::Baseline && inlineLevelBox.layoutBounds() == rootInlineBox.layoutBounds();
+            }
+            if (!shouldUseSimplifiedAlignment)
+                return false;
+        }
+        return true;
+    };
+
+    if (canUseSimplifiedAlignment())
         return simplifiedVerticalAlignment(lineBox);
     // This function (partially) implements:
     // 2.2. Layout Within Line Boxes

Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h (289154 => 289155)


--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h	2022-02-05 09:25:12 UTC (rev 289154)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h	2022-02-05 16:28:36 UTC (rev 289155)
@@ -35,10 +35,8 @@
 class LineBoxVerticalAligner {
 public:
     LineBoxVerticalAligner(const InlineFormattingContext&);
-    InlineLayoutUnit computeLogicalHeightAndAlign(LineBox&, bool useSimplifiedAlignment) const;
+    InlineLayoutUnit computeLogicalHeightAndAlign(LineBox&) const;
 
-    static bool canUseSimplifiedAlignmentForInlineLevelBox(const InlineLevelBox& rootInlineBox, const InlineLevelBox&, std::optional<const BoxGeometry> nlineLevelBoxGeometry);
-
 private:
     InlineLayoutUnit simplifiedVerticalAlignment(LineBox&) const;
 
@@ -47,8 +45,11 @@
     void alignInlineLevelBoxes(LineBox&, InlineLayoutUnit lineBoxLogicalHeight) const;
 
     const InlineFormattingGeometry& formattingGeometry() const { return m_inlineFormattingGeometry; }
+    const InlineFormattingContext& formattingContext() const { return m_inlineFormattingContext; }
+    const LayoutState& layoutState() const { return formattingContext().layoutState(); }
 
 private:
+    const InlineFormattingContext& m_inlineFormattingContext;
     const InlineFormattingGeometry m_inlineFormattingGeometry;
 };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to