Reviewed by Mitz.

        - 12.5% speedup on BenchJS test 6

        It turns out that calling documentVisibleRect on an NSScrollView is pretty expensive,
        and calling visibleRect even more so. Take measures to call them less often.

        * platform/mac/ScrollViewMac.mm:
        (WebCore::ScrollView::visibleContentRect): Use documentVisibleRect when possible.
        (WebCore::ScrollView::updateContents): Use visibleContentRect to be able to use
        documentVisibleRect when possible.
        * rendering/RenderView.cpp:
        (WebCore::RenderView::repaintViewRectangle): Don't get or intersect with viewRect
        if we don't have a parent frame, since the ScrollView will do that anyway. Also,
        don't get contentX and contentY separately since they are in the viewRect already.
        (WebCore::RenderView::viewRect): Use visibleContentRect instead of getting each
        coordinate individually, to avoid calling documentVisibleRect repeatedly.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@19828 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 30cc2664865e20f88cef10a3bd1fede9c8270f72..dc079e128e031241d519ee25322ac007f743d481 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2007-02-23  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Mitz.
+
+        - 12.5% speedup on BenchJS test 6
+        
+        It turns out that calling documentVisibleRect on an NSScrollView is pretty expensive,
+        and calling visibleRect even more so. Take measures to call them less often.
+
+        * platform/mac/ScrollViewMac.mm:
+        (WebCore::ScrollView::visibleContentRect): Use documentVisibleRect when possible.
+        (WebCore::ScrollView::updateContents): Use visibleContentRect to be able to use
+        documentVisibleRect when possible.
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::repaintViewRectangle): Don't get or intersect with viewRect
+        if we don't have a parent frame, since the ScrollView will do that anyway. Also,
+        don't get contentX and contentY separately since they are in the viewRect already.
+        (WebCore::RenderView::viewRect): Use visibleContentRect instead of getting each
+        coordinate individually, to avoid calling documentVisibleRect repeatedly.
+
 2007-02-23  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Mitz.

diff --git a/WebCore/platform/mac/ScrollViewMac.mm b/WebCore/platform/mac/ScrollViewMac.mm
index b8149ac330531d5cb9b9ce91adc6d2b806a27bff..3f08b6a2af7e5834bb27d2a4be1c826f255661bc 100644 (file)
--- a/WebCore/platform/mac/ScrollViewMac.mm
+++ b/WebCore/platform/mac/ScrollViewMac.mm
@@ -79,10 +79,15 @@ int ScrollView::visibleHeight() const
 
 FloatRect ScrollView::visibleContentRect() const
 {
+    NSScrollView *view = (NSScrollView *)getView();
+    
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
-    if (NSView *docView = getDocumentView())
-        return [docView visibleRect];
+    if ([view isKindOfClass:[NSScrollView class]])
+        return [view documentVisibleRect];
+    else
+        return [view visibleRect];
     END_BLOCK_OBJC_EXCEPTIONS;
+
     return FloatRect();
 }
 
@@ -330,14 +335,15 @@ void ScrollView::updateContents(const IntRect &rect, bool now)
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
 
     NSView *view = getView();
-
     if ([view isKindOfClass:[NSScrollView class]])
         view = getDocumentView();
 
+    NSRect visibleRect = visibleContentRect();
+
     // Checking for rect visibility is an important optimization for the case of
     // Select All of a large document. AppKit does not do this check, and so ends
     // up building a large complicated NSRegion if we don't perform the check.
-    NSRect dirtyRect = NSIntersectionRect(rect, [view visibleRect]);
+    NSRect dirtyRect = NSIntersectionRect(rect, visibleRect);
     if (!NSIsEmptyRect(dirtyRect)) {
         [view setNeedsDisplayInRect:dirtyRect];
         if (now) {

diff --git a/WebCore/rendering/RenderView.cpp b/WebCore/rendering/RenderView.cpp
index 683c1776816e23c19dfe5d1b091b69b9fa0d0390..61cd3144162f9470576748879f371363d07dd766 100644 (file)
--- a/WebCore/rendering/RenderView.cpp
+++ b/WebCore/rendering/RenderView.cpp
@@ -183,26 +183,28 @@ void RenderView::repaintViewRectangle(const IntRect& ur, bool immediate)
     if (printing() || ur.width() == 0 || ur.height() == 0)
         return;
 
-    IntRect vr = viewRect();
-    if (m_frameView && ur.intersects(vr)) {
-        // We always just invalidate the root view, since we could be an iframe that is clipped out
-        // or even invisible.
+    if (!m_frameView)
+        return;
+
+    // We always just invalidate the root view, since we could be an iframe that is clipped out
+    // or even invisible.
+    Element* elt = element()->document()->ownerElement();
+    if (!elt)
+        m_frameView->repaintRectangle(ur, immediate);
+    else if (RenderObject* obj = elt->renderer()) {
+        IntRect vr = viewRect();
         IntRect r = intersection(ur, vr);
-        Element* elt = element()->document()->ownerElement();
-        if (!elt)
-            m_frameView->repaintRectangle(r, immediate);
-        else if (RenderObject* obj = elt->renderer()) {
-            // Subtract out the contentsX and contentsY offsets to get our coords within the viewing
-            // rectangle.
-            r.move(-m_frameView->contentsX(), -m_frameView->contentsY());
-
-            // FIXME: Hardcoded offsets here are not good.
-            int yFrameOffset = m_frameView->hasBorder() ? 2 : 0;
-            int xFrameOffset = m_frameView->hasBorder() ? 1 : 0;
-            r.move(obj->borderLeft() + obj->paddingLeft() + xFrameOffset,
-                   obj->borderTop() + obj->paddingTop() + yFrameOffset);
-            obj->repaintRectangle(r, immediate);
-        }
+        
+        // Subtract out the contentsX and contentsY offsets to get our coords within the viewing
+        // rectangle.
+        r.move(-vr.x(), -vr.y());
+        
+        // FIXME: Hardcoded offsets here are not good.
+        int yFrameOffset = m_frameView->hasBorder() ? 2 : 0;
+        int xFrameOffset = m_frameView->hasBorder() ? 1 : 0;
+        r.move(obj->borderLeft() + obj->paddingLeft() + xFrameOffset,
+               obj->borderTop() + obj->paddingTop() + yFrameOffset);
+        obj->repaintRectangle(r, immediate);
     }
 }
 
@@ -464,10 +466,7 @@ IntRect RenderView::viewRect() const
     if (printing())
         return IntRect(0, 0, m_width, m_height);
     if (m_frameView)
-        return IntRect(m_frameView->contentsX(),
-                       m_frameView->contentsY(),
-                       m_frameView->visibleWidth(),
-                       m_frameView->visibleHeight());
+        return enclosingIntRect(m_frameView->visibleContentRect());
     return IntRect();
 }