Attachment 461445 Details for Bug 243621 – Patch

[patch] Patch

bug-243621-20220806141227.patch (text/plain), 12.75 KB, created by alan on 2022-08-06 14:12:28 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: alan

Created: 2022-08-06 14:12:28 PDT

Size: 12.75 KB

patch

obsolete

>From 0cb03d7c8166c4337bf27eced5e69c392da63d7f Mon Sep 17 00:00:00 2001
>From: Alan Bujtas <zalan@apple.com>
>Date: Sat, 6 Aug 2022 14:10:39 -0700
>Subject: [PATCH] [Painting] Do not call textRunOffsetFromBottomMost() unless
> the resolved underline position value is TextUnderlinePosition::Under
> https://bugs.webkit.org/show_bug.cgi?id=243621
>
>Reviewed by Antti Koivisto.
>
>textRunOffsetFromBottomMost() walks the line and looks at each text run's logical bottom to find the bottom most position. This bottom most value is used to compute the position of the underline when "text-underline-position" resolves to "under". Any other "text-underline-position" ignores the bottom most position.
>
>This patch ensures that we only call textRunOffsetFromBottomMost() when needed.
>1. Precompute the "underline position" value and call textRunOffsetFromBottomMost() only when it resolves to TextUnderlinePosition::Under.
>2. Pass in this resolved value to computedUnderlineOffset().
>
>* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:
>(WebCore::Layout::InlineDisplayContentBuilder::collectInkOverflowForTextDecorations):
>* Source/WebCore/style/InlineTextBoxStyle.cpp:
>(WebCore::resolvedUnderlinePosition):
>(WebCore::computedUnderlineOffset):
>(WebCore::visualOverflowForDecorations):
>(WebCore::underlineOffsetForTextBoxPainting):
>(WebCore::computeUnderlineOffset): Deleted.
>* Source/WebCore/style/InlineTextBoxStyle.h:
>---
> .../display/InlineDisplayContentBuilder.cpp   |  2 +-
> Source/WebCore/style/InlineTextBoxStyle.cpp   | 84 +++++++++++--------
> Source/WebCore/style/InlineTextBoxStyle.h     |  3 +-
> 3 files changed, 53 insertions(+), 36 deletions(-)
>
>diff --git a/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp
>index 053e6be3dd71..86181c5abe95 100644
>--- a/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp
>+++ b/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp
>@@ -880,7 +880,7 @@ void InlineDisplayContentBuilder::collectInkOverflowForTextDecorations(DisplayBo
>             if (!logicalBottomForTextDecoration)
>                 logicalBottomForTextDecoration = logicalBottomForTextDecorationContent(boxes, isHorizontalWritingMode);
>             auto textRunLogicalOffsetFromLineBottom = *logicalBottomForTextDecoration - (isHorizontalWritingMode ? displayBox.bottom() : displayBox.right());
>-            return visualOverflowForDecorations(style, { displayLine.baselineType(), displayBox.height(), textRunLogicalOffsetFromLineBottom });
>+            return visualOverflowForDecorations(style, displayLine.baselineType(), { displayBox.height(), textRunLogicalOffsetFromLineBottom });
>         }();
> 
>         if (!decorationOverflow.isEmpty()) {
>diff --git a/Source/WebCore/style/InlineTextBoxStyle.cpp b/Source/WebCore/style/InlineTextBoxStyle.cpp
>index b6c3b2464de3..0f9764f14247 100644
>--- a/Source/WebCore/style/InlineTextBoxStyle.cpp
>+++ b/Source/WebCore/style/InlineTextBoxStyle.cpp
>@@ -39,6 +39,7 @@ namespace WebCore {
> 
> struct UnderlineOffsetArguments {
>     const RenderStyle& lineStyle;
>+    TextUnderlinePosition resolvedUnderlinePosition { TextUnderlinePosition::Auto };
>     float defaultGap { 0 };
>     std::optional<TextUnderlinePositionUnder> textUnderlinePositionUnder { };
> };
>@@ -130,11 +131,22 @@ static inline float defaultGap(const RenderStyle& style)
>     return style.computedFontSize() / textDecorationBaseFontSize;
> }
> 
>-static float computeUnderlineOffset(const UnderlineOffsetArguments& context)
>+static inline TextUnderlinePosition resolvedUnderlinePosition(const RenderStyle& decoratingBoxStyle, std::optional<FontBaseline> baselineType)
>+{
>+    auto underlinePosition = decoratingBoxStyle.textUnderlinePosition();
>+    if (underlinePosition == TextUnderlinePosition::Auto && decoratingBoxStyle.textUnderlineOffset().isAuto()) {
>+        if (baselineType)
>+            return *baselineType == IdeographicBaseline ? TextUnderlinePosition::Under : TextUnderlinePosition::Auto;
>+        return TextUnderlinePosition::Auto;
>+    }
>+    return underlinePosition;
>+}
>+
>+static float computedUnderlineOffset(const UnderlineOffsetArguments& context)
> {
>     // This represents the gap between the baseline and the closest edge of the underline.
>-    float gap = std::max<int>(1, std::ceil(context.defaultGap / 2.0f));
>-    
>+    auto gap = std::max(1.f, std::ceil(context.defaultGap / 2.f));
>+
>     // FIXME: The code for visual overflow detection passes in a null inline text box. This means it is now
>     // broken for the case where auto needs to behave like "under".
>     
>@@ -142,37 +154,31 @@ static float computeUnderlineOffset(const UnderlineOffsetArguments& context)
>     // scripts where it would not be appropriate (e.g., ideographs).
>     // Strictly speaking this can occur whenever the line contains ideographs
>     // even if it is horizontal, but detecting this has performance implications. For now we only work with
>-    // vertical text, since we already determined the baseline type to be ideographic in that
>-    // case.
>-    auto underlinePosition = context.lineStyle.textUnderlinePosition();
>+    // vertical text, since we already determined the baseline type to be ideographic in that case.
>     auto underlineOffset = context.lineStyle.textUnderlineOffset();
>     auto& fontMetrics = context.lineStyle.metricsOfPrimaryFont();
> 
>-    auto resolvedUnderlinePosition = underlinePosition;
>-    if (resolvedUnderlinePosition == TextUnderlinePosition::Auto && underlineOffset.isAuto()) {
>-        if (context.textUnderlinePositionUnder)
>-            resolvedUnderlinePosition = context.textUnderlinePositionUnder->baselineType == IdeographicBaseline ? TextUnderlinePosition::Under : TextUnderlinePosition::Auto;
>-        else
>-            resolvedUnderlinePosition = TextUnderlinePosition::Auto;
>-    }
>-
>-    switch (resolvedUnderlinePosition) {
>+    float computedUnderlineOffset = fontMetrics.ascent() + gap;
>+    switch (context.resolvedUnderlinePosition) {
>     case TextUnderlinePosition::Auto:
>-        if (underlineOffset.isAuto())
>-            return fontMetrics.ascent() + gap;
>-        return fontMetrics.ascent() + underlineOffset.lengthValue();
>+        computedUnderlineOffset = fontMetrics.ascent() + underlineOffset.lengthOr(gap);
>+        break;
>     case TextUnderlinePosition::FromFont:
>-        return fontMetrics.ascent() + fontMetrics.underlinePosition() + underlineOffset.lengthOr(0);
>+        computedUnderlineOffset = fontMetrics.ascent() + fontMetrics.underlinePosition() + underlineOffset.lengthOr(0.f);
>+        break;
>     case TextUnderlinePosition::Under: {
>         ASSERT(context.textUnderlinePositionUnder);
>         // Position underline relative to the bottom edge of the lowest element's content box.
>-        auto desiredOffset = context.textUnderlinePositionUnder->textRunLogicalHeight + gap + std::max(context.textUnderlinePositionUnder->textRunOffsetFromBottomMost, 0.0f) + underlineOffset.lengthOr(0);
>-        return std::max<float>(desiredOffset, fontMetrics.ascent());
>+        auto desiredOffset = context.textUnderlinePositionUnder->textRunLogicalHeight + gap + std::max(context.textUnderlinePositionUnder->textRunOffsetFromBottomMost, 0.f) + underlineOffset.lengthOr(0.f);
>+        computedUnderlineOffset = std::max<float>(desiredOffset, fontMetrics.ascent());
>+        break;
>     }
>+    default:
>+        ASSERT_NOT_REACHED();
>+        break;
>     }
> 
>-    ASSERT_NOT_REACHED();
>-    return fontMetrics.ascent() + gap;
>+    return computedUnderlineOffset;
> }
> 
> WavyStrokeParameters getWavyStrokeParameters(float fontSize)
>@@ -254,16 +260,24 @@ static GlyphOverflow computedVisualOverflowForDecorations(const RenderStyle& lin
> GlyphOverflow visualOverflowForDecorations(const InlineIterator::LineBoxIterator& lineBox, const RenderText& renderer, float textBoxLogicalTop, float textBoxLogicalBottom)
> {
>     auto& style = lineBox->isFirst() ? renderer.firstLineStyle() : renderer.style();
>+    auto underlinePositionValue = resolvedUnderlinePosition(style, lineBox->baselineType());
>+    auto textUnderlinePositionUnder = std::optional<TextUnderlinePositionUnder> { };
>+
>+    if (underlinePositionValue == TextUnderlinePosition::Under) {
>+        auto textRunOffset = textRunOffsetFromBottomMost(lineBox, renderer, textBoxLogicalTop, textBoxLogicalBottom);
>+        textUnderlinePositionUnder = TextUnderlinePositionUnder { textBoxLogicalBottom - textBoxLogicalTop, textRunOffset };
>+    }
>+
>     auto underlineOffset = style.textDecorationsInEffect().contains(TextDecorationLine::Underline)
>-        ? std::make_optional(computeUnderlineOffset({ style, defaultGap(style), TextUnderlinePositionUnder { lineBox->baselineType(), textBoxLogicalBottom - textBoxLogicalTop, textRunOffsetFromBottomMost(lineBox, renderer, textBoxLogicalTop, textBoxLogicalBottom) } }))
>+        ? std::make_optional(computedUnderlineOffset({ style, underlinePositionValue, defaultGap(style), textUnderlinePositionUnder }))
>         : std::nullopt;
>     return computedVisualOverflowForDecorations(style, underlineOffset);
> }
> 
>-GlyphOverflow visualOverflowForDecorations(const RenderStyle& style, TextUnderlinePositionUnder textUnderlinePositionUnder)
>+GlyphOverflow visualOverflowForDecorations(const RenderStyle& style, FontBaseline baselineType, TextUnderlinePositionUnder textUnderlinePositionUnder)
> {
>     auto underlineOffset = style.textDecorationsInEffect().contains(TextDecorationLine::Underline)
>-        ? std::make_optional(computeUnderlineOffset({ style, defaultGap(style), textUnderlinePositionUnder }))
>+        ? std::make_optional(computedUnderlineOffset({ style, resolvedUnderlinePosition(style, baselineType), defaultGap(style), textUnderlinePositionUnder }))
>         : std::nullopt;
>     return computedVisualOverflowForDecorations(style, underlineOffset);
> }
>@@ -271,19 +285,23 @@ GlyphOverflow visualOverflowForDecorations(const RenderStyle& style, TextUnderli
> GlyphOverflow visualOverflowForDecorations(const RenderStyle& style)
> {
>     auto underlineOffset = style.textDecorationsInEffect().contains(TextDecorationLine::Underline)
>-        ? std::make_optional(computeUnderlineOffset({ style, defaultGap(style), { } }))
>+        ? std::make_optional(computedUnderlineOffset({ style, resolvedUnderlinePosition(style, { }), defaultGap(style), { } }))
>         : std::nullopt;
>     return computedVisualOverflowForDecorations(style, underlineOffset);
> }
> 
> float underlineOffsetForTextBoxPainting(const RenderStyle& style, const InlineIterator::TextBoxIterator& textBox)
> {
>-    auto textUnderlinePositionUnder = TextUnderlinePositionUnder {
>-        textBox->lineBox()->baselineType(),
>-        textBox->logicalBottom() - textBox->logicalTop(),
>-        textRunOffsetFromBottomMost(textBox->lineBox(), textBox->renderer(), textBox->logicalTop(), textBox->logicalBottom())
>-    };
>-    return computeUnderlineOffset({ style , defaultGap(style), textUnderlinePositionUnder });
>+    auto lineBox = textBox->lineBox();
>+    auto textUnderlinePositionUnder = std::optional<TextUnderlinePositionUnder> { };
>+    auto underlinePositionValue = resolvedUnderlinePosition(style, lineBox->baselineType());
>+
>+    if (underlinePositionValue == TextUnderlinePosition::Under) {
>+        auto textRunOffset = textRunOffsetFromBottomMost(lineBox, textBox->renderer(), textBox->logicalTop(), textBox->logicalBottom());
>+        textUnderlinePositionUnder = TextUnderlinePositionUnder { textBox->logicalBottom() - textBox->logicalTop(), textRunOffset };
>+    }
>+
>+    return computedUnderlineOffset({ style, underlinePositionValue, defaultGap(style), textUnderlinePositionUnder });
> }
> 
> }
>diff --git a/Source/WebCore/style/InlineTextBoxStyle.h b/Source/WebCore/style/InlineTextBoxStyle.h
>index 8b07c5a929ba..3843209d626b 100644
>--- a/Source/WebCore/style/InlineTextBoxStyle.h
>+++ b/Source/WebCore/style/InlineTextBoxStyle.h
>@@ -55,14 +55,13 @@ struct WavyStrokeParameters {
> WavyStrokeParameters getWavyStrokeParameters(float fontSize);
> 
> struct TextUnderlinePositionUnder {
>-    FontBaseline baselineType { AlphabeticBaseline };
>     float textRunLogicalHeight { 0 };
>     // This offset value is the distance between the current text run's logical bottom and the lowest position of all the text runs
>     // on line that belong to the same decoration box.
>     float textRunOffsetFromBottomMost { 0 };
> };
> GlyphOverflow visualOverflowForDecorations(const RenderStyle&);
>-GlyphOverflow visualOverflowForDecorations(const RenderStyle&, TextUnderlinePositionUnder);
>+GlyphOverflow visualOverflowForDecorations(const RenderStyle&, FontBaseline, TextUnderlinePositionUnder);
> GlyphOverflow visualOverflowForDecorations(const InlineIterator::LineBoxIterator&, const RenderText&, float textBoxLogicalTop, float textBoxLogicalBottom);
> 
> float underlineOffsetForTextBoxPainting(const RenderStyle&, const InlineIterator::TextBoxIterator&);

Actions: View | Formatted Diff | Diff

Attachments on bug 243621: 461436 | 461445