Revert "Revert "Go back to using dual source blending for lcd src-over even with non-opaque color""

This reverts commit 7d6fe0b996.

Reason for revert: Relanding with fix

Original change's description:
> Revert "Go back to using dual source blending for lcd src-over even with non-opaque color"
> 
> This reverts commit b54bdef86e.
> 
> Reason for revert: breaking some bots
> Original change's description:
> > Go back to using dual source blending for lcd src-over even with non-opaque color
> > 
> > This is change is currently still safe since earlier in Skia we are still requiring
> > the dst to be opaque. The change is a workaround to spots where trying to read the
> > dst to do in shader blending is failing for some reason. This also should give back
> > a little performance since doing dual source blending should be better than shader
> > blends.
> > 
> > Bug: chromium:732341
> > Change-Id: I795f8a520f87f3fbf5d63a9509fbd9f394ea2b29
> > Reviewed-on: https://skia-review.googlesource.com/19703
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> > Commit-Queue: Greg Daniel <egdaniel@google.com>
> 
> TBR=egdaniel@google.com,bsalomon@google.com
> 
> Change-Id: Ibb9bc1ef4ec5967dabcd62c81f62c0989c14fbb8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: chromium:732341
> Reviewed-on: https://skia-review.googlesource.com/19815
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>

TBR=egdaniel@google.com,bsalomon@google.com
Bug: chromium:732341

Change-Id: I7481755a9aa64364371d8149af4458fc2c15c8aa
Reviewed-on: https://skia-review.googlesource.com/19840
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>

src/gpu/effects/GrPorterDuffXferProcessor.cpp

@@ -764,7 +764,8 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::makeXferProcessor(
     BlendFormula blendFormula;
     bool isLCD = coverage == GrProcessorAnalysisCoverage::kLCD;
     if (isLCD) {
-        if (SkBlendMode::kSrcOver == fBlendMode && color.isConstant() && color.isOpaque() &&
+        // See comment in MakeSrcOverXferProcessor about color.isOpaque here
+        if (SkBlendMode::kSrcOver == fBlendMode && color.isConstant() && /*color.isOpaque() &&*/
             !caps.shaderCaps()->dualSourceBlendingSupport() &&
             !caps.shaderCaps()->dstReadInShaderSupport()) {
             // If we don't have dual source blending or in shader dst reads, we fall back to this
@@ -779,7 +780,7 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::makeXferProcessor(
     }
 
     if ((blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport()) ||
-        (isLCD && (SkBlendMode::kSrcOver != fBlendMode || !color.isOpaque()))) {
+        (isLCD && (SkBlendMode::kSrcOver != fBlendMode /*|| !color.isOpaque()*/))) {
         return sk_sp<const GrXferProcessor>(new ShaderPDXferProcessor(hasMixedSamples, fBlendMode,
                                                                       coverage));
     }
@@ -817,8 +818,8 @@ static inline GrXPFactory::AnalysisProperties analysis_properties(
             // and DstATop would also work). We also fall into the dst read case for src-over if we
             // do not have dual source blending.
             if (SkBlendMode::kSrcOver != mode ||
-                !color.isOpaque() ||
+                /*!color.isOpaque() ||*/ // See comment in MakeSrcOverXferProcessor about isOpaque.
-                !caps.shaderCaps()->dualSourceBlendingSupport()) {
+                (formula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport())) {
                 props |= AnalysisProperties::kReadsDstInShader;
             }
         }
@@ -903,8 +904,10 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::MakeSrcOverXferProcessor(
 
     // Currently up the stack Skia is requiring that the dst is opaque or that the client has said
     // the opaqueness doesn't matter. Thus for src-over we don't need to worry about the src color
-    // being opaque or not. For now we disable the check for opaqueness, but in the future we should
+    // being opaque or not. This allows us to use faster code paths as well as avoid various bugs
-    // pass down the knowledge about dst opaqueness and make the correct decision here.
+    // that occur with dst reads in the shader blending. For now we disable the check for
+    // opaqueness, but in the future we should pass down the knowledge about dst opaqueness and make
+    // the correct decision here.
     //
     // This also fixes a chrome bug on macs where we are getting random fuzziness when doing
     // blending in the shader for non opaque sources.
@@ -919,7 +922,8 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::MakeSrcOverXferProcessor(
 
     BlendFormula blendFormula;
     blendFormula = get_lcd_blend_formula(SkBlendMode::kSrcOver);
-    if (!color.isOpaque() ||
+    // See comment above regarding why the opaque check is commented out here.
+    if (/*!color.isOpaque() ||*/
         (blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport())) {
         return sk_sp<GrXferProcessor>(
                 new ShaderPDXferProcessor(hasMixedSamples, SkBlendMode::kSrcOver, coverage));

tests/GrPorterDuffTest.cpp

@@ -1032,8 +1032,8 @@ static void test_lcd_coverage_fallback_case(skiatest::Reporter* reporter, const
     // Test with non-opaque alpha
     color = GrColorPackRGBA(123, 45, 67, 221);
     coverage = GrProcessorAnalysisCoverage::kLCD;
-    TEST_ASSERT(GrXPFactory::GetAnalysisProperties(xpf, color, coverage, caps) &
+    TEST_ASSERT(!(GrXPFactory::GetAnalysisProperties(xpf, color, coverage, caps) &
-                GrXPFactory::AnalysisProperties::kRequiresDstTexture);
+                GrXPFactory::AnalysisProperties::kRequiresDstTexture));
     sk_sp<const GrXferProcessor> xp(
             GrXPFactory::MakeXferProcessor(xpf, color, coverage, false, caps));
     if (!xp) {