Revert of Patch to remove constant attributes (patchset #8 id:120002 of https://codereview.chromium.org/678073005/)

Reason for revert:
I'll checkin tonight when the tree is quieter

Original issue's description:
> Working patch to remove constant attributes.  This may cause some gm mismatches, I will rebaseline tonight.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/84c94c0dfd1e12e97d8a835882dda575f36e41d2
>
> Committed: https://skia.googlesource.com/skia/+/95f5194abce19e8ed875f3495fd16c79a9b931b4

TBR=bsalomon@google.com,egdaniel@google.com,joshualitt@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/683203002

expectations/gm/ignored-tests.txt

@@ -43,6 +43,3 @@ dropshadowimagefilter
 # senorblanco https://codereview.chromium.org/637283009/
 # quality improvements to imagemagnifier GM
 imagemagnifier
-
-#joshualitt single pixel mismatch in msaa16
-gradients_view_perspective

include/gpu/gl/GrGLConfig.h

@@ -46,6 +46,12 @@
  * GR_GL_CHECK_ERROR_START: controls the initial value of gCheckErrorGL
  * when GR_GL_CHECK_ERROR is 1.  Defaults to 1.
  *
+ * GR_GL_NO_CONSTANT_ATTRIBUTES: if this evaluates to true then the GL backend
+ * will use uniforms instead of attributes in all cases when there is not
+ * per-vertex data. This is important when the underlying GL implementation
+ * doesn't actually support immediate style attribute values (e.g. when
+ * the GL stream is converted to DX as in ANGLE on Chrome). Defaults to 0.
+ *
  * GR_GL_USE_BUFFER_DATA_NULL_HINT: When specifing new data for a vertex/index
  * buffer that replaces old data Ganesh can give a hint to the driver that the
  * previous data will not be used in future draws like this:
@@ -120,6 +126,10 @@
     #define GR_GL_CHECK_ERROR_START                     1
 #endif
 
+#if !defined(GR_GL_NO_CONSTANT_ATTRIBUTES)
+    #define GR_GL_NO_CONSTANT_ATTRIBUTES                0
+#endif
+
 #if !defined(GR_GL_USE_BUFFER_DATA_NULL_HINT)
     #define GR_GL_USE_BUFFER_DATA_NULL_HINT             1
 #endif

include/gpu/gl/GrGLConfig_chrome.h

@@ -12,12 +12,16 @@
 #define GR_GL_CHECK_ERROR_START                     0
 
 #if defined(SK_BUILD_FOR_WIN32)
+// ANGLE creates a temp VB for vertex attributes not specified per-vertex.
+#define GR_GL_NO_CONSTANT_ATTRIBUTES                1
+
 // For RGBA teximage/readpixels ANGLE will sw-convert to/from BGRA.
 #define GR_GL_RGBA_8888_PIXEL_OPS_SLOW              1
 
 // ANGLE can go faster if the entire fbo is read rather than a subrect
 #define GR_GL_FULL_READPIXELS_FASTER_THAN_PARTIAL   1
 #else
+#define GR_GL_NO_CONSTANT_ATTRIBUTES                0
 #define GR_GL_RGBA_8888_PIXEL_OPS_SLOW              0
 #define GR_GL_FULL_READPIXELS_FASTER_THAN_PARTIAL   0
 #endif

src/gpu/effects/GrYUVtoRGBEffect.cpp

@@ -109,7 +109,7 @@ private:
     virtual void onComputeInvariantOutput(InvariantOutput* inout) const SK_OVERRIDE {
         // YUV is opaque
         inout->setToOther(kA_GrColorComponentFlag, 0xFF << GrColor_SHIFT_A,
-                          InvariantOutput::kWillNot_ReadInput);
+                          InvariantOutput::kWill_ReadInput);
     }
 
     GrCoordTransform fCoordTransform;

src/gpu/gl/GrGLProgram.cpp

@@ -129,12 +129,13 @@ void GrGLProgram::bindTextures(const GrGLInstalledProc* ip, const GrProcessor& p
 
 void GrGLProgram::setData(const GrOptDrawState& optState,
                           GrGpu::DrawType drawType,
-                          const GrDeviceCoordTexture* dstCopy) {
+                          const GrDeviceCoordTexture* dstCopy,
+                          SharedGLState* sharedState) {
     GrColor color = optState.getColor();
     GrColor coverage = optState.getCoverageColor();
 
-    this->setColor(optState, color);
+    this->setColor(optState, color, sharedState);
-    this->setCoverage(optState, coverage);
+    this->setCoverage(optState, coverage, sharedState);
     this->setMatrixAndRenderTargetHeight(drawType, optState);
 
     if (dstCopy) {
@@ -200,49 +201,80 @@ void GrGLProgram::didSetData(GrGpu::DrawType drawType) {
     SkASSERT(!GrGpu::IsPathRenderingDrawType(drawType));
 }
 
-void GrGLProgram::setColor(const GrOptDrawState& optState, GrColor color) {
+void GrGLProgram::setColor(const GrOptDrawState& optState,
+                           GrColor color,
+                           SharedGLState* sharedState) {
     const GrGLProgramDesc::KeyHeader& header = fDesc.getHeader();
-    switch (header.fColorInput) {
+    if (!optState.hasColorVertexAttribute()) {
-        case GrGLProgramDesc::kAttribute_ColorInput:
+        switch (header.fColorInput) {
-            // Attribute case is handled in GrGpuGL::setupGeometry
+            case GrGLProgramDesc::kAttribute_ColorInput:
-            break;
+                SkASSERT(-1 != header.fColorAttributeIndex);
-        case GrGLProgramDesc::kUniform_ColorInput:
+                if (sharedState->fConstAttribColor != color ||
-            if (fColor != color && fBuiltinUniformHandles.fColorUni.isValid()) {
+                    sharedState->fConstAttribColorIndex != header.fColorAttributeIndex) {
-                // OpenGL ES doesn't support unsigned byte varieties of glUniform
+                    // OpenGL ES only supports the float varieties of glVertexAttrib
-                GrGLfloat c[4];
+                    GrGLfloat c[4];
-                GrColorToRGBAFloat(color, c);
+                    GrColorToRGBAFloat(color, c);
-                fProgramDataManager.set4fv(fBuiltinUniformHandles.fColorUni, 1, c);
+                    GL_CALL(VertexAttrib4fv(header.fColorAttributeIndex, c));
-                fColor = color;
+                    sharedState->fConstAttribColor = color;
-            }
+                    sharedState->fConstAttribColorIndex = header.fColorAttributeIndex;
-            break;
+                }
-        case GrGLProgramDesc::kAllOnes_ColorInput:
+                break;
-            // Handled by shader creation
+            case GrGLProgramDesc::kUniform_ColorInput:
-            break;
+                if (fColor != color && fBuiltinUniformHandles.fColorUni.isValid()) {
-        default:
+                    // OpenGL ES doesn't support unsigned byte varieties of glUniform
-            SkFAIL("Unexpected color type.");
+                    GrGLfloat c[4];
+                    GrColorToRGBAFloat(color, c);
+                    fProgramDataManager.set4fv(fBuiltinUniformHandles.fColorUni, 1, c);
+                    fColor = color;
+                }
+                sharedState->fConstAttribColorIndex = -1;
+                break;
+            case GrGLProgramDesc::kAllOnes_ColorInput:
+                sharedState->fConstAttribColorIndex = -1;
+                break;
+            default:
+                SkFAIL("Unexpected color type.");
+        }
+    } else {
+        sharedState->fConstAttribColorIndex = -1;
     }
 }
 
-void GrGLProgram::setCoverage(const GrOptDrawState& optState, GrColor coverage) {
+void GrGLProgram::setCoverage(const GrOptDrawState& optState,
+                              GrColor coverage,
+                              SharedGLState* sharedState) {
     const GrGLProgramDesc::KeyHeader& header = fDesc.getHeader();
-    switch (header.fCoverageInput) {
+    if (!optState.hasCoverageVertexAttribute()) {
-        case GrGLProgramDesc::kAttribute_ColorInput:
+        switch (header.fCoverageInput) {
-            // Attribute case is handled in GrGpuGL::setupGeometry
+            case GrGLProgramDesc::kAttribute_ColorInput:
-            break;
+                if (sharedState->fConstAttribCoverage != coverage ||
-        case GrGLProgramDesc::kUniform_ColorInput:
+                    sharedState->fConstAttribCoverageIndex != header.fCoverageAttributeIndex) {
-            if (fCoverage != coverage) {
+                    // OpenGL ES only supports the float varieties of  glVertexAttrib
-                // OpenGL ES doesn't support unsigned byte varieties of glUniform
+                    GrGLfloat c[4];
-                GrGLfloat c[4];
+                    GrColorToRGBAFloat(coverage, c);
-                GrColorToRGBAFloat(coverage, c);
+                    GL_CALL(VertexAttrib4fv(header.fCoverageAttributeIndex, c));
-                fProgramDataManager.set4fv(fBuiltinUniformHandles.fCoverageUni, 1, c);
+                    sharedState->fConstAttribCoverage = coverage;
-                fCoverage = coverage;
+                    sharedState->fConstAttribCoverageIndex = header.fCoverageAttributeIndex;
-            }
+                }
-            break;
+                break;
-        case GrGLProgramDesc::kAllOnes_ColorInput:
+            case GrGLProgramDesc::kUniform_ColorInput:
-            // Handled by shader creation
+                if (fCoverage != coverage) {
-            break;
+                    // OpenGL ES doesn't support unsigned byte varieties of glUniform
-        default:
+                    GrGLfloat c[4];
-            SkFAIL("Unexpected coverage type.");
+                    GrColorToRGBAFloat(coverage, c);
+                    fProgramDataManager.set4fv(fBuiltinUniformHandles.fCoverageUni, 1, c);
+                    fCoverage = coverage;
+                }
+                sharedState->fConstAttribCoverageIndex = -1;
+                break;
+            case GrGLProgramDesc::kAllOnes_ColorInput:
+                sharedState->fConstAttribCoverageIndex = -1;
+                break;
+            default:
+                SkFAIL("Unexpected coverage type.");
+        }
+    } else {
+        sharedState->fConstAttribCoverageIndex = -1;
     }
 }
 

src/gpu/gl/GrGLProgram.h

@@ -59,6 +59,27 @@ public:
      */
     virtual bool hasVertexShader() const { return true; }
 
+    /**
+     * Some GL state that is relevant to programs is not stored per-program. In particular color
+     * and coverage attributes can be global state. This struct is read and updated by
+     * GrGLProgram::setColor and GrGLProgram::setCoverage to allow us to avoid setting this state
+     * redundantly.
+     */
+    struct SharedGLState {
+        GrColor fConstAttribColor;
+        int     fConstAttribColorIndex;
+        GrColor fConstAttribCoverage;
+        int     fConstAttribCoverageIndex;
+
+        SharedGLState() { this->invalidate(); }
+        void invalidate() {
+            fConstAttribColor = GrColor_ILLEGAL;
+            fConstAttribColorIndex = -1;
+            fConstAttribCoverage = GrColor_ILLEGAL;
+            fConstAttribCoverageIndex = -1;
+        }
+    };
+
     /**
      * The GrDrawState's view matrix along with the aspects of the render target determine the
      * matrix sent to GL. The size of the render target affects the GL matrix because we must
@@ -131,7 +152,8 @@ public:
      */
     void setData(const GrOptDrawState&,
                  GrGpu::DrawType,
-                 const GrDeviceCoordTexture* dstCopy /* can be NULL*/);
+                 const GrDeviceCoordTexture* dstCopy, // can be NULL
+                 SharedGLState*);
 
 protected:
     typedef GrGLProgramDataManager::UniformHandle UniformHandle;
@@ -151,11 +173,11 @@ protected:
 
     // Helper for setData(). Makes GL calls to specify the initial color when there is not
     // per-vertex colors.
-    void setColor(const GrOptDrawState&, GrColor color);
+    void setColor(const GrOptDrawState&, GrColor color, SharedGLState*);
 
     // Helper for setData(). Makes GL calls to specify the initial coverage when there is not
     // per-vertex coverages.
-    void setCoverage(const GrOptDrawState&, GrColor coverage);
+    void setCoverage(const GrOptDrawState&, GrColor coverage, SharedGLState*);
 
     // A templated helper to loop over effects, set the transforms(via subclass) and bind textures
     void setFragmentData(const GrOptDrawState&);

src/gpu/gl/GrGLProgramDesc.cpp

@@ -257,27 +257,25 @@ bool GrGLProgramDesc::Build(const GrOptDrawState& optState,
 
     header->fEmitsPointSize = GrGpu::kDrawPoints_DrawType == drawType;
 
-    bool isPathRendering = GrGpu::IsPathRenderingDrawType(drawType);
+    if (gpu->caps()->pathRenderingSupport() &&
-    if (gpu->caps()->pathRenderingSupport() && isPathRendering) {
+        GrGpu::IsPathRenderingDrawType(drawType) &&
-        header->fUseNvpr = true;
+        gpu->glPathRendering()->texturingMode() == GrGLPathRendering::FixedFunction_TexturingMode) {
+        header->fUseFragShaderOnly = true;
         SkASSERT(!optState.hasGeometryProcessor());
     } else {
-        header->fUseNvpr = false;
+        header->fUseFragShaderOnly = false;
     }
 
-    bool hasUniformColor = inputColorIsUsed &&
+    bool defaultToUniformInputs = GrGpu::IsPathRenderingDrawType(drawType) ||
-                           (isPathRendering || !optState.hasColorVertexAttribute());
+                                  GR_GL_NO_CONSTANT_ATTRIBUTES;
-
-    bool hasUniformCoverage = inputCoverageIsUsed &&
-                              (isPathRendering || !optState.hasCoverageVertexAttribute());
 
     if (!inputColorIsUsed) {
         header->fColorInput = kAllOnes_ColorInput;
-    } else if (hasUniformColor) {
+    } else if (defaultToUniformInputs && !optState.hasColorVertexAttribute()) {
         header->fColorInput = kUniform_ColorInput;
     } else {
         header->fColorInput = kAttribute_ColorInput;
-        SkASSERT(!header->fUseNvpr);
+        SkASSERT(!header->fUseFragShaderOnly);
     }
 
     bool covIsSolidWhite = !optState.hasCoverageVertexAttribute() &&
@@ -285,11 +283,11 @@ bool GrGLProgramDesc::Build(const GrOptDrawState& optState,
 
     if (covIsSolidWhite || !inputCoverageIsUsed) {
         header->fCoverageInput = kAllOnes_ColorInput;
-    } else if (hasUniformCoverage) {
+    } else if (defaultToUniformInputs && !optState.hasCoverageVertexAttribute()) {
         header->fCoverageInput = kUniform_ColorInput;
     } else {
         header->fCoverageInput = kAttribute_ColorInput;
-        SkASSERT(!header->fUseNvpr);
+        SkASSERT(!header->fUseFragShaderOnly);
     }
 
     if (optState.readsDst()) {

src/gpu/gl/GrGLProgramDesc.h

@@ -94,7 +94,7 @@ private:
                                                         // effects that read the fragment position.
                                                         // Otherwise, 0.
 
-        SkBool8                     fUseNvpr;
+        SkBool8                     fUseFragShaderOnly;
         SkBool8                     fEmitsPointSize;
 
         ColorInput                       fColorInput : 8;

src/gpu/gl/GrGpuGL.cpp

@@ -342,6 +342,7 @@ void GrGpuGL::onResetContext(uint32_t resetBits) {
 
     if (resetBits & kProgram_GrGLBackendState) {
         fHWProgramID = 0;
+        fSharedGLProgramState.invalidate();
     }
 }
 

src/gpu/gl/GrGpuGL.h

@@ -288,6 +288,8 @@ private:
     int                         fHWActiveTextureUnitIdx;
     GrGLuint                    fHWProgramID;
 
+    GrGLProgram::SharedGLState  fSharedGLProgramState;
+
     enum TriState {
         kNo_TriState,
         kYes_TriState,

src/gpu/gl/GrGpuGL_program.cpp

@@ -256,7 +256,7 @@ bool GrGpuGL::flushGraphicsState(DrawType type,
 
         this->flushBlend(*optState.get(), kDrawLines_DrawType == type, srcCoeff, dstCoeff);
 
-        fCurrentProgram->setData(*optState.get(), type, dstCopy);
+        fCurrentProgram->setData(*optState.get(), type, dstCopy, &fSharedGLProgramState);
     }
 
     GrGLRenderTarget* glRT = static_cast<GrGLRenderTarget*>(optState->getRenderTarget());

src/gpu/gl/builders/GrGLProgramBuilder.cpp

@@ -55,9 +55,7 @@ GrGLProgram* GrGLProgramBuilder::CreateProgram(const GrOptDrawState& optState,
 
     // if we have a vertex shader(we don't only if we are using NVPR or NVPR ES), then we may have
     // to setup a few more things like builtin vertex attributes
-    bool hasVertexShader = !(header.fUseNvpr &&
+    bool hasVertexShader = !header.fUseFragShaderOnly;
-                             gpu->glPathRendering()->texturingMode() ==
-                             GrGLPathRendering::FixedFunction_TexturingMode);
     if (hasVertexShader) {
         pb->fVS.setupLocalCoords();
         pb->fVS.transformGLToSkiaCoords();
@@ -94,15 +92,18 @@ GrGLProgramBuilder::CreateProgramBuilder(const GrGLProgramDesc& desc,
                                          GrGpu::DrawType drawType,
                                          bool hasGeometryProcessor,
                                          GrGpuGL* gpu) {
-    if (desc.getHeader().fUseNvpr) {
+    if (desc.getHeader().fUseFragShaderOnly) {
         SkASSERT(gpu->glCaps().pathRenderingSupport());
+        SkASSERT(gpu->glPathRendering()->texturingMode() ==
+                 GrGLPathRendering::FixedFunction_TexturingMode);
         SkASSERT(!hasGeometryProcessor);
-        if (gpu->glPathRendering()->texturingMode() ==
+        return SkNEW_ARGS(GrGLLegacyNvprProgramBuilder, (gpu, optState, desc));
-            GrGLPathRendering::FixedFunction_TexturingMode) {
+    } else if (GrGpu::IsPathRenderingDrawType(drawType)) {
-            return SkNEW_ARGS(GrGLLegacyNvprProgramBuilder, (gpu, optState, desc));
+        SkASSERT(gpu->glCaps().pathRenderingSupport());
-        } else {
+        SkASSERT(gpu->glPathRendering()->texturingMode() ==
-            return SkNEW_ARGS(GrGLNvprProgramBuilder, (gpu, optState, desc));
+                 GrGLPathRendering::SeparableShaders_TexturingMode);
-        }
+        SkASSERT(!hasGeometryProcessor);
+        return SkNEW_ARGS(GrGLNvprProgramBuilder, (gpu, optState, desc));
     } else {
         return SkNEW_ARGS(GrGLProgramBuilder, (gpu, optState, desc));
     }
@@ -419,9 +420,7 @@ GrGLProgram* GrGLProgramBuilder::finalize() {
         this->cleanupProgram(programID, shadersToDelete);
         return NULL;
     }
-    if (!(this->header().fUseNvpr &&
+    if (!this->header().fUseFragShaderOnly) {
-          fGpu->glPathRendering()->texturingMode() ==
-          GrGLPathRendering::FixedFunction_TexturingMode)) {
         if (!fVS.compileAndAttachShaders(programID, &shadersToDelete)) {
             this->cleanupProgram(programID, shadersToDelete);
             return NULL;