From 1806ba39468dc44b20b79701f4ec4d54b9ed9d9b Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Wed, 17 Jul 2013 23:24:15 -0400 Subject: [PATCH] Bug 875232 - Workaround glReadPixels being broken with framebuffers backed by an IOSurface by copying to a temporary texture first. r=jgilbert --- gfx/gl/GLContext.h | 41 +++++++++++++++++++++++++++++++++++++- gfx/gl/GLScreenBuffer.cpp | 25 +++++++++++++++++++++++ gfx/gl/GLScreenBuffer.h | 10 ++++++++++ gfx/gl/SharedSurfaceGL.h | 5 +++++ gfx/gl/SharedSurfaceIO.cpp | 23 +++++++++++++++++++++ gfx/gl/SharedSurfaceIO.h | 3 +++ 6 files changed, 106 insertions(+), 1 deletion(-) diff --git a/gfx/gl/GLContext.h b/gfx/gl/GLContext.h index 1bcb6cb03284..0c1b5fcb8456 100644 --- a/gfx/gl/GLContext.h +++ b/gfx/gl/GLContext.h @@ -15,6 +15,7 @@ #include #include #include +#include #ifdef WIN32 #include @@ -718,7 +719,16 @@ public: y = FixYValue(y, height); BeforeGLReadCall(); - raw_fReadPixels(x, y, width, height, format, type, pixels); + + bool didReadPixels = false; + if (mScreen) { + didReadPixels = mScreen->ReadPixels(x, y, width, height, format, type, pixels); + } + + if (!didReadPixels) { + raw_fReadPixels(x, y, width, height, format, type, pixels); + } + AfterGLReadCall(); } @@ -1182,6 +1192,8 @@ protected: int32_t mRenderer; public: + std::map mFBOMapping; + enum { DebugEnabled = 1 << 0, DebugTrace = 1 << 1, @@ -3183,6 +3195,33 @@ protected: } }; +struct ScopedTexture + : public ScopedGLWrapper +{ + friend struct ScopedGLWrapper; + +protected: + GLuint mTexture; + +public: + ScopedTexture(GLContext* gl) + : ScopedGLWrapper(gl) + { + mGL->fGenTextures(1, &mTexture); + } + + GLuint Texture() { return mTexture; } + +protected: + void UnwrapImpl() { + // Check that we're not falling out of scope after + // the current context changed. + MOZ_ASSERT(mGL->IsCurrent()); + + mGL->fDeleteTextures(1, &mTexture); + } +}; + struct ScopedBindTexture : public ScopedGLWrapper { diff --git a/gfx/gl/GLScreenBuffer.cpp b/gfx/gl/GLScreenBuffer.cpp index a6a6dc08d382..b3771db14b26 100644 --- a/gfx/gl/GLScreenBuffer.cpp +++ b/gfx/gl/GLScreenBuffer.cpp @@ -288,6 +288,28 @@ GLScreenBuffer::BeforeReadCall() AssureBlitted(); } +bool +GLScreenBuffer::ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, + GLenum format, GLenum type, GLvoid *pixels) +{ + // If the currently bound framebuffer is backed by a SharedSurface_GL + // then it might want to override how we read pixel data from it. + // This is normally only the default framebuffer, but we can also + // have SharedSurfaces bound to other framebuffers when doing + // readback for BasicLayers. + SharedSurface_GL* surf; + if (GetReadFB() == 0) { + surf = SharedSurf(); + } else { + surf = mGL->mFBOMapping[GetReadFB()]; + } + if (surf) { + return surf->ReadPixels(x, y, width, height, format, type, pixels); + } + + return false; +} + void GLScreenBuffer::RequireBlit() { @@ -593,6 +615,7 @@ ReadBuffer::Create(GLContext* gl, GLuint fb = 0; gl->fGenFramebuffers(1, &fb); gl->AttachBuffersToFB(colorTex, colorRB, depthRB, stencilRB, fb, target); + gl->mFBOMapping[fb] = surf; MOZ_ASSERT(gl->IsFramebufferComplete(fb)); @@ -613,6 +636,7 @@ ReadBuffer::~ReadBuffer() mGL->fDeleteFramebuffers(1, &fb); mGL->fDeleteRenderbuffers(2, rbs); + mGL->mFBOMapping.erase(mFB); } void @@ -641,6 +665,7 @@ ReadBuffer::Attach(SharedSurface_GL* surf) } mGL->AttachBuffersToFB(colorTex, colorRB, 0, 0, mFB, target); + mGL->mFBOMapping[mFB] = surf; MOZ_ASSERT(mGL->IsFramebufferComplete(mFB)); } diff --git a/gfx/gl/GLScreenBuffer.h b/gfx/gl/GLScreenBuffer.h index 0fd6f27b1c71..593a48787450 100644 --- a/gfx/gl/GLScreenBuffer.h +++ b/gfx/gl/GLScreenBuffer.h @@ -244,6 +244,16 @@ public: void AfterDrawCall(); void BeforeReadCall(); + /** + * Attempts to read pixels from the current bound framebuffer, if + * it is backed by a SharedSurface_GL. + * + * Returns true if the pixel data has been read back, false + * otherwise. + */ + bool ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, + GLenum format, GLenum type, GLvoid *pixels); + /* Morph swaps out our SurfaceStream mechanism and replaces it with * one best suited to our platform and compositor configuration. * diff --git a/gfx/gl/SharedSurfaceGL.h b/gfx/gl/SharedSurfaceGL.h index b79b345c95ca..c56df229099d 100644 --- a/gfx/gl/SharedSurfaceGL.h +++ b/gfx/gl/SharedSurfaceGL.h @@ -59,6 +59,11 @@ public: return (SharedSurface_GL*)surf; } + virtual bool ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, + GLenum format, GLenum type, GLvoid *pixels) { + return false; + } + virtual void LockProd(); virtual void UnlockProd(); diff --git a/gfx/gl/SharedSurfaceIO.cpp b/gfx/gl/SharedSurfaceIO.cpp index 0b85db9ea864..92c74109368d 100644 --- a/gfx/gl/SharedSurfaceIO.cpp +++ b/gfx/gl/SharedSurfaceIO.cpp @@ -27,6 +27,29 @@ SharedSurface_IOSurface::Fence() mGL->fFlush(); } +bool +SharedSurface_IOSurface::ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, + GLenum format, GLenum type, GLvoid *pixels) +{ + // Calling glReadPixels when an IOSurface is bound to the current framebuffer + // can cause corruption in following glReadPixel calls (even if they aren't + // reading from an IOSurface). + // We workaround this by copying to a temporary texture, and doing the readback + // from that. + ScopedTexture texture(mGL); + ScopedBindTexture bindTex(mGL, texture.Texture()); + mGL->fCopyTexImage2D(LOCAL_GL_TEXTURE_2D, 0, + HasAlpha() ? LOCAL_GL_RGBA : LOCAL_GL_RGB, + x, y, + width, height, 0); + + ScopedFramebufferForTexture fb(mGL, texture.Texture()); + ScopedBindFramebuffer bindFB(mGL, fb.FB()); + + mGL->fReadPixels(0, 0, width, height, format, type, pixels); + return true; +} + SharedSurface_IOSurface::SharedSurface_IOSurface(MacIOSurface* surface, GLContext* gl, const gfxIntSize& size, diff --git a/gfx/gl/SharedSurfaceIO.h b/gfx/gl/SharedSurfaceIO.h index dee1fc9dba80..8f3bcbb36072 100644 --- a/gfx/gl/SharedSurfaceIO.h +++ b/gfx/gl/SharedSurfaceIO.h @@ -27,6 +27,9 @@ public: virtual void Fence(); virtual bool WaitSync() { return true; } + virtual bool ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, + GLenum format, GLenum type, GLvoid *pixels) MOZ_OVERRIDE; + virtual GLuint Texture() const { return mTexture;