Fix gpu lcd blending to semi-correctly handle alpha coverage

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

diff --git a/src/gpu/GrXferProcessor.cpp b/src/gpu/GrXferProcessor.cpp
index 27c8a96..8c0568b 100644 (file)
--- a/src/gpu/GrXferProcessor.cpp
+++ b/src/gpu/GrXferProcessor.cpp
@@ -9,11 +9,16 @@
 #include "GrPipeline.h"
 #include "gl/GrGLCaps.h"
 
-GrXferProcessor::GrXferProcessor() : fWillReadDstColor(false), fDstReadUsesMixedSamples(false) {}
+GrXferProcessor::GrXferProcessor()
+        : fWillReadDstColor(false)
+        , fDstReadUsesMixedSamples(false)
+        , fIsLCD(false) {}
 
-GrXferProcessor::GrXferProcessor(bool willReadDstColor, bool hasMixedSamples)
+GrXferProcessor::GrXferProcessor(bool willReadDstColor, bool hasMixedSamples,
+                                 GrProcessorAnalysisCoverage coverage)
         : fWillReadDstColor(willReadDstColor)
-        , fDstReadUsesMixedSamples(willReadDstColor && hasMixedSamples) {}
+        , fDstReadUsesMixedSamples(willReadDstColor && hasMixedSamples)
+        , fIsLCD(GrProcessorAnalysisCoverage::kLCD == coverage) {}
 
 bool GrXferProcessor::hasSecondaryOutput() const {
     if (!this->willReadDstColor()) {
@@ -45,6 +50,9 @@ void GrXferProcessor::getGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorK
             key |= 0x8;
         }
     }
+    if (fIsLCD) {
+        key |= 0x10;
+    }
     b->add32(key);
     this->onGetGLSLProcessorKey(caps, b);
 }

diff --git a/src/gpu/GrXferProcessor.h b/src/gpu/GrXferProcessor.h
index a739c8b..ed8e707 100644 (file)
--- a/src/gpu/GrXferProcessor.h
+++ b/src/gpu/GrXferProcessor.h
@@ -153,6 +153,8 @@ public:
      */
     bool hasSecondaryOutput() const;
 
+    bool isLCD() const { return fIsLCD; }
+
     /** Returns true if this and other processor conservatively draw identically. It can only return
         true when the two processor are of the same subclass (i.e. they return the same object from
         from getFactory()).
@@ -160,7 +162,7 @@ public:
         A return value of true from isEqual() should not be used to test whether the processor would
         generate the same shader code. To test for identical code generation use getGLSLProcessorKey
       */
-    
+
     bool isEqual(const GrXferProcessor& that) const {
         if (this->classID() != that.classID()) {
             return false;
@@ -171,12 +173,15 @@ public:
         if (this->fDstReadUsesMixedSamples != that.fDstReadUsesMixedSamples) {
             return false;
         }
+        if (fIsLCD != that.fIsLCD) {
+            return false;
+        }
         return this->onIsEqual(that);
     }
 
 protected:
     GrXferProcessor();
-    GrXferProcessor(bool willReadDstColor, bool hasMixedSamples);
+    GrXferProcessor(bool willReadDstColor, bool hasMixedSamples, GrProcessorAnalysisCoverage);
 
 private:
     /**
@@ -201,8 +206,9 @@ private:
 
     virtual bool onIsEqual(const GrXferProcessor&) const = 0;
 
-    bool                    fWillReadDstColor;
-    bool                    fDstReadUsesMixedSamples;
+    bool fWillReadDstColor;
+    bool fDstReadUsesMixedSamples;
+    bool fIsLCD;
 
     typedef GrFragmentProcessor INHERITED;
 };

diff --git a/src/gpu/effects/GrCustomXfermode.cpp b/src/gpu/effects/GrCustomXfermode.cpp
index 970c289..a2302e0 100644 (file)
--- a/src/gpu/effects/GrCustomXfermode.cpp
+++ b/src/gpu/effects/GrCustomXfermode.cpp
@@ -73,13 +73,13 @@ static bool can_use_hw_blend_equation(GrBlendEquation equation,
 class CustomXP : public GrXferProcessor {
 public:
     CustomXP(SkBlendMode mode, GrBlendEquation hwBlendEquation)
-        : fMode(mode),
-          fHWBlendEquation(hwBlendEquation) {
+        : fMode(mode)
+        , fHWBlendEquation(hwBlendEquation) {
         this->initClassID<CustomXP>();
     }
 
-    CustomXP(bool hasMixedSamples, SkBlendMode mode)
-            : INHERITED(true, hasMixedSamples)
+    CustomXP(bool hasMixedSamples, SkBlendMode mode, GrProcessorAnalysisCoverage coverage)
+            : INHERITED(true, hasMixedSamples, coverage)
             , fMode(mode)
             , fHWBlendEquation(static_cast<GrBlendEquation>(-1)) {
         this->initClassID<CustomXP>();
@@ -242,7 +242,7 @@ sk_sp<const GrXferProcessor> CustomXPFactory::makeXferProcessor(
     if (can_use_hw_blend_equation(fHWBlendEquation, coverage, caps)) {
         return sk_sp<GrXferProcessor>(new CustomXP(fMode, fHWBlendEquation));
     }
-    return sk_sp<GrXferProcessor>(new CustomXP(hasMixedSamples, fMode));
+    return sk_sp<GrXferProcessor>(new CustomXP(hasMixedSamples, fMode, coverage));
 }
 
 GrXPFactory::AnalysisProperties CustomXPFactory::analysisProperties(

diff --git a/src/gpu/effects/GrPorterDuffXferProcessor.cpp b/src/gpu/effects/GrPorterDuffXferProcessor.cpp
index e3548ad..d3ef717 100644 (file)
--- a/src/gpu/effects/GrPorterDuffXferProcessor.cpp
+++ b/src/gpu/effects/GrPorterDuffXferProcessor.cpp
@@ -392,7 +392,9 @@ static BlendFormula get_lcd_blend_formula(SkBlendMode xfermode) {
 
 class PorterDuffXferProcessor : public GrXferProcessor {
 public:
-    PorterDuffXferProcessor(BlendFormula blendFormula) : fBlendFormula(blendFormula) {
+    PorterDuffXferProcessor(BlendFormula blendFormula, GrProcessorAnalysisCoverage coverage)
+            : INHERITED(false, false, coverage)
+            , fBlendFormula(blendFormula) {
         this->initClassID<PorterDuffXferProcessor>();
     }
 
@@ -501,8 +503,9 @@ GrGLSLXferProcessor* PorterDuffXferProcessor::createGLSLInstance() const {
 
 class ShaderPDXferProcessor : public GrXferProcessor {
 public:
-    ShaderPDXferProcessor(bool hasMixedSamples, SkBlendMode xfermode)
-            : INHERITED(true, hasMixedSamples), fXfermode(xfermode) {
+    ShaderPDXferProcessor(bool hasMixedSamples, SkBlendMode xfermode,
+                          GrProcessorAnalysisCoverage coverage)
+            : INHERITED(true, hasMixedSamples, coverage), fXfermode(xfermode) {
         this->initClassID<ShaderPDXferProcessor>();
     }
 
@@ -649,7 +652,8 @@ private:
 ///////////////////////////////////////////////////////////////////////////////
 
 PDLCDXferProcessor::PDLCDXferProcessor(GrColor blendConstant, uint8_t alpha)
-    : fBlendConstant(blendConstant)
+    : INHERITED(false, false, GrProcessorAnalysisCoverage::kLCD)
+    , fBlendConstant(blendConstant)
     , fAlpha(alpha) {
     this->initClassID<PDLCDXferProcessor>();
 }
@@ -754,8 +758,9 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::makeXferProcessor(
         const GrProcessorAnalysisColor& color, GrProcessorAnalysisCoverage coverage,
         bool hasMixedSamples, const GrCaps& caps) const {
     BlendFormula blendFormula;
-    if (coverage == GrProcessorAnalysisCoverage::kLCD) {
-        if (SkBlendMode::kSrcOver == fBlendMode && color.isConstant() &&
+    bool isLCD = coverage == GrProcessorAnalysisCoverage::kLCD;
+    if (isLCD) {
+        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
@@ -769,10 +774,12 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::makeXferProcessor(
                                   hasMixedSamples, fBlendMode);
     }
 
-    if (blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport()) {
-        return sk_sp<const GrXferProcessor>(new ShaderPDXferProcessor(hasMixedSamples, fBlendMode));
+    if ((blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport()) ||
+        (isLCD && !color.isOpaque())) {
+        return sk_sp<const GrXferProcessor>(new ShaderPDXferProcessor(hasMixedSamples, fBlendMode,
+                                                                      coverage));
     }
-    return sk_sp<const GrXferProcessor>(new PorterDuffXferProcessor(blendFormula));
+    return sk_sp<const GrXferProcessor>(new PorterDuffXferProcessor(blendFormula, coverage));
 }
 
 static inline GrXPFactory::AnalysisProperties analysis_properties(
@@ -781,7 +788,13 @@ static inline GrXPFactory::AnalysisProperties analysis_properties(
     using AnalysisProperties = GrXPFactory::AnalysisProperties;
     AnalysisProperties props = AnalysisProperties::kNone;
     bool hasCoverage = GrProcessorAnalysisCoverage::kNone != coverage;
-    auto formula = gBlendTable[color.isOpaque()][hasCoverage][(int)mode];
+    bool isLCD = GrProcessorAnalysisCoverage::kLCD == coverage;
+    BlendFormula formula;
+    if (isLCD) {
+        formula = gLCDBlendTable[(int)mode];
+    } else {
+        formula = gBlendTable[color.isOpaque()][hasCoverage][(int)mode];
+    }
     if (formula.canTweakAlphaForCoverage()) {
         props |= AnalysisProperties::kCompatibleWithAlphaAsCoverage;
     }
@@ -794,7 +807,7 @@ static inline GrXPFactory::AnalysisProperties analysis_properties(
         if (GrProcessorAnalysisCoverage::kLCD == coverage) {
             // Check for special case of srcover with a known color which can be done using the
             // blend constant.
-            if (SkBlendMode::kSrcOver == mode && color.isConstant()) {
+            if (SkBlendMode::kSrcOver == mode && color.isConstant() && color.isOpaque()) {
                 props |= AnalysisProperties::kIgnoresInputColor;
             } else {
                 if (get_lcd_blend_formula(mode).hasSecondaryOutput()) {
@@ -804,6 +817,12 @@ static inline GrXPFactory::AnalysisProperties analysis_properties(
         } else if (formula.hasSecondaryOutput()) {
             props |= AnalysisProperties::kReadsDstInShader;
         }
+    } else {
+        // For LCD blending, if the color is not opaque we must read the dst in shader even if we
+        // have dual source blending.
+        if (isLCD && !color.isOpaque()) {
+            props |= AnalysisProperties::kReadsDstInShader;
+        }
     }
     if (!formula.modifiesDst() || !formula.usesInputColor()) {
         props |= AnalysisProperties::kIgnoresInputColor;
@@ -851,7 +870,8 @@ void GrPorterDuffXPFactory::TestGetXPOutputTypes(const GrXferProcessor* xp,
 const GrXferProcessor& GrPorterDuffXPFactory::SimpleSrcOverXP() {
     static BlendFormula gSrcOverBlendFormula =
             MakeCoeffFormula(kOne_GrBlendCoeff, kISA_GrBlendCoeff);
-    static PorterDuffXferProcessor gSrcOverXP(gSrcOverBlendFormula);
+    static PorterDuffXferProcessor gSrcOverXP(gSrcOverBlendFormula,
+                                              GrProcessorAnalysisCoverage::kSingleChannel);
     return gSrcOverXP;
 }
 
@@ -870,7 +890,7 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::MakeSrcOverXferProcessor(
         return nullptr;
     }
 
-    if (color.isConstant() && !caps.shaderCaps()->dualSourceBlendingSupport() &&
+    if (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 trick for rendering SrcOver LCD text instead of doing a
@@ -880,16 +900,17 @@ sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::MakeSrcOverXferProcessor(
 
     BlendFormula blendFormula;
     blendFormula = get_lcd_blend_formula(SkBlendMode::kSrcOver);
-    if (blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport()) {
+    if (!color.isOpaque() ||
+        (blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport())) {
         return sk_sp<GrXferProcessor>(
-                new ShaderPDXferProcessor(hasMixedSamples, SkBlendMode::kSrcOver));
+                new ShaderPDXferProcessor(hasMixedSamples, SkBlendMode::kSrcOver, coverage));
     }
-    return sk_sp<GrXferProcessor>(new PorterDuffXferProcessor(blendFormula));
+    return sk_sp<GrXferProcessor>(new PorterDuffXferProcessor(blendFormula, coverage));
 }
 
 sk_sp<const GrXferProcessor> GrPorterDuffXPFactory::MakeNoCoverageXP(SkBlendMode blendmode) {
     BlendFormula formula = get_blend_formula(false, false, false, blendmode);
-    return sk_make_sp<PorterDuffXferProcessor>(formula);
+    return sk_make_sp<PorterDuffXferProcessor>(formula, GrProcessorAnalysisCoverage::kNone);
 }
 
 GrXPFactory::AnalysisProperties GrPorterDuffXPFactory::SrcOverAnalysisProperties(

diff --git a/src/gpu/glsl/GrGLSLXferProcessor.cpp b/src/gpu/glsl/GrGLSLXferProcessor.cpp
index 545a9fd..52656b1 100644 (file)
--- a/src/gpu/glsl/GrGLSLXferProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLXferProcessor.cpp
@@ -13,8 +13,22 @@
 #include "glsl/GrGLSLProgramDataManager.h"
 #include "glsl/GrGLSLUniformHandler.h"
 
+// This is only called for cases where we are doing LCD coverage and not using in shader blending.
+// For these cases we assume the the src alpha is 1, thus we can just use the max for the alpha
+// coverage since src alpha will always be greater than or equal to dst alpha.
+static void adjust_for_lcd_coverage(GrGLSLXPFragmentBuilder* fragBuilder,
+                                    const char* srcCoverage,
+                                    const GrXferProcessor& proc) {
+    if (srcCoverage && proc.isLCD()) {
+        fragBuilder->codeAppendf("%s.a = max(max(%s.r, %s.g), %s.b);",
+                                 srcCoverage, srcCoverage, srcCoverage, srcCoverage);
+    }
+}
+
+
 void GrGLSLXferProcessor::emitCode(const EmitArgs& args) {
     if (!args.fXP.willReadDstColor()) {
+        adjust_for_lcd_coverage(args.fXPFragBuilder, args.fInputCoverage, args.fXP);
         this->emitOutputsForBlendState(args);
         return;
     }
@@ -111,13 +125,26 @@ void GrGLSLXferProcessor::DefaultCoverageModulation(GrGLSLXPFragmentBuilder* fra
                                                     const GrXferProcessor& proc) {
     if (proc.dstReadUsesMixedSamples()) {
         if (srcCoverage) {
+            SkASSERT(!proc.isLCD());
             fragBuilder->codeAppendf("%s *= %s;", outColor, srcCoverage);
             fragBuilder->codeAppendf("%s = %s;", outColorSecondary, srcCoverage);
         } else {
             fragBuilder->codeAppendf("%s = vec4(1.0);", outColorSecondary);
         }
     } else if (srcCoverage) {
+        if (proc.isLCD()) {
+            fragBuilder->codeAppendf("float lerpRed = mix(%s.a, %s.a, %s.r);",
+                                     dstColor, outColor, srcCoverage);
+            fragBuilder->codeAppendf("float lerpBlue = mix(%s.a, %s.a, %s.g);",
+                                     dstColor, outColor, srcCoverage);
+            fragBuilder->codeAppendf("float lerpGreen = mix(%s.a, %s.a, %s.b);",
+                                     dstColor, outColor, srcCoverage);
+        }
         fragBuilder->codeAppendf("%s = %s * %s + (vec4(1.0) - %s) * %s;",
                                  outColor, srcCoverage, outColor, srcCoverage, dstColor);
+        if (proc.isLCD()) {
+            fragBuilder->codeAppendf("%s.a = max(max(lerpRed, lerpBlue), lerpGreen);", outColor);
+        }
     }
 }
+

diff --git a/tests/GrPorterDuffTest.cpp b/tests/GrPorterDuffTest.cpp
index 780573d..f000077 100644 (file)
--- a/tests/GrPorterDuffTest.cpp
+++ b/tests/GrPorterDuffTest.cpp
@@ -104,7 +104,7 @@ public:
 };
 
 static void test_lcd_coverage(skiatest::Reporter* reporter, const GrCaps& caps) {
-    GrProcessorAnalysisColor inputColor = GrProcessorAnalysisColor::Opaque::kNo;
+    GrProcessorAnalysisColor inputColor = GrProcessorAnalysisColor::Opaque::kYes;
     GrProcessorAnalysisCoverage inputCoverage = GrProcessorAnalysisCoverage::kLCD;
 
     for (int m = 0; m <= (int)SkBlendMode::kLastCoeffMode; m++) {
@@ -1004,10 +1004,26 @@ static void test_color_opaque_no_coverage(skiatest::Reporter* reporter, const Gr
 
 static void test_lcd_coverage_fallback_case(skiatest::Reporter* reporter, const GrCaps& caps) {
     const GrXPFactory* xpf = GrPorterDuffXPFactory::Get(SkBlendMode::kSrcOver);
-    GrProcessorAnalysisColor color = GrColorPackRGBA(123, 45, 67, 221);
+    GrProcessorAnalysisColor color = GrColorPackRGBA(123, 45, 67, 255);
     GrProcessorAnalysisCoverage coverage = GrProcessorAnalysisCoverage::kLCD;
-    SkASSERT(!(GrXPFactory::GetAnalysisProperties(xpf, color, coverage, caps) &
-               GrXPFactory::AnalysisProperties::kRequiresDstTexture));
+    TEST_ASSERT(!(GrXPFactory::GetAnalysisProperties(xpf, color, coverage, caps) &
+                  GrXPFactory::AnalysisProperties::kRequiresDstTexture));
+    sk_sp<const GrXferProcessor> xp_opaque(
+            GrXPFactory::MakeXferProcessor(xpf, color, coverage, false, caps));
+    if (!xp_opaque) {
+        ERRORF(reporter, "Failed to create an XP with LCD coverage.");
+        return;
+    }
+
+    GrXferProcessor::BlendInfo blendInfo;
+    xp_opaque->getBlendInfo(&blendInfo);
+    TEST_ASSERT(blendInfo.fWriteColor);
+
+    // Test with non-opaque alpha
+    color = GrColorPackRGBA(123, 45, 67, 221);
+    coverage = GrProcessorAnalysisCoverage::kLCD;
+    TEST_ASSERT(GrXPFactory::GetAnalysisProperties(xpf, color, coverage, caps) &
+                GrXPFactory::AnalysisProperties::kRequiresDstTexture);
     sk_sp<const GrXferProcessor> xp(
             GrXPFactory::MakeXferProcessor(xpf, color, coverage, false, caps));
     if (!xp) {
@@ -1015,7 +1031,6 @@ static void test_lcd_coverage_fallback_case(skiatest::Reporter* reporter, const
         return;
     }
 
-    GrXferProcessor::BlendInfo blendInfo;
     xp->getBlendInfo(&blendInfo);
     TEST_ASSERT(blendInfo.fWriteColor);
 }