From c8c8475e5cc9cdfa7803969a9fd74d67cb28fd8f Mon Sep 17 00:00:00 2001 From: Jeff Gilbert Date: Tue, 21 Aug 2012 16:13:26 -0700 Subject: [PATCH] Bug 782785 - Use temp surfaces to ReadPixels with correct stride - r=bjacob --- content/canvas/src/WebGLContext.cpp | 2 +- gfx/gl/GLContext.cpp | 61 ++++++++++----------------- gfx/gl/GLContext.h | 13 +++--- gfx/layers/basic/BasicCanvasLayer.cpp | 61 +++++++++++++++++++-------- gfx/layers/opengl/LayerManagerOGL.cpp | 2 +- 5 files changed, 76 insertions(+), 63 deletions(-) diff --git a/content/canvas/src/WebGLContext.cpp b/content/canvas/src/WebGLContext.cpp index 4b0f95fc048..ef8abfc8087 100644 --- a/content/canvas/src/WebGLContext.cpp +++ b/content/canvas/src/WebGLContext.cpp @@ -610,7 +610,7 @@ WebGLContext::Render(gfxContext *ctx, gfxPattern::GraphicsFilter f, PRUint32 aFl 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; diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp index 75196a620e2..23a50e0e88a 100644 --- a/gfx/gl/GLContext.cpp +++ b/gfx/gl/GLContext.cpp @@ -1769,16 +1769,15 @@ GLContext::MarkDestroyed() 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 YInvertImageSurface(gfxImageSurface* aSurf) @@ -2005,33 +2004,25 @@ GLContext::ReadScreenIntoImageSurface(gfxImageSurface* dest) 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, ¤tPackAlignment); @@ -2043,20 +2034,14 @@ GLContext::ReadPixelsIntoImageSurface(GLint aX, GLint aY, GetOptimalReadFormats(this, format, datatype); - fReadPixels(0, 0, aWidth, aHeight, + fReadPixels(0, 0, + dest->Width(), dest->Height(), format, datatype, - aDest->Data()); + dest->Data()); - // Output should be in BGRA, so swap if RGBA + // 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) diff --git a/gfx/gl/GLContext.h b/gfx/gl/GLContext.h index 99872c6f017..80f4e3de914 100644 --- a/gfx/gl/GLContext.h +++ b/gfx/gl/GLContext.h @@ -1385,15 +1385,16 @@ public: already_AddRefed 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); /** diff --git a/gfx/layers/basic/BasicCanvasLayer.cpp b/gfx/layers/basic/BasicCanvasLayer.cpp index 34ebffb8114..cf239621322 100644 --- a/gfx/layers/basic/BasicCanvasLayer.cpp +++ b/gfx/layers/basic/BasicCanvasLayer.cpp @@ -77,6 +77,7 @@ protected: mCachedFormat = aFormat; } + MOZ_ASSERT(mCachedTempSurface->Stride() == mCachedTempSurface->Width() * 4); return mCachedTempSurface; } @@ -155,41 +156,67 @@ BasicCanvasLayer::UpdateSurface(gfxASurface* aDestSurface, Layer* aMaskLayer) return; } #endif - nsRefPtr isurf; + + gfxIntSize readSize(mBounds.width, mBounds.height); + gfxImageFormat format = (GetContentFlags() & CONTENT_OPAQUE) + ? gfxASurface::ImageFormatRGB24 + : gfxASurface::ImageFormatARGB32; + + nsRefPtr readSurf; + nsRefPtr resultSurf; + + bool usingTempSurface = false; + if (aDestSurface) { - DiscardTempSurface(); - isurf = static_cast(aDestSurface); + resultSurf = static_cast(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; } } } diff --git a/gfx/layers/opengl/LayerManagerOGL.cpp b/gfx/layers/opengl/LayerManagerOGL.cpp index be400c3593e..b52c35a442a 100644 --- a/gfx/layers/opengl/LayerManagerOGL.cpp +++ b/gfx/layers/opengl/LayerManagerOGL.cpp @@ -1073,7 +1073,7 @@ LayerManagerOGL::CopyToTarget(gfxContext *aTarget) 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);