Bug 1128769 (Part 2) - Check if we invalidated for a sync decode and never painted before invalidating for sync decoding again. r=tn a=lmandel
authorSeth Fowler <seth@mozilla.com>
Mon, 09 Feb 2015 23:27:39 -0800
changeset 250150 69da299d5e49
parent 250149 759ad062242f
child 250151 d80f4050e348
push id4514
push usermfowler@mozilla.com
push date2015-03-02 22:11 +0000
treeherdermozilla-beta@7b3c7ba30dfe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, lmandel
bugs1128769
milestone37.0
Bug 1128769 (Part 2) - Check if we invalidated for a sync decode and never painted before invalidating for sync decoding again. r=tn a=lmandel
layout/base/nsDisplayListInvalidation.cpp
layout/base/nsDisplayListInvalidation.h
layout/generic/nsBulletFrame.cpp
layout/generic/nsImageFrame.cpp
layout/xul/nsImageBoxFrame.cpp
--- a/layout/base/nsDisplayListInvalidation.cpp
+++ b/layout/base/nsDisplayListInvalidation.cpp
@@ -19,16 +19,22 @@ nsDisplayItemGeometry::~nsDisplayItemGeo
   MOZ_COUNT_DTOR(nsDisplayItemGeometry);
 }
 
 nsDisplayItemGenericGeometry::nsDisplayItemGenericGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder)
   : nsDisplayItemGeometry(aItem, aBuilder)
   , mBorderRect(aItem->GetBorderRect())
 {}
 
+bool
+ShouldSyncDecodeImages(nsDisplayListBuilder* aBuilder)
+{
+  return aBuilder->ShouldSyncDecodeImages();
+}
+
 void
 nsDisplayItemGenericGeometry::MoveBy(const nsPoint& aOffset)
 {
   nsDisplayItemGeometry::MoveBy(aOffset);
   mBorderRect.MoveBy(aOffset);
 }
 
 nsDisplayItemBoundsGeometry::nsDisplayItemBoundsGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder)
--- a/layout/base/nsDisplayListInvalidation.h
+++ b/layout/base/nsDisplayListInvalidation.h
@@ -68,70 +68,101 @@ class nsDisplayItemGenericGeometry : pub
 public:
   nsDisplayItemGenericGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder);
 
   virtual void MoveBy(const nsPoint& aOffset) MOZ_OVERRIDE;
 
   nsRect mBorderRect;
 };
 
+bool ShouldSyncDecodeImages(nsDisplayListBuilder* aBuilder);
+
 /**
  * nsImageGeometryMixin is a mixin for geometry items that draw images. Geometry
  * items that include this mixin can track drawing results and use that
  * information to inform invalidation decisions.
  *
  * This mixin uses CRTP; its template parameter should be the type of the class
  * that is inheriting from it. See nsDisplayItemGenericImageGeometry for an
  * example.
  */
 template <typename T>
 class nsImageGeometryMixin
 {
 public:
-  explicit nsImageGeometryMixin(nsDisplayItem* aItem)
+  nsImageGeometryMixin(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder)
     : mLastDrawResult(mozilla::image::DrawResult::NOT_READY)
+    , mWaitingForPaint(false)
   {
+    // Transfer state from the previous version of this geometry item.
     auto lastGeometry =
       static_cast<T*>(mozilla::FrameLayerBuilder::GetMostRecentGeometry(aItem));
     if (lastGeometry) {
-      mLastDrawResult = lastGeometry->LastDrawResult();
+      mLastDrawResult = lastGeometry->mLastDrawResult;
+      mWaitingForPaint = lastGeometry->mWaitingForPaint;
+    }
+
+    // If our display item is going to invalidate to trigger sync decoding of
+    // images, mark ourselves as waiting for a paint. If we actually get
+    // painted, UpdateDrawResult will get called, and we'll clear the flag.
+    if (ShouldSyncDecodeImages(aBuilder) &&
+        ShouldInvalidateToSyncDecodeImages()) {
+      mWaitingForPaint = true;
     }
   }
 
   static void UpdateDrawResult(nsDisplayItem* aItem,
                                mozilla::image::DrawResult aResult)
   {
     auto lastGeometry =
       static_cast<T*>(mozilla::FrameLayerBuilder::GetMostRecentGeometry(aItem));
     if (lastGeometry) {
       lastGeometry->mLastDrawResult = aResult;
+      lastGeometry->mWaitingForPaint = false;
     }
   }
 
-  mozilla::image::DrawResult LastDrawResult() const { return mLastDrawResult; }
+  bool ShouldInvalidateToSyncDecodeImages() const
+  {
+    if (mWaitingForPaint) {
+      // We previously invalidated for sync decoding and haven't gotten painted
+      // since them. This suggests that our display item is completely occluded
+      // and there's no point in invalidating again - and because the reftest
+      // harness takes a new snapshot every time we invalidate, doing so might
+      // lead to an invalidation loop if we're in a reftest.
+      return false;
+    }
+
+    if (mLastDrawResult == mozilla::image::DrawResult::SUCCESS) {
+      return false;
+    }
+
+    return true;
+  }
 
 private:
   mozilla::image::DrawResult mLastDrawResult;
+  bool mWaitingForPaint;
 };
 
 /**
  * nsDisplayItemGenericImageGeometry is a generic geometry item class that
  * includes nsImageGeometryMixin.
  *
  * This should be sufficient for most display items that draw images.
  */
 class nsDisplayItemGenericImageGeometry
   : public nsDisplayItemGenericGeometry
   , public nsImageGeometryMixin<nsDisplayItemGenericImageGeometry>
 {
 public:
   nsDisplayItemGenericImageGeometry(nsDisplayItem* aItem,
                                     nsDisplayListBuilder* aBuilder)
     : nsDisplayItemGenericGeometry(aItem, aBuilder)
-    , nsImageGeometryMixin(aItem)
+    , nsImageGeometryMixin(aItem, aBuilder)
   { }
 };
 
 class nsDisplayItemBoundsGeometry : public nsDisplayItemGeometry
 {
 public:
   nsDisplayItemBoundsGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder);
 
--- a/layout/generic/nsBulletFrame.cpp
+++ b/layout/generic/nsBulletFrame.cpp
@@ -172,17 +172,17 @@ nsBulletFrame::DidSetStyleContext(nsStyl
 
 class nsDisplayBulletGeometry
   : public nsDisplayItemGenericGeometry
   , public nsImageGeometryMixin<nsDisplayBulletGeometry>
 {
 public:
   nsDisplayBulletGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder)
     : nsDisplayItemGenericGeometry(aItem, aBuilder)
-    , nsImageGeometryMixin(aItem)
+    , nsImageGeometryMixin(aItem, aBuilder)
   {
     nsBulletFrame* f = static_cast<nsBulletFrame*>(aItem->Frame());
     mOrdinal = f->GetOrdinal();
   }
 
   int32_t mOrdinal;
 };
 
@@ -234,17 +234,17 @@ public:
     if (f->GetOrdinal() != geometry->mOrdinal) {
       bool snap;
       aInvalidRegion->Or(geometry->mBounds, GetBounds(aBuilder, &snap));
       return;
     }
 
     nsCOMPtr<imgIContainer> image = f->GetImage();
     if (aBuilder->ShouldSyncDecodeImages() && image &&
-        geometry->LastDrawResult() != DrawResult::SUCCESS) {
+        geometry->ShouldInvalidateToSyncDecodeImages()) {
       bool snap;
       aInvalidRegion->Or(*aInvalidRegion, GetBounds(aBuilder, &snap));
     }
 
     return nsDisplayItem::ComputeInvalidationRegion(aBuilder, aGeometry, aInvalidRegion);
   }
 };
 
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -1377,17 +1377,17 @@ void
 nsDisplayImage::ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
                                           const nsDisplayItemGeometry* aGeometry,
                                           nsRegion* aInvalidRegion)
 {
   auto geometry =
     static_cast<const nsDisplayItemGenericImageGeometry*>(aGeometry);
 
   if (aBuilder->ShouldSyncDecodeImages() &&
-      geometry->LastDrawResult() != DrawResult::SUCCESS) {
+      geometry->ShouldInvalidateToSyncDecodeImages()) {
     bool snap;
     aInvalidRegion->Or(*aInvalidRegion, GetBounds(aBuilder, &snap));
   }
 
   nsDisplayImageContainer::ComputeInvalidationRegion(aBuilder, aGeometry, aInvalidRegion);
 }
 
 already_AddRefed<ImageContainer>
--- a/layout/xul/nsImageBoxFrame.cpp
+++ b/layout/xul/nsImageBoxFrame.cpp
@@ -383,17 +383,17 @@ nsDisplayXULImage::ComputeInvalidationRe
                                              nsRegion* aInvalidRegion)
 {
   auto boxFrame = static_cast<nsImageBoxFrame*>(mFrame);
   auto geometry =
     static_cast<const nsDisplayItemGenericImageGeometry*>(aGeometry);
 
   if (aBuilder->ShouldSyncDecodeImages() &&
       boxFrame->mImageRequest &&
-      geometry->LastDrawResult() != DrawResult::SUCCESS) {
+      geometry->ShouldInvalidateToSyncDecodeImages()) {
       bool snap;
       aInvalidRegion->Or(*aInvalidRegion, GetBounds(aBuilder, &snap));
   }
 
   nsDisplayImageContainer::ComputeInvalidationRegion(aBuilder, aGeometry, aInvalidRegion);
 }
 
 void