Bug 656824: changed WebGL robustness timer to not get reset after each draw operation r=bjacob

This patch should improve performance of the robustness timer by not resetting
it each time a draw operation happens. It still checks if there's any activity
and, if not, it will stop firing it. It includes a single extra timer firing
after activity dies to make sure we don't miss anything. Also includes a fix for
robustness being marked as existing when it really isn't.

content/canvas/src/WebGLContext.cpp

@@ -301,16 +301,17 @@ WebGLContext::WebGLContext()
 
     mContextLost = false;
     mAllowRestore = false;
+    mRobustnessTimerRunning = false;
+    mDrawSinceRobustnessTimerSet = false;
+    mContextRestorer = do_CreateInstance("@mozilla.org/timer;1");
 }
 
 WebGLContext::~WebGLContext()
 {
     DestroyResourcesAndContext();
     WebGLMemoryReporter::RemoveWebGLContext(this);
-    if (mContextRestorer) {
-        mContextRestorer->Cancel();
-        mContextRestorer = NULL;
-    }
+    TerminateRobustnessTimer();
+    mContextRestorer = nsnull;
 }
 
 static PLDHashOperator
@@ -709,6 +710,8 @@ WebGLContext::SetDimensions(PRInt32 width, PRInt32 height)
     mResetLayer = true;
     mOptionsFrozen = true;
 
+    mHasRobustness = gl->HasRobustness();
+
     // increment the generation number
     ++mGeneration;
 
@@ -1147,13 +1150,8 @@ WebGLContext::EnsureBackbufferClearedAsNeeded()
 NS_IMETHODIMP
 WebGLContext::Notify(nsITimer* timer)
 {
+    TerminateRobustnessTimer();
     MaybeRestoreContext();
-
-    if (mContextRestorer) {
-        mContextRestorer->Cancel();
-        mContextRestorer = NULL;
-    }
-
     return NS_OK;
 }
 
@@ -1163,6 +1161,7 @@ WebGLContext::MaybeRestoreContext()
     if (mContextLost || mAllowRestore)
         return;
 
+    gl->MakeCurrent();
     GLContext::ContextResetARB resetStatus = 
         (GLContext::ContextResetARB) gl->fGetGraphicsResetStatus();
     
@@ -1174,6 +1173,11 @@ WebGLContext::MaybeRestoreContext()
 
     switch (resetStatus) {
         case GLContext::CONTEXT_NO_ERROR:
+            // If there has been activity since the timer was set, it's possible
+            // that we did or are going to miss something, so clear this flag and
+            // run it again some time later.
+            if (mDrawSinceRobustnessTimerSet)
+                SetupRobustnessTimer();
             return;
         case GLContext::CONTEXT_GUILTY_CONTEXT_RESET_ARB:
             NS_WARNING("WebGL content on the page caused the graphics card to reset; not restoring the context");
@@ -1191,10 +1195,7 @@ WebGLContext::MaybeRestoreContext()
 void
 WebGLContext::ForceLoseContext()
 {
-    if (mContextRestorer) {
-        mContextRestorer->Cancel();
-        mContextRestorer = NULL;
-    }
+    TerminateRobustnessTimer();
 
     mWebGLError = LOCAL_GL_CONTEXT_LOST;
 

content/canvas/src/WebGLContext.h

@@ -449,17 +449,30 @@ public:
     // Sets up the GL_ARB_robustness timer if it isn't already, so that if the
     // driver gets restarted, the context may get reset with it.
     void SetupRobustnessTimer() {
-        if (mContextLost)
+        if (mContextLost || !mHasRobustness)
             return;
 
-        if (!mContextRestorer)
-            mContextRestorer = do_CreateInstance("@mozilla.org/timer;1");
+        // If the timer was already running, don't restart it here. Instead,
+        // wait until the previous call is done, then fire it one more time.
+        // This is an optimization to prevent unnecessary cross-communication
+        // between threads.
+        if (mRobustnessTimerRunning) {
+            mDrawSinceRobustnessTimerSet = true;
+            return;
+        }
         
-        // As long as there's still activity, we reset the timer each time that
-        // this function gets called.
         mContextRestorer->InitWithCallback(static_cast<nsITimerCallback*>(this),
                                            PR_MillisecondsToInterval(1000),
                                            nsITimer::TYPE_ONE_SHOT);
+        mRobustnessTimerRunning = true;
+        mDrawSinceRobustnessTimerSet = false;
+    }
+
+    void TerminateRobustnessTimer() {
+        if (mRobustnessTimerRunning) {
+            mContextRestorer->Cancel();
+            mRobustnessTimerRunning = false;
+        }
     }
 
 protected:
@@ -504,6 +517,7 @@ protected:
     bool mOptionsFrozen;
     bool mMinCapability;
     bool mDisableExtensions;
+    bool mHasRobustness;
 
     WebGLuint mActiveTexture;
     WebGLenum mWebGLError;
@@ -723,6 +737,8 @@ protected:
     nsCOMPtr<nsITimer> mContextRestorer;
     bool mContextLost;
     bool mAllowRestore;
+    bool mRobustnessTimerRunning;
+    bool mDrawSinceRobustnessTimerSet;
 
 public:
     // console logging helpers

content/canvas/src/WebGLContextGL.cpp

@@ -1622,8 +1622,6 @@ WebGLContext::DrawArrays(GLenum mode, WebGLint first, WebGLsizei count)
     if (!ValidateBuffers(&maxAllowedCount, "drawArrays"))
         return NS_OK;
 
-    SetupRobustnessTimer();
-
     CheckedInt32 checked_firstPlusCount = CheckedInt32(first) + count;
 
     if (!checked_firstPlusCount.valid())
@@ -1645,6 +1643,7 @@ WebGLContext::DrawArrays(GLenum mode, WebGLint first, WebGLsizei count)
     if (!DoFakeVertexAttrib0(checked_firstPlusCount.value()))
         return NS_OK;
 
+    SetupRobustnessTimer();
     gl->fDrawArrays(mode, first, count);
 
     UndoFakeVertexAttrib0();
@@ -1675,8 +1674,6 @@ WebGLContext::DrawElements(WebGLenum mode, WebGLsizei count, WebGLenum type, Web
     if (count == 0)
         return NS_OK;
 
-    SetupRobustnessTimer();
-
     CheckedUint32 checked_byteCount;
 
     if (type == LOCAL_GL_UNSIGNED_SHORT) {
@@ -1757,6 +1754,7 @@ WebGLContext::DrawElements(WebGLenum mode, WebGLsizei count, WebGLenum type, Web
     if (!DoFakeVertexAttrib0(checked_maxIndexPlusOne.value()))
         return NS_OK;
 
+    SetupRobustnessTimer();
     gl->fDrawElements(mode, count, type, (GLvoid*) (byteOffset));
 
     UndoFakeVertexAttrib0();

gfx/gl/GLContext.cpp

@@ -71,6 +71,38 @@ const ContextFormat ContextFormat::BasicRGBA32Format(ContextFormat::BasicRGBA32)
 #define MAX_SYMBOL_LENGTH 128
 #define MAX_SYMBOL_NAMES 5
 
+// should match the order of GLExtensions
+static const char *sExtensionNames[] = {
+    "GL_EXT_framebuffer_object",
+    "GL_ARB_framebuffer_object",
+    "GL_ARB_texture_rectangle",
+    "GL_EXT_bgra",
+    "GL_EXT_texture_format_BGRA8888",
+    "GL_OES_depth24",
+    "GL_OES_depth32",
+    "GL_OES_stencil8",
+    "GL_OES_texture_npot",
+    "GL_OES_depth_texture",
+    "GL_OES_packed_depth_stencil",
+    "GL_IMG_read_format",
+    "GL_EXT_read_format_bgra",
+    "GL_APPLE_client_storage",
+    "GL_ARB_texture_non_power_of_two",
+    "GL_ARB_pixel_buffer_object",
+    "GL_ARB_ES2_compatibility",
+    "GL_OES_texture_float",
+    "GL_ARB_texture_float",
+    "GL_EXT_unpack_subimage",
+    "GL_OES_standard_derivatives",
+    "GL_EXT_framebuffer_blit",
+    "GL_ANGLE_framebuffer_blit",
+    "GL_EXT_framebuffer_multisample",
+    "GL_ANGLE_framebuffer_multisample",
+    "GL_OES_rgb8_rgba8",
+    "GL_ARB_robustness",
+    NULL
+};
+
 bool
 LibrarySymbolLoader::OpenLibrary(const char *library)
 {
@@ -174,8 +206,6 @@ GLContext::InitWithPrefix(const char *prefix, bool trygl)
         return true;
     }
 
-    mHasRobustness = IsExtensionSupported(ARB_robustness);
-
     SymLoadStruct symbols[] = {
         { (PRFuncPtr*) &mSymbols.fActiveTexture, { "ActiveTexture", "ActiveTextureARB", NULL } },
         { (PRFuncPtr*) &mSymbols.fAttachShader, { "AttachShader", "AttachShaderARB", NULL } },
@@ -327,9 +357,6 @@ GLContext::InitWithPrefix(const char *prefix, bool trygl)
         { mIsGLES2 ? (PRFuncPtr*) NULL : (PRFuncPtr*) &mSymbols.fUnmapBuffer,
           { mIsGLES2 ? NULL : "UnmapBuffer", NULL } },
 
-        { mHasRobustness ? (PRFuncPtr*) &mSymbols.fGetGraphicsResetStatus : (PRFuncPtr*) NULL,
-          { mHasRobustness ? "GetGraphicsResetStatusARB" : NULL, NULL } },
-
         { NULL, { NULL } },
 
     };
@@ -382,6 +409,20 @@ GLContext::InitWithPrefix(const char *prefix, bool trygl)
                      (mSymbols.fMapBuffer && mSymbols.fUnmapBuffer),
                      "ARB_pixel_buffer_object supported without glMapBuffer/UnmapBuffer being available!");
 
+        if (SupportsRobustness() && IsExtensionSupported(ARB_robustness)) {
+            SymLoadStruct robustnessSymbols[] = {
+                { (PRFuncPtr*) &mSymbols.fGetGraphicsResetStatus, { "GetGraphicsResetStatusARB", NULL } },
+                { NULL, { NULL } },
+            };
+
+            if (!LoadSymbols(&robustnessSymbols[0], trygl, prefix)) {
+                NS_RUNTIMEABORT("GL supports ARB_robustness without supplying GetGraphicsResetStatusARB.");
+                mInitialized = false;
+            }
+
+            mHasRobustness = true;
+        }
+
         // Check for aux symbols based on extensions
         if (IsExtensionSupported(GLContext::ANGLE_framebuffer_blit) ||
             IsExtensionSupported(GLContext::EXT_framebuffer_blit)) {
@@ -448,38 +489,6 @@ GLContext::InitWithPrefix(const char *prefix, bool trygl)
     return mInitialized;
 }
 
-// should match the order of GLExtensions
-static const char *sExtensionNames[] = {
-    "GL_EXT_framebuffer_object",
-    "GL_ARB_framebuffer_object",
-    "GL_ARB_texture_rectangle",
-    "GL_EXT_bgra",
-    "GL_EXT_texture_format_BGRA8888",
-    "GL_OES_depth24",
-    "GL_OES_depth32",
-    "GL_OES_stencil8",
-    "GL_OES_texture_npot",
-    "GL_OES_depth_texture",
-    "GL_OES_packed_depth_stencil",
-    "GL_IMG_read_format",
-    "GL_EXT_read_format_bgra",
-    "GL_APPLE_client_storage",
-    "GL_ARB_texture_non_power_of_two",
-    "GL_ARB_pixel_buffer_object",
-    "GL_ARB_ES2_compatibility",
-    "GL_OES_texture_float",
-    "GL_ARB_texture_float",
-    "GL_EXT_unpack_subimage",
-    "GL_OES_standard_derivatives",
-    "GL_EXT_framebuffer_blit",
-    "GL_ANGLE_framebuffer_blit",
-    "GL_EXT_framebuffer_multisample",
-    "GL_ANGLE_framebuffer_multisample",
-    "GL_OES_rgb8_rgba8",
-    "GL_ARB_robustness",
-    NULL
-};
-
 void
 GLContext::InitExtensions()
 {

gfx/gl/GLContext.h

@@ -541,6 +541,7 @@ public:
         mIsGLES2(false),
 #endif
         mIsGlobalSharedContext(false),
+        mHasRobustness(false),
         mVendor(-1),
         mDebugMode(0),
         mCreationFormat(aFormat),
@@ -661,6 +662,12 @@ public:
         return mIsGLES2 || IsExtensionSupported(ARB_ES2_compatibility);
     }
 
+    /**
+     * The derived class is expected to provide information on whether or not it
+     * supports robustness.
+     */
+    virtual bool SupportsRobustness() = 0;
+
     enum {
         VendorIntel,
         VendorNVIDIA,
@@ -1309,6 +1316,10 @@ public:
         CONTEXT_UNKNOWN_CONTEXT_RESET_ARB = 0x8255,
     };
 
+    bool HasRobustness() {
+        return mHasRobustness;
+    }
+
 protected:
     bool mInitialized;
     bool mIsOffscreen;

gfx/gl/GLContextProviderCGL.mm

@@ -188,6 +188,11 @@ public:
       return gUseDoubleBufferedWindows; 
     }
 
+    bool SupportsRobustness()
+    {
+        return false;
+    }
+
     bool SwapBuffers()
     {
       [mContext flushBuffer];

gfx/gl/GLContextProviderEGL.cpp

@@ -764,6 +764,11 @@ public:
         mIsDoubleBuffered = aIsDB;
     }
 
+    bool SupportsRobustness()
+    {
+        return false;
+    }
+
 #if defined(MOZ_X11) && defined(MOZ_EGL_XRENDER_COMPOSITE)
     gfxASurface* GetOffscreenPixmapSurface()
     {

gfx/gl/GLContextProviderGLX.cpp

@@ -256,15 +256,9 @@ GLXLibrary::EnsureInitialized()
         NS_WARNING("Texture from pixmap disabled");
     }
 
-    if (HasExtension(extensionsStr, "GL_ARB_robustness")) {
-        if (!LibrarySymbolLoader::LoadSymbols(mOGLLibrary, symbols_robustness)) {
-            // We have no easy way of checking whether or not this extension
-            // exists, so it's best to just try to load it and accept that it
-            // might fail.
-            //NS_WARNING("Couldn't load ARB_robustness symbols");
-        } else {
-            mHasRobustness = true;
-        }
+    if (HasExtension(extensionsStr, "GLX_ARB_create_context_robustness") &&
+        LibrarySymbolLoader::LoadSymbols(mOGLLibrary, symbols_robustness)) {
+        mHasRobustness = true;
     }
 
     gIsATI = serverVendor && DoesVendorStringMatch(serverVendor, "ATI");
@@ -828,6 +822,11 @@ TRY_AGAIN_NO_SHARING:
         return mDoubleBuffered;
     }
 
+    bool SupportsRobustness()
+    {
+        return sGLXLibrary.HasRobustness();
+    }
+
     bool SwapBuffers()
     {
         if (!mDoubleBuffered)

gfx/gl/GLContextProviderOSMesa.cpp

@@ -240,6 +240,11 @@ public:
         }
     }
 
+    bool SupportsRobustness()
+    {
+        return false;
+    }
+
 private:
     nsRefPtr<gfxImageSurface> mThebesSurface;
     PrivateOSMesaContext mContext;

gfx/gl/GLContextProviderWGL.cpp

@@ -307,6 +307,11 @@ public:
         return mIsDoubleBuffered;
     }
 
+    bool SupportsRobustness()
+    {
+        return false;
+    }
+
     virtual bool SwapBuffers() {
         if (!mIsDoubleBuffered)
             return false;