Bug 782785 - Use temp surfaces to ReadPixels with correct stride - r=bjacob
authorJeff Gilbert <jgilbert@mozilla.com>
Tue, 21 Aug 2012 16:13:26 -0700
changeset 102981 854c1028d1472713efeca722e01f51ffc5282dd0
parent 102980 581cdaf67a25a491c3471ab8c72c983ac42865d8
child 102982 69c5fcb15368dadecba94bc1ba906e46eb66de05
push id13754
push userjgilbert@mozilla.com
push dateTue, 21 Aug 2012 23:13:50 +0000
treeherdermozilla-inbound@854c1028d147 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbjacob
bugs782785
milestone17.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 782785 - Use temp surfaces to ReadPixels with correct stride - r=bjacob
content/canvas/src/WebGLContext.cpp
gfx/gl/GLContext.cpp
gfx/gl/GLContext.h
gfx/layers/basic/BasicCanvasLayer.cpp
gfx/layers/opengl/LayerManagerOGL.cpp
--- a/content/canvas/src/WebGLContext.cpp
+++ b/content/canvas/src/WebGLContext.cpp
@@ -605,17 +605,17 @@ WebGLContext::Render(gfxContext *ctx, gf
     if (!gl)
         return NS_OK;
 
     nsRefPtr<gfxImageSurface> surf = new gfxImageSurface(gfxIntSize(mWidth, mHeight),
                                                          gfxASurface::ImageFormatARGB32);
     if (surf->CairoStatus() != 0)
         return NS_ERROR_FAILURE;
 
-    gl->ReadPixelsIntoImageSurface(0, 0, mWidth, mHeight, surf);
+    gl->ReadPixelsIntoImageSurface(surf);
 
     bool srcPremultAlpha = mOptions.premultipliedAlpha;
     bool dstPremultAlpha = aFlags & RenderFlagPremultAlpha;
 
     if (!srcPremultAlpha && dstPremultAlpha) {
         gfxUtils::PremultiplyImageSurface(surf);
     } else if (srcPremultAlpha && !dstPremultAlpha) {
         gfxUtils::UnpremultiplyImageSurface(surf);
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -1764,26 +1764,25 @@ GLContext::MarkDestroyed()
         mBlitFramebuffer = 0;
     } else {
         NS_WARNING("MakeCurrent() failed during MarkDestroyed! Skipping GL object teardown.");
     }
 
     mSymbols.Zero();
 }
 
-static void SwapRAndBComponents(gfxImageSurface* aSurf)
+static void SwapRAndBComponents(gfxImageSurface* surf)
 {
-  gfxIntSize size = aSurf->GetSize();
-  for (int j = 0; j < size.height; ++j) {
-    PRUint32 *row = (PRUint32*) (aSurf->Data() + aSurf->Stride() * j);
-    for (int i = 0; i < size.width; ++i) {
-      *row = (*row & 0xff00ff00) | ((*row & 0xff) << 16) | ((*row & 0xff0000) >> 16);
-      row++;
+    for (int j = 0; j < surf->Height(); ++j) {
+        uint32_t* row = (uint32_t*)(surf->Data() + surf->Stride() * j);
+        for (int i = 0; i < surf->Width(); ++i) {
+            *row = (*row & 0xff00ff00) | ((*row & 0xff) << 16) | ((*row & 0xff0000) >> 16);
+            row++;
+        }
     }
-  }
 }
 
 static already_AddRefed<gfxImageSurface> YInvertImageSurface(gfxImageSurface* aSurf)
 {
   gfxIntSize size = aSurf->GetSize();
   nsRefPtr<gfxImageSurface> temp = new gfxImageSurface(size, aSurf->Format());
   nsRefPtr<gfxContext> ctx = new gfxContext(temp);
   ctx->SetOperator(gfxContext::OPERATOR_SOURCE);
@@ -2000,68 +1999,54 @@ GetOptimalReadFormats(GLContext* gl, GLe
 
 void
 GLContext::ReadScreenIntoImageSurface(gfxImageSurface* dest)
 {
     GLuint boundFB = 0;
     fGetIntegerv(LOCAL_GL_FRAMEBUFFER_BINDING, (GLint*)&boundFB);
     fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, 0);
 
-    ReadPixelsIntoImageSurface(0, 0, dest->Width(), dest->Height(), dest);
+    ReadPixelsIntoImageSurface(dest);
 
     fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, boundFB);
 }
 
 void
-GLContext::ReadPixelsIntoImageSurface(GLint aX, GLint aY,
-                                      GLsizei aWidth, GLsizei aHeight,
-                                      gfxImageSurface *aDest)
+GLContext::ReadPixelsIntoImageSurface(gfxImageSurface* dest)
 {
+    MOZ_ASSERT(dest->Format() == gfxASurface::ImageFormatARGB32 ||
+               dest->Format() == gfxASurface::ImageFormatRGB24);
+
+    MOZ_ASSERT(dest->Stride() == dest->Width() * 4);
+    MOZ_ASSERT(dest->Format() == gfxASurface::ImageFormatARGB32 ||
+               dest->Format() == gfxASurface::ImageFormatRGB24);
+
+    MOZ_ASSERT(dest->Stride() == dest->Width() * 4);
+
     MakeCurrent();
 
-    if (aDest->Format() != gfxASurface::ImageFormatARGB32 &&
-        aDest->Format() != gfxASurface::ImageFormatRGB24)
-    {
-        NS_WARNING("ReadPixelsIntoImageSurface called with invalid image format");
-        return;
-    }
-
-    if (aDest->Width() != aWidth ||
-        aDest->Height() != aHeight ||
-        aDest->Stride() != aWidth * 4)
-    {
-        NS_WARNING("ReadPixelsIntoImageSurface called with wrong size or stride surface");
-        return;
-    }
-
     GLint currentPackAlignment = 0;
     fGetIntegerv(LOCAL_GL_PACK_ALIGNMENT, &currentPackAlignment);
 
     if (currentPackAlignment != 4)
         fPixelStorei(LOCAL_GL_PACK_ALIGNMENT, 4);
 
     GLenum format;
     GLenum datatype;
 
     GetOptimalReadFormats(this, format, datatype);
 
-    fReadPixels(0, 0, aWidth, aHeight,
+    fReadPixels(0, 0,
+                dest->Width(), dest->Height(),
                 format, datatype,
-                aDest->Data());
-
-    // Output should be in BGRA, so swap if RGBA
+                dest->Data());
+
+    // Output should be in BGRA, so swap if RGBA.
     if (format == LOCAL_GL_RGBA) {
-        // swap B and R bytes
-        for (int j = 0; j < aHeight; ++j) {
-            PRUint32 *row = (PRUint32*) (aDest->Data() + aDest->Stride() * j);
-            for (int i = 0; i < aWidth; ++i) {
-                *row = (*row & 0xff00ff00) | ((*row & 0xff) << 16) | ((*row & 0xff0000) >> 16);
-                row++;
-            }
-        }
+        SwapRAndBComponents(dest);
     }
 
     if (currentPackAlignment != 4)
         fPixelStorei(LOCAL_GL_PACK_ALIGNMENT, currentPackAlignment);
 }
 
 void
 GLContext::BlitTextureImage(TextureImage *aSrc, const nsIntRect& aSrcRect,
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -1380,25 +1380,26 @@ public:
     already_AddRefed<gfxImageSurface> ReadTextureImage(GLuint aTexture,
                                                        const gfxIntSize& aSize,
                                                        GLenum aTextureFormat,
                                                        bool aYInvert = false);
 
     already_AddRefed<gfxImageSurface> GetTexImage(GLuint aTexture, bool aYInvert, ShaderProgramType aShader);
 
     /**
-     * Call ReadPixels into an existing gfxImageSurface for the given bounds.
-     * The image surface must be using image format RGBA32 or RGB24.
+     * Call ReadPixels into an existing gfxImageSurface.
+     * The image surface must be using image format RGBA32 or RGB24,
+     * and must have stride == width*4.
+     * Note that neither ReadPixelsIntoImageSurface nor
+     * ReadScreenIntoImageSurface call dest->Flush/MarkDirty.
      */
-    void THEBES_API ReadPixelsIntoImageSurface(GLint aX, GLint aY,
-                                    GLsizei aWidth, GLsizei aHeight,
-                                    gfxImageSurface *aDest);
+    void THEBES_API ReadPixelsIntoImageSurface(gfxImageSurface* dest);
 
     // Similar to ReadPixelsIntoImageSurface, but pulls from the screen
-    // instead of the currenly bound framebuffer.
+    // instead of the currently bound framebuffer.
     void ReadScreenIntoImageSurface(gfxImageSurface* dest);
 
     /**
      * Copy a rectangle from one TextureImage into another.  The
      * source and destination are given in integer coordinates, and
      * will be converted to texture coordinates.
      *
      * For the source texture, the wrap modes DO apply -- it's valid
--- a/gfx/layers/basic/BasicCanvasLayer.cpp
+++ b/gfx/layers/basic/BasicCanvasLayer.cpp
@@ -72,16 +72,17 @@ protected:
         aSize.height != mCachedSize.height ||
         aFormat != mCachedFormat)
     {
       mCachedTempSurface = new gfxImageSurface(aSize, aFormat);
       mCachedSize = aSize;
       mCachedFormat = aFormat;
     }
 
+    MOZ_ASSERT(mCachedTempSurface->Stride() == mCachedTempSurface->Width() * 4);
     return mCachedTempSurface;
   }
 
   void DiscardTempSurface()
   {
     mCachedTempSurface = nullptr;
   }
 };
@@ -150,51 +151,77 @@ BasicCanvasLayer::UpdateSurface(gfxASurf
     // XRender can only blend premuliplied alpha, so only allow xrender
     // path if we have premultiplied alpha or opaque content.
     if (offscreenSurface && (mGLBufferIsPremultiplied || (GetContentFlags() & CONTENT_OPAQUE))) {  
         mSurface = offscreenSurface;
         mNeedsYFlip = false;
         return;
     }
 #endif
-    nsRefPtr<gfxImageSurface> isurf;
+
+    gfxIntSize readSize(mBounds.width, mBounds.height);
+    gfxImageFormat format = (GetContentFlags() & CONTENT_OPAQUE)
+                              ? gfxASurface::ImageFormatRGB24
+                              : gfxASurface::ImageFormatARGB32;
+
+    nsRefPtr<gfxImageSurface> readSurf;
+    nsRefPtr<gfxImageSurface> resultSurf;
+
+    bool usingTempSurface = false;
+
     if (aDestSurface) {
-      DiscardTempSurface();
-      isurf = static_cast<gfxImageSurface*>(aDestSurface);
+      resultSurf = static_cast<gfxImageSurface*>(aDestSurface);
+
+      if (resultSurf->GetSize() != readSize ||
+          resultSurf->Stride() != resultSurf->Width() * 4)
+      {
+        readSurf = GetTempSurface(readSize, format);
+        usingTempSurface = true;
+      }
     } else {
-      nsIntSize size(mBounds.width, mBounds.height);
-      gfxImageFormat format = (GetContentFlags() & CONTENT_OPAQUE)
-                                ? gfxASurface::ImageFormatRGB24
-                                : gfxASurface::ImageFormatARGB32;
-
-      isurf = GetTempSurface(size, format);
+      resultSurf = GetTempSurface(readSize, format);
+      usingTempSurface = true;
     }
 
+    if (!usingTempSurface)
+      DiscardTempSurface();
 
-    if (!isurf || isurf->CairoStatus() != 0) {
+    if (!readSurf)
+      readSurf = resultSurf;
+
+    if (!resultSurf || resultSurf->CairoStatus() != 0)
       return;
-    }
 
-    NS_ASSERTION(isurf->Stride() == mBounds.width * 4, "gfxImageSurface stride isn't what we expect!");
+    MOZ_ASSERT(readSurf);
+    MOZ_ASSERT(readSurf->Stride() == mBounds.width * 4, "gfxImageSurface stride isn't what we expect!");
 
     // We need to Flush() the surface before modifying it outside of cairo.
-    isurf->Flush();
-    mGLContext->ReadScreenIntoImageSurface(isurf);
-    isurf->MarkDirty();
+    readSurf->Flush();
+    mGLContext->ReadScreenIntoImageSurface(readSurf);
+    readSurf->MarkDirty();
 
     // If the underlying GLContext doesn't have a framebuffer into which
     // premultiplied values were written, we have to do this ourselves here.
     // Note that this is a WebGL attribute; GL itself has no knowledge of
     // premultiplied or unpremultiplied alpha.
     if (!mGLBufferIsPremultiplied)
-      gfxUtils::PremultiplyImageSurface(isurf);
+      gfxUtils::PremultiplyImageSurface(readSurf);
+
+    if (readSurf != resultSurf) {
+      MOZ_ASSERT(resultSurf->Width() >= readSurf->Width());
+      MOZ_ASSERT(resultSurf->Height() >= readSurf->Height());
+
+      resultSurf->Flush();
+      resultSurf->CopyFrom(readSurf);
+      resultSurf->MarkDirty();
+    }
 
     // stick our surface into mSurface, so that the Paint() path is the same
     if (!aDestSurface) {
-      mSurface = isurf;
+      mSurface = resultSurf;
     }
   }
 }
 
 void
 BasicCanvasLayer::Paint(gfxContext* aContext, Layer* aMaskLayer)
 {
   if (IsHidden())
--- a/gfx/layers/opengl/LayerManagerOGL.cpp
+++ b/gfx/layers/opengl/LayerManagerOGL.cpp
@@ -1068,17 +1068,17 @@ LayerManagerOGL::CopyToTarget(gfxContext
     else {
       mGLContext->fReadBuffer(LOCAL_GL_COLOR_ATTACHMENT0);
     }
   }
 
   NS_ASSERTION(imageSurface->Stride() == width * 4,
                "Image Surfaces being created with weird stride!");
 
-  mGLContext->ReadPixelsIntoImageSurface(0, 0, width, height, imageSurface);
+  mGLContext->ReadPixelsIntoImageSurface(imageSurface);
 
   aTarget->SetOperator(gfxContext::OPERATOR_SOURCE);
   aTarget->Scale(1.0, -1.0);
   aTarget->Translate(-gfxPoint(0.0, height));
   aTarget->SetSource(imageSurface);
   aTarget->Paint();
 }