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

Changeset 99212 in webkit


Ignore:
Timestamp:
Nov 3, 2011, 10:20:01 AM (15 years ago)
Author:
jchaffraix@webkit.org
Message:

Stop abusing RenderTableSection::needsRecalcCells logic
​https://bugs.webkit.org/show_bug.cgi?id=71420

Reviewed by Darin Adler.

Source/WebCore:

Change covered by existing tests like fast/repaint/table-extra-bottom-grow.html
and fast/table/row-height-recalc* (among others).

Cell recalculation is very expensive and should only be called when the section's structure
changed in a way that requires a safe update to its structure (like removing a row as our
column split may not be appropriate anymore).

The current code would abuse cell recalculation to actually reset the logical height on the
RowStruct. This change makes it do the right thing.

  • rendering/RenderTableCell.h:
  • rendering/RenderTableRow.h:

Removed styleWillChange override as it was unneeded.

  • rendering/RenderTableCell.cpp:

(WebCore::RenderTableCell::styleDidChange):

  • rendering/RenderTableRow.cpp:

(WebCore::RenderTableRow::styleDidChange):
Move the code from styleWillChange to styleDidChange.

  • rendering/RenderTableSection.cpp:

(WebCore::RenderTableSection::rowLogicalHeightChanged):
This function just reset the height on the |RowStruct| which is the
only part of recalcCells that we would need.

(WebCore::RenderTableSection::rowIndexForRenderer):
Added this function to find out which index a column has (strangely
RenderTableRow does not have this information).

  • rendering/RenderTableSection.h: Added the 2 previous functions.

LayoutTests:

  • platform/chromium-linux/fast/repaint/table-extra-bottom-grow-expected.png:

Update this test as this is a progression: we are not over-repainting the table
anymore.

  • platform/chromium/test_expectations.txt:
  • platform/efl/Skipped:
  • platform/mac/Skipped:
  • platform/qt/test_expectations.txt:

Skipped the test here as it needs a rebaseline.

Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r99211 r99212  
     12011-11-03  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Stop abusing RenderTableSection::needsRecalcCells logic
     4        https://bugs.webkit.org/show_bug.cgi?id=71420
     5
     6        Reviewed by Darin Adler.
     7
     8        * platform/chromium-linux/fast/repaint/table-extra-bottom-grow-expected.png:
     9        Update this test as this is a progression: we are not over-repainting the table
     10        anymore.
     11
     12        * platform/chromium/test_expectations.txt:
     13        * platform/efl/Skipped:
     14        * platform/mac/Skipped:
     15        * platform/qt/test_expectations.txt:
     16        Skipped the test here as it needs a rebaseline.
     17
    1182011-11-03  Kentaro Hara  <haraken@chromium.org>
    219

  • trunk/LayoutTests/platform/chromium/test_expectations.txt

    r99191 r99212  
    38873887BUGWK71451 LINUX : http/tests/security/contentSecurityPolicy/policy-does-not-affect-child.html = TEXT PASS
    38883888BUGWK71451 LINUX : http/tests/security/contentSecurityPolicy/object-src-none-allowed.html = TEXT PASS
    3889  
     3889
     3890// This just needs a rebaseline.
     3891BUGWK71420 WIN : fast/repaint/table-extra-bottom-grow.html = IMAGE

  • trunk/LayoutTests/platform/efl/Skipped

    r99155 r99212  
    19241924# https://bugs.webkit.org/show_bug.cgi?id=64731
    19251925http/tests/media/media-source/
     1926
     1927# Needs a rebaseline
     1928fast/repaint/table-extra-bottom-grow.html

  • trunk/LayoutTests/platform/mac/Skipped

    r99131 r99212  
    468468fast/canvas/webgl/attrib-location-length-limits.html
    469469fast/canvas/webgl/uniform-location-length-limits.html
     470
     471# https://bugs.webkit.org/show_bug.cgi?id=71420
     472# It needs a rebaseline
     473fast/repaint/table-extra-bottom-grow.html

  • trunk/LayoutTests/platform/qt/test_expectations.txt

    r98984 r99212  
    2626
    2727BUGWK62662 DEBUG : inspector/cookie-parser.html = CRASH PASS
     28
     29// Needs a rebaseline
     30BUGWK71420 : fast/repaint/table-extra-bottom-grow.html = IMAGE

  • trunk/Source/WebCore/ChangeLog

    r99204 r99212  
     12011-11-03  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Stop abusing RenderTableSection::needsRecalcCells logic
     4        https://bugs.webkit.org/show_bug.cgi?id=71420
     5
     6        Reviewed by Darin Adler.
     7
     8        Change covered by existing tests like fast/repaint/table-extra-bottom-grow.html
     9        and fast/table/row-height-recalc* (among others).
     10
     11        Cell recalculation is very expensive and should only be called when the section's structure
     12        changed in a way that requires a safe update to its structure (like removing a row as our
     13        column split may not be appropriate anymore).
     14
     15        The current code would abuse cell recalculation to actually reset the logical height on the
     16        RowStruct. This change makes it do the right thing.
     17
     18        * rendering/RenderTableCell.h:
     19        * rendering/RenderTableRow.h:
     20        Removed styleWillChange override as it was unneeded.
     21
     22        * rendering/RenderTableCell.cpp:
     23        (WebCore::RenderTableCell::styleDidChange):
     24        * rendering/RenderTableRow.cpp:
     25        (WebCore::RenderTableRow::styleDidChange):
     26        Move the code from styleWillChange to styleDidChange.
     27
     28        * rendering/RenderTableSection.cpp:
     29        (WebCore::RenderTableSection::rowLogicalHeightChanged):
     30        This function just reset the height on the |RowStruct| which is the
     31        only part of recalcCells that we would need.
     32
     33        (WebCore::RenderTableSection::rowIndexForRenderer):
     34        Added this function to find out which index a column has (strangely
     35        RenderTableRow does not have this information).
     36
     37        * rendering/RenderTableSection.h: Added the 2 previous functions.
     38
    1392011-11-03  Andreas Kling  <kling@webkit.org>
    240

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

    r99020 r99212  
    311311}
    312312
    313 void RenderTableCell::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
    314 {
    315     if (parent() && section() && style() && style()->height() != newStyle->height())
    316         section()->setNeedsCellRecalc();
    317 
    318     ASSERT(newStyle->display() == TABLE_CELL);
    319 
    320     RenderBlock::styleWillChange(diff, newStyle);
    321 }
    322 
    323313void RenderTableCell::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
    324314{
     315    ASSERT(style()->display() == TABLE_CELL);
     316
    325317    RenderBlock::styleDidChange(diff, oldStyle);
    326318    setHasBoxDecorations(true);
     319
     320    if (parent() && section() && oldStyle && style()->height() != oldStyle->height())
     321        section()->rowLogicalHeightChanged(row());
    327322
    328323    // If border was changed, notify table.

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

    r99020 r99212  
    154154
    155155protected:
    156     virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle);
    157156    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
    158157

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

    r98767 r99212  
    5454}
    5555
    56 void RenderTableRow::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
    57 {
    58     if (section() && style() && style()->logicalHeight() != newStyle->logicalHeight())
    59         section()->setNeedsCellRecalc();
    60 
    61     ASSERT(newStyle->display() == TABLE_ROW);
    62 
    63     RenderBox::styleWillChange(diff, newStyle);
    64 }
    65 
    6656void RenderTableRow::updateBeforeAndAfterContent()
    6757{
    … …  
    7464void RenderTableRow::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
    7565{
     66    ASSERT(style()->display() == TABLE_ROW);
     67
    7668    RenderBox::styleDidChange(diff, oldStyle);
    7769    propagateStyleToAnonymousChildren();
    … …  
    7971    if (parent())
    8072        updateBeforeAndAfterContent();
     73
     74    if (section() && oldStyle && style()->logicalHeight() != oldStyle->logicalHeight())
     75        section()->rowLogicalHeightChanged(section()->rowIndexForRenderer(this));
    8176
    8277    // If border was changed, notify table.

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

    r98767 r99212  
    6464    virtual void imageChanged(WrappedImagePtr, const IntRect* = 0);
    6565
    66     virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle);
    6766    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
    6867

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

    r98997 r99212  
    11591159}
    11601160
     1161void RenderTableSection::rowLogicalHeightChanged(unsigned rowIndex)
     1162{
     1163    setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[rowIndex]);
     1164}
     1165
    11611166void RenderTableSection::setNeedsCellRecalc()
    11621167{
    … …  
    13011306}
    13021307
     1308unsigned RenderTableSection::rowIndexForRenderer(const RenderTableRow* row) const
     1309{
     1310    for (size_t i = 0; i < m_grid.size(); ++i) {
     1311        if (m_grid[i].rowRenderer == row)
     1312            return i;
     1313    }
     1314    ASSERT_NOT_REACHED();
     1315    return 0;
     1316}
     1317
    13031318} // namespace WebCore

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

    r98738 r99212  
    119119    LayoutUnit getBaseline(int row) { return m_grid[row].baseline; }
    120120
     121    void rowLogicalHeightChanged(unsigned rowIndex);
     122
     123    unsigned rowIndexForRenderer(const RenderTableRow*) const;
     124
    121125protected:
    122126    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
Note: See TracChangeset for help on using the changeset viewer.