Changeset 91112 in webkit

Timestamp:

Jul 15, 2011 2:22:45 PM (13 years ago)

Author:

kbr@google.com

Message:

Don't restore WebGL context if it was guilty of a graphics reset
​https://bugs.webkit.org/show_bug.cgi?id=64497

Reviewed by Stephen White.

Source/WebCore:

Use getGraphicsResetStatusARB already defined in Extensions3D to
determine why the context was lost, and respond to guilty context
notifications by forbidding restoration of the context.

It isn't currently possible to write an automated test for this.
We might consider extending the WEBKIT_lose_context extension to
allow a reason to be provided why the context was lost. It was
tested manually in Chromium on Windows and Linux with some test
cases that provoke actual graphics card resets.

• html/canvas/WebGLRenderingContext.cpp:

(WebCore::WebGLRenderingContext::WebGLRenderingContextRestoreTimer::fired):
(WebCore::WebGLRenderingContext::forceLostContext):
(WebCore::WebGLRenderingContext::maybeRestoreContext):

• html/canvas/WebGLRenderingContext.h:

Source/WebKit/chromium:

Actually implement getGraphicsResetStatusARB rather than inferring
the status based on whether the context has been lost.

• public/WebGraphicsContext3D.h:

(WebKit::WebGraphicsContext3D::getGraphicsResetStatusARB):

• src/Extensions3DChromium.cpp:

(WebCore::Extensions3DChromium::getGraphicsResetStatusARB):

• src/GraphicsContext3DChromium.cpp:
• src/GraphicsContext3DInternal.h:

LayoutTests:

Updated expectations for context-lost test and added comment about
the order of event delivery. Per the specification of the
WEBKIT_lose_context extension, the previous expectations were wrong.

• fast/canvas/webgl/context-lost-expected.txt:
• fast/canvas/webgl/context-lost.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/canvas/webgl/context-lost-expected.txt (modified) (2 diffs)
• LayoutTests/fast/canvas/webgl/context-lost.html (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/canvas/WebGLRenderingContext.cpp (modified) (3 diffs)
• Source/WebCore/html/canvas/WebGLRenderingContext.h (modified) (1 diff)
• Source/WebKit/chromium/ChangeLog (modified) (1 diff)
• Source/WebKit/chromium/public/WebGraphicsContext3D.h (modified) (1 diff)
• Source/WebKit/chromium/src/Extensions3DChromium.cpp (modified) (1 diff)
• Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp (modified) (1 diff)
• Source/WebKit/chromium/src/GraphicsContext3DInternal.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-07-15  Kenneth Russell  <kbr@google.com>
+
+        Don't restore WebGL context if it was guilty of a graphics reset
+        https://bugs.webkit.org/show_bug.cgi?id=64497
+
+        Reviewed by Stephen White.
+
+        Updated expectations for context-lost test and added comment about
+        the order of event delivery. Per the specification of the
+        WEBKIT_lose_context extension, the previous expectations were wrong.
+
+        * fast/canvas/webgl/context-lost-expected.txt:
+        * fast/canvas/webgl/context-lost.html:
+
 2011-07-15  Adrienne Walker  <enne@google.com>
 

trunk/LayoutTests/fast/canvas/webgl/context-lost-expected.txt

@@ -16 +16 @@
 
 Lose context
-PASS extension.loseContext() was expected value: NO_ERROR.
-
 Test lost context
 PASS gl.isContextLost() is true
@@ -180 +178 @@
 PASS gl.isTexture(texture) is false
 PASS gl.getError() is gl.NO_ERROR
+
+PASS extension.loseContext() was expected value: NO_ERROR.
+
 PASS successfullyParsed is true
 

trunk/LayoutTests/fast/canvas/webgl/context-lost.html

@@ -60 +60 @@
     debug("");
     debug("Lose context");
+
+    // Note: this will cause the context to be lost, and the
+    // webglcontextlost event listener to be called, immediately.
     shouldGenerateGLError(gl, gl.NO_ERROR, "extension.loseContext()");
     debug("");
@@ -293 +296 @@
     shouldBe("gl.getError()", "gl.NO_ERROR");
 
+    debug("");
+
     finish();
 }

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-07-15  Kenneth Russell  <kbr@google.com>
+
+        Don't restore WebGL context if it was guilty of a graphics reset
+        https://bugs.webkit.org/show_bug.cgi?id=64497
+
+        Reviewed by Stephen White.
+
+        Use getGraphicsResetStatusARB already defined in Extensions3D to
+        determine why the context was lost, and respond to guilty context
+        notifications by forbidding restoration of the context.
+
+        It isn't currently possible to write an automated test for this.
+        We might consider extending the WEBKIT_lose_context extension to
+        allow a reason to be provided why the context was lost. It was
+        tested manually in Chromium on Windows and Linux with some test
+        cases that provoke actual graphics card resets.
+
+        * html/canvas/WebGLRenderingContext.cpp:
+        (WebCore::WebGLRenderingContext::WebGLRenderingContextRestoreTimer::fired):
+        (WebCore::WebGLRenderingContext::forceLostContext):
+        (WebCore::WebGLRenderingContext::maybeRestoreContext):
+        * html/canvas/WebGLRenderingContext.h:
+
 2011-07-15  Chris Marrin  <cmarrin@apple.com>
 

trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp

@@ -340 +340 @@
 void WebGLRenderingContext::WebGLRenderingContextRestoreTimer::fired()
 {
-    // Timer is started when m_contextLost is false.  It will first call
-    // onLostContext, which will set m_contextLost to true.  Then it will keep
-    // calling restoreContext and reschedule itself until m_contextLost is back
-    // to false.
-    if (!m_context->m_contextLost) {
-        m_context->onLostContext();
-        startOneShot(secondsBetweenRestoreAttempts);
-    } else {
-        // The rendering context is not restored if there is no handler for
-        // the context restored event.
-        if (!m_context->canvas()->hasEventListeners(eventNames().webglcontextrestoredEvent))
-            return;
-
-        m_context->restoreContext();
-        if (m_context->m_contextLost)
-            startOneShot(secondsBetweenRestoreAttempts);
-    }
+    m_context->maybeRestoreContext();
 }
 
@@ -3941 +3925 @@
     }
 
-    m_restoreTimer.startOneShot(0);
+    maybeRestoreContext();
 }
 
@@ -4795 +4779 @@
 }
 
+void WebGLRenderingContext::maybeRestoreContext()
+{
+    // Timer is started when m_contextLost is false. It will first call
+    // onLostContext, which will set m_contextLost to true. Then it will keep
+    // calling restoreContext and reschedule itself until m_contextLost is back
+    // to false.
+    bool shouldStartTimer = false;
+    bool shouldAttemptRestoreNow = true;
+
+    if (!m_contextLost) {
+        onLostContext();
+        shouldStartTimer = true;
+        shouldAttemptRestoreNow = false;
+    }
+
+    // The rendering context is not restored if there is no handler for
+    // the context restored event.
+    if (!canvas()->hasEventListeners(eventNames().webglcontextrestoredEvent))
+        return;
+
+    int contextLostReason = m_context->getExtensions()->getGraphicsResetStatusARB();
+
+    switch (contextLostReason) {
+    case GraphicsContext3D::NO_ERROR:
+        // The GraphicsContext3D implementation might not fully
+        // support GL_ARB_robustness semantics yet. Alternatively, the
+        // WebGL WEBKIT_lose_context extension might have been used to
+        // force a lost context.
+        break;
+    case Extensions3D::GUILTY_CONTEXT_RESET_ARB:
+        // The rendering context is not restored if this context was
+        // guilty of causing the graphics reset.
+        printWarningToConsole("WARNING: WebGL content on the page caused the graphics card to reset; not restoring the context");
+        return;
+    case Extensions3D::INNOCENT_CONTEXT_RESET_ARB:
+        // Always allow the context to be restored.
+        break;
+    case Extensions3D::UNKNOWN_CONTEXT_RESET_ARB:
+        // Warn. Ideally, prompt the user telling them that WebGL
+        // content on the page might have caused the graphics card to
+        // reset and ask them whether they want to continue running
+        // the content. Only if they say "yes" should we start
+        // attempting to restore the context.
+        printWarningToConsole("WARNING: WebGL content on the page might have caused the graphics card to reset");
+        break;
+    }
+
+    if (shouldAttemptRestoreNow) {
+        restoreContext();
+        if (m_contextLost)
+            shouldStartTimer = true;
+    }
+
+    if (shouldStartTimer)
+        m_restoreTimer.startOneShot(secondsBetweenRestoreAttempts);
+}
+
 WebGLRenderingContext::LRUImageBufferCache::LRUImageBufferCache(int capacity)
     : m_buffers(adoptArrayPtr(new OwnPtr<ImageBuffer>[capacity]))

trunk/Source/WebCore/html/canvas/WebGLRenderingContext.h

@@ -611 +611 @@
     void restoreStatesAfterVertexAttrib0Simulation();
 
+    // Helper for restoration after context lost.
+    void maybeRestoreContext();
+
     friend class WebGLStateRestorer;
 };

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2011-07-15  Kenneth Russell  <kbr@google.com>
+
+        Don't restore WebGL context if it was guilty of a graphics reset
+        https://bugs.webkit.org/show_bug.cgi?id=64497
+
+        Reviewed by Stephen White.
+
+        Actually implement getGraphicsResetStatusARB rather than inferring
+        the status based on whether the context has been lost.
+
+        * public/WebGraphicsContext3D.h:
+        (WebKit::WebGraphicsContext3D::getGraphicsResetStatusARB):
+        * src/Extensions3DChromium.cpp:
+        (WebCore::Extensions3DChromium::getGraphicsResetStatusARB):
+        * src/GraphicsContext3DChromium.cpp:
+        * src/GraphicsContext3DInternal.h:
+
 2011-07-15  Dan Bernstein  <mitz@apple.com>
 

trunk/Source/WebKit/chromium/public/WebGraphicsContext3D.h

@@ -357 +357 @@
 
     virtual void setContextLostCallback(WebGraphicsContextLostCallback* callback) {}
+    // GL_ARB_robustness
+    //
+    // This entry point must provide slightly different semantics than
+    // the GL_ARB_robustness extension; specifically, the lost context
+    // state is sticky, rather than reported only once.
+    virtual WGC3Denum getGraphicsResetStatusARB() { return 0; /* GL_NO_ERROR */ }
 };
 

trunk/Source/WebKit/chromium/src/Extensions3DChromium.cpp

@@ -65 +65 @@
 int Extensions3DChromium::getGraphicsResetStatusARB()
 {
-    return m_internal->isContextLost() ? static_cast<int>(Extensions3D::UNKNOWN_CONTEXT_RESET_ARB) : static_cast<int>(GraphicsContext3D::NO_ERROR);
+    return static_cast<int>(m_internal->getGraphicsResetStatusARB());
 }
 

trunk/Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp

@@ -841 +841 @@
 
 DELEGATE_TO_IMPL(rateLimitOffscreenContextCHROMIUM)
+DELEGATE_TO_IMPL_R(getGraphicsResetStatusARB, GC3Denum)
 
 //----------------------------------------------------------------------

trunk/Source/WebKit/chromium/src/GraphicsContext3DInternal.h

@@ -293 +293 @@
     void rateLimitOffscreenContextCHROMIUM();
 
+    // GL_ARB_robustness
+    GC3Denum getGraphicsResetStatusARB();
+
 private:
     OwnPtr<WebKit::WebGraphicsContext3D> m_impl;