Changeset 99752 in webkit


Ignore:
Timestamp:
Nov 9, 2011 12:35:23 PM (12 years ago)
Author:
jchaffraix@webkit.org
Message:

REGRESSION (r93614): Content remains despite parent element being scrolled off page using javascript.
​https://bugs.webkit.org/show_bug.cgi?id=71550

Reviewed by Simon Fraser.

Source/WebCore:

Tests: fast/repaint/overflow-auto-in-overflow-auto-scrolled.html

fast/repaint/overflow-hidden-in-overflow-hidden-scrolled.html
fast/repaint/overflow-scroll-in-overflow-scroll-scrolled.html
fast/repaint/scroll-inside-table-cell.html
fast/repaint/scroll-relative-table-inside-table-cell.html
fast/repaint/table-overflow-auto-in-overflow-auto-scrolled.html
fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled.html
fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled.html

The issue was that updateLayerPositionsAfterScroll would not update some layer's
repaint rectangles. Thus we would not properly repaint (if at all).

The optimization added in 93614 was short-sighted and missed the fact that tables are a
special case when it comes to updating the repaint rectangles. When we scroll a layer
with an overflow clip, most layers *do* need to update their repaint rectangles.

This changes keeps the optimization for cells as this is the hotest case for tables
with overflow: hidden on cells but is much more conservative: if we encounter an
overflow clip, we update the descendant's repaint rectangles.

  • rendering/RenderLayer.cpp:

(WebCore::RenderLayer::RenderLayer):
(WebCore::RenderLayer::updateLayerPositionsAfterScroll):
Refactored the code to use some flags (HasSeenFixedPositionedAncestor and
HasSeenAncestorWithOverflowClip). Also added an exception if
m_canSkipRepaintRectsUpdateOnScroll is set.

  • rendering/RenderLayer.h:

Added a new field when it is fine to not update our repaint rects on scroll.

LayoutTests:

Added some repaint tests on the same idea (overflow: scroll / auto / hidden +/- table).

  • fast/repaint/overflow-auto-in-overflow-auto-scrolled-expected.txt: Added.
  • fast/repaint/overflow-auto-in-overflow-auto-scrolled.html: Added.
  • fast/repaint/overflow-hidden-in-overflow-hidden-scrolled-expected.png: Added.
  • fast/repaint/overflow-hidden-in-overflow-hidden-scrolled-expected.txt: Added.
  • fast/repaint/overflow-hidden-in-overflow-hidden-scrolled.html: Added.
  • fast/repaint/overflow-scroll-in-overflow-scroll-scrolled-expected.txt: Added.
  • fast/repaint/overflow-scroll-in-overflow-scroll-scrolled.html: Added.
  • fast/repaint/scroll-inside-table-cell-expected.txt: Added.
  • fast/repaint/scroll-inside-table-cell.html: Added.
  • fast/repaint/scroll-relative-table-inside-table-cell-expected.txt: Added.
  • fast/repaint/scroll-relative-table-inside-table-cell.html: Added.
  • fast/repaint/table-overflow-auto-in-overflow-auto-scrolled-expected.txt: Added.
  • fast/repaint/table-overflow-auto-in-overflow-auto-scrolled.html: Added.
  • fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled-expected.txt: Added.
  • fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled.html: Added.
  • fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled-expected.txt: Added.
  • fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled.html: Added.
  • platform/chromium-linux/fast/repaint/overflow-auto-in-overflow-auto-scrolled-expected.png: Added.
  • platform/chromium-linux/fast/repaint/overflow-scroll-in-overflow-scroll-scrolled-expected.png: Added.
  • platform/chromium-linux/fast/repaint/scroll-inside-table-cell-expected.png: Added.
  • platform/chromium-linux/fast/repaint/scroll-relative-table-inside-table-cell-expected.png: Added.
  • platform/chromium-linux/fast/repaint/table-overflow-auto-in-overflow-auto-scrolled-expected.png: Added.
  • platform/chromium-linux/fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled-expected.png: Added.
  • platform/chromium-linux/fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled-expected.png: Added.
Location:
trunk
Files:
24 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r99751 r99752  
     12011-11-09  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        REGRESSION (r93614): Content remains despite parent element being scrolled off page using javascript.
     4        https://bugs.webkit.org/show_bug.cgi?id=71550
     5
     6        Reviewed by Simon Fraser.
     7
     8        Added some repaint tests on the same idea (overflow: scroll / auto / hidden +/- table).
     9
     10        * fast/repaint/overflow-auto-in-overflow-auto-scrolled-expected.txt: Added.
     11        * fast/repaint/overflow-auto-in-overflow-auto-scrolled.html: Added.
     12        * fast/repaint/overflow-hidden-in-overflow-hidden-scrolled-expected.png: Added.
     13        * fast/repaint/overflow-hidden-in-overflow-hidden-scrolled-expected.txt: Added.
     14        * fast/repaint/overflow-hidden-in-overflow-hidden-scrolled.html: Added.
     15        * fast/repaint/overflow-scroll-in-overflow-scroll-scrolled-expected.txt: Added.
     16        * fast/repaint/overflow-scroll-in-overflow-scroll-scrolled.html: Added.
     17        * fast/repaint/scroll-inside-table-cell-expected.txt: Added.
     18        * fast/repaint/scroll-inside-table-cell.html: Added.
     19        * fast/repaint/scroll-relative-table-inside-table-cell-expected.txt: Added.
     20        * fast/repaint/scroll-relative-table-inside-table-cell.html: Added.
     21        * fast/repaint/table-overflow-auto-in-overflow-auto-scrolled-expected.txt: Added.
     22        * fast/repaint/table-overflow-auto-in-overflow-auto-scrolled.html: Added.
     23        * fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled-expected.txt: Added.
     24        * fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled.html: Added.
     25        * fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled-expected.txt: Added.
     26        * fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled.html: Added.
     27        * platform/chromium-linux/fast/repaint/overflow-auto-in-overflow-auto-scrolled-expected.png: Added.
     28        * platform/chromium-linux/fast/repaint/overflow-scroll-in-overflow-scroll-scrolled-expected.png: Added.
     29        * platform/chromium-linux/fast/repaint/scroll-inside-table-cell-expected.png: Added.
     30        * platform/chromium-linux/fast/repaint/scroll-relative-table-inside-table-cell-expected.png: Added.
     31        * platform/chromium-linux/fast/repaint/table-overflow-auto-in-overflow-auto-scrolled-expected.png: Added.
     32        * platform/chromium-linux/fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled-expected.png: Added.
     33        * platform/chromium-linux/fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled-expected.png: Added.
     34
    1352011-11-08  Andrew Scherkus  <scherkus@chromium.org>
    236

  • trunk/Source/WebCore/ChangeLog

    r99750 r99752  
     12011-11-09  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        REGRESSION (r93614): Content remains despite parent element being scrolled off page using javascript.
     4        https://bugs.webkit.org/show_bug.cgi?id=71550
     5
     6        Reviewed by Simon Fraser.
     7
     8        Tests: fast/repaint/overflow-auto-in-overflow-auto-scrolled.html
     9               fast/repaint/overflow-hidden-in-overflow-hidden-scrolled.html
     10               fast/repaint/overflow-scroll-in-overflow-scroll-scrolled.html
     11               fast/repaint/scroll-inside-table-cell.html
     12               fast/repaint/scroll-relative-table-inside-table-cell.html
     13               fast/repaint/table-overflow-auto-in-overflow-auto-scrolled.html
     14               fast/repaint/table-overflow-hidden-in-overflow-hidden-scrolled.html
     15               fast/repaint/table-overflow-scroll-in-overflow-scroll-scrolled.html
     16
     17        The issue was that updateLayerPositionsAfterScroll would not update some layer's
     18        repaint rectangles. Thus we would not properly repaint (if at all).
     19
     20        The optimization added in 93614 was short-sighted and missed the fact that tables are a
     21        special case when it comes to updating the repaint rectangles. When we scroll a layer
     22        with an overflow clip, most layers *do* need to update their repaint rectangles.
     23
     24        This changes keeps the optimization for cells as this is the hotest case for tables
     25        with overflow: hidden on cells but is much more conservative: if we encounter an
     26        overflow clip, we update the descendant's repaint rectangles.
     27
     28        * rendering/RenderLayer.cpp:
     29        (WebCore::RenderLayer::RenderLayer):
     30        (WebCore::RenderLayer::updateLayerPositionsAfterScroll):
     31        Refactored the code to use some flags (HasSeenFixedPositionedAncestor and
     32        HasSeenAncestorWithOverflowClip). Also added an exception if
     33        m_canSkipRepaintRectsUpdateOnScroll is set.
     34
     35        * rendering/RenderLayer.h:
     36        Added a new field when it is fine to not update our repaint rects on scroll.
     37
    1382011-11-09  Kentaro Hara  <haraken@chromium.org>
    239

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

    r99511 r99752  
    157157#endif
    158158    , m_containsDirtyOverlayScrollbars(false)
     159    , m_canSkipRepaintRectsUpdateOnScroll(renderer->isTableCell())
    159160    , m_renderer(renderer)
    160161    , m_parent(0)
    … …  
    424425}
    425426
    426 void RenderLayer::updateLayerPositionsAfterScroll(bool fixed)
     427void RenderLayer::updateLayerPositionsAfterScroll(UpdateLayerPositionsAfterScrollFlags flags)
    427428{
    428429    ASSERT(!m_visibleContentStatusDirty);
    … …  
    436437    updateLayerPosition();
    437438
    438     if (fixed || renderer()->style()->position() == FixedPosition) {
     439    if ((flags & HasSeenFixedPositionedAncestor) || renderer()->style()->position() == FixedPosition) {
    439440        // FIXME: Is it worth passing the offsetFromRoot around like in updateLayerPositions?
    440441        computeRepaintRects();
    441         fixed = true;
     442        flags |= HasSeenFixedPositionedAncestor;
    442443    } else if (renderer()->hasTransform() && !renderer()->isRenderView()) {
    443444        // Transforms act as fixed position containers, so nothing inside a
    … …  
    445446        // transformed element is not fixed itself or child of a fixed element.
    446447        return;
    447     }
     448    } else if ((flags & HasSeenAncestorWithOverflowClip) && !m_canSkipRepaintRectsUpdateOnScroll) {
     449        // If we have seen an overflow clip, we should update our repaint rects as clippedOverflowRectForRepaint
     450        // intersects it with our ancestor overflow clip that may have moved.
     451        computeRepaintRects();
     452    }
     453
     454    if (renderer()->hasOverflowClip())
     455        flags |= HasSeenAncestorWithOverflowClip;
    448456
    449457    for (RenderLayer* child = firstChild(); child; child = child->nextSibling())
    450         child->updateLayerPositionsAfterScroll(fixed);
     458        child->updateLayerPositionsAfterScroll(flags);
    451459
    452460    // We don't update our reflection as scrolling is a translation which does not change the size()

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

    r99511 r99752  
    469469    LayoutRect repaintRect() const { return m_repaintRect; }
    470470    LayoutRect repaintRectIncludingDescendants() const;
    471     void updateLayerPositionsAfterScroll(bool fixed = false);
     471
     472    enum UpdateLayerPositionsAfterScrollFlag {
     473        NoFlag = 0,
     474        HasSeenFixedPositionedAncestor = 1 << 0,
     475        HasSeenAncestorWithOverflowClip = 1 << 1
     476    };
     477
     478    typedef unsigned UpdateLayerPositionsAfterScrollFlags;
     479    void updateLayerPositionsAfterScroll(UpdateLayerPositionsAfterScrollFlags = NoFlag);
    472480    void setNeedsFullRepaint(bool f = true) { m_needsFullRepaint = f; }
    473481   
    … …  
    738746    bool m_containsDirtyOverlayScrollbars : 1;
    739747
     748    // This is an optimization added for <table>.
     749    // Currently cells do not need to update their repaint rectangles when scrolling. This also
     750    // saves a lot of time when scrolling on a table.
     751    bool m_canSkipRepaintRectsUpdateOnScroll : 1;
     752
    740753    RenderBoxModelObject* m_renderer;
    741754
Note: See TracChangeset for help on using the changeset viewer.