From c6dbf554496191a0cfe4c8f5dbe8c96031d1445b Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Mon, 20 Jan 2020 22:35:36 +0100 Subject: Issue #1354 - Clear the current context when MakeCurrent() fails. --- dom/canvas/WebGLContextValidate.cpp | 7 ++++++ gfx/gl/GLContext.cpp | 13 +++++----- gfx/gl/GLContext.h | 6 ++--- gfx/gl/GLContextCGL.h | 2 +- gfx/gl/GLContextGLX.h | 1 + gfx/gl/GLContextProviderCGL.mm | 22 ++++++----------- gfx/gl/GLContextProviderEAGL.mm | 41 ++++++++++++-------------------- gfx/gl/GLContextProviderEGL.cpp | 47 +++++++++++++++++++------------------ gfx/gl/GLContextProviderGLX.cpp | 24 +++++++++---------- gfx/gl/GLContextProviderWGL.cpp | 13 ++++------ 10 files changed, 81 insertions(+), 95 deletions(-) diff --git a/dom/canvas/WebGLContextValidate.cpp b/dom/canvas/WebGLContextValidate.cpp index ebf0aa8c2..30d4c6522 100644 --- a/dom/canvas/WebGLContextValidate.cpp +++ b/dom/canvas/WebGLContextValidate.cpp @@ -440,6 +440,13 @@ WebGLContext::InitAndValidateGL(FailureReason* const out_failReason) { MOZ_RELEASE_ASSERT(gl, "GFX: GL not initialized"); + if (!gl->MakeCurrent(true)) { + MOZ_ASSERT(false); + *out_failReason = { "FEATURE_FAILURE_WEBGL_MAKECURRENT", + "Failed to MakeCurrent for init." }; + return false; + } + // Unconditionally create a new format usage authority. This is // important when restoring contexts and extensions need to add // formats back into the authority. diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp index 1515b8627..3fb87822d 100644 --- a/gfx/gl/GLContext.cpp +++ b/gfx/gl/GLContext.cpp @@ -578,6 +578,10 @@ GLContext::LoadFeatureSymbols(const char* prefix, bool trygl, const SymLoadStruc bool GLContext::InitWithPrefixImpl(const char* prefix, bool trygl) { + // wglGetProcAddress requires a current context. + if (!MakeCurrent(true)) + return false; + mWorkAroundDriverBugs = gfxPrefs::WorkAroundDriverBugs(); const SymLoadStruct coreSymbols[] = { @@ -714,7 +718,6 @@ GLContext::InitWithPrefixImpl(const char* prefix, bool trygl) //////////////// - MakeCurrent(); MOZ_ASSERT(mProfile != ContextProfile::Unknown); uint32_t version = 0; @@ -2253,13 +2256,11 @@ GLContext::MarkDestroyed() mBlitHelper = nullptr; mReadTexImageHelper = nullptr; - if (MakeCurrent()) { + mIsDestroyed = true; + mSymbols.Zero(); + if (MakeCurrent(true)) { mTexGarbageBin->GLContextTeardown(); - } else { - NS_WARNING("MakeCurrent() failed during MarkDestroyed! Skipping GL object teardown."); } - - mSymbols.Zero(); } #ifdef MOZ_GL_DEBUG diff --git a/gfx/gl/GLContext.h b/gfx/gl/GLContext.h index 6e3e22207..4c1f4f55c 100644 --- a/gfx/gl/GLContext.h +++ b/gfx/gl/GLContext.h @@ -353,6 +353,7 @@ public: protected: bool mIsOffscreen; bool mContextLost; + bool mIsDestroyed = false; /** * mVersion store the OpenGL's version, multiplied by 100. For example, if @@ -3265,7 +3266,7 @@ public: #endif return MakeCurrentImpl(aForce); } - + virtual bool Init() = 0; virtual bool SetupLookupFunction() = 0; @@ -3273,8 +3274,7 @@ public: virtual void ReleaseSurface() {} bool IsDestroyed() { - // MarkDestroyed will mark all these as null. - return mSymbols.fUseProgram == nullptr; + return mIsDestroyed; } GLContext* GetSharedContext() { return mSharedContext; } diff --git a/gfx/gl/GLContextCGL.h b/gfx/gl/GLContextCGL.h index 1a29f3d15..23616d861 100644 --- a/gfx/gl/GLContextCGL.h +++ b/gfx/gl/GLContextCGL.h @@ -24,7 +24,7 @@ class GLContextCGL : public GLContext { friend class GLContextProviderCGL; - NSOpenGLContext* mContext; + NSOpenGLContext* const mContext; public: MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(GLContextCGL, override) diff --git a/gfx/gl/GLContextGLX.h b/gfx/gl/GLContextGLX.h index 1f2cee08d..0463669e9 100644 --- a/gfx/gl/GLContextGLX.h +++ b/gfx/gl/GLContextGLX.h @@ -86,6 +86,7 @@ private: GLXContext mContext; Display* mDisplay; GLXDrawable mDrawable; + Maybe mOverrideDrawable; bool mDeleteDrawable; bool mDoubleBuffered; diff --git a/gfx/gl/GLContextProviderCGL.mm b/gfx/gl/GLContextProviderCGL.mm index ceab3046c..7cc89eac9 100644 --- a/gfx/gl/GLContextProviderCGL.mm +++ b/gfx/gl/GLContextProviderCGL.mm @@ -83,26 +83,13 @@ GLContextCGL::GLContextCGL(CreateContextFlags flags, const SurfaceCaps& caps, GLContextCGL::~GLContextCGL() { MarkDestroyed(); - - if (mContext) { - if ([NSOpenGLContext currentContext] == mContext) { - // Clear the current context before releasing. If we don't do - // this, the next time we call [NSOpenGLContext currentContext], - // "invalid context" will be printed to the console. - [NSOpenGLContext clearCurrentContext]; - } - [mContext release]; - } - + [mContext release]; } bool GLContextCGL::Init() { - if (!InitWithPrefix("gl", true)) - return false; - - return true; + return InitWithPrefix("gl", true); } CGLContextObj @@ -114,6 +101,11 @@ GLContextCGL::GetCGLContext() const bool GLContextCGL::MakeCurrentImpl(bool aForce) { + if (IsDestroyed()) { + [NSOpenGLContext clearCurrentContext]; + return false; + } + if (!aForce && [NSOpenGLContext currentContext] == mContext) { return true; } diff --git a/gfx/gl/GLContextProviderEAGL.mm b/gfx/gl/GLContextProviderEAGL.mm index 507616e2f..11c7cce77 100644 --- a/gfx/gl/GLContextProviderEAGL.mm +++ b/gfx/gl/GLContextProviderEAGL.mm @@ -36,35 +36,24 @@ GLContextEAGL::GLContextEAGL(CreateContextFlags flags, const SurfaceCaps& caps, GLContextEAGL::~GLContextEAGL() { - MakeCurrent(); - - if (mBackbufferFB) { - fDeleteFramebuffers(1, &mBackbufferFB); - } - - if (mBackbufferRB) { - fDeleteRenderbuffers(1, &mBackbufferRB); + if (MakeCurrent()) { + if (mBackbufferFB) { + fDeleteFramebuffers(1, &mBackbufferFB); + } + if (mBackbufferRB) { + fDeleteRenderbuffers(1, &mBackbufferRB); + } } + mLayer = nil; MarkDestroyed(); - - if (mLayer) { - mLayer = nil; - } - - if (mContext) { - [EAGLContext setCurrentContext:nil]; - [mContext release]; - } + [mContext release]; } bool GLContextEAGL::Init() { - if (!InitWithPrefix("gl", true)) - return false; - - return true; + return InitWithPrefix("gl", true); } bool @@ -120,12 +109,12 @@ GLContextEAGL::MakeCurrentImpl(bool aForce) return true; } - if (mContext) { - if(![EAGLContext setCurrentContext:mContext]) { - return false; - } + if (IsDestroyed()) { + [EAGLContext setCurrentContext:nil]; + return false; } - return true; + + return [EAGLContext setCurrentContext:mContext]; } bool diff --git a/gfx/gl/GLContextProviderEGL.cpp b/gfx/gl/GLContextProviderEGL.cpp index 7979f3bf0..23fc3472d 100644 --- a/gfx/gl/GLContextProviderEGL.cpp +++ b/gfx/gl/GLContextProviderEGL.cpp @@ -150,13 +150,12 @@ is_power_of_two(int v) } static void -DestroySurface(EGLSurface oldSurface) { - if (oldSurface != EGL_NO_SURFACE) { - sEGLLibrary.fMakeCurrent(EGL_DISPLAY(), - EGL_NO_SURFACE, EGL_NO_SURFACE, - EGL_NO_CONTEXT); - sEGLLibrary.fDestroySurface(EGL_DISPLAY(), oldSurface); +DestroySurface(const EGLSurface surf) { + if (!surf) { + // Nothing to do. + return; } + sEGLLibrary.fDestroySurface(EGL_DISPLAY(), surf); } static EGLSurface @@ -223,7 +222,7 @@ GLContextEGL::~GLContextEGL() sEGLLibrary.fDestroyContext(EGL_DISPLAY(), mContext); sEGLLibrary.UnsetCachedCurrentContext(); - mozilla::gl::DestroySurface(mSurface); + DestroySurface(mSurface); } bool @@ -247,12 +246,7 @@ GLContextEGL::Init() if (!InitWithPrefix("gl", true)) return false; - bool current = MakeCurrent(); - if (!current) { - gfx::LogFailure(NS_LITERAL_CSTRING( - "Couldn't get device attachments for device.")); - return false; - } + MOZ_ASSERT(IsCurrent()); static_assert(sizeof(GLint) >= sizeof(int32_t), "GLint is smaller than int32_t"); mMaxTextureImageSize = INT32_MAX; @@ -303,7 +297,10 @@ GLContextEGL::ReleaseTexImage() } void -GLContextEGL::SetEGLSurfaceOverride(EGLSurface surf) { +GLContextEGL::SetEGLSurfaceOverride(const EGLSurface surf) { + + MOZ_ASSERT(!surf || surf != mSurface); + if (Screen()) { /* Blit `draw` to `read` if we need to, before we potentially juggle * `read` around. If we don't, we might attach a different `read`, @@ -314,12 +311,17 @@ GLContextEGL::SetEGLSurfaceOverride(EGLSurface surf) { } mSurfaceOverride = surf; - DebugOnly ok = MakeCurrent(true); - MOZ_ASSERT(ok); + MOZ_ALWAYS_TRUE(MakeCurrent(true)); } bool GLContextEGL::MakeCurrentImpl(bool aForce) { + if (IsDestroyed()) { + //Clear and exit + sEGLLibrary.fMakeCurrent(EGL_DISPLAY(), nullptr, nullptr, nullptr); + return false; + } + bool succeeded = true; // Assume that EGL has the same problem as WGL does, @@ -373,7 +375,7 @@ GLContextEGL::IsCurrent() { } bool -GLContextEGL::RenewSurface(nsIWidget* aWidget) { +GLContextEGL::RenewSurface(nsIWidget* const aWidget) { if (!mOwnsContext) { return false; } @@ -389,13 +391,12 @@ GLContextEGL::RenewSurface(nsIWidget* aWidget) { void GLContextEGL::ReleaseSurface() { - if (mOwnsContext) { - mozilla::gl::DestroySurface(mSurface); - } - if (mSurface == mSurfaceOverride) { - mSurfaceOverride = EGL_NO_SURFACE; + if (!mOwnsContext) { + return; } - mSurface = EGL_NO_SURFACE; + + DestroySurface(mSurface); + mSurface = nullptr; } bool diff --git a/gfx/gl/GLContextProviderGLX.cpp b/gfx/gl/GLContextProviderGLX.cpp index 539520a8c..178052f8c 100644 --- a/gfx/gl/GLContextProviderGLX.cpp +++ b/gfx/gl/GLContextProviderGLX.cpp @@ -894,20 +894,12 @@ GLContextGLX::~GLContextGLX() return; } - // see bug 659842 comment 76 -#ifdef DEBUG - bool success = -#endif - mGLX->xMakeCurrent(mDisplay, X11None, nullptr); - MOZ_ASSERT(success, - "glXMakeCurrent failed to release GL context before we call " - "glXDestroyContext!"); - mGLX->xDestroyContext(mDisplay, mContext); if (mDeleteDrawable) { mGLX->xDestroyPixmap(mDisplay, mDrawable); } + MOZ_ASSERT(!mOverrideDrawable); } @@ -944,6 +936,10 @@ GLContextGLX::MakeCurrentImpl(bool aForce) // DRI2InvalidateBuffers event before drawing. See bug 1280653. Unused << XPending(mDisplay); } + if (IsDestroyed()) { + MOZ_ALWAYS_TRUE(mGLX->xMakeCurrent(mDisplay, X11None, nullptr); + return false; // We did not MakeCurrent mContext, but that's what we wanted! + } succeeded = mGLX->xMakeCurrent(mDisplay, mDrawable, mContext); NS_ASSERTION(succeeded, "Failed to make GL context current!"); @@ -1017,16 +1013,18 @@ GLContextGLX::GetWSIInfo(nsCString* const out) const bool GLContextGLX::OverrideDrawable(GLXDrawable drawable) { - if (Screen()) + if (Screen()) { Screen()->AssureBlitted(); - Bool result = mGLX->xMakeCurrent(mDisplay, drawable, mContext); - return result; + } + mOverrideDrawable = Some(drawable); + return MakeCurrent(true); } bool GLContextGLX::RestoreDrawable() { - return mGLX->xMakeCurrent(mDisplay, mDrawable, mContext); + mOverrideDrawable = Nothing(); + return MakeCurrent(true); } GLContextGLX::GLContextGLX( diff --git a/gfx/gl/GLContextProviderWGL.cpp b/gfx/gl/GLContextProviderWGL.cpp index 35957259d..da8c93d10 100644 --- a/gfx/gl/GLContextProviderWGL.cpp +++ b/gfx/gl/GLContextProviderWGL.cpp @@ -314,20 +314,17 @@ GLContextWGL::Init() if (!mDC || !mContext) return false; - // see bug 929506 comment 29. wglGetProcAddress requires a current context. - if (!sWGLLib.fMakeCurrent(mDC, mContext)) - return false; - SetupLookupFunction(); - if (!InitWithPrefix("gl", true)) - return false; - - return true; + return InitWithPrefix("gl", true); } bool GLContextWGL::MakeCurrentImpl(bool aForce) { + if (IsDestroyed()) { + MOZ_ALWAYS_TRUE(sWGLLib.fMakeCurrent(0, 0)); + } + BOOL succeeded = true; // wglGetCurrentContext seems to just pull the HGLRC out -- cgit v1.2.3