Attachment 467573 Details for Bug 261212 – Patch

[patch] Patch

bug-261212-20230906115318.patch (text/plain), 13.28 KB, created by alan on 2023-09-06 11:53:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: alan

Created: 2023-09-06 11:53:19 PDT

Size: 13.28 KB

patch

obsolete

>From 7193d8abb7c533270b1f24a6ae9657eb29552afb Mon Sep 17 00:00:00 2001
>From: Alan Baradlay <zalan@apple.com>
>Date: Wed, 6 Sep 2023 11:51:34 -0700
>Subject: [PATCH] [IFC] Remove integral round/floor/ceil
> https://bugs.webkit.org/show_bug.cgi?id=261212
>
>Reviewed by NOBODY (OOPS!).
>
>* Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h:
>(WebCore::Layout::InlineLevelBox::setLogicalHeight):
>(WebCore::Layout::InlineLevelBox::setLogicalTop):
>(WebCore::Layout::InlineLevelBox::setAscentAndDescent):
>(WebCore::Layout::InlineLevelBox::setLayoutBounds):
>(WebCore::Layout::InlineLevelBox::preferredLineHeight const):
>(WebCore::Layout::InlineLevelBox::AscentAndDescent::round): Deleted.
>* Source/WebCore/layout/formattingContexts/inline/InlineLevelBoxInlines.h:
>* Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:
>(WebCore::Layout::LineBoxBuilder::enclosingAscentDescentWithFallbackFonts const):
>(WebCore::Layout::LineBoxBuilder::setLayoutBoundsForInlineBox const):
>(WebCore::Layout::LineBoxBuilder::setVerticalPropertiesForInlineLevelBox const):
>(WebCore::Layout::LineBoxBuilder::adjustInlineBoxHeightsForLineBoxContainIfApplicable):
>(WebCore::Layout::LineBoxBuilder::adjustIdeographicBaselineIfApplicable):
>* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:
>(WebCore::Layout::InlineDisplayContentBuilder::appendTextDisplayBox):
>* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:
>(WebCore::LayoutIntegration::LineLayout::updateLayoutBoxDimensions):
>* Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
>(LayoutTestRunner._interrupt_if_at_failure_limits):
>---
> .../inline/InlineLevelBox.h                   | 22 +++++--------------
> .../inline/InlineLevelBoxInlines.h            |  2 +-
> .../inline/InlineLineBoxBuilder.cpp           | 15 +++++--------
> .../display/InlineDisplayContentBuilder.cpp   |  6 ++---
> .../inline/LayoutIntegrationLineLayout.cpp    |  2 +-
> .../controllers/layout_test_runner.py         |  1 +
> 6 files changed, 18 insertions(+), 30 deletions(-)
>
>diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h b/Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h
>index 7a3b65885b7e..d6af808c344d 100644
>--- a/Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h
>+++ b/Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h
>@@ -55,9 +55,6 @@ public:
> 
>         InlineLayoutUnit height() const { return ascent + descent; }
>         friend bool operator==(const AscentAndDescent&, const AscentAndDescent&) = default;
>-        // FIXME: Remove this.
>-        // We need floor/ceil to match legacy layout integral positioning.
>-        void round();
>     };
>     InlineLayoutUnit ascent() const { return m_ascentAndDescent.ascent; }
>     InlineLayoutUnit descent() const { return m_ascentAndDescent.descent; }
>@@ -134,13 +131,12 @@ private:
>     InlineLayoutUnit logicalWidth() const { return m_logicalRect.width(); }
>     InlineLayoutUnit logicalHeight() const { return m_logicalRect.height(); }
> 
>-    // FIXME: Remove legacy rounding.
>     void setLogicalWidth(InlineLayoutUnit logicalWidth) { m_logicalRect.setWidth(logicalWidth); }
>-    void setLogicalHeight(InlineLayoutUnit logicalHeight) { m_logicalRect.setHeight(roundToInt(logicalHeight)); }
>-    void setLogicalTop(InlineLayoutUnit logicalTop) { m_logicalRect.setTop(logicalTop >= 0 ? roundToInt(logicalTop) : -roundToInt(-logicalTop)); }
>+    void setLogicalHeight(InlineLayoutUnit logicalHeight) { m_logicalRect.setHeight(logicalHeight); }
>+    void setLogicalTop(InlineLayoutUnit logicalTop) { m_logicalRect.setTop(logicalTop); }
>     void setLogicalLeft(InlineLayoutUnit logicalLeft) { m_logicalRect.setLeft(logicalLeft); }
>-    void setAscentAndDescent(AscentAndDescent ascentAndDescent) { m_ascentAndDescent = { InlineLayoutUnit(roundToInt(ascentAndDescent.ascent)), InlineLayoutUnit(roundToInt(ascentAndDescent.descent)) }; }
>-    void setLayoutBounds(const AscentAndDescent& layoutBounds) { m_layoutBounds = { InlineLayoutUnit(roundToInt(layoutBounds.ascent)), InlineLayoutUnit(roundToInt(layoutBounds.descent)) }; }
>+    void setAscentAndDescent(AscentAndDescent ascentAndDescent) { m_ascentAndDescent = { InlineLayoutUnit(ascentAndDescent.ascent), InlineLayoutUnit(ascentAndDescent.descent) }; }
>+    void setLayoutBounds(const AscentAndDescent& layoutBounds) { m_layoutBounds = { InlineLayoutUnit(layoutBounds.ascent), InlineLayoutUnit(layoutBounds.descent) }; }
>     void setInlineBoxContentOffsetForTextBoxTrim(InlineLayoutUnit offset) { m_inlineBoxContentOffsetForTextBoxTrim = offset; }
> 
>     void setIsFirstBox() { m_isFirstWithinLayoutBox = true; }
>@@ -191,8 +187,8 @@ inline InlineLayoutUnit InlineLevelBox::preferredLineHeight() const
>         return primarymetricsOfPrimaryFont().lineSpacing();
> 
>     if (m_style.lineHeight.isPercentOrCalculated())
>-        return floorf(minimumValueForLength(m_style.lineHeight, fontSize()));
>-    return floorf(m_style.lineHeight.value());
>+        return minimumValueForLength(m_style.lineHeight, fontSize());
>+    return m_style.lineHeight.value();
> }
> 
> inline bool InlineLevelBox::hasLineBoxRelativeAlignment() const
>@@ -201,12 +197,6 @@ inline bool InlineLevelBox::hasLineBoxRelativeAlignment() const
>     return verticalAlignment == VerticalAlign::Top || verticalAlignment == VerticalAlign::Bottom;
> }
> 
>-inline void InlineLevelBox::AscentAndDescent::round()
>-{
>-    ascent = floorf(ascent);
>-    descent = ceilf(descent);
>-}
>-
> }
> }
> 
>diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineLevelBoxInlines.h b/Source/WebCore/layout/formattingContexts/inline/InlineLevelBoxInlines.h
>index 18c31ac5ca40..e0f912e5f8d1 100644
>--- a/Source/WebCore/layout/formattingContexts/inline/InlineLevelBoxInlines.h
>+++ b/Source/WebCore/layout/formattingContexts/inline/InlineLevelBoxInlines.h
>@@ -67,7 +67,7 @@ inline InlineLevelBox::InlineLevelBox(const Box& layoutBox, const RenderStyle& s
>         }
> 
>         if (hasAboveTextEmphasis || hasUnderTextEmphasis) {
>-            InlineLayoutUnit annotationSize = roundToInt(style.fontCascade().floatEmphasisMarkHeight(style.textEmphasisMarkString()));
>+            InlineLayoutUnit annotationSize = style.fontCascade().floatEmphasisMarkHeight(style.textEmphasisMarkString());
>             m_annotation = { hasAboveTextEmphasis ? annotationSize : 0, hasAboveTextEmphasis ? 0 : annotationSize };
>         }
>     };
>diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp
>index b1f044cb262b..a39bc02fa0b7 100644
>--- a/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp
>+++ b/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp
>@@ -179,8 +179,7 @@ InlineLevelBox::AscentAndDescent LineBoxBuilder::enclosingAscentDescentWithFallb
>         maxAscent = std::max(maxAscent, ascentAndDescent.ascent);
>         maxDescent = std::max(maxDescent, ascentAndDescent.descent);
>     }
>-    // We need floor/ceil to match legacy layout integral positioning.
>-    return { floorf(maxAscent), ceilf(maxDescent) };
>+    return { maxAscent, maxDescent };
> }
> 
> void LineBoxBuilder::setLayoutBoundsForInlineBox(InlineLevelBox& inlineBox, FontBaseline fontBaseline) const
>@@ -223,14 +222,12 @@ void LineBoxBuilder::setLayoutBoundsForInlineBox(InlineLevelBox& inlineBox, Font
>         ascent += inlineBoxGeometry.marginBorderAndPaddingBefore();
>         descent += inlineBoxGeometry.marginBorderAndPaddingAfter();
>     }
>-    inlineBox.setLayoutBounds({ floorf(ascent), ceilf(descent) });
>+    inlineBox.setLayoutBounds({ ascent, descent });
> }
> 
> void LineBoxBuilder::setVerticalPropertiesForInlineLevelBox(const LineBox& lineBox, InlineLevelBox& inlineLevelBox) const
> {
>-    auto setVerticalProperties = [&] (InlineLevelBox::AscentAndDescent ascentAndDescent, bool applyLegacyRounding = true) {
>-        if (applyLegacyRounding)
>-            ascentAndDescent.round();
>+    auto setVerticalProperties = [&] (InlineLevelBox::AscentAndDescent ascentAndDescent) {
>         inlineLevelBox.setAscentAndDescent(ascentAndDescent);
>         inlineLevelBox.setLayoutBounds(ascentAndDescent);
>         inlineLevelBox.setLogicalHeight(ascentAndDescent.height());
>@@ -314,7 +311,7 @@ void LineBoxBuilder::setVerticalPropertiesForInlineLevelBox(const LineBox& lineB
>             }
>             return marginBoxHeight;
>         }();
>-        setVerticalProperties({ ascent, marginBoxHeight - ascent }, false);
>+        setVerticalProperties({ ascent, marginBoxHeight - ascent });
>         return;
>     }
>     ASSERT_NOT_REACHED();
>@@ -538,7 +535,7 @@ void LineBoxBuilder::adjustInlineBoxHeightsForLineBoxContainIfApplicable(LineBox
>         auto mayShrinkLineBox = inlineBox->isRootInlineBox() ? !lineBoxContain.contains(LineBoxContain::Block) : true;
>         auto ascent = mayShrinkLineBox ? enclosingAscentDescentForInlineBox.ascent : std::max(enclosingAscentDescentForInlineBox.ascent, inlineBoxLayoutBounds.ascent);
>         auto descent = mayShrinkLineBox ? enclosingAscentDescentForInlineBox.descent : std::max(enclosingAscentDescentForInlineBox.descent, inlineBoxLayoutBounds.descent);
>-        inlineBox->setLayoutBounds({ ceilf(ascent), ceilf(descent) });
>+        inlineBox->setLayoutBounds({ ascent, descent });
>     }
> }
> 
>@@ -583,7 +580,7 @@ void LineBoxBuilder::adjustIdeographicBaselineIfApplicable(LineBox& lineBox)
>             setVerticalPropertiesForInlineLevelBox(lineBox, inlineLevelBox);
>         else if (inlineLevelBox.isAtomicInlineLevelBox()) {
>             auto inlineLevelBoxHeight = inlineLevelBox.logicalHeight();
>-            InlineLayoutUnit ideographicBaseline = roundToInt(inlineLevelBoxHeight / 2);
>+            InlineLayoutUnit ideographicBaseline = inlineLevelBoxHeight / 2;
>             // Move the baseline position but keep the same logical height.
>             inlineLevelBox.setAscentAndDescent({ ideographicBaseline, inlineLevelBoxHeight - ideographicBaseline });
>             inlineLevelBox.setLayoutBounds({ ideographicBaseline, inlineLevelBoxHeight - ideographicBaseline });
>diff --git a/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp
>index e6999ef5350e..4ff6672058d6 100644
>--- a/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp
>+++ b/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp
>@@ -185,7 +185,7 @@ void InlineDisplayContentBuilder::appendTextDisplayBox(const Line::Run& lineRun,
>         addLetterSpacingOverflow();
> 
>         auto addStrokeOverflow = [&] {
>-            inkOverflow.inflate(ceilf(style.computedStrokeWidth(m_initialContaingBlockSize)));
>+            inkOverflow.inflate(style.computedStrokeWidth(m_initialContaingBlockSize));
>         };
>         addStrokeOverflow();
> 
>@@ -203,8 +203,8 @@ void InlineDisplayContentBuilder::appendTextDisplayBox(const Line::Run& lineRun,
>             auto enclosingAscentAndDescent = TextUtil::enclosingGlyphBoundsForText(StringView(content).substring(text->start, text->length), style);
>             // FIXME: Take fallback fonts into account.
>             auto& fontMetrics = style.metricsOfPrimaryFont();
>-            auto topOverflow = std::max(0.f, ceilf(-enclosingAscentAndDescent.ascent) - fontMetrics.ascent());
>-            auto bottomOverflow = std::max(0.f, ceilf(enclosingAscentAndDescent.descent) - fontMetrics.descent());
>+            auto topOverflow = std::max(0.f, -enclosingAscentAndDescent.ascent - fontMetrics.ascent());
>+            auto bottomOverflow = std::max(0.f, enclosingAscentAndDescent.descent - fontMetrics.descent());
>             inkOverflow.inflate(topOverflow, { }, bottomOverflow, { });
>         };
>         addGlyphOverflow();
>diff --git a/Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp b/Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp
>index 552e543cf814..42c87b3ed09a 100644
>--- a/Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp
>+++ b/Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp
>@@ -426,7 +426,7 @@ void LineLayout::updateLayoutBoxDimensions(const RenderBox& replacedOrInlineBloc
>     }();
>     if (hasNonSyntheticBaseline) {
>         auto baseline = replacedOrInlineBlock.baselinePosition(AlphabeticBaseline, false /* firstLine */, blockFlowDirection == BlockFlowDirection::TopToBottom ? HorizontalLine : VerticalLine, PositionOnContainingLine);
>-        layoutBox.setBaselineForIntegration(roundToInt(baseline));
>+        layoutBox.setBaselineForIntegration(baseline);
>     }
> 
>     if (auto* shapeOutsideInfo = replacedOrInlineBlock.shapeOutsideInfo())
>diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py b/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>index ea2c5833f500..928c9bd6ef0d 100644
>--- a/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
>@@ -212,6 +212,7 @@ class LayoutTestRunner(object):
>                 self._mark_interrupted_tests_as_skipped(run_results)
>                 raise TestRunInterruptedException(message)
> 
>+        self._options.exit_after_n_failures = None
>         interrupt_if_at_failure_limit(
>             self._options.exit_after_n_failures,
>             run_results.unexpected_failures,

Flags:

ews-feeder: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 261212: 467571 | 467573