From 25068bf858312f9dcc76aa7b0f3636f2e955076d Mon Sep 17 00:00:00 2001 From: Benoit Girard Date: Fri, 24 Feb 2012 18:17:27 -0500 Subject: [PATCH] Bug 730079 - Move sCurrentGLContext to TLS to support off main thread GLDebug. r=joe --- gfx/gl/GLContext.cpp | 3 +-- gfx/gl/GLContext.h | 35 ++++++++++++++++++--------------- gfx/gl/GLContextProviderEGL.cpp | 9 --------- gfx/thebes/gfxPlatform.cpp | 4 ++++ 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp index 1de053b1f44a..2dbcbd4a2097 100644 --- a/gfx/gl/GLContext.cpp +++ b/gfx/gl/GLContext.cpp @@ -60,8 +60,7 @@ namespace mozilla { namespace gl { #ifdef DEBUG -// see comment near declaration in GLContext.h. Should be thread-local. -GLContext* GLContext::sCurrentGLContext = nsnull; +PRUintn GLContext::sCurrentGLContextTLS = -1; #endif PRUint32 GLContext::sDebugMode = 0; diff --git a/gfx/gl/GLContext.h b/gfx/gl/GLContext.h index 8d226917cc5b..506590bb9b51 100644 --- a/gfx/gl/GLContext.h +++ b/gfx/gl/GLContext.h @@ -602,9 +602,15 @@ public: virtual bool MakeCurrentImpl(bool aForce = false) = 0; +#ifdef DEBUG + static void StaticInit() { + PR_NewThreadPrivateIndex(&sCurrentGLContextTLS, NULL); + } +#endif + bool MakeCurrent(bool aForce = false) { #ifdef DEBUG - sCurrentGLContext = this; + PR_SetThreadPrivate(sCurrentGLContextTLS, this); #endif return MakeCurrentImpl(aForce); } @@ -1505,10 +1511,12 @@ protected: GLContextSymbols mSymbols; #ifdef DEBUG - // this should be thread-local, but that is slightly annoying to implement because on Mac - // we don't have any __thread-like keyword. So for now, MOZ_GL_DEBUG assumes (and asserts) - // that only the main thread is doing OpenGL calls. - static THEBES_API GLContext* sCurrentGLContext; + // 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(); @@ -1632,21 +1640,16 @@ public: void BeforeGLCall(const char* glFunction) { if (DebugMode()) { - // since the static member variable sCurrentGLContext is not thread-local as it should, - // we have to assert that we're in the main thread. Note that sCurrentGLContext is only used - // for the OpenGL debug mode. - if (!NS_IsMainThread()) { - NS_ERROR("OpenGL call from non-main thread. While this is fine in itself, " - "the OpenGL debug mode, which is currently enabled, doesn't support this. " - "It needs to be patched by making GLContext::sCurrentGLContext be thread-local.\n"); - NS_ABORT(); - } + GLContext *currentGLContext = NULL; + + currentGLContext = (GLContext*)PR_GetThreadPrivate(sCurrentGLContextTLS); + if (DebugMode() & DebugTrace) printf_stderr("[gl:%p] > %s\n", this, glFunction); - if (this != sCurrentGLContext) { + if (this != currentGLContext) { printf_stderr("Fatal: %s called on non-current context %p. " "The current context for this thread is %p.\n", - glFunction, this, sCurrentGLContext); + glFunction, this, currentGLContext); NS_ABORT(); } } diff --git a/gfx/gl/GLContextProviderEGL.cpp b/gfx/gl/GLContextProviderEGL.cpp index 2b4ad969e8f7..966295972466 100644 --- a/gfx/gl/GLContextProviderEGL.cpp +++ b/gfx/gl/GLContextProviderEGL.cpp @@ -267,15 +267,6 @@ is_power_of_two(int v) static void BeforeGLCall(const char* glFunction) { if (GLContext::DebugMode()) { - // since the static member variable sCurrentGLContext is not thread-local as it should, - // we have to assert that we're in the main thread. Note that sCurrentGLContext is only used - // for the OpenGL debug mode. - if (!NS_IsMainThread()) { - NS_ERROR("OpenGL call from non-main thread. While this is fine in itself, " - "the OpenGL debug mode, which is currently enabled, doesn't support this. " - "It needs to be patched by making GLContext::sCurrentGLContext be thread-local.\n"); - NS_ABORT(); - } if (GLContext::DebugMode() & GLContext::DebugTrace) printf_stderr("[egl] > %s\n", glFunction); } diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 9fde85d5ceb9..4a0a31606507 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -298,6 +298,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