From c64f3440ffca0a7848459a49ca755a98061799ec Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sun, 1 Sep 2019 22:42:19 +0000 Subject: [PATCH] Bug 1578099 - Change CompositingRenderTargetOGL creation API. r=mattwoodrow Depends on D44326 Differential Revision: https://phabricator.services.mozilla.com/D44327 --HG-- extra : moz-landing-system : lando --- .../opengl/CompositingRenderTargetOGL.cpp | 3 +- .../opengl/CompositingRenderTargetOGL.h | 79 ++++++++++++------- gfx/layers/opengl/CompositorOGL.cpp | 29 ++++--- 3 files changed, 65 insertions(+), 46 deletions(-) diff --git a/gfx/layers/opengl/CompositingRenderTargetOGL.cpp b/gfx/layers/opengl/CompositingRenderTargetOGL.cpp index 1c9512301b77..9ade49ba070c 100644 --- a/gfx/layers/opengl/CompositingRenderTargetOGL.cpp +++ b/gfx/layers/opengl/CompositingRenderTargetOGL.cpp @@ -17,7 +17,8 @@ using namespace mozilla::gfx; using namespace mozilla::gl; CompositingRenderTargetOGL::~CompositingRenderTargetOGL() { - if (mGL && mGL->MakeCurrent()) { + if (mGLResourceOwnership == GLResourceOwnership::OWNED_BY_RENDER_TARGET && + mGL && mGL->MakeCurrent()) { mGL->fDeleteTextures(1, &mTextureHandle); mGL->fDeleteFramebuffers(1, &mFBO); } diff --git a/gfx/layers/opengl/CompositingRenderTargetOGL.h b/gfx/layers/opengl/CompositingRenderTargetOGL.h index 1be5efad9abd..0060a0375b6b 100644 --- a/gfx/layers/opengl/CompositingRenderTargetOGL.h +++ b/gfx/layers/opengl/CompositingRenderTargetOGL.h @@ -40,6 +40,16 @@ class CompositingRenderTargetOGL : public CompositingRenderTarget { friend class CompositorOGL; + enum class GLResourceOwnership : uint8_t { + // Framebuffer and texture will be deleted when the RenderTarget is + // destroyed. + OWNED_BY_RENDER_TARGET, + + // Framebuffer and texture are only used by the RenderTarget, but never + // deleted. + EXTERNALLY_OWNED + }; + // For lazy initialisation of the GL stuff struct InitParams { InitParams() @@ -67,20 +77,6 @@ class CompositingRenderTargetOGL : public CompositingRenderTarget { }; public: - CompositingRenderTargetOGL(CompositorOGL* aCompositor, - const gfx::IntPoint& aOrigin, - const gfx::IntPoint& aClipSpaceOrigin, - GLuint aTexure, GLuint aFBO) - : CompositingRenderTarget(aOrigin), - mInitParams(), - mCompositor(aCompositor), - mGL(aCompositor->gl()), - mClipSpaceOrigin(aClipSpaceOrigin), - mTextureHandle(aTexure), - mFBO(aFBO) { - MOZ_ASSERT(mGL); - } - ~CompositingRenderTargetOGL(); const char* Name() const override { return "CompositingRenderTargetOGL"; } @@ -89,27 +85,29 @@ class CompositingRenderTargetOGL : public CompositingRenderTarget { * Create a render target around the default FBO, for rendering straight to * the window. */ - static already_AddRefed RenderTargetForWindow( + static already_AddRefed CreateForWindow( CompositorOGL* aCompositor, const gfx::IntSize& aSize) { RefPtr result = new CompositingRenderTargetOGL( - aCompositor, gfx::IntPoint(), gfx::IntPoint(), 0, 0); + aCompositor, gfx::IntPoint(), gfx::IntPoint(), + GLResourceOwnership::EXTERNALLY_OWNED, 0, 0); result->mInitParams = InitParams(aSize, aSize, 0, INIT_MODE_NONE); result->mInitParams.mStatus = InitParams::INITIALIZED; return result.forget(); } - /** - * Some initialisation work on the backing FBO and texture. - * We do this lazily so that when we first set this render target on the - * compositor we do not have to re-bind the FBO after unbinding it, or - * alternatively leave the FBO bound after creation. - */ - void Initialize(const gfx::IntSize& aSize, const gfx::IntSize& aPhySize, - GLenum aFBOTextureTarget, SurfaceInitMode aInit) { - MOZ_ASSERT(mInitParams.mStatus == InitParams::NO_PARAMS, - "Initialized twice?"); - // postpone initialization until we actually want to use this render target - mInitParams = InitParams(aSize, aPhySize, aFBOTextureTarget, aInit); + static already_AddRefed + CreateForNewFBOAndTakeOwnership(CompositorOGL* aCompositor, GLuint aTexture, + GLuint aFBO, const gfx::IntRect& aRect, + const gfx::IntPoint& aClipSpaceOrigin, + const gfx::IntSize& aPhySize, + GLenum aFBOTextureTarget, + SurfaceInitMode aInit) { + RefPtr result = new CompositingRenderTargetOGL( + aCompositor, aRect.TopLeft(), aClipSpaceOrigin, + GLResourceOwnership::OWNED_BY_RENDER_TARGET, aTexture, aFBO); + result->mInitParams = + InitParams(aRect.Size(), aPhySize, aFBOTextureTarget, aInit); + return result.forget(); } void BindTexture(GLenum aTextureUnit, GLenum aTextureTarget); @@ -161,9 +159,29 @@ class CompositingRenderTargetOGL : public CompositingRenderTarget { const gfx::IntSize& GetInitSize() const { return mInitParams.mSize; } private: + CompositingRenderTargetOGL(CompositorOGL* aCompositor, + const gfx::IntPoint& aOrigin, + const gfx::IntPoint& aClipSpaceOrigin, + GLResourceOwnership aGLResourceOwnership, + GLuint aTexure, GLuint aFBO) + : CompositingRenderTarget(aOrigin), + mInitParams(), + mCompositor(aCompositor), + mGL(aCompositor->gl()), + mClipSpaceOrigin(aClipSpaceOrigin), + mGLResourceOwnership(aGLResourceOwnership), + mTextureHandle(aTexure), + mFBO(aFBO) { + MOZ_ASSERT(mGL); + } + /** - * Actually do the initialisation. Note that we leave our FBO bound, and so - * calling this method is only suitable when about to use this render target. + * Actually do the initialisation. + * We do this lazily so that when we first set this render target on the + * compositor we do not have to re-bind the FBO after unbinding it, or + * alternatively leave the FBO bound after creation. Note that we leave our + * FBO bound, and so calling this method is only suitable when about to use + * this render target. */ void InitializeImpl(); @@ -177,6 +195,7 @@ class CompositingRenderTargetOGL : public CompositingRenderTarget { RefPtr mGL; Maybe mClipRect; gfx::IntPoint mClipSpaceOrigin; + GLResourceOwnership mGLResourceOwnership; GLuint mTextureHandle; GLuint mFBO; }; diff --git a/gfx/layers/opengl/CompositorOGL.cpp b/gfx/layers/opengl/CompositorOGL.cpp index 2c7422399e75..f8277c767d52 100644 --- a/gfx/layers/opengl/CompositorOGL.cpp +++ b/gfx/layers/opengl/CompositorOGL.cpp @@ -640,12 +640,11 @@ already_AddRefed CompositorOGL::CreateRenderTarget( GLuint tex = 0; GLuint fbo = 0; IntRect rect = aRect; - IntSize FBOSize; - CreateFBOWithTexture(rect, false, 0, &fbo, &tex, &FBOSize); - RefPtr surface = new CompositingRenderTargetOGL( - this, aRect.TopLeft(), aRect.TopLeft(), tex, fbo); - surface->Initialize(aRect.Size(), FBOSize, mFBOTextureTarget, aInit); - return surface.forget(); + IntSize fboSize; + CreateFBOWithTexture(rect, false, 0, &fbo, &tex, &fboSize); + return CompositingRenderTargetOGL::CreateForNewFBOAndTakeOwnership( + this, tex, fbo, aRect, aRect.TopLeft(), aRect.Size(), mFBOTextureTarget, + aInit); } already_AddRefed @@ -671,11 +670,9 @@ CompositorOGL::CreateRenderTargetFromSource( IntRect sourceRect(aSourcePoint, aRect.Size()); CreateFBOWithTexture(sourceRect, true, sourceSurface->GetFBO(), &fbo, &tex); - RefPtr surface = new CompositingRenderTargetOGL( - this, aRect.TopLeft(), aRect.TopLeft(), tex, fbo); - surface->Initialize(aRect.Size(), sourceRect.Size(), mFBOTextureTarget, - INIT_MODE_NONE); - return surface.forget(); + return CompositingRenderTargetOGL::CreateForNewFBOAndTakeOwnership( + this, tex, fbo, aRect, aRect.TopLeft(), sourceRect.Size(), + mFBOTextureTarget, INIT_MODE_NONE); } void CompositorOGL::SetRenderTarget(CompositingRenderTarget* aSurface) { @@ -808,9 +805,11 @@ void CompositorOGL::RegisterIOSurface(IOSurfacePtr aSurface) { LOCAL_GL_TEXTURE_RECTANGLE_ARB, tex, 0); } - RefPtr rt = new CompositingRenderTargetOGL( - this, gfx::IntPoint(), gfx::IntPoint(), tex, fbo); - rt->Initialize(size, size, LOCAL_GL_TEXTURE_RECTANGLE_ARB, INIT_MODE_NONE); + IntRect rect(IntPoint(), size); + RefPtr rt = + CompositingRenderTargetOGL::CreateForNewFBOAndTakeOwnership( + this, tex, fbo, rect, IntPoint(), size, + LOCAL_GL_TEXTURE_RECTANGLE_ARB, INIT_MODE_NONE); mRegisteredIOSurfaceRenderTargets.insert({aSurface, rt}); } @@ -1100,7 +1099,7 @@ Maybe CompositorOGL::BeginFrame(const nsIntRegion& aInvalidRegion, RefPtr rt; if (mCanRenderToDefaultFramebuffer) { - rt = CompositingRenderTargetOGL::RenderTargetForWindow(this, rect.Size()); + rt = CompositingRenderTargetOGL::CreateForWindow(this, rect.Size()); } else if (mTarget) { rt = CreateRenderTarget(rect, INIT_MODE_CLEAR); } else {