Changeset 162947 in webkit

Timestamp:

Jan 28, 2014, 11:40:05 AM (11 years ago)

Author:

Antti Koivisto

Message:

REGRESSION(r162837): 5% regression on html5-full-render and 3% regression in DoYouEvenBench
​https://bugs.webkit.org/show_bug.cgi?id=127722

Reviewed by Darin Adler.

Accumulate repaint region in RendeView instead of flushing rects directly to the system.

• dom/Document.cpp:

(WebCore::Document::recalcStyle):
(WebCore::Document::updateLayout):
(WebCore::Document::topDocument):

Make less silly.

• page/FrameView.cpp:

(WebCore::FrameView::layout):

• rendering/RenderView.cpp:

(WebCore::RenderView::repaintViewRectangle):
(WebCore::RenderView::flushAccumulatedRepaintRegion):
(WebCore::RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator):
(WebCore::RenderView::RepaintRegionAccumulator::~RepaintRegionAccumulator):

• rendering/RenderView.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (3 diffs)
• page/FrameView.cpp (modified) (1 diff)
• rendering/RenderView.cpp (modified) (2 diffs)
• rendering/RenderView.h (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-01-28  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION(r162837): 5% regression on html5-full-render and 3% regression in DoYouEvenBench
+        https://bugs.webkit.org/show_bug.cgi?id=127722
+
+        Reviewed by Darin Adler.
+
+        Accumulate repaint region in RendeView instead of flushing rects directly to the system.
+
+        * dom/Document.cpp:
+        (WebCore::Document::recalcStyle):
+        (WebCore::Document::updateLayout):
+        (WebCore::Document::topDocument):
+        
+            Make less silly.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layout):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::repaintViewRectangle):
+        (WebCore::RenderView::flushAccumulatedRepaintRegion):
+        (WebCore::RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator):
+        (WebCore::RenderView::RepaintRegionAccumulator::~RepaintRegionAccumulator):
+        * rendering/RenderView.h:
+
 2014-01-28  Jeremy Jones  <jeremyj@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -1721 +1721 @@
         return; // Guard against re-entrancy. -dwh
 
+    RenderView::RepaintRegionAccumulator repaintRegionAccumulator(renderView());
+
     // FIXME: We should update style on our ancestor chain before proceeding (especially for seamless),
     // however doing so currently causes several tests to crash, as Frame::setDocument calls Document::attach
@@ -1811 +1813 @@
         return;
     }
+
+    RenderView::RepaintRegionAccumulator repaintRegionAccumulator(renderView());
 
     if (Element* oe = ownerElement())
@@ -4310 +4314 @@
 Document* Document::topDocument() const
 {
-    // FIXME: Why does this walk up owner elements instead using the frame tree as parentDocument does?
-    // The frame tree even has a top() function.
-    Document* document = const_cast<Document*>(this);
-    while (Element* element = document->ownerElement())
-        document = &element->document();
-    return document;
+    return m_frame ? m_frame->mainFrame().document() : const_cast<Document*>(this);
 }
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -1277 +1277 @@
         }
         LayoutStateDisabler layoutStateDisabler(disableLayoutState ? &root->view() : 0);
+        RenderView::RepaintRegionAccumulator repaintRegionAccumulator(&root->view());
 
         ASSERT(m_layoutPhase == InPreLayout);

trunk/Source/WebCore/rendering/RenderView.cpp

@@ -606 +606 @@
 }
 
-void RenderView::repaintViewRectangle(const LayoutRect& ur, bool immediate) const
-{
-    if (!shouldRepaint(ur))
-        return;
-
-    // We always just invalidate the root view, since we could be an iframe that is clipped out
-    // or even invisible.
-    Element* elt = document().ownerElement();
-    if (!elt)
-        frameView().repaintContentRectangle(pixelSnappedIntRect(ur), immediate);
-    else if (RenderBox* obj = elt->renderBox()) {
-        LayoutRect vr = viewRect();
+void RenderView::repaintViewRectangle(const LayoutRect& repaintRect, bool immediate) const
+{
+    // FIXME: Get rid of the 'immediate' argument. It only works on Mac WK1 and should never be used.
+    if (!shouldRepaint(repaintRect))
+        return;
+
+    if (auto ownerElement = document().ownerElement()) {
+        if (!ownerElement->renderer())
+            return;
+        auto& ownerBox = toRenderBox(*ownerElement->renderer());
+        LayoutRect viewRect = this->viewRect();
 #if PLATFORM(IOS)
         // Don't clip using the visible rect since clipping is handled at a higher level on iPhone.
-        LayoutRect r = ur;
+        LayoutRect adjustedRect = repaintRect;
 #else
-        LayoutRect r = intersection(ur, vr);
-#endif
-        
-        // Subtract out the contentsX and contentsY offsets to get our coords within the viewing
-        // rectangle.
-        r.moveBy(-vr.location());
-
-        // FIXME: Hardcoded offsets here are not good.
-        r.moveBy(obj->contentBoxRect().location());
-        obj->repaintRectangle(r, immediate);
-    }
+        LayoutRect adjustedRect = intersection(repaintRect, viewRect);
+#endif
+        adjustedRect.moveBy(-viewRect.location());
+        adjustedRect.moveBy(ownerBox.contentBoxRect().location());
+        ownerBox.repaintRectangle(adjustedRect, immediate);
+        return;
+    }
+
+    IntRect pixelSnappedRect = pixelSnappedIntRect(repaintRect);
+    if (!m_accumulatedRepaintRegion || immediate) {
+        frameView().repaintContentRectangle(pixelSnappedRect, immediate);
+        return;
+    }
+    m_accumulatedRepaintRegion->unite(pixelSnappedRect);
+}
+
+void RenderView::flushAccumulatedRepaintRegion() const
+{
+    ASSERT(!document().ownerElement());
+    ASSERT(m_accumulatedRepaintRegion);
+    auto repaintRects = m_accumulatedRepaintRegion->rects();
+    for (auto& rect : repaintRects)
+        frameView().repaintContentRectangle(rect, false);
+    m_accumulatedRepaintRegion = nullptr;
 }
 
@@ -1254 +1266 @@
 }
 
+RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator(RenderView* view)
+    : m_rootView(view ? view->document().topDocument()->renderView() : nullptr)
+{
+    if (!m_rootView)
+        return;
+    m_wasAccumulatingRepaintRegion = !!m_rootView->m_accumulatedRepaintRegion;
+    if (!m_wasAccumulatingRepaintRegion)
+        m_rootView->m_accumulatedRepaintRegion = std::make_unique<Region>();
+}
+
+RenderView::RepaintRegionAccumulator::~RepaintRegionAccumulator()
+{
+    if (!m_rootView)
+        return;
+    if (m_wasAccumulatingRepaintRegion)
+        return;
+    m_rootView->flushAccumulatedRepaintRegion();
+}
+
 } // namespace WebCore

trunk/Source/WebCore/rendering/RenderView.h

@@ -26 +26 @@
 #include "LayoutState.h"
 #include "PODFreeListArena.h"
+#include "Region.h"
 #include "RenderBlockFlow.h"
 #include <wtf/OwnPtr.h>
@@ -235 +236 @@
     void didDestroyRenderer() { --m_rendererCount; }
 
+    class RepaintRegionAccumulator {
+        WTF_MAKE_NONCOPYABLE(RepaintRegionAccumulator);
+    public:
+        RepaintRegionAccumulator(RenderView*);
+        ~RepaintRegionAccumulator();
+
+    private:
+        RenderView* m_rootView;
+        bool m_wasAccumulatingRepaintRegion;
+    };
+
 protected:
     virtual void mapLocalToContainer(const RenderLayerModelObject* repaintContainer, TransformState&, MapCoordinatesFlags = ApplyContainerFlip, bool* wasFixed = 0) const override;
@@ -248 +260 @@
 
     bool shouldRepaint(const LayoutRect&) const;
+    void flushAccumulatedRepaintRegion() const;
 
     // These functions may only be accessed by LayoutStateMaintainer.
@@ -303 +316 @@
 
     uint64_t m_rendererCount;
+
+    mutable std::unique_ptr<Region> m_accumulatedRepaintRegion;
 
     // FIXME: Only used by embedded WebViews inside AppKit NSViews.  Find a way to remove.