Bug 1344971 - Part 2: Only modify mValidRegion if we actually invalidated something. r?mstange draft
authorMatt Woodrow <mwoodrow@mozilla.com>
Tue, 07 Mar 2017 14:07:32 +1300
changeset 494302 637e3ce6df84ea520f34b8a8d691fce5f792bf8b
parent 494301 be592d4e6902b9e1c587470b7fa1627a90a3215c
child 494303 6b0b18e0f7e22ef34c824e4d80349c42cd587049
push id48011
push usermwoodrow@mozilla.com
push dateTue, 07 Mar 2017 01:18:12 +0000
reviewersmstange
bugs1344971
milestone55.0a1
Bug 1344971 - Part 2: Only modify mValidRegion if we actually invalidated something. r?mstange MozReview-Commit-ID: EPTK3s3rnLM
gfx/layers/client/ClientPaintedLayer.h
gfx/layers/client/ClientTiledPaintedLayer.h
gfx/src/TiledRegion.cpp
gfx/src/TiledRegion.h
--- a/gfx/layers/client/ClientPaintedLayer.h
+++ b/gfx/layers/client/ClientPaintedLayer.h
@@ -55,18 +55,19 @@ public:
     NS_ASSERTION(ClientManager()->InConstruction(),
                  "Can only set properties in construction phase");
     PaintedLayer::SetVisibleRegion(aRegion);
   }
   virtual void InvalidateRegion(const nsIntRegion& aRegion) override
   {
     NS_ASSERTION(ClientManager()->InConstruction(),
                  "Can only set properties in construction phase");
-    mInvalidRegion.Add(aRegion);
-    mValidRegion.Sub(mValidRegion, mInvalidRegion.GetRegion());
+    if (mInvalidRegion.Add(aRegion)) {
+      mValidRegion.Sub(mValidRegion, mInvalidRegion.GetRegion());
+    }
   }
 
   virtual void RenderLayer() override { RenderLayerWithReadback(nullptr); }
 
   virtual void RenderLayerWithReadback(ReadbackProcessor *aReadback) override;
 
   virtual void ClearCachedResources() override
   {
--- a/gfx/layers/client/ClientTiledPaintedLayer.h
+++ b/gfx/layers/client/ClientTiledPaintedLayer.h
@@ -49,20 +49,21 @@ protected:
 
 public:
   // Override name to distinguish it from ClientPaintedLayer in layer dumps
   virtual const char* Name() const override { return "TiledPaintedLayer"; }
 
   // PaintedLayer
   virtual Layer* AsLayer() override { return this; }
   virtual void InvalidateRegion(const nsIntRegion& aRegion) override {
-    mInvalidRegion.Add(aRegion);
-    nsIntRegion invalidRegion = mInvalidRegion.GetRegion();
-    mValidRegion.Sub(mValidRegion, invalidRegion);
-    mLowPrecisionValidRegion.Sub(mLowPrecisionValidRegion, invalidRegion);
+    if (mInvalidRegion.Add(aRegion)) {
+      nsIntRegion invalidRegion = mInvalidRegion.GetRegion();
+      mValidRegion.Sub(mValidRegion, invalidRegion);
+      mLowPrecisionValidRegion.Sub(mLowPrecisionValidRegion, invalidRegion);
+    }
   }
 
   // Shadow methods
   virtual void FillSpecificAttributes(SpecificLayerAttributes& aAttrs) override;
   virtual ShadowableLayer* AsShadowableLayer() override { return this; }
 
   virtual void RenderLayer() override;
 
--- a/gfx/src/TiledRegion.cpp
+++ b/gfx/src/TiledRegion.cpp
@@ -273,46 +273,59 @@ UnionBoundsOfNonEmptyBoxes(const pixman_
                            const pixman_box32_t& aBox2)
 {
   return { std::min(aBox1.x1, aBox2.x1),
            std::min(aBox1.y1, aBox2.y1),
            std::max(aBox1.x2, aBox2.x2),
            std::max(aBox1.y2, aBox2.y2) };
 }
 
+bool operator!=(const pixman_box32_t& aBox1,
+                const pixman_box32_t& aBox2)
+{
+  return aBox1.x1 != aBox2.x1 ||
+         aBox1.y1 != aBox2.y1 ||
+         aBox1.x2 != aBox2.x2 ||
+         aBox1.y2 != aBox2.y2;
+}
+
 // Returns true when adding the rectangle was successful, and false if
 // allocation failed.
 // When this returns false, our internal state might not be consistent and we
 // need to be cleared.
 bool
-TiledRegionImpl::AddRect(const pixman_box32_t& aRect)
+TiledRegionImpl::AddRect(const pixman_box32_t& aRect, bool* aModified)
 {
   // We are adding a rectangle that can span multiple tiles.
   // For each empty tile that aRect intersects, we need to add the intersection
   // of aRect with that tile to mRects, respecting the order of mRects.
   // For each tile that already has a rectangle, we need to enlarge that
   // existing rectangle to include the intersection of aRect with the tile.
   return ProcessIntersectedTiles(aRect, mRects,
-    [&aRect](nsTArray<pixman_box32_t>& rects, size_t& rectIndex, TileRange emptyTiles) {
+    [&aRect, &aModified](nsTArray<pixman_box32_t>& rects, size_t& rectIndex, TileRange emptyTiles) {
       CheckedInt<size_t> newLength(rects.Length());
       newLength += emptyTiles.Length();
       if (!newLength.isValid() || newLength.value() >= kMaxTiles ||
           !rects.InsertElementsAt(rectIndex, emptyTiles.Length(), fallible)) {
         return IterationAction::STOP;
       }
+      *aModified = true;
       for (TileIterator tileIt = emptyTiles.Begin();
            tileIt != emptyTiles.End();
            ++tileIt, ++rectIndex) {
         rects[rectIndex] = tileIt.IntersectionWith(aRect);
       }
       return IterationAction::CONTINUE;
     },
-    [](nsTArray<pixman_box32_t>& rects, size_t rectIndex, const pixman_box32_t& rectIntersectionWithTile) {
-      rects[rectIndex] =
-        UnionBoundsOfNonEmptyBoxes(rects[rectIndex], rectIntersectionWithTile);
+    [&aModified](nsTArray<pixman_box32_t>& rects, size_t rectIndex, const pixman_box32_t& rectIntersectionWithTile) {
+      pixman_box32_t newBox = UnionBoundsOfNonEmptyBoxes(rects[rectIndex], rectIntersectionWithTile);
+      if (newBox != rects[rectIndex]) {
+        rects[rectIndex] = newBox;
+        *aModified = true;
+      }
       return IterationAction::CONTINUE;
     }) == IterationEndReason::NOT_STOPPED;
 }
 
 static bool
 NonEmptyBoxesIntersect(const pixman_box32_t& aBox1, const pixman_box32_t& aBox2)
 {
   return aBox1.x1 < aBox2.x2 && aBox2.x1 < aBox1.x2 &&
--- a/gfx/src/TiledRegion.h
+++ b/gfx/src/TiledRegion.h
@@ -15,17 +15,17 @@
 
 namespace mozilla {
 namespace gfx {
 
 // See TiledRegion.cpp for documentation on TiledRegionImpl.
 class TiledRegionImpl {
 public:
   void Clear() { mRects.Clear(); }
-  bool AddRect(const pixman_box32_t& aRect);
+  bool AddRect(const pixman_box32_t& aRect, bool* aModified);
   bool Intersects(const pixman_box32_t& aRect) const;
   bool Contains(const pixman_box32_t& aRect) const;
   operator ArrayView<pixman_box32_t>() const { return ArrayView<pixman_box32_t>(mRects); }
 
 private:
   nsTArray<pixman_box32_t> mRects;
 };
 
@@ -78,64 +78,69 @@ public:
     if (&aOther != this) {
       mBounds = aOther.mBounds;
       mImpl = aOther.mImpl;
       mCoversBounds = aOther.mCoversBounds;
     }
     return *this;
   }
 
-  void Add(const RectT& aRect)
+  bool Add(const RectT& aRect)
   {
     if (aRect.IsEmpty()) {
-      return;
+      return false;
     }
 
     Maybe<RectT> newBounds = mBounds.SafeUnion(aRect);
     if (!newBounds) {
-      return;
+      return false;
     }
     mBounds = newBounds.value();
     MOZ_ASSERT(!mBounds.Overflows());
 
     if (mCoversBounds) {
-      return;
+      return false;
     }
 
-    if (!mImpl.AddRect(RectToBox(aRect))) {
+    bool modified = false;
+    if (!mImpl.AddRect(RectToBox(aRect), &modified)) {
       FallBackToBounds();
+      return true;
     }
+    return modified;
   }
 
-  void Add(const RegionT& aRegion)
+  bool Add(const RegionT& aRegion)
   {
     Maybe<RectT> newBounds = mBounds.SafeUnion(aRegion.GetBounds());
     if (!newBounds) {
-      return;
+      return false;
     }
     mBounds = newBounds.value();
     MOZ_ASSERT(!mBounds.Overflows());
 
     if (mCoversBounds) {
-      return;
+      return false;
     }
 
+    bool modified = false;
     for (auto iter = aRegion.RectIter(); !iter.Done(); iter.Next()) {
       RectT r = iter.Get();
       if (r.IsEmpty() || r.Overflows()) {
         // This can happen if e.g. a negative-width rect was wrapped into a
         // region. Treat it the same as we would if such a rect was passed to
         // the Add(const RectT&) function.
         continue;
       }
-      if (!mImpl.AddRect(RectToBox(r))) {
+      if (!mImpl.AddRect(RectToBox(r), &modified)) {
         FallBackToBounds();
-        return;
+        return true;
       }
     }
+    return modified;
   }
 
   bool IsEmpty() const { return mBounds.IsEmpty(); }
 
   void SetEmpty()
   {
     mBounds.SetEmpty();
     mImpl.Clear();