Changeset 167568 in webkit


Ignore:
Timestamp:
Apr 20, 2014 10:43:54 AM (10 years ago)
Author:
Antti Koivisto
Message:

Text bounding box computation for simple line layout is wrong
​https://bugs.webkit.org/show_bug.cgi?id=131912

Reviewed by Andreas Kling.

Source/WebCore:

Top-left is currently the first line top-left which is not always correct.

  • WebCore.exp.in:
  • rendering/RenderText.cpp:

(WebCore::RenderText::firstRunLocation):
(WebCore::RenderText::firstRunOrigin): Deleted.
(WebCore::RenderText::firstRunX): Deleted.
(WebCore::RenderText::firstRunY): Deleted.

Keep just one accessor and rename it.
Encapsulate the line box and simple line versions.

  • rendering/RenderText.h:
  • rendering/RenderTextLineBoxes.cpp:

(WebCore::RenderTextLineBoxes::firstRunLocation):

Line box version.

  • rendering/RenderTextLineBoxes.h:
  • rendering/RenderTreeAsText.cpp:


Simplify RenderText dumping.

(WebCore::RenderTreeAsText::writeRenderObject):

  • rendering/SimpleLineLayoutFunctions.cpp:

(WebCore::SimpleLineLayout::computeTextBoundingBox):

Return the correct x position.

(WebCore::SimpleLineLayout::computeTextFirstRunLocation):

Simple line version.

  • rendering/SimpleLineLayoutFunctions.h:
  • rendering/svg/SVGRenderTreeAsText.cpp:

(WebCore::writeSVGInlineText):

LayoutTests:

Some dumped RenderText sizes change in table related test. These are progressions,
the new results match the contained lines. There are no visual changes.

  • platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.txt:
  • platform/mac/fast/table/multiple-captions-display-expected.txt:
  • platform/mac/tables/mozilla/marvin/body_col-expected.txt:
  • platform/mac/tables/mozilla/marvin/x_th_valign_baseline-expected.txt:
  • platform/mac/tables/mozilla/other/body_col-expected.txt:
  • platform/mac/tables/mozilla_expected_failures/bugs/bug10140-expected.txt:
  • platform/mac/tables/mozilla_expected_failures/bugs/bug10216-expected.txt:
  • platform/mac/tables/mozilla_expected_failures/core/captions3-expected.txt:
  • platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt:
Location:
trunk
Files:
23 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r167564 r167568  
     12014-04-20  Antti Koivisto  <antti@apple.com>
     2
     3        Text bounding box computation for simple line layout is wrong
     4        https://bugs.webkit.org/show_bug.cgi?id=131912
     5
     6        Reviewed by Andreas Kling.
     7       
     8        Some dumped RenderText sizes change in table related test. These are progressions,
     9        the new results match the contained lines. There are no visual changes.
     10
     11        * platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.txt:
     12        * platform/mac/fast/table/multiple-captions-display-expected.txt:
     13        * platform/mac/tables/mozilla/marvin/body_col-expected.txt:
     14        * platform/mac/tables/mozilla/marvin/x_th_valign_baseline-expected.txt:
     15        * platform/mac/tables/mozilla/other/body_col-expected.txt:
     16        * platform/mac/tables/mozilla_expected_failures/bugs/bug10140-expected.txt:
     17        * platform/mac/tables/mozilla_expected_failures/bugs/bug10216-expected.txt:
     18        * platform/mac/tables/mozilla_expected_failures/core/captions3-expected.txt:
     19        * platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt:
     20
    1212014-04-20  Commit Queue  <commit-queue@webkit.org>
    222

  • trunk/LayoutTests/platform/mac/fast/repaint/reflection-redraw-expected.txt

    r162553 r167568  
    88  RenderBlock (positioned) {DIV} at (285,10) size 150x100 [bgcolor=#808080] [border: (1px solid #000000)]
    99    RenderBlock {P} at (11,27) size 128x54 [color=#008000]
    10       RenderText {#text} at (12,0) size 125x54
     10      RenderText {#text} at (12,0) size 126x54
    1111        text run at (12,0) width 104: "The color of this"
    1212        text run at (1,18) width 126: "text in the reflection"
    … …  
    1515  RenderBlock (positioned) {DIV} at (210,120) size 150x100 [bgcolor=#808080] [border: (1px solid #000000)]
    1616    RenderBlock {P} at (11,27) size 128x54 [color=#008000]
    17       RenderText {#text} at (12,0) size 125x54
     17      RenderText {#text} at (12,0) size 126x54
    1818        text run at (12,0) width 104: "The color of this"
    1919        text run at (1,18) width 126: "text in the reflection"
    … …  
    2222  RenderBlock (positioned) {DIV} at (360,120) size 150x100 [bgcolor=#808080] [border: (1px solid #000000)]
    2323    RenderBlock {P} at (11,27) size 128x54 [color=#008000]
    24       RenderText {#text} at (12,0) size 125x54
     24      RenderText {#text} at (12,0) size 126x54
    2525        text run at (12,0) width 104: "The color of this"
    2626        text run at (1,18) width 126: "text in the reflection"
    … …  
    2929  RenderBlock (positioned) {DIV} at (285,230) size 150x100 [bgcolor=#808080] [border: (1px solid #000000)]
    3030    RenderBlock {P} at (11,27) size 128x54 [color=#008000]
    31       RenderText {#text} at (12,0) size 125x54
     31      RenderText {#text} at (12,0) size 126x54
    3232        text run at (12,0) width 104: "The color of this"
    3333        text run at (1,18) width 126: "text in the reflection"

  • trunk/LayoutTests/platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.txt

    r162553 r167568  
    66      RenderTable {table} at (0,0) size 46x72
    77        RenderBlock {caption} at (0,0) size 46x72
    8           RenderText {#text} at (1,0) size 47x72
     8          RenderText {#text} at (1,0) size 46x72
    99            text run at (1,0) width 44: "PASS:"
    1010            text run at (0,18) width 46: "Text in"

  • trunk/LayoutTests/platform/mac/fast/table/multiple-captions-display-expected.txt

    r162553 r167568  
    3737layer at (8,218) size 126x54
    3838  RenderBlock {caption} at (0,210) size 126x54
    39     RenderText {#text} at (25,0) size 117x54
     39    RenderText {#text} at (25,0) size 116x54
    4040      text run at (25,0) width 76: "PASS: First"
    4141      text run at (5,18) width 116: "Caption aligned to"
    … …  
    4747layer at (8,8) size 126x36
    4848  RenderBlock {caption} at (0,0) size 126x36
    49     RenderText {#text} at (14,0) size 103x36
     49    RenderText {#text} at (14,0) size 102x36
    5050      text run at (14,0) width 98: "PASS: Caption"
    5151      text run at (12,18) width 102: "with opacity 0.7"

  • trunk/LayoutTests/platform/mac/tables/mozilla/marvin/body_col-expected.txt

    r162553 r167568  
    1111          RenderTableRow {TR} at (0,2) size 155x40
    1212            RenderTableCell {TH} at (2,2) size 49x40 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
    13               RenderText {#text} at (15,2) size 36x36
     13              RenderText {#text} at (15,2) size 37x36
    1414                text run at (15,2) width 19: "La"
    1515                text run at (6,20) width 37: "Mesa"
    … …  
    1919          RenderTableRow {TR} at (0,44) size 155x40
    2020            RenderTableCell {TH} at (2,44) size 49x40 [border: (1px inset #808080)] [r=1 c=0 rs=1 cs=1]
    21               RenderText {#text} at (17,2) size 42x36
     21              RenderText {#text} at (17,2) size 43x36
    2222                text run at (17,2) width 15: "El"
    2323                text run at (3,20) width 43: "Cajon"

  • trunk/LayoutTests/platform/mac/tables/mozilla/marvin/x_th_valign_baseline-expected.txt

    r162553 r167568  
    1111          RenderTableRow {tr} at (0,2) size 782x234
    1212            RenderTableCell {th} at (2,24) size 214x94 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
    13               RenderText {#text} at (12,2) size 207x90
     13              RenderText {#text} at (12,2) size 206x90
    1414                text run at (12,2) width 190: "Compare the baseline of the"
    1515                text run at (4,20) width 76: "first line of "

  • trunk/LayoutTests/platform/mac/tables/mozilla/other/body_col-expected.txt

    r162553 r167568  
    1111          RenderTableRow {TR} at (0,2) size 155x40
    1212            RenderTableCell {TH} at (2,2) size 49x40 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
    13               RenderText {#text} at (15,2) size 36x36
     13              RenderText {#text} at (15,2) size 37x36
    1414                text run at (15,2) width 19: "La"
    1515                text run at (6,20) width 37: "Mesa"
    … …  
    1919          RenderTableRow {TR} at (0,44) size 155x40
    2020            RenderTableCell {TH} at (2,44) size 49x40 [border: (1px inset #808080)] [r=1 c=0 rs=1 cs=1]
    21               RenderText {#text} at (17,2) size 42x36
     21              RenderText {#text} at (17,2) size 43x36
    2222                text run at (17,2) width 15: "El"
    2323                text run at (3,20) width 43: "Cajon"

  • trunk/LayoutTests/platform/mac/tables/mozilla_expected_failures/bugs/bug10140-expected.txt

    r162553 r167568  
    186186      RenderTable {TABLE} at (0,1196) size 90x127 [border: (10px solid #008000)]
    187187        RenderBlock {CAPTION} at (20,0) size 70x60 [border: (3px solid #800080)]
    188           RenderText {#text} at (22,3) size 51x54
     188          RenderText {#text} at (22,3) size 50x54
    189189            text run at (22,3) width 26: "The"
    190190            text run at (15,21) width 40: "table's"
    … …  
    207207      RenderTable {TABLE} at (0,1322) size 90x127 [border: (10px solid #008000)]
    208208        RenderBlock {CAPTION} at (20,0) size 70x60 [border: (3px solid #800080)]
    209           RenderText {#text} at (22,3) size 51x54
     209          RenderText {#text} at (22,3) size 50x54
    210210            text run at (22,3) width 26: "The"
    211211            text run at (15,21) width 40: "table's"
    … …  
    356356      RenderTable {TABLE} at (0,2290) size 90x127 [border: (10px solid #008000)]
    357357        RenderBlock {CAPTION} at (0,0) size 70x60 [border: (3px solid #800080)]
    358           RenderText {#text} at (22,3) size 51x54
     358          RenderText {#text} at (22,3) size 50x54
    359359            text run at (22,3) width 26: "The"
    360360            text run at (15,21) width 40: "table's"
    … …  
    377377      RenderTable {TABLE} at (15,2416) size 90x127 [border: (10px solid #008000)]
    378378        RenderBlock {CAPTION} at (0,0) size 70x60 [border: (3px solid #800080)]
    379           RenderText {#text} at (22,3) size 51x54
     379          RenderText {#text} at (22,3) size 50x54
    380380            text run at (22,3) width 26: "The"
    381381            text run at (15,21) width 40: "table's"

  • trunk/LayoutTests/platform/mac/tables/mozilla_expected_failures/bugs/bug10216-expected.txt

    r162553 r167568  
    9898      RenderTable {TABLE} at (0,572) size 87x107 [border: (1px outset #808080)]
    9999        RenderBlock {CAPTION} at (0,0) size 87x54
    100           RenderText {#text} at (9,0) size 88x54
     100          RenderText {#text} at (9,0) size 87x54
    101101            text run at (9,0) width 69: "The table's"
    102102            text run at (2,18) width 83: "caption, with"

  • trunk/LayoutTests/platform/mac/tables/mozilla_expected_failures/core/captions3-expected.txt

    r162553 r167568  
    186186      RenderTable {TABLE} at (0,1196) size 90x127 [border: (10px solid #008000)]
    187187        RenderBlock {CAPTION} at (20,0) size 70x60 [border: (3px solid #800080)]
    188           RenderText {#text} at (22,3) size 51x54
     188          RenderText {#text} at (22,3) size 50x54
    189189            text run at (22,3) width 26: "The"
    190190            text run at (15,21) width 40: "table's"
    … …  
    207207      RenderTable {TABLE} at (0,1322) size 90x127 [border: (10px solid #008000)]
    208208        RenderBlock {CAPTION} at (20,0) size 70x60 [border: (3px solid #800080)]
    209           RenderText {#text} at (22,3) size 51x54
     209          RenderText {#text} at (22,3) size 50x54
    210210            text run at (22,3) width 26: "The"
    211211            text run at (15,21) width 40: "table's"
    … …  
    356356      RenderTable {TABLE} at (0,2290) size 90x127 [border: (10px solid #008000)]
    357357        RenderBlock {CAPTION} at (0,0) size 70x60 [border: (3px solid #800080)]
    358           RenderText {#text} at (22,3) size 51x54
     358          RenderText {#text} at (22,3) size 50x54
    359359            text run at (22,3) width 26: "The"
    360360            text run at (15,21) width 40: "table's"
    … …  
    377377      RenderTable {TABLE} at (15,2416) size 90x127 [border: (10px solid #008000)]
    378378        RenderBlock {CAPTION} at (0,0) size 70x60 [border: (3px solid #800080)]
    379           RenderText {#text} at (22,3) size 51x54
     379          RenderText {#text} at (22,3) size 50x54
    380380            text run at (22,3) width 26: "The"
    381381            text run at (15,21) width 40: "table's"

  • trunk/LayoutTests/platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt

    r162553 r167568  
    142142      RenderTable {TABLE} at (0,602) size 252x175 [border: (1px none #808080)]
    143143        RenderBlock {CAPTION} at (0,120) size 252x54
    144           RenderText {#text} at (11,0) size 239x54
     144          RenderText {#text} at (11,0) size 240x54
    145145            text run at (11,0) width 230: "Table 4 has this bottom caption. The"
    146146            text run at (6,18) width 240: "table has specified column widths and"

  • trunk/Source/WebCore/ChangeLog

    r167562 r167568  
     12014-04-20  Antti Koivisto  <antti@apple.com>
     2
     3        Text bounding box computation for simple line layout is wrong
     4        https://bugs.webkit.org/show_bug.cgi?id=131912
     5
     6        Reviewed by Andreas Kling.
     7
     8        Top-left is currently the first line top-left which is not always correct.
     9
     10        * WebCore.exp.in:
     11        * rendering/RenderText.cpp:
     12        (WebCore::RenderText::firstRunLocation):
     13        (WebCore::RenderText::firstRunOrigin): Deleted.
     14        (WebCore::RenderText::firstRunX): Deleted.
     15        (WebCore::RenderText::firstRunY): Deleted.
     16       
     17            Keep just one accessor and rename it.
     18            Encapsulate the line box and simple line versions.
     19
     20        * rendering/RenderText.h:
     21        * rendering/RenderTextLineBoxes.cpp:
     22        (WebCore::RenderTextLineBoxes::firstRunLocation):
     23
     24            Line box version.
     25
     26        * rendering/RenderTextLineBoxes.h:
     27        * rendering/RenderTreeAsText.cpp:
     28       
     29            Simplify RenderText dumping.
     30
     31        (WebCore::RenderTreeAsText::writeRenderObject):
     32        * rendering/SimpleLineLayoutFunctions.cpp:
     33        (WebCore::SimpleLineLayout::computeTextBoundingBox):
     34       
     35            Return the correct x position.
     36
     37        (WebCore::SimpleLineLayout::computeTextFirstRunLocation):
     38       
     39            Simple line version.
     40
     41        * rendering/SimpleLineLayoutFunctions.h:
     42        * rendering/svg/SVGRenderTreeAsText.cpp:
     43        (WebCore::writeSVGInlineText):
     44
    1452014-04-19  Zalan Bujtas  <zalan@apple.com>
    246

  • trunk/Source/WebCore/WebCore.exp.in

    r167553 r167568  
    14801480__ZNK7WebCore10FontGlyphs17realizeFontDataAtERKNS_15FontDescriptionEj
    14811481__ZNK7WebCore10PluginData16supportsMimeTypeERKN3WTF6StringENS0_18AllowedPluginTypesE
     1482__ZNK7WebCore10RenderText16firstRunLocationEv
    14821483__ZNK7WebCore10RenderText16linesBoundingBoxEv
    1483 __ZNK7WebCore10RenderText9firstRunXEv
    1484 __ZNK7WebCore10RenderText9firstRunYEv
    14851484__ZNK7WebCore10RenderView12documentRectEv
    14861485__ZNK7WebCore10RenderView20unscaledDocumentRectEv

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

    r167444 r167568  
    938938}
    939939
    940 FloatPoint RenderText::firstRunOrigin() const
    941 {
    942     return IntPoint(firstRunX(), firstRunY());
    943 }
    944 
    945 float RenderText::firstRunX() const
    946 {
    947     return firstTextBox() ? firstTextBox()->x() : 0;
    948 }
    949 
    950 float RenderText::firstRunY() const
    951 {
    952     return firstTextBox() ? firstTextBox()->y() : 0;
    953 }
    954    
     940IntPoint RenderText::firstRunLocation() const
     941{
     942    if (auto* layout = simpleLineLayout())
     943        return SimpleLineLayout::computeTextFirstRunLocation(*this, *layout);
     944
     945    return m_lineBoxes.firstRunLocation();
     946}
     947
    955948void RenderText::setSelectionState(SelectionState state)
    956949{

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

    r166120 r167568  
    100100    LayoutRect linesVisualOverflowBoundingBox() const;
    101101
    102     FloatPoint firstRunOrigin() const;
    103     float firstRunX() const;
    104     float firstRunY() const;
     102    IntPoint firstRunLocation() const;
    105103
    106104    virtual void setText(const String&, bool force = false);

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

    r164928 r167568  
    170170}
    171171
     172IntPoint RenderTextLineBoxes::firstRunLocation() const
     173{
     174    if (!m_first)
     175        return IntPoint();
     176    return IntPoint(m_first->topLeft());
     177}
     178
    172179LayoutRect RenderTextLineBoxes::visualOverflowBoundingBox(const RenderText& renderer) const
    173180{

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

    r160837 r167568  
    7171
    7272    IntRect boundingBox(const RenderText&) const;
     73    IntPoint firstRunLocation() const;
    7374    LayoutRect visualOverflowBoundingBox(const RenderText&) const;
    7475

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

    r166525 r167568  
    201201        // many test results.
    202202        const RenderText& text = toRenderText(o);
    203         IntRect linesBox = text.linesBoundingBox();
    204         if (text.simpleLineLayout()) {
    205             int y = linesBox.y();
    206             if (text.containingBlock()->isTableCell())
    207                 y -= toRenderTableCell(o.containingBlock())->intrinsicPaddingBefore();
    208             r = IntRect(linesBox.x(), y, linesBox.width(), linesBox.height());
    209         } else
    210             r = IntRect(text.firstRunX(), text.firstRunY(), linesBox.width(), linesBox.height());
    211         if (adjustForTableCells && !text.firstTextBox())
     203        r = IntRect(text.firstRunLocation(), text.linesBoundingBox().size());
     204        if (!text.firstTextBox() && !text.simpleLineLayout())
    212205            adjustForTableCells = false;
    213206    } else if (o.isBR()) {

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

    r166456 r167568  
    161161            bottom = rect.maxY();
    162162    }
    163     float x = firstLineRect.x();
     163    float x = left;
    164164    float y = firstLineRect.y();
    165165    float width = right - left;
    166166    float height = bottom - y;
    167167    return enclosingIntRect(FloatRect(x, y, width, height));
     168}
     169
     170IntPoint computeTextFirstRunLocation(const RenderText& textRenderer, const Layout& layout)
     171{
     172    auto resolver = runResolver(toRenderBlockFlow(*textRenderer.parent()), layout);
     173    auto begin = resolver.begin();
     174    if (begin == resolver.end())
     175        return IntPoint();
     176    return flooredIntPoint((*begin).rect().location());
    168177}
    169178

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

    r159345 r167568  
    5656unsigned findTextCaretMaximumOffset(const RenderText&, const Layout&);
    5757IntRect computeTextBoundingBox(const RenderText&, const Layout&);
     58IntPoint computeTextFirstRunLocation(const RenderText&, const Layout&);
    5859
    5960Vector<IntRect> collectTextAbsoluteRects(const RenderText&, const Layout&, const LayoutPoint& accumulatedOffset);

  • trunk/Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp

    r165676 r167568  
    583583{
    584584    writeStandardPrefix(ts, text, indent);
    585     ts << " " << enclosingIntRect(FloatRect(text.firstRunOrigin(), text.floatLinesBoundingBox().size())) << "\n";
     585    ts << " " << enclosingIntRect(FloatRect(text.firstRunLocation(), text.floatLinesBoundingBox().size())) << "\n";
    586586    writeResources(ts, text, indent);
    587587    writeSVGInlineTextBoxes(ts, text, indent);

  • trunk/Source/WebKit/mac/WebView/WebRenderNode.mm

    r165676 r167568  
    115115        // FIXME: Preserve old behavior even though it's strange.
    116116        RenderText* text = toRenderText(node);
    117         x = text->firstRunX();
    118         y = text->firstRunY();
     117        IntPoint firstRunLocation = text->firstRunLocation();
     118        x = firstRunLocation.x();
     119        y = firstRunLocation.y();
    119120        IntRect box = text->linesBoundingBox();
    120121        width = box.width();

  • trunk/Source/WebKit2/Shared/WebRenderObject.cpp

    r160608 r167568  
    9191    else if (renderer->isText()) {
    9292        m_frameRect = toRenderText(renderer)->linesBoundingBox();
    93         m_frameRect.setX(toRenderText(renderer)->firstRunX());
    94         m_frameRect.setY(toRenderText(renderer)->firstRunY());
     93        m_frameRect.setLocation(toRenderText(renderer)->firstRunLocation());
    9594    } else if (renderer->isRenderInline())
    9695        m_frameRect = toRenderBoxModelObject(renderer)->borderBoundingBox();
Note: See TracChangeset for help on using the changeset viewer.