Bug 1429754 - Trust the driver about floating point support. - r=daoshengmu
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 11 Jan 2018 17:53:52 -0800
changeset 453880 f6d0f79d687b1f427ebaf3018d82b55eda3e173a
parent 453832 8df2e3d57e945e3445b7aef5f6ffaffebb36e0e7
child 453881 0656d11f1328a01fa9d345e116126bb6519370de
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [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
dom/canvas/test/webgl-mochitest/mochitest.ini
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/dom/canvas/test/webgl-mochitest/mochitest.ini
+++ b/dom/canvas/test/webgl-mochitest/mochitest.ini
@@ -12,31 +12,31 @@ support-files =
   red-green.webmvp8.webm
   red-green.webmvp9.webm
 
 [ensure-exts/test_ANGLE_instanced_arrays.html]
 fail-if = (os == 'android') || (os == 'mac' && os_version == '10.6')
 [ensure-exts/test_EXT_blend_minmax.html]
 fail-if = (os == 'android')
 [ensure-exts/test_EXT_color_buffer_half_float.html]
-fail-if = (os == 'android') || (os == 'linux')
+fail-if = (os == 'android')
 [ensure-exts/test_EXT_disjoint_timer_query.html]
 fail-if = (os == 'android') || (os == 'win' && os_version == '5.1')
 [ensure-exts/test_EXT_frag_depth.html]
 fail-if = (os == 'android')
 [ensure-exts/test_EXT_sRGB.html]
 fail-if = (os == 'android') || (os == 'mac' && os_version == '10.6')
 [ensure-exts/test_EXT_shader_texture_lod.html]
 fail-if = (os == 'android')
 [ensure-exts/test_EXT_texture_filter_anisotropic.html]
 fail-if = (os == 'android') || (os == 'linux')
 [ensure-exts/test_OES_standard_derivatives.html]
 fail-if = (os == 'android')
 [ensure-exts/test_WEBGL_color_buffer_float.html]
-fail-if = (os == 'android') || (os == 'linux')
+fail-if = (os == 'android')
 [ensure-exts/test_WEBGL_compressed_texture_atc.html]
 fail-if = (os == 'android') || (os == 'linux') || (os == 'mac') || (os == 'win')
 [ensure-exts/test_WEBGL_compressed_texture_es3.html]
 fail-if = (os == 'android') || (os == 'mac') || (os == 'win')
 [ensure-exts/test_WEBGL_compressed_texture_etc1.html]
 fail-if = (os == 'linux') || (os == 'mac') || (os == 'win')
 [ensure-exts/test_WEBGL_compressed_texture_pvrtc.html]
 fail-if = (os == 'android') || (os == 'linux') || (os == 'mac') || (os == 'win')
--- 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);