Bug 1368386 - Make PaintedLayer::mValidRegion private so that its access can be tightly controlled. r=mattwoodrow

MozReview-Commit-ID: LdtH3hrXT5v

--HG--
extra : rebase_source : c1eff1ba44f9a762cadb028113576bbffee93715

gfx/layers/Layers.cpp

@@ -2101,8 +2101,9 @@ void
 PaintedLayer::PrintInfo(std::stringstream& aStream, const char* aPrefix)
 {
   Layer::PrintInfo(aStream, aPrefix);
-  if (!mValidRegion.IsEmpty()) {
-    AppendToString(aStream, mValidRegion, " [valid=", "]");
+  nsIntRegion validRegion = GetValidRegion();
+  if (!validRegion.IsEmpty()) {
+    AppendToString(aStream, validRegion, " [valid=", "]");
   }
 }
 
@@ -2114,8 +2115,9 @@ PaintedLayer::DumpPacket(layerscope::LayersPacket* aPacket, const void* aParent)
   using namespace layerscope;
   LayersPacket::Layer* layer = aPacket->mutable_layer(aPacket->layer_size()-1);
   layer->set_type(LayersPacket::Layer::PaintedLayer);
-  if (!mValidRegion.IsEmpty()) {
-    DumpRegion(layer->mutable_valid(), mValidRegion);
+  nsIntRegion validRegion = GetValidRegion();
+  if (!validRegion.IsEmpty()) {
+    DumpRegion(layer->mutable_valid(), validRegion);
   }
 }
 

gfx/layers/Layers.h

@@ -2010,6 +2010,20 @@ public:
     InvalidateRegion(GetValidRegion().GetBounds());
   }
 
+  void ClearValidRegion() { mValidRegion.SetEmpty(); }
+  void AddToValidRegion(const nsIntRegion& aRegion)
+  {
+    mValidRegion.OrWith(aRegion);
+  }
+  void SubtractFromValidRegion(const nsIntRegion& aRegion)
+  {
+    mValidRegion.SubOut(aRegion);
+  }
+  void UpdateValidRegionAfterInvalidRegionChanged()
+  {
+    SubtractFromValidRegion(mInvalidRegion.GetRegion());
+  }
+
   virtual PaintedLayer* AsPaintedLayer() override { return this; }
 
   MOZ_LAYER_DECL_NAME("PaintedLayer", TYPE_PAINTED)
@@ -2036,7 +2050,7 @@ public:
       NS_ASSERTION(-0.5 <= (&transformed)->x && (&transformed)->x < 0.5 &&
                    -0.5 <= (&transformed)->y && (&transformed)->y < 0.5,
                    "Residual translation out of range");
-      mValidRegion.SetEmpty();
+      ClearValidRegion();
     }
     ComputeEffectiveTransformForMaskLayers(aTransformToSurface);
   }
@@ -2083,7 +2097,11 @@ protected:
    * mEffectiveTransform to get the ideal transform.
    */
   gfxPoint mResidualTranslation;
+
+private:
   nsIntRegion mValidRegion;
+
+protected:
   /**
    * The creation hint that was used when constructing this layer.
    */

gfx/layers/basic/BasicPaintedLayer.cpp

@@ -58,7 +58,7 @@ BasicPaintedLayer::PaintThebes(gfxContext* aContext,
   CompositionOp effectiveOperator = GetEffectiveOperator(this);
 
   if (!BasicManager()->IsRetained()) {
-    mValidRegion.SetEmpty();
+    ClearValidRegion();
     mContentClient->Clear();
 
     nsIntRegion toDraw = IntersectWithClip(GetLocalVisibleRegion().ToUnknownRegion(), aContext);
@@ -164,7 +164,7 @@ BasicPaintedLayer::Validate(LayerManager::DrawPaintedLayerCallback aCallback,
   }
   PaintState state =
     mContentClient->BeginPaintBuffer(this, flags);
-  mValidRegion.Sub(mValidRegion, state.mRegionToInvalidate);
+  SubtractFromValidRegion(state.mRegionToInvalidate);
 
   DrawTarget* target = mContentClient->BorrowDrawTargetForPainting(state);
   if (target && target->IsValid()) {

gfx/layers/basic/BasicPaintedLayer.h

@@ -56,7 +56,7 @@ public:
     NS_ASSERTION(BasicManager()->InConstruction(),
                  "Can only set properties in construction phase");
     mInvalidRegion.Add(aRegion);
-    mValidRegion.Sub(mValidRegion, mInvalidRegion.GetRegion());
+    UpdateValidRegionAfterInvalidRegionChanged();
   }
 
   virtual void PaintThebes(gfxContext* aContext,
@@ -73,7 +73,7 @@ public:
     if (mContentClient) {
       mContentClient->Clear();
     }
-    mValidRegion.SetEmpty();
+    ClearValidRegion();
   }
 
   virtual void ComputeEffectiveTransforms(const gfx::Matrix4x4& aTransformToSurface) override
@@ -84,7 +84,7 @@ public:
       mEffectiveTransform = GetLocalTransform() * aTransformToSurface;
       if (gfxPoint(0,0) != mResidualTranslation) {
         mResidualTranslation = gfxPoint(0,0);
-        mValidRegion.SetEmpty();
+        ClearValidRegion();
       }
       ComputeEffectiveTransformForMaskLayers(aTransformToSurface);
       return;
@@ -120,7 +120,7 @@ protected:
     // representation of a region.)
     nsIntRegion tmp;
     tmp.Or(mVisibleRegion.ToUnknownRegion(), aExtendedRegionToDraw);
-    mValidRegion.Or(mValidRegion, tmp);
+    AddToValidRegion(tmp);
   }
 
   RefPtr<ContentClientBasic> mContentClient;

gfx/layers/client/ClientPaintedLayer.cpp

@@ -53,7 +53,7 @@ ClientPaintedLayer::PaintThebes(nsTArray<ReadbackProcessor::Update>* aReadbackUp
 #endif
   PaintState state =
     mContentClient->BeginPaintBuffer(this, flags);
-  mValidRegion.Sub(mValidRegion, state.mRegionToInvalidate);
+  SubtractFromValidRegion(state.mRegionToInvalidate);
 
   if (!state.mRegionToDraw.IsEmpty() && !ClientManager()->GetPaintedLayerCallback()) {
     ClientManager()->SetTransactionIncomplete();
@@ -101,7 +101,7 @@ ClientPaintedLayer::PaintThebes(nsTArray<ReadbackProcessor::Update>* aReadbackUp
   if (didUpdate) {
     Mutated();
 
-    mValidRegion.Or(mValidRegion, state.mRegionToDraw);
+    AddToValidRegion(state.mRegionToDraw);
 
     ContentClientRemote* contentClientRemote = static_cast<ContentClientRemote*>(mContentClient.get());
     MOZ_ASSERT(contentClientRemote->GetIPCHandle());

gfx/layers/client/ClientPaintedLayer.h

@@ -61,7 +61,7 @@ public:
     NS_ASSERTION(ClientManager()->InConstruction(),
                  "Can only set properties in construction phase");
     mInvalidRegion.Add(aRegion);
-    mValidRegion.Sub(mValidRegion, mInvalidRegion.GetRegion());
+    UpdateValidRegionAfterInvalidRegionChanged();
   }
 
   virtual void RenderLayer() override { RenderLayerWithReadback(nullptr); }
@@ -73,7 +73,7 @@ public:
     if (mContentClient) {
       mContentClient->Clear();
     }
-    mValidRegion.SetEmpty();
+    ClearValidRegion();
     DestroyBackBuffer();
   }
 

gfx/layers/client/ClientTiledPaintedLayer.cpp

@@ -51,7 +51,7 @@ ClientTiledPaintedLayer::ClearCachedResources()
   if (mContentClient) {
     mContentClient->ClearCachedResources();
   }
-  mValidRegion.SetEmpty();
+  ClearValidRegion();
   mContentClient = nullptr;
 }
 
@@ -324,25 +324,26 @@ ClientTiledPaintedLayer::RenderHighPrecision(const nsIntRegion& aInvalidRegion,
 
     nsIntRegion drawnRegion;
     bool updatedBuffer =
-      mContentClient->GetTiledBuffer()->ProgressiveUpdate(mValidRegion, aInvalidRegion,
+      mContentClient->GetTiledBuffer()->ProgressiveUpdate(GetValidRegion(), aInvalidRegion,
                       oldValidRegion, drawnRegion, &mPaintData, aCallback, aCallbackData);
-    mValidRegion.OrWith(drawnRegion);
+    AddToValidRegion(drawnRegion);
     return updatedBuffer;
   }
 
   // Otherwise do a non-progressive paint. We must do this even when
   // the region to paint is empty as the valid region may have shrunk.
 
-  mValidRegion = aVisibleRegion;
+  nsIntRegion validRegion = aVisibleRegion;
   if (mPaintData.mCriticalDisplayPort) {
-    mValidRegion.And(mValidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
+    validRegion.AndWith(mPaintData.mCriticalDisplayPort->ToUnknownRect());
   }
+  SetValidRegion(validRegion);
 
   TILING_LOG("TILING %p: Non-progressive paint invalid region %s\n", this, Stringify(aInvalidRegion).c_str());
-  TILING_LOG("TILING %p: Non-progressive paint new valid region %s\n", this, Stringify(mValidRegion).c_str());
+  TILING_LOG("TILING %p: Non-progressive paint new valid region %s\n", this, Stringify(GetValidRegion()).c_str());
 
   mContentClient->GetTiledBuffer()->SetFrameResolution(mPaintData.mResolution);
-  mContentClient->GetTiledBuffer()->PaintThebes(mValidRegion, aInvalidRegion, aInvalidRegion,
+  mContentClient->GetTiledBuffer()->PaintThebes(GetValidRegion(), aInvalidRegion, aInvalidRegion,
                                                 aCallback, aCallbackData);
   mPaintData.mPaintFinished = true;
   return true;
@@ -386,7 +387,7 @@ ClientTiledPaintedLayer::RenderLowPrecision(const nsIntRegion& aInvalidRegion,
 
     // Remove the valid high-precision region from the invalid low-precision
     // region. We don't want to spend time drawing things twice.
-    invalidRegion.SubOut(mValidRegion);
+    invalidRegion.SubOut(GetValidRegion());
 
     TILING_LOG("TILING %p: Progressive paint: low-precision invalid region is %s\n", this, Stringify(invalidRegion).c_str());
     TILING_LOG("TILING %p: Progressive paint: low-precision old valid region is %s\n", this, Stringify(oldValidRegion).c_str());
@@ -448,7 +449,7 @@ ClientTiledPaintedLayer::RenderLayer()
 
   if (mContentClient && mHaveSingleTiledContentClient && !wantSingleTiledContentClient) {
     mContentClient = nullptr;
-    mValidRegion.SetEmpty();
+    ClearValidRegion();
   }
 
   if (!mContentClient) {
@@ -466,12 +467,12 @@ ClientTiledPaintedLayer::RenderLayer()
   }
 
   if (mContentClient->GetTiledBuffer()->HasFormatChanged()) {
-    mValidRegion = nsIntRegion();
+    ClearValidRegion();
     mContentClient->GetTiledBuffer()->ResetPaintedAndValidState();
   }
 
   TILING_LOG("TILING %p: Initial visible region %s\n", this, Stringify(mVisibleRegion).c_str());
-  TILING_LOG("TILING %p: Initial valid region %s\n", this, Stringify(mValidRegion).c_str());
+  TILING_LOG("TILING %p: Initial valid region %s\n", this, Stringify(GetValidRegion()).c_str());
   TILING_LOG("TILING %p: Initial low-precision valid region %s\n", this, Stringify(mLowPrecisionValidRegion).c_str());
 
   nsIntRegion neededRegion = mVisibleRegion.ToUnknownRegion();
@@ -493,7 +494,7 @@ ClientTiledPaintedLayer::RenderLayer()
 #endif
 
   nsIntRegion invalidRegion;
-  invalidRegion.Sub(neededRegion, mValidRegion);
+  invalidRegion.Sub(neededRegion, GetValidRegion());
   if (invalidRegion.IsEmpty()) {
     EndPaint();
     return;
@@ -518,13 +519,15 @@ ClientTiledPaintedLayer::RenderLayer()
     // Make sure that tiles that fall outside of the visible region or outside of the
     // critical displayport are discarded on the first update. Also make sure that we
     // only draw stuff inside the critical displayport on the first update.
-    mValidRegion.And(mValidRegion, neededRegion);
+    nsIntRegion validRegion;
+    validRegion.And(GetValidRegion(), neededRegion);
     if (mPaintData.mCriticalDisplayPort) {
-      mValidRegion.And(mValidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
+      validRegion.AndWith(mPaintData.mCriticalDisplayPort->ToUnknownRect());
       invalidRegion.And(invalidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
     }
+    SetValidRegion(validRegion);
 
-    TILING_LOG("TILING %p: First-transaction valid region %s\n", this, Stringify(mValidRegion).c_str());
+    TILING_LOG("TILING %p: First-transaction valid region %s\n", this, Stringify(validRegion).c_str());
     TILING_LOG("TILING %p: First-transaction invalid region %s\n", this, Stringify(invalidRegion).c_str());
   } else {
     if (mPaintData.mCriticalDisplayPort) {
@@ -538,7 +541,7 @@ ClientTiledPaintedLayer::RenderLayer()
     // Calculate the invalid region for the low precision buffer. Make sure
     // to remove the valid high-precision area so we don't double-paint it.
     lowPrecisionInvalidRegion.Sub(neededRegion, mLowPrecisionValidRegion);
-    lowPrecisionInvalidRegion.Sub(lowPrecisionInvalidRegion, mValidRegion);
+    lowPrecisionInvalidRegion.Sub(lowPrecisionInvalidRegion, GetValidRegion());
   }
   TILING_LOG("TILING %p: Low-precision invalid region %s\n", this, Stringify(lowPrecisionInvalidRegion).c_str());
 

gfx/layers/client/ClientTiledPaintedLayer.h

@@ -55,9 +55,12 @@ public:
   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);
+    UpdateValidRegionAfterInvalidRegionChanged();
+    if (!mLowPrecisionValidRegion.IsEmpty()) {
+      // Also update mLowPrecisionValidRegion. Unfortunately we call
+      // mInvalidRegion.GetRegion() here, which is expensive.
+      mLowPrecisionValidRegion.SubOut(mInvalidRegion.GetRegion());
+    }
   }
 
   // Shadow methods

gfx/layers/wr/WebRenderPaintedLayer.cpp

@@ -124,7 +124,7 @@ WebRenderPaintedLayer::RenderLayer(wr::DisplayListBuilder& aBuilder,
   }
 
   nsIntRegion regionToPaint;
-  regionToPaint.Sub(mVisibleRegion.ToUnknownRegion(), mValidRegion);
+  regionToPaint.Sub(mVisibleRegion.ToUnknownRegion(), GetValidRegion());
 
   // We have something to paint but can't. This usually happens only in
   // empty transactions

gfx/layers/wr/WebRenderPaintedLayer.h

@@ -43,7 +43,7 @@ public:
   virtual void InvalidateRegion(const nsIntRegion& aRegion) override
   {
     mInvalidRegion.Add(aRegion);
-    mValidRegion.Sub(mValidRegion, mInvalidRegion.GetRegion());
+    UpdateValidRegionAfterInvalidRegionChanged();
   }
 
   Layer* GetLayer() override { return this; }

gfx/layers/wr/WebRenderPaintedLayerBlob.cpp

@@ -37,7 +37,7 @@ WebRenderPaintedLayerBlob::RenderLayer(wr::DisplayListBuilder& aBuilder,
   }
 
   nsIntRegion regionToPaint;
-  regionToPaint.Sub(mVisibleRegion.ToUnknownRegion(), mValidRegion);
+  regionToPaint.Sub(mVisibleRegion.ToUnknownRegion(), GetValidRegion());
 
   // We have something to paint but can't. This usually happens only in
   // empty transactions

gfx/layers/wr/WebRenderPaintedLayerBlob.h

@@ -46,7 +46,7 @@ public:
   virtual void InvalidateRegion(const nsIntRegion& aRegion) override
   {
     mInvalidRegion.Add(aRegion);
-    mValidRegion.Sub(mValidRegion, mInvalidRegion.GetRegion());
+    UpdateValidRegionAfterInvalidRegionChanged();
   }
 
   Layer* GetLayer() override { return this; }