Bug 1429754 - Trust the driver about floating point support. - r=daoshengmu draft
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 11 Jan 2018 17:53:52 -0800
changeset 719351 a81b0ab631575f07f56205f42b3128b83eff86e9
parent 719297 fe77befa5210fbd680660abe617145bc821f98c1
child 745772 d91ef7d1ef0dfe6c18eb8b70f2a47b94c7b10fc9
push id95232
push userbmo:jgilbert@mozilla.com
push dateFri, 12 Jan 2018 01:54:14 +0000
reviewersdaoshengmu
bugs1429754
milestone59.0a1
Bug 1429754 - Trust the driver about floating point support. - r=daoshengmu Remove the probe, and remove the cached value check. Also remove dead code which relies on this sometimes-clamping glGet query. MozReview-Commit-ID: JA1VgH8fLRB
dom/canvas/WebGLContextUtils.cpp
dom/canvas/WebGLExtensionColorBufferFloat.cpp
gfx/gl/GLContext.cpp
gfx/gl/GLContext.h
--- a/dom/canvas/WebGLContextUtils.cpp
+++ b/dom/canvas/WebGLContextUtils.cpp
@@ -799,29 +799,19 @@ WebGLContext::AssertCachedGlobalState() 
     ////////////////
 
     // Draw state
     MOZ_ASSERT(gl->fIsEnabled(LOCAL_GL_DITHER) == mDitherEnabled);
     MOZ_ASSERT_IF(IsWebGL2(),
                   gl->fIsEnabled(LOCAL_GL_RASTERIZER_DISCARD) == mRasterizerDiscardEnabled);
     MOZ_ASSERT(gl->fIsEnabled(LOCAL_GL_SCISSOR_TEST) == mScissorTestEnabled);
 
-    GLfloat colorClearValue[4] = {0.0f, 0.0f, 0.0f, 0.0f};
-    gl->fGetFloatv(LOCAL_GL_COLOR_CLEAR_VALUE, colorClearValue);
-    const bool ok = IsCacheCorrect(mColorClearValue[0], colorClearValue[0]) &&
-               IsCacheCorrect(mColorClearValue[1], colorClearValue[1]) &&
-               IsCacheCorrect(mColorClearValue[2], colorClearValue[2]) &&
-               IsCacheCorrect(mColorClearValue[3], colorClearValue[3]);
-    if (!ok) {
-        gfxCriticalNote << mColorClearValue[0] << " - " << colorClearValue[0] << " = " << (mColorClearValue[0] - colorClearValue[0]) << "\n"
-                        << mColorClearValue[1] << " - " << colorClearValue[1] << " = " << (mColorClearValue[1] - colorClearValue[1]) << "\n"
-                        << mColorClearValue[2] << " - " << colorClearValue[2] << " = " << (mColorClearValue[2] - colorClearValue[2]) << "\n"
-                        << mColorClearValue[3] << " - " << colorClearValue[3] << " = " << (mColorClearValue[3] - colorClearValue[3]);
-    }
-    MOZ_ASSERT(ok);
+    // Cannot trivially check COLOR_CLEAR_VALUE, since in old GL versions glGet may clamp
+    // based on whether the current framebuffer is floating-point or not.
+    // This also means COLOR_CLEAR_VALUE save+restore is dangerous!
 
     realGLboolean depthWriteMask = 0;
     gl->fGetBooleanv(LOCAL_GL_DEPTH_WRITEMASK, &depthWriteMask);
     MOZ_ASSERT(depthWriteMask == mDepthWriteMask);
 
     GLfloat depthClearValue = 0.0f;
     gl->fGetFloatv(LOCAL_GL_DEPTH_CLEAR_VALUE, &depthClearValue);
     MOZ_ASSERT(IsCacheCorrect(mDepthClearValue, depthClearValue));
--- a/dom/canvas/WebGLExtensionColorBufferFloat.cpp
+++ b/dom/canvas/WebGLExtensionColorBufferFloat.cpp
@@ -29,37 +29,16 @@ WebGLExtensionColorBufferFloat::WebGLExt
     };
 
 #define FOO(x) fnUpdateUsage(LOCAL_GL_ ## x, webgl::EffectiveFormat::x)
 
     // The extension doesn't actually add RGB32F; only RGBA32F.
     FOO(RGBA32F);
 
 #undef FOO
-
-#ifdef DEBUG
-    const auto gl = webgl->gl;
-    float was[4] = {};
-    gl->fGetFloatv(LOCAL_GL_COLOR_CLEAR_VALUE, was);
-
-    const float test[4] = {-1.0, 0, 2.0, 255.0};
-    gl->fClearColor(test[0], test[1], test[2], test[3]);
-
-    float now[4] = {};
-    gl->fGetFloatv(LOCAL_GL_COLOR_CLEAR_VALUE, now);
-    const bool ok = now[0] == test[0] && now[1] == test[1] &&
-                    now[2] == test[2] && now[3] == test[3];
-    if (!ok) {
-        printf_stderr("COLOR_CLEAR_VALUE: now{%f,%f,%f,%f} != test{%f,%f,%f,%f}\n",
-                      test[0], test[1], test[2], test[3],
-                      now[0], now[1], now[2], now[3]);
-        MOZ_ASSERT(false);
-    }
-    gl->fClearColor(was[0], was[1], was[2], was[3]);
-#endif
 }
 
 WebGLExtensionColorBufferFloat::~WebGLExtensionColorBufferFloat()
 {
 }
 
 bool
 WebGLExtensionColorBufferFloat::IsSupported(const WebGLContext* webgl)
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -749,40 +749,16 @@ GLContext::InitWithPrefixImpl(const char
         // this has been fixed in Mac OS X 10.9. See 907946
         // and it also works in 10.8.3 and higher.  See 1094338.
         if (Vendor() == gl::GLVendor::NVIDIA &&
             !nsCocoaFeatures::IsAtLeastVersion(10,8,3))
         {
             MarkUnsupported(GLFeature::depth_texture);
         }
 #endif
-        if (IsSupported(GLFeature::frag_color_float)) {
-            float was[4] = {};
-            fGetFloatv(LOCAL_GL_COLOR_CLEAR_VALUE, was);
-
-            const float test[4] = {-1.0, 0, 2.0, 255.0};
-            fClearColor(test[0], test[1], test[2], test[3]);
-
-            float now[4] = {};
-            fGetFloatv(LOCAL_GL_COLOR_CLEAR_VALUE, now);
-
-            fClearColor(was[0], was[1], was[2], was[3]);
-
-            const bool unclamped = now[0] == test[0] && now[1] == test[1] &&
-                                   now[2] == test[2] && now[3] == test[3];
-            if (!unclamped) {
-                printf_stderr("COLOR_CLEAR_VALUE: now{%f,%f,%f,%f} != test{%f,%f,%f,%f}\n",
-                              test[0], test[1], test[2], test[3],
-                              now[0], now[1], now[2], now[3]);
-                gfxCriticalNote << "GLFeature::frag_color_float failed support probe,"
-                                << " disabling. (RENDERER: "
-                                << (const char*)fGetString(LOCAL_GL_RENDERER) << ")";
-                MarkUnsupported(GLFeature::frag_color_float);
-            }
-        }
     }
 
     if (IsExtensionSupported(GLContext::ARB_pixel_buffer_object)) {
         MOZ_ASSERT((mSymbols.fMapBuffer && mSymbols.fUnmapBuffer),
                    "ARB_pixel_buffer_object supported without glMapBuffer/UnmapBuffer"
                    " being available!");
     }
 
@@ -2066,96 +2042,16 @@ GLContext::AssembleOffscreenFBs(const GL
         *readFB_out = readFB;
     } else if (readFB) {
         MOZ_CRASH("readFB created when not requested!");
     }
 
     return isComplete;
 }
 
-
-void
-GLContext::ClearSafely()
-{
-    // bug 659349 --- we must be very careful here: clearing a GL framebuffer is nontrivial, relies on a lot of state,
-    // and in the case of the backbuffer of a WebGL context, state is exposed to scripts.
-    //
-    // The code here is taken from WebGLContext::ForceClearFramebufferWithDefaultValues, but I didn't find a good way of
-    // sharing code with it. WebGL's code is somewhat performance-critical as it is typically called on every frame, so
-    // WebGL keeps track of GL state to avoid having to query it everytime, and also tries to only do work for actually
-    // present buffers (e.g. stencil buffer). Doing that here seems like premature optimization,
-    // as ClearSafely() is called only when e.g. a canvas is resized, not on every animation frame.
-
-    realGLboolean scissorTestEnabled;
-    realGLboolean ditherEnabled;
-    realGLboolean colorWriteMask[4];
-    realGLboolean depthWriteMask;
-    GLint stencilWriteMaskFront, stencilWriteMaskBack;
-    GLfloat colorClearValue[4];
-    GLfloat depthClearValue;
-    GLint stencilClearValue;
-
-    // save current GL state
-    fGetBooleanv(LOCAL_GL_SCISSOR_TEST, &scissorTestEnabled);
-    fGetBooleanv(LOCAL_GL_DITHER, &ditherEnabled);
-    fGetBooleanv(LOCAL_GL_COLOR_WRITEMASK, colorWriteMask);
-    fGetBooleanv(LOCAL_GL_DEPTH_WRITEMASK, &depthWriteMask);
-    fGetIntegerv(LOCAL_GL_STENCIL_WRITEMASK, &stencilWriteMaskFront);
-    fGetIntegerv(LOCAL_GL_STENCIL_BACK_WRITEMASK, &stencilWriteMaskBack);
-    fGetFloatv(LOCAL_GL_COLOR_CLEAR_VALUE, colorClearValue);
-    fGetFloatv(LOCAL_GL_DEPTH_CLEAR_VALUE, &depthClearValue);
-    fGetIntegerv(LOCAL_GL_STENCIL_CLEAR_VALUE, &stencilClearValue);
-
-    // prepare GL state for clearing
-    fDisable(LOCAL_GL_SCISSOR_TEST);
-    fDisable(LOCAL_GL_DITHER);
-
-    fColorMask(1, 1, 1, 1);
-    fClearColor(0.f, 0.f, 0.f, 0.f);
-
-    fDepthMask(1);
-    fClearDepth(1.0f);
-
-    fStencilMask(0xffffffff);
-    fClearStencil(0);
-
-    // do clear
-    fClear(LOCAL_GL_COLOR_BUFFER_BIT |
-           LOCAL_GL_DEPTH_BUFFER_BIT |
-           LOCAL_GL_STENCIL_BUFFER_BIT);
-
-    // restore GL state after clearing
-    fColorMask(colorWriteMask[0],
-               colorWriteMask[1],
-               colorWriteMask[2],
-               colorWriteMask[3]);
-    fClearColor(colorClearValue[0],
-                colorClearValue[1],
-                colorClearValue[2],
-                colorClearValue[3]);
-
-    fDepthMask(depthWriteMask);
-    fClearDepth(depthClearValue);
-
-    fStencilMaskSeparate(LOCAL_GL_FRONT, stencilWriteMaskFront);
-    fStencilMaskSeparate(LOCAL_GL_BACK, stencilWriteMaskBack);
-    fClearStencil(stencilClearValue);
-
-    if (ditherEnabled)
-        fEnable(LOCAL_GL_DITHER);
-    else
-        fDisable(LOCAL_GL_DITHER);
-
-    if (scissorTestEnabled)
-        fEnable(LOCAL_GL_SCISSOR_TEST);
-    else
-        fDisable(LOCAL_GL_SCISSOR_TEST);
-
-}
-
 void
 GLContext::MarkDestroyed()
 {
     if (IsDestroyed())
         return;
 
     // Null these before they're naturally nulled after dtor, as we want GLContext to
     // still be alive in *their* dtors.
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -3571,22 +3571,16 @@ public:
     bool IsOffscreen() const {
         return mIsOffscreen;
     }
 
     GLScreenBuffer* Screen() const {
         return mScreen.get();
     }
 
-    /* Clear to transparent black, with 0 depth and stencil,
-     * while preserving current ClearColor etc. values.
-     * Useful for resizing offscreen buffers.
-     */
-    void ClearSafely();
-
     bool WorkAroundDriverBugs() const { return mWorkAroundDriverBugs; }
 
     bool IsDrawingToDefaultFramebuffer();
 
     bool IsOffscreenSizeAllowed(const gfx::IntSize& aSize) const;
 
 protected:
     bool InitWithPrefix(const char* prefix, bool trygl);