Make GrSimpleMeshDrawOpHelper record whether coverage can be implemented as alpha

This also makes the blend table entry for opaque src over indicate a blend of (1, ISA) rather than (1, 0) to match the actual implementation of the global src-over XP.
Change-Id: I1b1f64d2546e4f0cf03c0239ce674d1baad655f6
Reviewed-on: https://skia-review.googlesource.com/18521
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

5 files changed, 41 insertions, 25 deletions

diff --git a/src/gpu/GrProcessorAnalysis.cpp b/src/gpu/GrProcessorAnalysis.cpp
index 317246d247..118e93da0b 100644
--- a/src/gpu/GrProcessorAnalysis.cpp
+++ b/src/gpu/GrProcessorAnalysis.cpp
@@ -27,14 +27,17 @@ void GrColorFragmentProcessorAnalysis::analyzeProcessors(
             // processors.
             fAllProcessorsCompatibleWithCoverageAsAlpha = true;
             fUsesLocalCoords = false;
-        } else if (fIsOpaque && !fp->preservesOpaqueInput()) {
-            fIsOpaque = false;
-        }
-        if (fAllProcessorsCompatibleWithCoverageAsAlpha && !fp->compatibleWithCoverageAsAlpha()) {
-            fAllProcessorsCompatibleWithCoverageAsAlpha = false;
-        }
-        if (fp->usesLocalCoords()) {
-            fUsesLocalCoords = true;
+        } else {
+            if (fIsOpaque && !fp->preservesOpaqueInput()) {
+                fIsOpaque = false;
+            }
+            if (fAllProcessorsCompatibleWithCoverageAsAlpha &&
+                !fp->compatibleWithCoverageAsAlpha()) {
+                fAllProcessorsCompatibleWithCoverageAsAlpha = false;
+            }
+            if (fp->usesLocalCoords()) {
+                fUsesLocalCoords = true;
+            }
         }
         ++fTotalProcessorsVisited;
     }
diff --git a/src/gpu/GrProcessorAnalysis.h b/src/gpu/GrProcessorAnalysis.h
index ad56ab4e2d..f5e4065ac2 100644
--- a/src/gpu/GrProcessorAnalysis.h
+++ b/src/gpu/GrProcessorAnalysis.h
@@ -117,16 +117,17 @@ public:
 
     /**
      * Are all the fragment processors compatible with conflating coverage with color prior to the
-     * the first fragment processor. This result does not consider processors that should be
-     * eliminated as indicated by initialProcessorsToEliminate().
+     * the first fragment processor. This result assumes that processors that should be eliminated
+     * as indicated by initialProcessorsToEliminate() are in fact eliminated.
      */
     bool allProcessorsCompatibleWithCoverageAsAlpha() const {
         return fAllProcessorsCompatibleWithCoverageAsAlpha;
     }
 
     /**
-     * Do any of the fragment processors require local coords. This result does not consider
-     * processors that should be eliminated as indicated by initialProcessorsToEliminate().
+     * Do any of the fragment processors require local coords. This result assumes that
+     * processors that should be eliminated as indicated by initialProcessorsToEliminate() are in
+     * fact eliminated.
      */
     bool usesLocalCoords() const { return fUsesLocalCoords; }
 
diff --git a/src/gpu/effects/GrPorterDuffXferProcessor.cpp b/src/gpu/effects/GrPorterDuffXferProcessor.cpp
index 9012d2028c..254b44b055 100644
--- a/src/gpu/effects/GrPorterDuffXferProcessor.cpp
+++ b/src/gpu/effects/GrPorterDuffXferProcessor.cpp
@@ -321,7 +321,7 @@ static MAYBE_CONSTEXPR BlendFormula gBlendTable[2][2][(int)SkBlendMode::kLastCoe
     /* clear */      MakeCoeffFormula(kZero_GrBlendCoeff, kZero_GrBlendCoeff),
     /* src */        MakeCoeffFormula(kOne_GrBlendCoeff,  kZero_GrBlendCoeff),
     /* dst */        MakeCoeffFormula(kZero_GrBlendCoeff, kOne_GrBlendCoeff),
-    /* src-over */   MakeCoeffFormula(kOne_GrBlendCoeff,  kZero_GrBlendCoeff),
+    /* src-over */   MakeCoeffFormula(kOne_GrBlendCoeff,  kISA_GrBlendCoeff), // see comment below
     /* dst-over */   MakeCoeffFormula(kIDA_GrBlendCoeff,  kOne_GrBlendCoeff),
     /* src-in */     MakeCoeffFormula(kDA_GrBlendCoeff,   kZero_GrBlendCoeff),
     /* dst-in */     MakeCoeffFormula(kZero_GrBlendCoeff, kOne_GrBlendCoeff),
@@ -352,6 +352,10 @@ static MAYBE_CONSTEXPR BlendFormula gBlendTable[2][2][(int)SkBlendMode::kLastCoe
     /* modulate */   MakeCoverageSrcCoeffZeroFormula(BlendFormula::kISCModulate_OutputType),
     /* screen */     MakeCoeffFormula(kOne_GrBlendCoeff,  kISC_GrBlendCoeff),
 }}};
+// In the above table src-over is not optimized to src mode when the color is opaque because we
+// found no advantage to doing so. Also, we are using a global src-over XP in most cases which is
+// not specialized for opaque input. If the table were set to use the src formula then we'd have to
+// change when we use this global XP to keep analysis and practice in sync.
 
 static MAYBE_CONSTEXPR BlendFormula gLCDBlendTable[(int)SkBlendMode::kLastCoeffMode + 1] = {
     /* clear */      MakeCoverageSrcCoeffZeroFormula(BlendFormula::kCoverage_OutputType),
diff --git a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
index c230257c1c..dcd8dd887e 100644
--- a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
+++ b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
@@ -41,7 +41,8 @@ public:
             , fPipelineFlags(args.fSRGBFlags)
             , fAAType((int)aaType)
             , fRequiresDstTexture(false)
-            , fUsesLocalCoords(false) {
+            , fUsesLocalCoords(false)
+            , fCompatibleWithAlphaAsCoveage(false) {
         SkASSERT(!stencilSettings);
         SkDEBUGCODE(fDidAnalysis = false);
         if (GrAATypeIsHW(aaType)) {
@@ -80,12 +81,16 @@ public:
                 }
             }
         }
-        return fPipelineFlags == that.fPipelineFlags && fAAType == that.fAAType;
+        bool result = fPipelineFlags == that.fPipelineFlags && fAAType == that.fAAType;
+        SkASSERT(!result || fCompatibleWithAlphaAsCoveage == that.fCompatibleWithAlphaAsCoveage);
+        SkASSERT(!result || fUsesLocalCoords == that.fUsesLocalCoords);
+        return result;
     }
 
     bool xpRequiresDstTexture(const GrCaps& caps, const GrAppliedClip* clip,
                               GrProcessorAnalysisCoverage geometryCoverage, GrColor* color) {
         SkDEBUGCODE(fDidAnalysis = true);
+        GrProcessorSet::Analysis analysis;
         if (fProcessors) {
             GrProcessorAnalysisCoverage coverage = geometryCoverage;
             if (GrProcessorAnalysisCoverage::kNone == coverage) {
@@ -94,15 +99,14 @@ public:
                                    : GrProcessorAnalysisCoverage::kNone;
             }
             bool isMixedSamples = this->aaType() == GrAAType::kMixedSamples;
-            GrProcessorSet::Analysis analysis =
-                    fProcessors->finalize(*color, coverage, clip, isMixedSamples, caps, color);
-            fRequiresDstTexture = analysis.requiresDstTexture();
-            fUsesLocalCoords = analysis.usesLocalCoords();
-            return analysis.requiresDstTexture();
+            analysis = fProcessors->finalize(*color, coverage, clip, isMixedSamples, caps, color);
         } else {
-            fUsesLocalCoords = GrProcessorSet::EmptySetAnalysis().usesLocalCoords();
-            return GrProcessorSet::EmptySetAnalysis().requiresDstTexture();
+            analysis = GrProcessorSet::EmptySetAnalysis();
         }
+        fRequiresDstTexture = analysis.requiresDstTexture();
+        fUsesLocalCoords = analysis.usesLocalCoords();
+        fCompatibleWithAlphaAsCoveage = analysis.isCompatibleWithCoverageAsAlpha();
+        return analysis.requiresDstTexture();
     }
 
     bool usesLocalCoords() const {
@@ -110,6 +114,8 @@ public:
         return fUsesLocalCoords;
     }
 
+    bool compatibleWithAlphaAsCoverage() const { return fCompatibleWithAlphaAsCoveage; }
+
     GrPipeline* makePipeline(GrMeshDrawOp::Target* target) const {
         return target->allocPipeline(this->pipelineInitArgs(target));
     }
@@ -149,6 +155,7 @@ private:
     unsigned fAAType : 2;
     unsigned fRequiresDstTexture : 1;
     unsigned fUsesLocalCoords : 1;
+    unsigned fCompatibleWithAlphaAsCoveage : 1;
     SkDEBUGCODE(unsigned fDidAnalysis : 1;)
 };
 
diff --git a/tests/GrPorterDuffTest.cpp b/tests/GrPorterDuffTest.cpp
index bf325981b4..78497d2b31 100644
--- a/tests/GrPorterDuffTest.cpp
+++ b/tests/GrPorterDuffTest.cpp
@@ -873,14 +873,15 @@ static void test_color_opaque_no_coverage(skiatest::Reporter* reporter, const Gr
                 TEST_ASSERT(!xpi.fBlendInfo.fWriteColor);
                 break;
             case SkBlendMode::kSrcOver:
-                TEST_ASSERT(xpi.fCanCombineOverlappedStencilAndCover);
+                // We don't specialize opaque src-over. See note in GrPorterDuffXferProcessor.cpp
+                TEST_ASSERT(!xpi.fCanCombineOverlappedStencilAndCover);
                 TEST_ASSERT(!xpi.fIgnoresInputColor);
-                TEST_ASSERT(!xpi.fCompatibleWithCoverageAsAlpha);
+                TEST_ASSERT(xpi.fCompatibleWithCoverageAsAlpha);
                 TEST_ASSERT(kModulate_OutputType == xpi.fPrimaryOutputType);
                 TEST_ASSERT(kNone_OutputType == xpi.fSecondaryOutputType);
                 TEST_ASSERT(kAdd_GrBlendEquation == xpi.fBlendInfo.fEquation);
                 TEST_ASSERT(kOne_GrBlendCoeff == xpi.fBlendInfo.fSrcBlend);
-                TEST_ASSERT(kZero_GrBlendCoeff == xpi.fBlendInfo.fDstBlend);
+                TEST_ASSERT(kISA_GrBlendCoeff == xpi.fBlendInfo.fDstBlend);
                 TEST_ASSERT(xpi.fBlendInfo.fWriteColor);
                 break;
             case SkBlendMode::kDstOver: