Bug 622072. Part 1: Remove rect parameter from Updated() and change implementations to defer updates to render time. r=bas

The rectangle parameter is currently not used --- all callers always pass the full canvas bounds. In the long term,
we probably won't want this parameter since all implementations should be doing accelerated drawing direct to buffers
with no intermediate copies, hence there will be no need to optimize the size of those copies. Plus, performance-sensitive
testcases tend to paint most or all of the canvas on every frame anyway.

content/canvas/src/WebGLContext.cpp

@@ -627,7 +627,7 @@ WebGLContext::GetCanvasLayer(CanvasLayer *aOldLayer,
         aOldLayer->HasUserData(&gWebGLLayerUserData)) {
         NS_ADDREF(aOldLayer);
         if (mInvalidated) {
-            aOldLayer->Updated(nsIntRect(0, 0, mWidth, mHeight));
+            aOldLayer->Updated();
             mInvalidated = PR_FALSE;
             HTMLCanvasElement()->GetPrimaryCanvasFrame()->MarkLayersActive();
         }
@@ -661,7 +661,7 @@ WebGLContext::GetCanvasLayer(CanvasLayer *aOldLayer,
     canvasLayer->Initialize(data);
     PRUint32 flags = gl->CreationFormat().alpha == 0 ? Layer::CONTENT_OPAQUE : 0;
     canvasLayer->SetContentFlags(flags);
-    canvasLayer->Updated(nsIntRect(0, 0, mWidth, mHeight));
+    canvasLayer->Updated();
 
     mInvalidated = PR_FALSE;
     mResetLayer = PR_FALSE;

content/canvas/src/nsCanvasRenderingContext2D.cpp

@@ -4110,9 +4110,7 @@ nsCanvasRenderingContext2D::GetCanvasLayer(CanvasLayer *aOldLayer,
         aOldLayer->HasUserData(&g2DContextLayerUserData)) {
         NS_ADDREF(aOldLayer);
         if (mIsEntireFrameInvalid || mInvalidateCount > 0) {
-            // XXX Need to just update the changed area here; we should keep track
-            // of the rectangle based on Redraw args.
-            aOldLayer->Updated(nsIntRect(0, 0, mWidth, mHeight));
+            aOldLayer->Updated();
             MarkContextClean();
             HTMLCanvasElement()->GetPrimaryCanvasFrame()->MarkLayersActive();
         }
@@ -4135,7 +4133,7 @@ nsCanvasRenderingContext2D::GetCanvasLayer(CanvasLayer *aOldLayer,
     canvasLayer->Initialize(data);
     PRUint32 flags = mOpaque ? Layer::CONTENT_OPAQUE : 0;
     canvasLayer->SetContentFlags(flags);
-    canvasLayer->Updated(nsIntRect(0, 0, mWidth, mHeight));
+    canvasLayer->Updated();
 
     mResetLayer = PR_FALSE;
 

gfx/layers/Layers.h

@@ -1176,13 +1176,10 @@ public:
   virtual void Initialize(const Data& aData) = 0;
 
   /**
-   * CONSTRUCTION PHASE ONLY
-   * Notify this CanvasLayer that the rectangle given by aRect
-   * has been updated, and any work that needs to be done
-   * to bring the contents from the Surface/GLContext to the
-   * Layer in preparation for compositing should be performed.
+   * Notify this CanvasLayer that the canvas surface contents have
+   * changed (or will change) before the next transaction.
    */
-  virtual void Updated(const nsIntRect& aRect) = 0;
+  void Updated() { mDirty = PR_TRUE; }
 
   /**
    * CONSTRUCTION PHASE ONLY
@@ -1207,7 +1204,7 @@ public:
 
 protected:
   CanvasLayer(LayerManager* aManager, void* aImplData)
-    : Layer(aManager, aImplData), mFilter(gfxPattern::FILTER_GOOD) {}
+    : Layer(aManager, aImplData), mFilter(gfxPattern::FILTER_GOOD), mDirty(PR_FALSE) {}
 
   virtual nsACString& PrintInfo(nsACString& aTo, const char* aPrefix);
 
@@ -1216,6 +1213,10 @@ protected:
    */
   nsIntRect mBounds;
   gfxPattern::GraphicsFilter mFilter;
+  /**
+   * Set to true in Updated(), cleared during a transaction.
+   */
+  PRPackedBool mDirty;
 };
 
 }

gfx/layers/basic/BasicLayers.cpp

@@ -893,7 +893,6 @@ public:
   }
 
   virtual void Initialize(const Data& aData);
-  virtual void Updated(const nsIntRect& aRect);
   virtual void Paint(gfxContext* aContext);
 
   virtual void PaintWithOpacity(gfxContext* aContext,
@@ -904,13 +903,12 @@ protected:
   {
     return static_cast<BasicLayerManager*>(mManager);
   }
+  void UpdateSurface();
 
   nsRefPtr<gfxASurface> mSurface;
   nsRefPtr<mozilla::gl::GLContext> mGLContext;
   PRUint32 mCanvasFramebuffer;
 
-  nsIntRect mUpdatedRect;
-
   PRPackedBool mGLBufferIsPremultiplied;
   PRPackedBool mNeedsYFlip;
 };
@@ -920,8 +918,6 @@ BasicCanvasLayer::Initialize(const Data& aData)
 {
   NS_ASSERTION(mSurface == nsnull, "BasicCanvasLayer::Initialize called twice!");
 
-  mUpdatedRect.Empty();
-
   if (aData.mSurface) {
     mSurface = aData.mSurface;
     NS_ASSERTION(aData.mGLContext == nsnull,
@@ -941,12 +937,11 @@ BasicCanvasLayer::Initialize(const Data& aData)
 }
 
 void
-BasicCanvasLayer::Updated(const nsIntRect& aRect)
+BasicCanvasLayer::UpdateSurface()
 {
-  NS_ASSERTION(mUpdatedRect.IsEmpty(),
-               "CanvasLayer::Updated called more than once in a transaction!");
-
-  mUpdatedRect.UnionRect(mUpdatedRect, aRect);
+  if (!mDirty)
+    return;
+  mDirty = PR_FALSE;
 
   if (mGLContext) {
     nsRefPtr<gfxImageSurface> isurf =
@@ -976,9 +971,6 @@ BasicCanvasLayer::Updated(const nsIntRect& aRect)
     if (currentFramebuffer != mCanvasFramebuffer)
       mGLContext->fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mCanvasFramebuffer);
 
-    // For simplicity, we read the entire framebuffer for now -- in
-    // the future we should use mUpdatedRect, though with WebGL we don't
-    // have an easy way to generate one.
     mGLContext->ReadPixelsIntoImageSurface(0, 0,
                                            mBounds.width, mBounds.height,
                                            isurf);
@@ -997,15 +989,12 @@ BasicCanvasLayer::Updated(const nsIntRect& aRect)
     // stick our surface into mSurface, so that the Paint() path is the same
     mSurface = isurf;
   }
-
-  // sanity
-  NS_ASSERTION(mUpdatedRect.IsEmpty() || mBounds.Contains(mUpdatedRect),
-               "CanvasLayer: Updated rect bigger than bounds!");
 }
 
 void
 BasicCanvasLayer::Paint(gfxContext* aContext)
 {
+  UpdateSurface();
   PaintWithOpacity(aContext, GetEffectiveOpacity());
 }
 
@@ -1037,8 +1026,6 @@ BasicCanvasLayer::PaintWithOpacity(gfxContext* aContext,
   if (mNeedsYFlip) {
     aContext->SetMatrix(m);
   }
-
-  mUpdatedRect.Empty();
 }
 
 class BasicReadbackLayer : public ReadbackLayer,
@@ -2557,9 +2544,6 @@ public:
 
   virtual void Initialize(const Data& aData);
 
-  virtual void Updated(const nsIntRect& aRect)
-  {}
-
   virtual already_AddRefed<gfxSharedImageSurface>
   Swap(gfxSharedImageSurface* newFront);
 

gfx/layers/d3d10/CanvasLayerD3D10.cpp

@@ -114,8 +114,12 @@ CanvasLayerD3D10::Initialize(const Data& aData)
 }
 
 void
-CanvasLayerD3D10::Updated(const nsIntRect& aRect)
+CanvasLayerD3D10::UpdateSurface()
 {
+  if (!mDirty)
+    return;
+  mDirty = PR_FALSE;
+
   if (mIsD2DTexture) {
     mSurface->Flush();
     return;
@@ -161,9 +165,6 @@ CanvasLayerD3D10::Updated(const nsIntRect& aRect)
     if (currentFramebuffer != mCanvasFramebuffer)
       mGLContext->fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mCanvasFramebuffer);
 
-    // For simplicity, we read the entire framebuffer for now -- in
-    // the future we should use aRect, though with WebGL we don't
-    // have an easy way to generate one.
     nsRefPtr<gfxImageSurface> tmpSurface =
       new gfxImageSurface(destination,
                           gfxIntSize(mBounds.width, mBounds.height),
@@ -189,10 +190,10 @@ CanvasLayerD3D10::Updated(const nsIntRect& aRect)
     mTexture->Unmap(0);
   } else if (mSurface) {
     RECT r;
-    r.left = aRect.x;
-    r.top = aRect.y;
-    r.right = aRect.XMost();
-    r.bottom = aRect.YMost();
+    r.left = 0;
+    r.top = 0;
+    r.right = mBounds.width;
+    r.bottom = mBounds.height;
 
     D3D10_MAPPED_TEXTURE2D map;
     HRESULT hr = mTexture->Map(0, D3D10_MAP_WRITE_DISCARD, 0, &map);
@@ -202,17 +203,13 @@ CanvasLayerD3D10::Updated(const nsIntRect& aRect)
       return;
     }
 
-    PRUint8 *startBits;
-    PRUint32 sourceStride;
-
     nsRefPtr<gfxImageSurface> dstSurface;
 
     dstSurface = new gfxImageSurface((unsigned char*)map.pData,
-                                     gfxIntSize(aRect.width, aRect.height),
+                                     gfxIntSize(mBounds.width, mBounds.height),
                                      map.RowPitch,
                                      gfxASurface::ImageFormatARGB32);
     nsRefPtr<gfxContext> ctx = new gfxContext(dstSurface);
-    ctx->Translate(gfxPoint(-aRect.x, -aRect.y));
     ctx->SetOperator(gfxContext::OPERATOR_SOURCE);
     ctx->SetSource(mSurface);
     ctx->Paint();
@@ -230,6 +227,8 @@ CanvasLayerD3D10::GetLayer()
 void
 CanvasLayerD3D10::RenderLayer()
 {
+  UpdateSurface();
+
   if (!mTexture) {
     return;
   }

gfx/layers/d3d10/CanvasLayerD3D10.h

@@ -64,7 +64,6 @@ public:
 
   // CanvasLayer implementation
   virtual void Initialize(const Data& aData);
-  virtual void Updated(const nsIntRect& aRect);
 
   // LayerD3D10 implementation
   virtual Layer* GetLayer();
@@ -73,6 +72,8 @@ public:
 private:
   typedef mozilla::gl::GLContext GLContext;
 
+  void UpdateSurface();
+
   nsRefPtr<gfxASurface> mSurface;
   nsRefPtr<GLContext> mGLContext;
   nsRefPtr<ID3D10Texture2D> mTexture;

gfx/layers/d3d9/CanvasLayerD3D9.cpp

@@ -79,8 +79,12 @@ CanvasLayerD3D9::Initialize(const Data& aData)
 }
 
 void
-CanvasLayerD3D9::Updated(const nsIntRect& aRect)
+CanvasLayerD3D9::UpdateSurface()
 {
+  if (!mDirty)
+    return;
+  mDirty = PR_FALSE;
+
   if (!mTexture) {
     CreateTexture();
     NS_WARNING("CanvasLayerD3D9::Updated called but no texture present!");
@@ -117,9 +121,6 @@ CanvasLayerD3D9::Updated(const nsIntRect& aRect)
     if (currentFramebuffer != mCanvasFramebuffer)
       mGLContext->fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mCanvasFramebuffer);
 
-    // For simplicity, we read the entire framebuffer for now -- in
-    // the future we should use aRect, though with WebGL we don't
-    // have an easy way to generate one.
     nsRefPtr<gfxImageSurface> tmpSurface =
       new gfxImageSurface(destination,
                           gfxIntSize(mBounds.width, mBounds.height),
@@ -145,10 +146,10 @@ CanvasLayerD3D9::Updated(const nsIntRect& aRect)
     mTexture->UnlockRect(0);
   } else if (mSurface) {
     RECT r;
-    r.left = aRect.x;
-    r.top = aRect.y;
-    r.right = aRect.XMost();
-    r.bottom = aRect.YMost();
+    r.left = mBounds.x;
+    r.top = mBounds.y;
+    r.right = mBounds.XMost();
+    r.bottom = mBounds.YMost();
 
     D3DLOCKED_RECT lockedRect;
     HRESULT hr = mTexture->LockRect(0, &lockedRect, &r, 0);
@@ -158,16 +159,10 @@ CanvasLayerD3D9::Updated(const nsIntRect& aRect)
       return;
     }
 
-    PRUint8 *startBits;
-    PRUint32 sourceStride;
-
     nsRefPtr<gfxImageSurface> sourceSurface;
 
     if (mSurface->GetType() == gfxASurface::SurfaceTypeWin32) {
       sourceSurface = mSurface->GetAsImageSurface();
-      startBits = sourceSurface->Data() + sourceSurface->Stride() * aRect.y +
-                  aRect.x * 4;
-      sourceStride = sourceSurface->Stride();
     } else if (mSurface->GetType() == gfxASurface::SurfaceTypeImage) {
       sourceSurface = static_cast<gfxImageSurface*>(mSurface.get());
       if (sourceSurface->Format() != gfxASurface::ImageFormatARGB32 &&
@@ -176,31 +171,28 @@ CanvasLayerD3D9::Updated(const nsIntRect& aRect)
         mTexture->UnlockRect(0);
         return;
       }
-      startBits = sourceSurface->Data() + sourceSurface->Stride() * aRect.y +
-                  aRect.x * 4;
-      sourceStride = sourceSurface->Stride();
     } else {
-      sourceSurface = new gfxImageSurface(gfxIntSize(aRect.width, aRect.height),
+      sourceSurface = new gfxImageSurface(gfxIntSize(mBounds.width, mBounds.height),
                                           gfxASurface::ImageFormatARGB32);
       nsRefPtr<gfxContext> ctx = new gfxContext(sourceSurface);
-      ctx->Translate(gfxPoint(-aRect.x, -aRect.y));
       ctx->SetOperator(gfxContext::OPERATOR_SOURCE);
       ctx->SetSource(mSurface);
       ctx->Paint();
-      startBits = sourceSurface->Data();
-      sourceStride = sourceSurface->Stride();
     }
 
+    PRUint8 *startBits = sourceSurface->Data();
+    PRUint32 sourceStride = sourceSurface->Stride();
+
     if (sourceSurface->Format() != gfxASurface::ImageFormatARGB32) {
       mHasAlpha = false;
     } else {
       mHasAlpha = true;
     }
 
-    for (int y = 0; y < aRect.height; y++) {
+    for (int y = 0; y < mBounds.height; y++) {
       memcpy((PRUint8*)lockedRect.pBits + lockedRect.Pitch * y,
              startBits + sourceStride * y,
-             aRect.width * 4);
+             mBounds.width * 4);
     }
 
     mTexture->UnlockRect(0);
@@ -216,9 +208,9 @@ CanvasLayerD3D9::GetLayer()
 void
 CanvasLayerD3D9::RenderLayer()
 {
-  if (!mTexture) {
-    Updated(mBounds);
-  }
+  UpdateSurface();
+  if (!mTexture)
+    return;
 
   /*
    * We flip the Y axis here, note we can only do this because we are in 

gfx/layers/d3d9/CanvasLayerD3D9.h

@@ -66,7 +66,6 @@ public:
 
   // CanvasLayer implementation
   virtual void Initialize(const Data& aData);
-  virtual void Updated(const nsIntRect& aRect);
 
   // LayerD3D9 implementation
   virtual Layer* GetLayer();
@@ -79,6 +78,8 @@ public:
 protected:
   typedef mozilla::gl::GLContext GLContext;
 
+  void UpdateSurface();
+
   nsRefPtr<gfxASurface> mSurface;
   nsRefPtr<GLContext> mGLContext;
   nsRefPtr<IDirect3DTexture9> mTexture;

gfx/layers/ipc/PLayers.ipdl

@@ -172,7 +172,6 @@ struct OpPaintThebesBuffer {
 
 struct OpPaintCanvas  {
   PLayer layer;
-  nsIntRect updated;
   Shmem newFrontBuffer;
 };
 

gfx/layers/ipc/ShadowLayers.cpp

@@ -301,7 +301,6 @@ ShadowLayerForwarder::PaintedCanvas(ShadowableLayer* aCanvas,
                                     gfxSharedImageSurface* aNewFrontSurface)
 {
   mTxn->AddPaint(OpPaintCanvas(NULL, Shadow(aCanvas),
-                               nsIntRect(),
                                aNewFrontSurface->GetShmem()));
 }
 

gfx/layers/ipc/ShadowLayersParent.cpp

@@ -422,7 +422,7 @@ ShadowLayersParent::RecvUpdate(const InfallibleTArray<Edit>& cset,
         newFront.forget();
       }
 
-      canvas->Updated(op.updated());
+      canvas->Updated();
 
       replyv.push_back(OpBufferSwap(shadow, NULL,
                                     newBack->GetShmem()));

gfx/layers/opengl/CanvasLayerOGL.cpp

@@ -135,19 +135,18 @@ CanvasLayerOGL::MakeTexture()
 }
 
 void
-CanvasLayerOGL::Updated(const nsIntRect& aRect)
+CanvasLayerOGL::UpdateSurface()
 {
+  if (!mDirty)
+    return;
+  mDirty = PR_FALSE;
+
   if (mDestroyed || mDelayedUpdates) {
     return;
   }
 
-  NS_ASSERTION(mUpdatedRect.IsEmpty(),
-               "CanvasLayer::Updated called more than once during a transaction!");
-
   mOGLManager->MakeCurrent();
 
-  mUpdatedRect.UnionRect(mUpdatedRect, aRect);
-
   if (mCanvasGLContext &&
       mCanvasGLContext->GetContextType() == gl()->GetContextType())
   {
@@ -157,41 +156,35 @@ CanvasLayerOGL::Updated(const nsIntRect& aRect)
       MakeTexture();
     }
   } else {
-    if (!mTexture) {
-      mUpdatedRect = mBounds;
-    }
-
     nsRefPtr<gfxASurface> updatedAreaSurface;
     if (mCanvasSurface) {
       updatedAreaSurface = mCanvasSurface;
     } else if (mCanvasGLContext) {
       nsRefPtr<gfxImageSurface> updatedAreaImageSurface =
-        new gfxImageSurface(gfxIntSize(mUpdatedRect.width, mUpdatedRect.height),
+        new gfxImageSurface(gfxIntSize(mBounds.width, mBounds.height),
                             gfxASurface::ImageFormatARGB32);
-      mCanvasGLContext->ReadPixelsIntoImageSurface(mUpdatedRect.x, mUpdatedRect.y,
-                                                   mUpdatedRect.width,
-                                                   mUpdatedRect.height,
+      mCanvasGLContext->ReadPixelsIntoImageSurface(0, 0,
+                                                   mBounds.width,
+                                                   mBounds.height,
                                                    updatedAreaImageSurface);
       updatedAreaSurface = updatedAreaImageSurface;
     }
 
     mLayerProgram =
       gl()->UploadSurfaceToTexture(updatedAreaSurface,
-                                   mUpdatedRect,
+                                   mBounds,
                                    mTexture,
                                    false,
-                                   mUpdatedRect.TopLeft());
+                                   nsIntPoint(0, 0));
   }
-
-  // sanity
-  NS_ASSERTION(mBounds.Contains(mUpdatedRect),
-               "CanvasLayer: Updated rect bigger than bounds!");
 }
 
 void
 CanvasLayerOGL::RenderLayer(int aPreviousDestination,
                             const nsIntPoint& aOffset)
 {
+  UpdateSurface();
+
   mOGLManager->MakeCurrent();
 
   // XXX We're going to need a different program depending on if
@@ -248,8 +241,6 @@ CanvasLayerOGL::RenderLayer(int aPreviousDestination,
   if (useGLContext) {
     gl()->UnbindTex2DOffscreen(mCanvasGLContext);
   }
-
-  mUpdatedRect.Empty();
 }
 
 

gfx/layers/opengl/CanvasLayerOGL.h

@@ -66,7 +66,6 @@ public:
 
   // CanvasLayer implementation
   virtual void Initialize(const Data& aData);
-  virtual void Updated(const nsIntRect& aRect);
 
   // LayerOGL implementation
   virtual void Destroy();
@@ -75,6 +74,8 @@ public:
                            const nsIntPoint& aOffset);
 
 protected:
+  void UpdateSurface();
+
   nsRefPtr<gfxASurface> mCanvasSurface;
   nsRefPtr<GLContext> mCanvasGLContext;
   gl::ShaderProgramType mLayerProgram;
@@ -82,8 +83,6 @@ protected:
   void MakeTexture();
   GLuint mTexture;
 
-  nsIntRect mUpdatedRect;
-
   PRPackedBool mDelayedUpdates;
   PRPackedBool mGLBufferIsPremultiplied;
   PRPackedBool mNeedsYFlip;