From af96fce87937aba4aa5d394ee6316ba790308490 Mon Sep 17 00:00:00 2001 From: ericrk Date: Mon, 19 Oct 2015 14:41:11 -0700 Subject: [PATCH] The SkArithmeticMode_gpu GL filters support clamping to valid premultiplied colors, however the flag for whether or not to do this, which is present in their parent filters, is dropped when creating the GL implementations. This change adds logic to forward the value from the parent filter to the GL implementation. This makes GPU behavior match software and fixes a WebKit LayoutTest. See referenced bug. BUG=473186 Review URL: https://codereview.chromium.org/1410553002 --- gm/arithmode.cpp | 36 +++++++++++++++++++++++++++- src/effects/SkArithmeticMode_gpu.cpp | 9 +++---- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/gm/arithmode.cpp b/gm/arithmode.cpp index 53385ea6ab..1e50e892f7 100644 --- a/gm/arithmode.cpp +++ b/gm/arithmode.cpp @@ -78,7 +78,7 @@ protected: return SkString("arithmode"); } - virtual SkISize onISize() { return SkISize::Make(640, 480); } + virtual SkISize onISize() { return SkISize::Make(640, 572); } virtual void onDraw(SkCanvas* canvas) { SkBitmap src = make_src(); @@ -122,6 +122,40 @@ protected: k += 4; y += SkIntToScalar(src.height() + 12); } + + // Draw two special cases to test enforcePMColor. In these cases, we + // draw the dst bitmap twice, the first time it is halved and inverted, + // leading to invalid premultiplied colors. If we enforcePMColor, these + // invalid values should be clamped, and will not contribute to the + // second draw. + for (int i = 0; i < 2; i++) { + const bool enforcePMColor = (i == 0); + SkScalar x = gap; + canvas->drawBitmap(dst, x, y, nullptr); + x += gap; + SkRect rect = SkRect::MakeXYWH(x, y, SkIntToScalar(WW), SkIntToScalar(HH)); + canvas->saveLayer(&rect, nullptr); + SkXfermode* xfer1 = SkArithmeticMode::Create(0, -one / 2, 0, 1, enforcePMColor); + SkPaint paint1; + paint1.setXfermode(xfer1)->unref(); + canvas->drawBitmap(dst, x, y, &paint1); + SkXfermode* xfer2 = SkArithmeticMode::Create(0, one / 2, -one, 1); + SkPaint paint2; + paint2.setXfermode(xfer2)->unref(); + canvas->drawBitmap(dst, x, y, &paint2); + canvas->restore(); + x += gap; + + // Label + SkPaint paint; + paint.setTextSize(SkIntToScalar(24)); + paint.setAntiAlias(true); + sk_tool_utils::set_portable_typeface(&paint); + SkString str(enforcePMColor ? "enforcePM" : "no enforcePM"); + canvas->drawText(str.c_str(), str.size(), x, y + paint.getTextSize(), paint); + + y += SkIntToScalar(src.height() + 12); + } } private: diff --git a/src/effects/SkArithmeticMode_gpu.cpp b/src/effects/SkArithmeticMode_gpu.cpp index 96e8c0db96..e72ead8db8 100644 --- a/src/effects/SkArithmeticMode_gpu.cpp +++ b/src/effects/SkArithmeticMode_gpu.cpp @@ -54,7 +54,8 @@ static void add_arithmetic_code(GrGLFragmentBuilder* fsBuilder, class GLArithmeticFP : public GrGLFragmentProcessor { public: - GLArithmeticFP(const GrProcessor&) : fEnforcePMColor(true) {} + GLArithmeticFP(const GrArithmeticFP& arithmeticFP) + : fEnforcePMColor(arithmeticFP.enforcePMColor()) {} ~GLArithmeticFP() override {} @@ -191,8 +192,8 @@ private: class GLArithmeticXP : public GrGLXferProcessor { public: - GLArithmeticXP(const GrProcessor&) - : fEnforcePMColor(true) { + GLArithmeticXP(const ArithmeticXP& arithmeticXP) + : fEnforcePMColor(arithmeticXP.enforcePMColor()) { } ~GLArithmeticXP() override {} @@ -260,7 +261,7 @@ GrXferProcessor::OptFlags ArithmeticXP::onGetOptimizations(const GrProcOptInfo& /////////////////////////////////////////////////////////////////////////////// GrArithmeticXPFactory::GrArithmeticXPFactory(float k1, float k2, float k3, float k4, - bool enforcePMColor) + bool enforcePMColor) : fK1(k1), fK2(k2), fK3(k3), fK4(k4), fEnforcePMColor(enforcePMColor) { this->initClassID(); }