Add gl_SampleMask functionality to fragment builders

src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp

Index: src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
index 8651827841a103dae9f21fa598e3bf682e49556c..903b03af9594b1a4477d6562f3ad167902f5ccdb 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -77,25 +77,46 @@ GrGLSLFragmentShaderBuilder::GrGLSLFragmentShaderBuilder(GrGLSLProgramBuilder* p
 }
 
 bool GrGLSLFragmentShaderBuilder::enableFeature(GLSLFeature feature) {
+    const GrGLSLCaps& glslCaps = *fProgramBuilder->glslCaps();
     switch (feature) {
-        case kStandardDerivatives_GLSLFeature: {
-            if (!fProgramBuilder->glslCaps()->shaderDerivativeSupport()) {
+        case kStandardDerivatives_GLSLFeature:
+            if (!glslCaps.shaderDerivativeSupport()) {
                 return false;
             }
-            const char* extension = fProgramBuilder->glslCaps()->shaderDerivativeExtensionString();
-            if (extension) {
+            if (const char* extension = glslCaps.shaderDerivativeExtensionString()) {
                 this->addFeature(1 << kStandardDerivatives_GLSLFeature, extension);
             }
             return true;
-        }
-        case kPixelLocalStorage_GLSLFeature: {
+        case kPixelLocalStorage_GLSLFeature:
             if (fProgramBuilder->glslCaps()->pixelLocalStorageSize() <= 0) {
                 return false;
             }
             this->addFeature(1 << kPixelLocalStorage_GLSLFeature,
                              "GL_EXT_shader_pixel_local_storage");
             return true;
-        }
+        case kSampleVariables_GLSLFeature:
+            if (!glslCaps.sampleVariablesSupport()) {
+                return false;
+            }
+            if (const char* extension = glslCaps.sampleVariablesExtensionString()) {
+                this->addFeature(1 << kSampleVariables_GLSLFeature, extension);
+            }
+            return true;
+        case kSampleMaskOverrideCoverage_GLSLFeature:
+            if (!glslCaps.sampleMaskOverrideCoverageSupport()) {
+                return false;
+            }
+            if (!this->enableFeature(kSampleVariables_GLSLFeature)) {
+                return false;
+            }
+            if (this->addFeature(1 << kSampleMaskOverrideCoverage_GLSLFeature,
+                                 "GL_NV_sample_mask_override_coverage")) {
+                // Redeclare gl_SampleMask with layout(override_coverage) if we haven't already.
+                fOutputs.push_back().set(kInt_GrSLType, GrShaderVar::kOut_TypeModifier,
+                                         "gl_SampleMask", 1, kHigh_GrSLPrecision,
+                                         "override_coverage");

[Chris Dalton, 2016/02/17 09:39:12]

As I think about it more, I don't like this being here.

This is different than the other enables because enabling the other features
doesn't change the semantic meaning of the program. It just makes sure the
program will compile.

Here, however, calling enableFeature(kSampleMaskOverrideCoverage) causes the
draw to produce different results, and therefore the caller needs to account for
this in their program key.

I have a few alternative options in mind and would like to hear your feedback on
which you like better:

1) Just simplify it and let the GrPrimitiveProcessor add new outputs. It would
then add this redefinition of gl_SampleMask.

2) Add kShaderOverrideCoverage_Flag to GrPipeline(Builder)::Flags, then let the
framework handle putting this in the program key as well as redefining
gl_SampleMask.

3) Add features that can be enabled in GrProcessorKeyBuilder for things like
this that affect the program key.

I don't like #3.

We will also need to enable sample locations in the fragment shader, and for
that I think I like #2 best. However, it doesn't seem like as good of a fit for
sample_mask_override_coverage because this extension is pretty GLSL specific. So
maybe it doesn't belong in a "Gr" class.

#1 is simple, however it seems to violate the shader abstractions if we just let
anything define new outputs.

What do you think? Maybe a 4th option could be to add a new method to the
builder that redefines gl_SampleMask, and it is understood that the caller is
responsible for including this in their key?

+            }
+            return true;
         default:
             SkFAIL("Unexpected GLSLFeature requested.");
             return false;