From 6ebe4b9dbeab68ca3b6da61fd08f22cdc080267d Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Fri, 19 May 2017 10:56:46 -0400 Subject: [PATCH] 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 Commit-Queue: Greg Daniel --- src/gpu/GrXferProcessor.cpp | 14 +++++-- src/gpu/GrXferProcessor.h | 14 +++++-- src/gpu/effects/GrCustomXfermode.cpp | 10 ++--- src/gpu/effects/GrPorterDuffXferProcessor.cpp | 55 ++++++++++++++++++--------- src/gpu/glsl/GrGLSLXferProcessor.cpp | 27 +++++++++++++ tests/GrPorterDuffTest.cpp | 25 +++++++++--- 6 files changed, 111 insertions(+), 34 deletions(-) diff --git a/src/gpu/GrXferProcessor.cpp b/src/gpu/GrXferProcessor.cpp index 27c8a96..8c0568b 100644 --- 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 --- 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 --- 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(bool hasMixedSamples, SkBlendMode mode) - : INHERITED(true, hasMixedSamples) + CustomXP(bool hasMixedSamples, SkBlendMode mode, GrProcessorAnalysisCoverage coverage) + : INHERITED(true, hasMixedSamples, coverage) , fMode(mode) , fHWBlendEquation(static_cast(-1)) { this->initClassID(); @@ -242,7 +242,7 @@ sk_sp CustomXPFactory::makeXferProcessor( if (can_use_hw_blend_equation(fHWBlendEquation, coverage, caps)) { return sk_sp(new CustomXP(fMode, fHWBlendEquation)); } - return sk_sp(new CustomXP(hasMixedSamples, fMode)); + return sk_sp(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 --- 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(); } @@ -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(); } @@ -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(); } @@ -754,8 +758,9 @@ sk_sp 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 GrPorterDuffXPFactory::makeXferProcessor( hasMixedSamples, fBlendMode); } - if (blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport()) { - return sk_sp(new ShaderPDXferProcessor(hasMixedSamples, fBlendMode)); + if ((blendFormula.hasSecondaryOutput() && !caps.shaderCaps()->dualSourceBlendingSupport()) || + (isLCD && !color.isOpaque())) { + return sk_sp(new ShaderPDXferProcessor(hasMixedSamples, fBlendMode, + coverage)); } - return sk_sp(new PorterDuffXferProcessor(blendFormula)); + return sk_sp(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 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 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( - new ShaderPDXferProcessor(hasMixedSamples, SkBlendMode::kSrcOver)); + new ShaderPDXferProcessor(hasMixedSamples, SkBlendMode::kSrcOver, coverage)); } - return sk_sp(new PorterDuffXferProcessor(blendFormula)); + return sk_sp(new PorterDuffXferProcessor(blendFormula, coverage)); } sk_sp GrPorterDuffXPFactory::MakeNoCoverageXP(SkBlendMode blendmode) { BlendFormula formula = get_blend_formula(false, false, false, blendmode); - return sk_make_sp(formula); + return sk_make_sp(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 --- 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 --- 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 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 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); } -- 2.7.4