Bug 1429754 - Trust the driver about floating point support. - r=daoshengmu
☠☠ backed out by 5150651a530b ☠ ☠
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 11 Jan 2018 17:53:52 -0800
changeset 450710 add48ffcf30322b8ecf135e03e3bf333b8847c2c
parent 450709 2fa8c2b1c9917c035e0d9cc7f7b18549d67e5ade
child 450711 47e8a8cd8c6cfe2c0a905bd5fda6ccb7524ab6a6
push id8531
push userryanvm@gmail.com
push dateFri, 12 Jan 2018 16:47:01 +0000
treeherdermozilla-beta@0bc627ade5a0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaoshengmu
bugs1429754
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);