Changeset 109273 in webkit

Timestamp:

Feb 29, 2012 3:10:06 PM (12 years ago)

Author:

Beth Dakin

Message:

​https://bugs.webkit.org/show_bug.cgi?id=79705
didNewFirstVisuallyNonEmptyLayout should be enhanced to look at size instead
of a raw tally
-and corresponding-
<rdar://problem/10821314>

Reviewed by Dave Hyatt.

Instead of firing didNewFirstVisuallyNonEmptyLayout() when a raw tally of
relevant painted objects has reached a port-defined threshold, this patch
looks at the size of those objects with respect to the view area. The patch
also looks at relevant objects that cannot yet be fully painted, such as
incrementally loading images.

We no longer need a HashSet for the relevant painted objects since Region can
do all of the heavy lifting. We now have a Region for the painted and
unpainted regions. We do need a HashSet for the unpainted objects though,
because we need to know if a painted object needs to be subtracted from the
unpainted region before being added to the painted region.

• page/Page.cpp:

(WebCore):
(WebCore::Page::isCountingRelevantRepaintedObjects):
(WebCore::Page::startCountingRelevantRepaintedObjects):
(WebCore::Page::resetRelevantPaintedObjectCounter):
(WebCore::Page::addRelevantRepaintedObject):
(WebCore::Page::addRelevantUnpaintedObject):

• page/Page.h:

(Page):

New function on Region iterates through the rects and calculates the total
area.

• platform/graphics/Region.cpp:

(WebCore::Region::totalArea):
(WebCore):

• platform/graphics/Region.h:

(Region):

Remove code from these classes since they are not actually relevant objects.

• rendering/InlineBox.cpp:

(WebCore::InlineBox::paint):

• rendering/RenderRegion.cpp:

(WebCore::RenderRegion::paintReplaced):

• rendering/RenderReplaced.cpp:

(WebCore::RenderReplaced::paint):

All of these other callers send a rect that actually represents their size
(usually the visualOverflowRect) instead of the paintInfo's paintRect, and
they call addRelevantUnpaintedObject when appropriate.

• rendering/InlineTextBox.cpp:

(WebCore::InlineTextBox::paint):

• rendering/RenderEmbeddedObject.cpp:

(WebCore::RenderEmbeddedObject::paint):
(WebCore::RenderEmbeddedObject::paintReplaced):

• rendering/RenderHTMLCanvas.cpp:

(WebCore::RenderHTMLCanvas::paintReplaced):

• rendering/RenderImage.cpp:

(WebCore::RenderImage::paintReplaced):

• rendering/RenderVideo.cpp:

(WebCore::RenderVideo::paintReplaced):

• rendering/svg/RenderSVGRoot.cpp:

(WebCore::RenderSVGRoot::paintReplaced):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/Page.cpp (modified) (3 diffs)
• page/Page.h (modified) (3 diffs)
• platform/graphics/Region.cpp (modified) (1 diff)
• platform/graphics/Region.h (modified) (1 diff)
• rendering/InlineBox.cpp (modified) (1 diff)
• rendering/InlineTextBox.cpp (modified) (1 diff)
• rendering/RenderEmbeddedObject.cpp (modified) (2 diffs)
• rendering/RenderHTMLCanvas.cpp (modified) (1 diff)
• rendering/RenderImage.cpp (modified) (2 diffs)
• rendering/RenderRegion.cpp (modified) (1 diff)
• rendering/RenderReplaced.cpp (modified) (1 diff)
• rendering/RenderVideo.cpp (modified) (1 diff)
• rendering/svg/RenderSVGRoot.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-02-29  Beth Dakin  <bdakin@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=79705
+        didNewFirstVisuallyNonEmptyLayout should be enhanced to look at size instead 
+        of a raw tally
+        -and corresponding-
+        <rdar://problem/10821314>
+
+        Reviewed by Dave Hyatt.
+
+        Instead of firing didNewFirstVisuallyNonEmptyLayout() when a raw tally of 
+        relevant painted objects has reached a port-defined threshold, this patch 
+        looks at the size of those objects with respect to the view area. The patch 
+        also looks at relevant objects that cannot yet be fully painted, such as 
+        incrementally loading images. 
+
+        We no longer need a HashSet for the relevant painted objects since Region can 
+        do all of the heavy lifting. We now have a Region for the painted and 
+        unpainted regions. We do need a HashSet for the unpainted objects though, 
+        because we need to know if a painted object needs to be subtracted from the 
+        unpainted region before being added to the painted region.
+        * page/Page.cpp:
+        (WebCore):
+        (WebCore::Page::isCountingRelevantRepaintedObjects):
+        (WebCore::Page::startCountingRelevantRepaintedObjects):
+        (WebCore::Page::resetRelevantPaintedObjectCounter):
+        (WebCore::Page::addRelevantRepaintedObject):
+        (WebCore::Page::addRelevantUnpaintedObject):
+        * page/Page.h:
+        (Page):
+
+        New function on Region iterates through the rects and calculates the total 
+        area.
+        * platform/graphics/Region.cpp:
+        (WebCore::Region::totalArea):
+        (WebCore):
+        * platform/graphics/Region.h:
+        (Region):
+
+        Remove code from these classes since they are not actually relevant objects.
+        * rendering/InlineBox.cpp:
+        (WebCore::InlineBox::paint):
+        * rendering/RenderRegion.cpp:
+        (WebCore::RenderRegion::paintReplaced):
+        * rendering/RenderReplaced.cpp:
+        (WebCore::RenderReplaced::paint):
+
+        All of these other callers send a rect that actually represents their size 
+        (usually the visualOverflowRect) instead of the paintInfo's paintRect, and 
+        they call addRelevantUnpaintedObject when appropriate.
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paint):
+        * rendering/RenderEmbeddedObject.cpp:
+        (WebCore::RenderEmbeddedObject::paint):
+        (WebCore::RenderEmbeddedObject::paintReplaced):
+        * rendering/RenderHTMLCanvas.cpp:
+        (WebCore::RenderHTMLCanvas::paintReplaced):
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::paintReplaced):
+        * rendering/RenderVideo.cpp:
+        (WebCore::RenderVideo::paintReplaced):
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::paintReplaced):
+
 2012-02-29  Joshua Bell  <jsbell@chromium.org>
 

trunk/Source/WebCore/page/Page.cpp

@@ -1010 +1010 @@
 #endif
 
+// FIXME: gPaintedObjectCounterThreshold is no longer used for calculating relevant repainted areas,
+// and it should be removed. For the time being, it is useful because it allows us to avoid doing
+// any of this work for ports that don't make sure of didNewFirstVisuallyNonEmptyLayout. We should
+// remove this when we resolve <rdar://problem/10791680> Need to merge didFirstVisuallyNonEmptyLayout 
+// and didNewFirstVisuallyNonEmptyLayout
 static uint64_t gPaintedObjectCounterThreshold = 0;
 
+// These are magical constants that might be tweaked over time.
+static double gMinimumPaintedAreaRatio = 0.1;
+static double gMaximumUnpaintedAreaRatio = 0.1;
+
 void Page::setRelevantRepaintedObjectsCounterThreshold(uint64_t threshold)
 {
@@ -1017 +1026 @@
 }
 
+bool Page::isCountingRelevantRepaintedObjects() const
+{
+    return m_isCountingRelevantRepaintedObjects && gPaintedObjectCounterThreshold > 0;
+}
+
 void Page::startCountingRelevantRepaintedObjects()
 {
     m_isCountingRelevantRepaintedObjects = true;
 
-    // Clear the HashSet in case we didn't hit the threshold last time.
-    m_relevantPaintedRenderObjects.clear();
+    // Reset everything in case we didn't hit the threshold last time.
+    resetRelevantPaintedObjectCounter();
+}
+
+void Page::resetRelevantPaintedObjectCounter()
+{
+    m_relevantUnpaintedRenderObjects.clear();
+    m_relevantPaintedRegion = Region();
+    m_relevantUnpaintedRegion = Region();
 }
 
 void Page::addRelevantRepaintedObject(RenderObject* object, const IntRect& objectPaintRect)
 {
-    if (!m_isCountingRelevantRepaintedObjects)
-        return;
-
-    // We don't need to do anything if there is no counter threshold.
-    if (!gPaintedObjectCounterThreshold)
+    if (!isCountingRelevantRepaintedObjects())
         return;
 
@@ -1040 +1057 @@
     }
 
-    m_relevantPaintedRenderObjects.add(object);
-
-    if (m_relevantPaintedRenderObjects.size() == static_cast<int>(gPaintedObjectCounterThreshold)) {
+    // If this object was previously counted as an unpainted object, remove it from that HashSet
+    // and corresponding Region. FIXME: This doesn't do the right thing if the objects overlap.
+    if (m_relevantUnpaintedRenderObjects.contains(object)) {
+        m_relevantUnpaintedRenderObjects.remove(object);
+        m_relevantUnpaintedRegion.subtract(objectPaintRect);
+    }
+
+    m_relevantPaintedRegion.unite(objectPaintRect);
+
+    RenderView* view = object->view();
+    if (!view)
+        return;
+    
+    float viewArea = view->viewRect().width() * view->viewRect().height();
+    float ratioOfViewThatIsPainted = m_relevantPaintedRegion.totalArea() / viewArea;
+    float ratioOfViewThatIsUnpainted = m_relevantUnpaintedRegion.totalArea() / viewArea;
+
+    if (ratioOfViewThatIsPainted > gMinimumPaintedAreaRatio && ratioOfViewThatIsUnpainted < gMaximumUnpaintedAreaRatio) {
         m_isCountingRelevantRepaintedObjects = false;
-        m_relevantPaintedRenderObjects.clear();
+        resetRelevantPaintedObjectCounter();
         if (Frame* frame = mainFrame())
             frame->loader()->didNewFirstVisuallyNonEmptyLayout();
     }
+}
+
+void Page::addRelevantUnpaintedObject(RenderObject* object, const IntRect& objectPaintRect)
+{
+    if (!isCountingRelevantRepaintedObjects())
+        return;
+
+    // The objects are only relevant if they are being painted within the viewRect().
+    if (RenderView* view = object->view()) {
+        if (!objectPaintRect.intersects(pixelSnappedIntRect(view->viewRect())))
+            return;
+    }
+
+    m_relevantUnpaintedRenderObjects.add(object);
+    m_relevantUnpaintedRegion.unite(objectPaintRect);
 }
 

trunk/Source/WebCore/page/Page.h

@@ -28 +28 @@
 #include "PlatformScreen.h"
 #include "PlatformString.h"
+#include "Region.h"
 #include "Supplementable.h"
 #include "ViewportArguments.h"
@@ -321 +322 @@
         PlatformDisplayID displayID() const { return m_displayID; }
 
+        bool isCountingRelevantRepaintedObjects() const;
         void setRelevantRepaintedObjectsCounterThreshold(uint64_t);
         void startCountingRelevantRepaintedObjects();
+        void resetRelevantPaintedObjectCounter();
         void addRelevantRepaintedObject(RenderObject*, const IntRect& objectPaintRect);
+        void addRelevantUnpaintedObject(RenderObject*, const IntRect& objectPaintRect);
 
     private:
@@ -419 +423 @@
         PlatformDisplayID m_displayID;
 
-        HashSet<RenderObject*> m_relevantPaintedRenderObjects;
+        HashSet<RenderObject*> m_relevantUnpaintedRenderObjects;
+        Region m_relevantPaintedRegion;
+        Region m_relevantUnpaintedRegion;
         bool m_isCountingRelevantRepaintedObjects;
     };

trunk/Source/WebCore/platform/graphics/Region.cpp

@@ -78 +78 @@
 }
 
+unsigned Region::totalArea() const
+{
+    Vector<IntRect> rects = this->rects();
+    size_t size = rects.size();
+    unsigned totalArea = 0;
+
+    for (size_t i = 0; i < size; ++i) {
+        IntRect rect = rects[i];
+        totalArea += (rect.width() * rect.height());
+    }
+
+    return totalArea;
+}
+
 Region::Shape::Shape()
 {

trunk/Source/WebCore/platform/graphics/Region.h

@@ -52 +52 @@
 
     bool contains(const IntPoint&) const;
+
+    unsigned totalArea() const;
 
 #ifndef NDEBUG

trunk/Source/WebCore/rendering/InlineBox.cpp

@@ -214 +214 @@
         return;
 
-    if (Frame* frame = renderer()->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(renderer(), paintInfo.rect);
-    }
-
     LayoutPoint childPoint = paintOffset;
     if (parent()->renderer()->style()->isFlippedBlocksWritingMode()) // Faster than calling containingBlock().

trunk/Source/WebCore/rendering/InlineTextBox.cpp

@@ -498 +498 @@
 
     if (Frame* frame = renderer()->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(renderer(), paintInfo.rect);
+        if (Page* page = frame->page()) {
+            // FIXME: Right now, InlineTextBoxes never call addRelevantUnpaintedObject() even though they might 
+            // legitimately be unpainted if they are waiting on a slow-loading web font. We should fix that, and
+            // when we do, we will have to account for the fact the InlineTextBoxes do not always have unique
+            // renderers and Page currently relies on each unpainted object having a unique renderer.
+            if (paintInfo.phase == PaintPhaseForeground)
+                page->addRelevantRepaintedObject(renderer(), IntRect(adjustedPaintOffset.x(), adjustedPaintOffset.y(), logicalWidth(), logicalHeight()));
+        }
     }
 

trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp

@@ -140 +140 @@
 void RenderEmbeddedObject::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
+    Page* page = 0;
+    if (Frame* frame = this->frame())
+        page = frame->page();
+
     if (pluginCrashedOrWasMissing()) {
+        if (page && paintInfo.phase == PaintPhaseForeground)
+            page->addRelevantUnpaintedObject(this, visualOverflowRect());
         RenderReplaced::paint(paintInfo, paintOffset);
         return;
     }
-    
+
+    if (page && paintInfo.phase == PaintPhaseForeground)
+        page->addRelevantRepaintedObject(this, visualOverflowRect());
+
     RenderPart::paint(paintInfo, paintOffset);
 }
@@ -168 +177 @@
     if (!getReplacementTextGeometry(paintOffset, contentRect, path, replacementTextRect, font, run, textWidth))
         return;
-
-    if (Frame* frame = this->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(this, paintInfo.rect);
-    }
     
     GraphicsContextStateSaver stateSaver(*context);

trunk/Source/WebCore/rendering/RenderHTMLCanvas.cpp

@@ -59 +59 @@
 void RenderHTMLCanvas::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
+    LayoutRect rect = contentBoxRect();
+    rect.moveBy(paintOffset);
+
     if (Frame* frame = this->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(this, paintInfo.rect);
+        if (Page* page = frame->page()) {
+            if (paintInfo.phase == PaintPhaseForeground)
+                page->addRelevantRepaintedObject(this, rect);
+        }
     }
 
-    LayoutRect rect = contentBoxRect();
-    rect.moveBy(paintOffset);
     bool useLowQualityScale = style()->imageRendering() == ImageRenderingOptimizeContrast;
     static_cast<HTMLCanvasElement*>(node())->paint(paintInfo.context, rect, useLowQualityScale);

trunk/Source/WebCore/rendering/RenderImage.cpp

@@ -265 +265 @@
     GraphicsContext* context = paintInfo.context;
 
+    Page* page = 0;
+    if (Frame* frame = this->frame())
+        page = frame->page();
+
     if (!m_imageResource->hasImage() || m_imageResource->errorOccurred()) {
         if (paintInfo.phase == PaintPhaseSelection)
             return;
+
+        if (page && paintInfo.phase == PaintPhaseForeground)
+            page->addRelevantUnpaintedObject(this, visualOverflowRect());
 
         if (cWidth > 2 && cHeight > 2) {
@@ -326 +333 @@
     } else if (m_imageResource->hasImage() && cWidth > 0 && cHeight > 0) {
         RefPtr<Image> img = m_imageResource->image(cWidth, cHeight);
-        if (!img || img->isNull())
+        if (!img || img->isNull()) {
+            if (page && paintInfo.phase == PaintPhaseForeground)
+                page->addRelevantUnpaintedObject(this, visualOverflowRect());
             return;
-
-    if (Frame* frame = this->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(this, paintInfo.rect);
+        }
+
+    if (page && paintInfo.phase == PaintPhaseForeground) {
+        // For now, count images as unpainted if they are still progressively loading. We may want 
+        // to refine this in the future to account for the portion of the image that has painted.
+        if (cachedImage()->isLoading())
+            page->addRelevantUnpaintedObject(this, visualOverflowRect());
+        else
+            page->addRelevantRepaintedObject(this, visualOverflowRect());
     }
 

trunk/Source/WebCore/rendering/RenderRegion.cpp

@@ -140 +140 @@
         return;
 
-    if (Frame* frame = this->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(this, paintInfo.rect);
-    }
-
     setRegionBoxesRegionStyle();
     m_flowThread->paintIntoRegion(paintInfo, this, LayoutPoint(paintOffset.x() + borderLeft() + paddingLeft(), paintOffset.y() + borderTop() + paddingTop()));

trunk/Source/WebCore/rendering/RenderReplaced.cpp

@@ -138 +138 @@
     }
 
-    if (Frame* frame = this->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(this, paintInfo.rect);
-    }
-
     bool completelyClippedOut = false;
     if (style()->hasBorderRadius()) {

trunk/Source/WebCore/rendering/RenderVideo.cpp

@@ -199 +199 @@
     bool displayingPoster = videoElement()->shouldDisplayPosterImage();
 
+    Page* page = 0;
+    if (Frame* frame = this->frame())
+        page = frame->page();
+
     if (!displayingPoster) {
-        if (!mediaPlayer)
+        if (!mediaPlayer) {
+            if (page && paintInfo.phase == PaintPhaseForeground)
+                page->addRelevantUnpaintedObject(this, visualOverflowRect());
             return;
+        }
         updatePlayer();
     }
 
     LayoutRect rect = videoBox();
-    if (rect.isEmpty())
-        return;
+    if (rect.isEmpty()) {
+        if (page && paintInfo.phase == PaintPhaseForeground)
+            page->addRelevantUnpaintedObject(this, visualOverflowRect());
+        return;
+    }
     rect.moveBy(paintOffset);
 
-    if (Frame* frame = this->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(this, paintInfo.rect);
-    }
+    if (page && paintInfo.phase == PaintPhaseForeground)
+        page->addRelevantRepaintedObject(this, visualOverflowRect());
 
     if (displayingPoster)

trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp

@@ -250 +250 @@
         return;
 
+    Page* page = 0;
+    if (Frame* frame = this->frame())
+        page = frame->page();
+
     // Don't paint if we don't have kids, except if we have filters we should paint those.
     if (!firstChild()) {
         SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
-        if (!resources || !resources->filter())
+        if (!resources || !resources->filter()) {
+            if (page && paintInfo.phase == PaintPhaseForeground)
+                page->addRelevantUnpaintedObject(this, visualOverflowRect());
             return;
-    }
-
-    if (Frame* frame = this->frame()) {
-        if (Page* page = frame->page())
-            page->addRelevantRepaintedObject(this, paintInfo.rect);
-    }
+        }
+    }
+
+    if (page && paintInfo.phase == PaintPhaseForeground)
+        page->addRelevantRepaintedObject(this, visualOverflowRect());
 
     // Make a copy of the PaintInfo because applyTransform will modify the damage rect.