diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp index 53252838ffed..e5aae8a73df5 100644 --- a/gfx/gl/GLContext.cpp +++ b/gfx/gl/GLContext.cpp @@ -64,8 +64,9 @@ using namespace mozilla::gfx; namespace mozilla { namespace gl { -tls::key GLContextTLSStorage::sTLSKey; -bool GLContextTLSStorage::sTLSKeyAlreadyCreated = false; +#ifdef DEBUG +PRUintn GLContext::sCurrentGLContextTLS = -1; +#endif PRUint32 GLContext::sDebugMode = 0; diff --git a/gfx/gl/GLContext.h b/gfx/gl/GLContext.h index 9f8d256e0e9f..38924561ed40 100644 --- a/gfx/gl/GLContext.h +++ b/gfx/gl/GLContext.h @@ -67,7 +67,6 @@ #include "nsRegion.h" #include "nsAutoPtr.h" #include "nsThreadUtils.h" -#include "thread_helper.h" typedef char realGLboolean; @@ -525,77 +524,6 @@ struct THEBES_API ContextFormat int colorBits() const { return red + green + blue; } }; -/* - * This is a helper class to do the little bit of TLS storage that we need - * to allow GLContext to keep track of the current GLContext for a given thread. - * - * This is mostly an optimization to avoid calling MakeCurrent on an - * already-current context,which depending on OpenGL libraries/drivers can be - * very expensive. An earlier optimization consisted in calling - * getCurrentContext to check if the context was already current, but - * even that was shown to be very slow at least on Mac and on Linux NVIDIA, - * see bug 749678. - * - * In a general setting, we would have to do a TLS lookup on every MakeCurrent - * call. But in GLContext, we currently assume that we only ever make GL calls - * on a given GLContext in the same thread that created it (the "owning thread"). - * That assumption allows us to avoid doing a TLS lookup on every MakeCurrent - * call. It's checked by assertions in MOZ_GL_DEBUG mode. - * - * The way this works is that inside each GLContext, we store a pointer to the - * TLS pointer to the current context for this thread. This pointer-to-pointer - * (mStorage->mCurrentGLContext) is set during GL context creation: that's where - * we rely on the assumption that all GL calls on a given context are made on - * the same thread that created that context. - */ -class GLContextTLSStorage -{ - struct Storage - { - GLContext *mCurrentGLContext; - - NS_INLINE_DECL_REFCOUNTING(Storage) - - Storage() : mCurrentGLContext(nsnull) {} - - ~Storage() { - // avoid keeping a dangling TLS pointer to the Storage object being destroyed - tls::set(sTLSKey, nsnull); - } - }; - - nsRefPtr mStorage; - static tls::key sTLSKey; - static bool sTLSKeyAlreadyCreated; - -public: - - GLContextTLSStorage() { - if (!sTLSKeyAlreadyCreated) { - tls::create(&sTLSKey); - sTLSKeyAlreadyCreated = true; - } - - mStorage = tls::get(sTLSKey); - - if (!mStorage) { - mStorage = new Storage; - tls::set(sTLSKey, mStorage); - } - } - - ~GLContextTLSStorage() { - } - - GLContext *CurrentGLContext() const { - return mStorage->mCurrentGLContext; - } - - void SetCurrentGLContext(GLContext *c) { - mStorage->mCurrentGLContext = c; - } -}; - class GLContext : public GLLibraryLoader { @@ -647,7 +575,7 @@ public: } virtual ~GLContext() { - NS_ABORT_IF_FALSE(IsDestroyed(), "GLContext implementation must call MarkDestroyed in destructor!"); + NS_ASSERTION(IsDestroyed(), "GLContext implementation must call MarkDestroyed in destructor!"); #ifdef DEBUG if (mSharedContext) { GLContext *tip = mSharedContext; @@ -657,8 +585,6 @@ public: tip->ReportOutstandingNames(); } #endif - if (this == CurrentGLContext()) - SetCurrentGLContext(nsnull); } enum ContextFlags { @@ -677,43 +603,19 @@ public: virtual GLContextType GetContextType() { return ContextTypeUnknown; } - virtual bool MakeCurrentImpl() = 0; + virtual bool MakeCurrentImpl(bool aForce = false) = 0; - void CheckOwningThreadInDebugMode() { #ifdef DEBUG - if (!NS_GetCurrentThread()) { - // happens during shutdown. Drop this check in that case. - return; - } - if (!IsOwningThreadCurrent()) - { - printf_stderr( - "This GL context (%p) is owned by thread %p, but the current thread is %p. " - "That's fine by itself, but our current code in GLContext::MakeCurrent, checking " - "if the context is already current, relies on the assumption that GL calls on a given " - "GLContext are only made by the thread that created that GLContext. If you want to " - "start making GL calls from non-owning threads, you'll have to change a few things " - "around here, see Bug 749678 comments 13 and 15.\n", - this, mOwningThread.get(), NS_GetCurrentThread()); - NS_ABORT(); - } -#endif + static void StaticInit() { + PR_NewThreadPrivateIndex(&sCurrentGLContextTLS, NULL); } +#endif bool MakeCurrent(bool aForce = false) { - CheckOwningThreadInDebugMode(); - - if (!aForce && - this == CurrentGLContext()) - { - return true; - } - - bool success = MakeCurrentImpl(); - if (success) { - SetCurrentGLContext(this); - } - return success; +#ifdef DEBUG + PR_SetThreadPrivate(sCurrentGLContextTLS, this); +#endif + return MakeCurrentImpl(aForce); } bool IsContextLost() { return mContextLost; } @@ -1200,16 +1102,6 @@ public: } private: - - GLContext *CurrentGLContext() const { - return mTLSStorage.CurrentGLContext(); - } - - void SetCurrentGLContext(GLContext *c) { - mTLSStorage.SetCurrentGLContext(c); - } - - bool mOffscreenFBOsDirty; void GetShaderPrecisionFormatNonES2(GLenum shadertype, GLenum precisiontype, GLint* range, GLint* precision) { @@ -1737,6 +1629,15 @@ protected: GLContextSymbols mSymbols; +#ifdef DEBUG + // GLDebugMode will check that we don't send call + // to a GLContext that isn't current on the current + // thread. + // Store the current context when binding to thread local + // storage to support DebugMode on an arbitrary thread. + static PRUintn sCurrentGLContextTLS; +#endif + void UpdateActualFormat(); ContextFormat mActualFormat; @@ -1745,8 +1646,6 @@ protected: GLuint mOffscreenTexture; bool mFlipped; - GLContextTLSStorage mTLSStorage; - // lazy-initialized things GLuint mBlitProgram, mBlitFramebuffer; void UseBlitProgram(); @@ -1924,13 +1823,16 @@ public: void BeforeGLCall(const char* glFunction) { if (DebugMode()) { + GLContext *currentGLContext = NULL; + + currentGLContext = (GLContext*)PR_GetThreadPrivate(sCurrentGLContextTLS); + if (DebugMode() & DebugTrace) printf_stderr("[gl:%p] > %s\n", this, glFunction); - CheckOwningThreadInDebugMode(); - if (this != CurrentGLContext()) { + if (this != currentGLContext) { printf_stderr("Fatal: %s called on non-current context %p. " "The current context for this thread is %p.\n", - glFunction, this, CurrentGLContext()); + glFunction, this, currentGLContext); NS_ABORT(); } } diff --git a/gfx/gl/GLContextProviderCGL.mm b/gfx/gl/GLContextProviderCGL.mm index dfba67dd8066..4abd2bb75ba6 100644 --- a/gfx/gl/GLContextProviderCGL.mm +++ b/gfx/gl/GLContextProviderCGL.mm @@ -171,8 +171,12 @@ public: } } - bool MakeCurrentImpl() + bool MakeCurrentImpl(bool aForce = false) { + if (!aForce && [NSOpenGLContext currentContext] == mContext) { + return true; + } + if (mContext) { [mContext makeCurrentContext]; } diff --git a/gfx/gl/GLContextProviderEGL.cpp b/gfx/gl/GLContextProviderEGL.cpp index 654000d70d69..2f8a46b20961 100644 --- a/gfx/gl/GLContextProviderEGL.cpp +++ b/gfx/gl/GLContextProviderEGL.cpp @@ -420,7 +420,7 @@ public: return true; } - bool MakeCurrentImpl() { + bool MakeCurrentImpl(bool aForce = false) { bool succeeded = true; // Assume that EGL has the same problem as WGL does, @@ -442,24 +442,25 @@ public: return succeeded; } #endif - + if (aForce || sEGLLibrary.fGetCurrentContext() != mContext) { #ifdef MOZ_WIDGET_QT - // Shared Qt GL context need to be informed about context switch - if (mSharedContext) { - QGLContext* qglCtx = static_cast(static_cast(mSharedContext.get())->mPlatformContext); - if (qglCtx) { - qglCtx->doneCurrent(); + // Shared Qt GL context need to be informed about context switch + if (mSharedContext) { + QGLContext* qglCtx = static_cast(static_cast(mSharedContext.get())->mPlatformContext); + if (qglCtx) { + qglCtx->doneCurrent(); + } } - } #endif - succeeded = sEGLLibrary.fMakeCurrent(EGL_DISPLAY(), - mSurface, mSurface, - mContext); - if (!succeeded && sEGLLibrary.fGetError() == LOCAL_EGL_CONTEXT_LOST) { - mContextLost = true; - NS_WARNING("EGL context has been lost."); + succeeded = sEGLLibrary.fMakeCurrent(EGL_DISPLAY(), + mSurface, mSurface, + mContext); + if (!succeeded && sEGLLibrary.fGetError() == LOCAL_EGL_CONTEXT_LOST) { + mContextLost = true; + NS_WARNING("EGL context has been lost."); + } + NS_ASSERTION(succeeded, "Failed to make GL context current!"); } - NS_ASSERTION(succeeded, "Failed to make GL context current!"); return succeeded; } diff --git a/gfx/gl/GLContextProviderGLX.cpp b/gfx/gl/GLContextProviderGLX.cpp index 23331becdcbc..8956ac25fdb4 100644 --- a/gfx/gl/GLContextProviderGLX.cpp +++ b/gfx/gl/GLContextProviderGLX.cpp @@ -793,10 +793,20 @@ TRY_AGAIN_NO_SHARING: return true; } - bool MakeCurrentImpl() + bool MakeCurrentImpl(bool aForce = false) { - bool succeeded = sGLXLibrary.xMakeCurrent(mDisplay, mDrawable, mContext); - NS_ASSERTION(succeeded, "Failed to make GL context current!"); + bool succeeded = true; + + // With the ATI FGLRX driver, glxMakeCurrent is very slow even when the context doesn't change. + // (This is not the case with other drivers such as NVIDIA). + // So avoid calling it more than necessary. Since GLX documentation says that: + // "glXGetCurrentContext returns client-side information. + // It does not make a round trip to the server." + // I assume that it's not worth using our own TLS slot here. + if (aForce || sGLXLibrary.xGetCurrentContext() != mContext) { + succeeded = sGLXLibrary.xMakeCurrent(mDisplay, mDrawable, mContext); + NS_ASSERTION(succeeded, "Failed to make GL context current!"); + } return succeeded; } diff --git a/gfx/gl/GLContextProviderOSMesa.cpp b/gfx/gl/GLContextProviderOSMesa.cpp index 86d9b0706aea..f4e027dfbe60 100644 --- a/gfx/gl/GLContextProviderOSMesa.cpp +++ b/gfx/gl/GLContextProviderOSMesa.cpp @@ -213,7 +213,7 @@ public: return InitWithPrefix("gl", true); } - bool MakeCurrentImpl() + bool MakeCurrentImpl(bool aForce = false) { bool succeeded = sOSMesaLibrary.fMakeCurrent(mContext, mThebesSurface->Data(), diff --git a/gfx/gl/GLContextProviderWGL.cpp b/gfx/gl/GLContextProviderWGL.cpp index de36a1c4988b..8bcabfe7a5f4 100644 --- a/gfx/gl/GLContextProviderWGL.cpp +++ b/gfx/gl/GLContextProviderWGL.cpp @@ -333,10 +333,18 @@ public: return true; } - bool MakeCurrentImpl() + bool MakeCurrentImpl(bool aForce = false) { - bool succeeded = sWGLLibrary.fMakeCurrent(mDC, mContext); - NS_ASSERTION(succeeded, "Failed to make GL context current!"); + BOOL succeeded = true; + + // wglGetCurrentContext seems to just pull the HGLRC out + // of its TLS slot, so no need to do our own tls slot. + // You would think that wglMakeCurrent would avoid doing + // work if mContext was already current, but not so much.. + if (aForce || sWGLLibrary.fGetCurrentContext() != mContext) { + succeeded = sWGLLibrary.fMakeCurrent(mDC, mContext); + NS_ASSERTION(succeeded, "Failed to make GL context current!"); + } return succeeded; } diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 5b06de6de681..a66a4a48d027 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -301,6 +301,10 @@ gfxPlatform::Init() #error "No gfxPlatform implementation available" #endif +#ifdef DEBUG + mozilla::gl::GLContext::StaticInit(); +#endif + nsresult rv; #if defined(XP_MACOSX) || defined(XP_WIN) || defined(ANDROID) // temporary, until this is implemented on others