Re-enable advanced blend with a blacklist

src/gpu/gl/GrGLCaps.cpp

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "GrGLCaps.h"
 
@@ skipping 212 matching lines @@
         fTwoSidedStencilSupport = (ctxInfo.version() >= GR_GL_VER(2,0));
         // supported on GL 1.4 and higher or by extension
         fStencilWrapOpsSupport = (ctxInfo.version() >= GR_GL_VER(1,4)) ||
                                   ctxInfo.hasExtension("GL_EXT_stencil_wrap");
     } else {
         // ES 2 has two sided stencil and stencil wrap
         fTwoSidedStencilSupport = true;
         fStencilWrapOpsSupport = true;
     }
 
-// Disabling advanced blend until we can resolve various bugs
-#if 0
-    if (kIntel_GrGLVendor != ctxInfo.vendor()) {
+    // Only allowing advanced blend on NVIDIA until we can resolve various bugs.
+    if (kNVIDIA_GrGLDriver == ctxInfo.driver()) {

[egdaniel, 2015/06/05 18:18:04]

Outside of intel which I believe you had said had major bugs before, why not
turn on for other drivers? I think most worked before and I'd rather blacklist a
driver if it ends up being problematic. I think we could leave Dodge and Burn
blacklisted on all because I do remember those showing artifacts on others as
well (not surprising since the implementation in the original spec was
numerically unstable and I assume most just copied it). Thoughts?

[nv_mark, 2015/06/05 19:09:38]

I question if blacking listing the individual Dodge and Burn modes (lines
245-249) is really worth it.

I've expedited pushing the fix for Dodge and Burn into public drivers sooner.

I think we should be smart about how much black listing we do.  In this
particular case, the actual issue is extremely limited in scope and severity

Excessive blacklisting logic has its own pitfalls as it makes testing that much
harder and can bite you later.  Pragmatically, this is a case where I think we
are better served avoiding blacklisting and letting driver updates fix the
problem.  I fully expect in the very term you aren't going to want Skia
cluttered with blacklisting for this minor issue.

My rationale thinking this:  Very little real content uses Dodge or Burn.  Also
the NVIDIA GPUs that are affected are very new and these are devices where
driver updates are pushed quickly done by end-users.  And it has very low
severity; it isn't a crash or non-deterministic rendering issue.  The
numerically unstable math occurs in a narrow range of the domain of the 4D blend
mode function input space for Dodge and Burn.  The problem region is when the
source & destination alpha are approximately equal.  Skia's test is structured
to exercise the problem range well (kudos on the good testing coverage) but the
vast 4D range of Dodge and Burn works fine.

I think we'd be better off not implementing blacklisting for this particular bug
because it is extremely limited in scope, very low severity, and the driver fix
is coming quickly.  To be clear, I'm advocating dropping out the check in lines
245-249.

To be clear, the 238-243 blacklisting I do support.

         if (ctxInfo.hasExtension("GL_KHR_blend_equation_advanced_coherent") ||
             ctxInfo.hasExtension("GL_NV_blend_equation_advanced_coherent")) {
             fBlendEquationSupport = kAdvancedCoherent_BlendEquationSupport;
-        } else if (ctxInfo.hasExtension("GL_KHR_blend_equation_advanced") ||
-                   ctxInfo.hasExtension("GL_NV_blend_equation_advanced")) {
-            fBlendEquationSupport = kAdvanced_BlendEquationSupport;
-        } else {
-            fBlendEquationSupport = kBasic_BlendEquationSupport;
+        } else if (ctxInfo.driverVersion() >= GR_GL_DRIVER_VER(337,00)) {
+            // Non-coherent advanced blend has an issue before 337.00.
+            if (ctxInfo.hasExtension("GL_KHR_blend_equation_advanced") ||
+                ctxInfo.hasExtension("GL_NV_blend_equation_advanced")) {
+                fBlendEquationSupport = kAdvanced_BlendEquationSupport;
+            }
         }
-    } else {
-        // On Intel platforms, KHR_blend_equation_advanced is not conformant.
-        fBlendEquationSupport = kBasic_BlendEquationSupport;
+        if (this->advancedBlendEquationSupport()) {
+            // Blacklist color-dodge and color-burn until the NVIDIA fix is released.
+            fAdvBlendEqBlacklist |= (1 << kColorDodge_GrBlendEquation) |
+                                    (1 << kColorBurn_GrBlendEquation);
+        }
     }
-#endif
+
     if (kGL_GrGLStandard == standard) {
         fMapBufferFlags = kCanMap_MapFlag; // we require VBO support and the desktop VBO
                                             // extension includes glMapBuffer.
         if (version >= GR_GL_VER(3, 0) || ctxInfo.hasExtension("GL_ARB_map_buffer_range")) {
             fMapBufferFlags |= kSubset_MapFlag;
             fMapBufferType = kMapBufferRange_MapBufferType;
         } else {
             fMapBufferType = kMapBuffer_MapBufferType;
         }
     } else {
@@ skipping 861 matching lines @@
     if (fGeometryShaderSupport) {
         for (int p = 0; p < kGrSLPrecisionCount; ++p) {
             fFloatPrecisions[kGeometry_GrShaderType][p] = fFloatPrecisions[kVertex_GrShaderType][p];
         }
     }
 }