⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 282129 in webkit


Ignore:
Timestamp:
Sep 7, 2021, 11:12:01 PM (5 years ago)
Author:
Antti Koivisto
Message:

Disable inline culling
​https://bugs.webkit.org/show_bug.cgi?id=229993

Reviewed by Alan Bujtas.

LayoutTests/imported/w3c:

  • web-platform-tests/css/cssom-view/cssom-getClientRects-002-expected.txt:
  • web-platform-tests/css/cssom-view/elementFromPoint-mixed-font-sizes-expected.txt:
  • web-platform-tests/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint-expected.txt:

Source/WebCore:

Inline culling is an optimization that avoids creating LegacyInlineFlowBoxes for inline
elements under certain circumstances (basically if they don't affect rendering).

The optimization is is complex and requires a ton of code. It is a constant source of bugs.
Meanwhile the kind of content where this is beneficial is already mostly taken over by LFC.
It is time to remove it.

This patch disables the optimization but doesn't yet remove the code.

  • editing/SimplifyMarkupCommand.cpp:

(WebCore::SimplifyMarkupCommand::doApply):

  • rendering/LegacyEllipsisBox.cpp:

(WebCore::LegacyEllipsisBox::markupBox const):

  • rendering/RenderInline.cpp:

(WebCore::RenderInline::mayAffectRendering const):
(WebCore::RenderInline::updateAlwaysCreateLineBoxes):
(WebCore::RenderInline::shouldCreateLineBoxes const): Deleted.

  • rendering/RenderInline.h:

(WebCore::RenderInline::alwaysCreateLineBoxes const):

  • rendering/RenderTreeAsText.cpp:

(WebCore::hasNonEmptySibling):

LayoutTests:

  • fast/flexbox/line-clamp-link-after-ellipsis.html:
  • platform/mac/fast/multicol/table-vertical-align-expected.txt:
  • platform/mac/fast/multicol/vertical-lr/float-multicol-expected.txt:
  • platform/mac/fast/multicol/vertical-rl/float-multicol-expected.txt:
  • platform/mac/tables/mozilla/bugs/bug1188-expected.txt:
Location:
trunk
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r282123 r282129  
     12021-09-07  Antti Koivisto  <antti@apple.com>
     2
     3        Disable inline culling
     4        https://bugs.webkit.org/show_bug.cgi?id=229993
     5
     6        Reviewed by Alan Bujtas.
     7
     8        * fast/flexbox/line-clamp-link-after-ellipsis.html:
     9        * platform/mac/fast/multicol/table-vertical-align-expected.txt:
     10        * platform/mac/fast/multicol/vertical-lr/float-multicol-expected.txt:
     11        * platform/mac/fast/multicol/vertical-rl/float-multicol-expected.txt:
     12        * platform/mac/tables/mozilla/bugs/bug1188-expected.txt:
     13
    1142021-09-07  Fujii Hironori  <Hironori.Fujii@sony.com>
    215

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r282089 r282129  
     12021-09-07  Antti Koivisto  <antti@apple.com>
     2
     3        Disable inline culling
     4        https://bugs.webkit.org/show_bug.cgi?id=229993
     5
     6        Reviewed by Alan Bujtas.
     7
     8        * web-platform-tests/css/cssom-view/cssom-getClientRects-002-expected.txt:
     9        * web-platform-tests/css/cssom-view/elementFromPoint-mixed-font-sizes-expected.txt:
     10        * web-platform-tests/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint-expected.txt:
     11
    1122021-09-07  Simon Fraser  <simon.fraser@apple.com>
    213

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/cssom-getClientRects-002-expected.txt

    r267650 r282129  
    22test test
    33
    4 FAIL CSSOM View - GetClientRects().length is the same regardless source new lines assert_equals: count2 expected 1 but got 2
     4PASS CSSOM View - GetClientRects().length is the same regardless source new lines
    55

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/elementFromPoint-mixed-font-sizes-expected.txt

    r236274 r282129  
    11XXX small YYY
    22
    3 FAIL document.elementFromPoint finds container SPAN in the empty region above a child SPAN with a smaller font size assert_equals: expected Element node <span id="target">
    4     XXX <span id="small" style="font-s... but got Element node <div style="font-size: 40px">
    5   <span id="target">
    6     XX...
     3PASS document.elementFromPoint finds container SPAN in the empty region above a child SPAN with a smaller font size
    74

  • trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint-expected.txt

    r267647 r282129  
    2828PASS document.elementsFromPoint and shadow.elementsFromPoint must return the shadow host and its ancestors of the hit-tested text node when the hit-tested text node is a direct child of the root and the host has display: block
    2929PASS document.elementsFromPoint and shadow.elementsFromPoint must return the shadow host and its ancestors of the hit-tested text node when the hit-tested text node is a direct child of the root and the host has display: inline-block
    30 FAIL document.elementsFromPoint and shadowRoot.elementsFromPoint must return the shadow host and its ancestors when the hit-tested text node is assigned to a slot and the host has display: inline assert_array_equals: expected property 0 to be Element node <test-element style="display: inline;">text</test-element> but got Element node <slot></slot> (expected array [Element node <test-element style="display: inline;">text</test-element>, Element node <div id="container"><test-element style="display: inline;..., Element node <body>
     30FAIL document.elementsFromPoint and shadowRoot.elementsFromPoint must return the shadow host and its ancestors when the hit-tested text node is assigned to a slot and the host has display: inline assert_array_equals: lengths differ, expected array [Element node <test-element style="display: inline;">text</test-element>, Element node <div id="container"><test-element style="display: inline;..., Element node <body>
    3131    <div id="container"><test-element style="displ..., Element node <html><head>
    32     <title>Shadow DOM and CSSOM View: Docume...] got [Element node <slot></slot>, Element node <div id="container"><test-element style="display: inline;..., Element node <body>
     32    <title>Shadow DOM and CSSOM View: Docume...] length 4, got [Element node <slot></slot>, Element node <test-element style="display: inline;">text</test-element>, Element node <div id="container"><test-element style="display: inline;..., Element node <body>
    3333    <div id="container"><test-element style="displ..., Element node <html><head>
    34     <title>Shadow DOM and CSSOM View: Docume...])
     34    <title>Shadow DOM and CSSOM View: Docume...] length 5
    3535FAIL document.elementsFromPoint and shadowRoot.elementsFromPoint must return the shadow host and its ancestors when the hit-tested text node is assigned to a slot and the host has display: block assert_array_equals: lengths differ, expected array [Element node <test-element style="display: block;">text</test-element>, Element node <div id="container"><test-element style="display: block;"..., Element node <body>
    3636    <div id="container"><test-element style="displ..., Element node <html><head>
    … …  
    4646PASS document.elementsFromPoint and shadowRoot.elementsFromPoint must return the element assigned to a slot and its non-shadow ancestors when hit-tested text node under an element is assigned to a slot in the shadow tree and the shadow host of the slot has display: block
    4747PASS document.elementsFromPoint and shadowRoot.elementsFromPoint must return the element assigned to a slot and its non-shadow ancestors when hit-tested text node under an element is assigned to a slot in the shadow tree and the shadow host of the slot has display: inline-block
    48 FAIL document.elementsFromPoint must return the shadow host and its ancestors of the hit-tested element under a shadow root andshadowRoot.elementsFromPoint must return the element parent and its non-shadow ancestors of the hit-tested text node under the point when the shadow host has display: inline assert_array_equals: lengths differ, expected array [Element node <span>text</span>, Element node <test-element style="display: inline;"></test-element>, Element node <div id="container"><test-element style="display: inline;..., Element node <body>
    49     <div id="container"><test-element style="displ..., Element node <html><head>
    50     <title>Shadow DOM and CSSOM View: Docume...] length 5, got [Element node <span>text</span>, Element node <div id="container"><test-element style="display: inline;..., Element node <body>
    51     <div id="container"><test-element style="displ..., Element node <html><head>
    52     <title>Shadow DOM and CSSOM View: Docume...] length 4
     48PASS document.elementsFromPoint must return the shadow host and its ancestors of the hit-tested element under a shadow root andshadowRoot.elementsFromPoint must return the element parent and its non-shadow ancestors of the hit-tested text node under the point when the shadow host has display: inline
    5349PASS document.elementsFromPoint must return the shadow host and its ancestors of the hit-tested element under a shadow root andshadowRoot.elementsFromPoint must return the element parent and its non-shadow ancestors of the hit-tested text node under the point when the shadow host has display: block
    5450PASS document.elementsFromPoint must return the shadow host and its ancestors of the hit-tested element under a shadow root andshadowRoot.elementsFromPoint must return the element parent and its non-shadow ancestors of the hit-tested text node under the point when the shadow host has display: inline-block

  • trunk/LayoutTests/platform/mac/fast/multicol/table-vertical-align-expected.txt

    r268520 r282129  
    270270            RenderBR {BR} at (56,1097) size 1x18
    271271          RenderTableCell {TD} at (142,478) size 233x207 [border: (1px inset #808080)] [r=0 c=1 rs=1 cs=1]
    272             RenderInline {SPAN} at (0,0) size 146x183
     272            RenderInline {SPAN} at (0,0) size 146x184
    273273              RenderText {#text} at (11,10) size 146x185
    274274                text run at (11,11) width 146: "Other"

  • trunk/LayoutTests/platform/mac/fast/multicol/vertical-lr/float-multicol-expected.txt

    r279673 r282129  
    147147            text run at (306,291) width 70: "build with "
    148148            text run at (306,360) width 63: "Talkback,"
    149           RenderInline {EM} at (0,0) size 18x106
     149          RenderInline {EM} at (0,0) size 19x106
    150150            RenderText {#text} at (324,291) size 19x106
    151151              text run at (324,291) width 46: "please "
    … …  
    163163            text run at (414,284) width 139: "If you find something"
    164164            text run at (432,0) width 317: "you think is a bug, check to see if it's not already "
    165           RenderInline {A} at (0,0) size 18x84 [color=#0000EE]
     165          RenderInline {A} at (0,0) size 19x84 [color=#0000EE]
    166166            RenderText {#text} at (432,316) size 19x84
    167167              text run at (432,316) width 84: "known about"
    … …  
    170170            text run at (450,0) width 76: "then please "
    171171            text run at (450,75) width 70: "follow the "
    172           RenderInline {A} at (0,0) size 18x169 [color=#0000EE]
     172          RenderInline {A} at (0,0) size 19x169 [color=#0000EE]
    173173            RenderText {#text} at (450,144) size 19x169
    174174              text run at (450,144) width 169: "bug submission procedure"

  • trunk/LayoutTests/platform/mac/fast/multicol/vertical-rl/float-multicol-expected.txt

    r279673 r282129  
    147147            text run at (306,291) width 70: "build with "
    148148            text run at (306,360) width 63: "Talkback,"
    149           RenderInline {EM} at (0,0) size 18x106
     149          RenderInline {EM} at (0,0) size 19x106
    150150            RenderText {#text} at (324,291) size 19x106
    151151              text run at (324,291) width 46: "please "
    … …  
    163163            text run at (414,284) width 139: "If you find something"
    164164            text run at (432,0) width 317: "you think is a bug, check to see if it's not already "
    165           RenderInline {A} at (0,0) size 18x84 [color=#0000EE]
     165          RenderInline {A} at (0,0) size 19x84 [color=#0000EE]
    166166            RenderText {#text} at (432,316) size 19x84
    167167              text run at (432,316) width 84: "known about"
    … …  
    170170            text run at (450,0) width 76: "then please "
    171171            text run at (450,75) width 70: "follow the "
    172           RenderInline {A} at (0,0) size 18x169 [color=#0000EE]
     172          RenderInline {A} at (0,0) size 19x169 [color=#0000EE]
    173173            RenderText {#text} at (450,144) size 19x169
    174174              text run at (450,144) width 169: "bug submission procedure"

  • trunk/LayoutTests/platform/mac/tables/mozilla/bugs/bug1188-expected.txt

    r278931 r282129  
    1414            RenderTableRow {TR} at (0,48) size 600x55
    1515              RenderTableCell {TD} at (2,55) size 595x41 [bgcolor=#99CCCC] [r=1 c=0 rs=1 cs=3]
    16                 RenderInline {FONT} at (0,0) size 128x15
    17                   RenderInline {B} at (0,0) size 128x15
     16                RenderInline {FONT} at (0,0) size 128x16
     17                  RenderInline {B} at (0,0) size 128x16
    1818                    RenderText {#text} at (48,4) size 128x17
    1919                      text run at (48,5) width 128: "Search the Web with"
    … …  
    3232                      text run at (0,0) width 37: "Search"
    3333                RenderBR {BR} at (545,2) size 1x20
    34                 RenderInline {SMALL} at (0,0) size 554x15
    35                   RenderInline {A} at (0,0) size 98x15 [color=#0000EE]
     34                RenderInline {SMALL} at (0,0) size 554x16
     35                  RenderInline {A} at (0,0) size 98x16 [color=#0000EE]
    3636                    RenderText {#text} at (20,23) size 98x17
    3737                      text run at (20,24) width 98: "Classifieds< /A>   "
    38                   RenderInline {A} at (0,0) size 58x15 [color=#0000EE]
     38                  RenderInline {A} at (0,0) size 58x16 [color=#0000EE]
    3939                    RenderText {#text} at (117,23) size 58x17
    4040                      text run at (117,24) width 23: "Net "
    … …  
    4242                  RenderText {#text} at (174,23) size 11x17
    4343                    text run at (174,24) width 11: "   "
    44                   RenderInline {A} at (0,0) size 80x15 [color=#0000EE]
     44                  RenderInline {A} at (0,0) size 80x16 [color=#0000EE]
    4545                    RenderText {#text} at (184,23) size 80x17
    4646                      text run at (184,24) width 55: "Find Web "
    … …  
    4949                    text run at (263,24) width 5: " "
    5050                    text run at (267,24) width 7: "  "
    51                   RenderInline {A} at (0,0) size 65x15 [color=#0000EE]
     51                  RenderInline {A} at (0,0) size 65x16 [color=#0000EE]
    5252                    RenderText {#text} at (273,23) size 65x17
    5353                      text run at (273,24) width 40: "What's "
    … …  
    5555                  RenderText {#text} at (337,23) size 11x17
    5656                    text run at (337,24) width 11: "   "
    57                   RenderInline {A} at (0,0) size 64x15 [color=#0000EE]
     57                  RenderInline {A} at (0,0) size 64x16 [color=#0000EE]
    5858                    RenderText {#text} at (347,23) size 64x17
    5959                      text run at (347,24) width 64: "What's New"
    6060                  RenderText {#text} at (410,23) size 11x17
    6161                    text run at (410,24) width 11: "   "
    62                   RenderInline {A} at (0,0) size 74x15 [color=#0000EE]
     62                  RenderInline {A} at (0,0) size 74x16 [color=#0000EE]
    6363                    RenderText {#text} at (420,23) size 74x17
    6464                      text run at (420,24) width 74: "People Finder"
    6565                  RenderText {#text} at (493,23) size 10x17
    6666                    text run at (493,24) width 10: "   "
    67                   RenderInline {A} at (0,0) size 72x15 [color=#0000EE]
     67                  RenderInline {A} at (0,0) size 72x16 [color=#0000EE]
    6868                    RenderText {#text} at (502,23) size 72x17
    6969                      text run at (502,24) width 72: "Yellow Pages"

  • trunk/Source/WebCore/ChangeLog

    r282126 r282129  
     12021-09-07  Antti Koivisto  <antti@apple.com>
     2
     3        Disable inline culling
     4        https://bugs.webkit.org/show_bug.cgi?id=229993
     5
     6        Reviewed by Alan Bujtas.
     7
     8        Inline culling is an optimization that avoids creating LegacyInlineFlowBoxes for inline
     9        elements under certain circumstances (basically if they don't affect rendering).
     10
     11        The optimization is is complex and requires a ton of code. It is a constant source of bugs.
     12        Meanwhile the kind of content where this is beneficial is already mostly taken over by LFC.
     13        It is time to remove it.
     14
     15        This patch disables the optimization but doesn't yet remove the code.
     16
     17        * editing/SimplifyMarkupCommand.cpp:
     18        (WebCore::SimplifyMarkupCommand::doApply):
     19        * rendering/LegacyEllipsisBox.cpp:
     20        (WebCore::LegacyEllipsisBox::markupBox const):
     21        * rendering/RenderInline.cpp:
     22        (WebCore::RenderInline::mayAffectRendering const):
     23        (WebCore::RenderInline::updateAlwaysCreateLineBoxes):
     24        (WebCore::RenderInline::shouldCreateLineBoxes const): Deleted.
     25        * rendering/RenderInline.h:
     26        (WebCore::RenderInline::alwaysCreateLineBoxes const):
     27        * rendering/RenderTreeAsText.cpp:
     28        (WebCore::hasNonEmptySibling):
     29
    1302021-09-07  Alex Christensen  <achristensen@webkit.org>
    231

  • trunk/Source/WebCore/editing/SimplifyMarkupCommand.cpp

    r232018 r282129  
    7272
    7373            auto* renderer = currentNode->renderer();
    74             if (!is<RenderInline>(renderer) || downcast<RenderInline>(*renderer).alwaysCreateLineBoxes())
     74            if (!is<RenderInline>(renderer) || downcast<RenderInline>(*renderer).mayAffectRendering())
    7575                continue;
    7676           

  • trunk/Source/WebCore/rendering/LegacyEllipsisBox.cpp

    r282092 r282129  
    9494    // If the last line-box on the last line of a block is a link, -webkit-line-clamp paints that box after the ellipsis.
    9595    // It does not actually move the link.
    96     LegacyInlineBox* anchorBox = lastLine->lastChild();
     96    LegacyInlineBox* anchorBox = lastLine->lastLeafDescendant();
    9797    if (!anchorBox || !anchorBox->renderer().style().isLink())
    9898        return 0;

  • trunk/Source/WebCore/rendering/RenderInline.cpp

    r282045 r282129  
    208208}
    209209
    210 bool RenderInline::shouldCreateLineBoxes() const
     210bool RenderInline::mayAffectRendering() const
    211211{
    212212    // Test if we can get away with culling.
    … …  
    215215    auto hasHardLineBreakChildOnly = firstChild() && firstChild() == lastChild() && firstChild()->isBR();
    216216    bool checkFonts = document().inNoQuirksMode();
    217     auto needsLineBoxes = (parentRenderInline && parentRenderInline->alwaysCreateLineBoxes())
     217    auto mayAffectRendering = (parentRenderInline && parentRenderInline->mayAffectRendering())
    218218        || (parentRenderInline && parentStyle->verticalAlign() != VerticalAlign::Baseline)
    219219        || style().verticalAlign() != VerticalAlign::Baseline
    … …  
    223223        || hasHardLineBreakChildOnly;
    224224
    225     if (!needsLineBoxes && checkFonts && view().usesFirstLineRules()) {
     225    if (!mayAffectRendering && checkFonts && view().usesFirstLineRules()) {
    226226        // Have to check the first line style as well.
    227227        parentStyle = &parent()->firstLineStyle();
    228228        auto& childStyle = firstLineStyle();
    229         needsLineBoxes = !parentStyle->fontCascade().fontMetrics().hasIdenticalAscentDescentAndLineGap(childStyle.fontCascade().fontMetrics())
     229        mayAffectRendering = !parentStyle->fontCascade().fontMetrics().hasIdenticalAscentDescentAndLineGap(childStyle.fontCascade().fontMetrics())
    230230            || childStyle.verticalAlign() != VerticalAlign::Baseline
    231231            || parentStyle->lineHeight() != childStyle.lineHeight();
    232232    }
    233     return needsLineBoxes;
     233    return mayAffectRendering;
    234234}
    235235
    … …  
    238238    // Once we have been tainted once, just assume it will happen again. This way effects like hover highlighting that change the
    239239    // background color will only cause a layout on the first rollover.
    240     if (alwaysCreateLineBoxes() || !shouldCreateLineBoxes())
     240    if (alwaysCreateLineBoxes() || !mayAffectRendering())
    241241        return;
    242242

  • trunk/Source/WebCore/rendering/RenderInline.h

    r281239 r282129  
    8383    void paintOutline(PaintInfo&, const LayoutPoint&);
    8484
    85     bool alwaysCreateLineBoxes() const { return renderInlineAlwaysCreatesLineBoxes(); }
     85    bool alwaysCreateLineBoxes() const { return true; }
    8686    void setAlwaysCreateLineBoxes() { setRenderInlineAlwaysCreatesLineBoxes(true); }
    87     bool shouldCreateLineBoxes() const;
     87    bool mayAffectRendering() const;
    8888    void updateAlwaysCreateLineBoxes(bool fullLayout);
    8989

  • trunk/Source/WebCore/rendering/RenderTreeAsText.cpp

    r279918 r282129  
    203203            return true;
    204204        auto& siblingRendererInline = downcast<RenderInline>(sibling);
    205         if (siblingRendererInline.shouldCreateLineBoxes() || !isRenderInlineEmpty(siblingRendererInline))
     205        if (siblingRendererInline.mayAffectRendering() || !isRenderInlineEmpty(siblingRendererInline))
    206206            return true;
    207207    }
Note: See TracChangeset for help on using the changeset viewer.