Re-enable processor optimization test with some fixes.

Enables on GL (for now)

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

diff --git a/include/gpu/GrFragmentProcessor.h b/include/gpu/GrFragmentProcessor.h
index 2ed187d..202e7fa 100644 (file)
--- a/include/gpu/GrFragmentProcessor.h
+++ b/include/gpu/GrFragmentProcessor.h
@@ -100,8 +100,8 @@ public:
 
     /**
      * True if the processor's output is a modulation of its input color or alpha with a computed
-     * color or alpha in the 0..1 range. If true and the blend mode allows it we may fold coverage
-     * into the first color fragment processor's input.
+     * premultiplied color or alpha in the 0..1 range. If true and the blend mode allows it we may
+     * fold coverage into the first color fragment processor's input.
      */
     bool modulatesInput() const { return SkToBool(fFlags & kModulatesInput_OptimizationFlag); }
 

diff --git a/include/gpu/GrRenderTargetContext.h b/include/gpu/GrRenderTargetContext.h
index 11b1c53..466424e 100644 (file)
--- a/include/gpu/GrRenderTargetContext.h
+++ b/include/gpu/GrRenderTargetContext.h
@@ -432,6 +432,8 @@ private:
     friend class GrMSAAPathRenderer;             // for access to addDrawOp
     friend class GrStencilAndCoverPathRenderer;  // for access to addDrawOp
     friend class GrTessellatingPathRenderer;     // for access to addDrawOp
+    // for a unit test
+    friend void test_draw_op(GrRenderTargetContext*, sk_sp<GrFragmentProcessor>, GrTexture*);
 
     void internalClear(const GrFixedClip&, const GrColor, bool canIgnoreClip);
 

diff --git a/src/effects/GrAlphaThresholdFragmentProcessor.cpp b/src/effects/GrAlphaThresholdFragmentProcessor.cpp
index a18fb9c..37ca3ef 100644 (file)
--- a/src/effects/GrAlphaThresholdFragmentProcessor.cpp
+++ b/src/effects/GrAlphaThresholdFragmentProcessor.cpp
@@ -174,8 +174,12 @@ GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrAlphaThresholdFragmentProcessor);
 sk_sp<GrFragmentProcessor> GrAlphaThresholdFragmentProcessor::TestCreate(GrProcessorTestData* d) {
     GrTexture* bmpTex = d->fTextures[GrProcessorUnitTest::kSkiaPMTextureIdx];
     GrTexture* maskTex = d->fTextures[GrProcessorUnitTest::kAlphaTextureIdx];
-    float innerThresh = d->fRandom->nextUScalar1();
-    float outerThresh = d->fRandom->nextUScalar1();
+    // Make the inner and outer thresholds be in (0, 1) exclusive and be sorted correctly.
+    float innerThresh = d->fRandom->nextUScalar1() * .99f + 0.005f;
+    float outerThresh = d->fRandom->nextUScalar1() * .99f + 0.005f;
+    if (innerThresh > outerThresh) {
+        SkTSwap(innerThresh, outerThresh);
+    }
     const int kMaxWidth = 1000;
     const int kMaxHeight = 1000;
     uint32_t width = d->fRandom->nextULessThan(kMaxWidth);

diff --git a/src/effects/SkAlphaThresholdFilter.cpp b/src/effects/SkAlphaThresholdFilter.cpp
index 0e38ccc..96f8365 100644 (file)
--- a/src/effects/SkAlphaThresholdFilter.cpp
+++ b/src/effects/SkAlphaThresholdFilter.cpp
@@ -65,6 +65,9 @@ sk_sp<SkImageFilter> SkAlphaThresholdFilter::Make(const SkRegion& region,
                                                   const SkImageFilter::CropRect* cropRect) {
     innerThreshold = pin_0_1(innerThreshold);
     outerThreshold = pin_0_1(outerThreshold);
+    if (innerThreshold > outerThreshold) {
+        return nullptr;
+    }
     if (!SkScalarIsFinite(innerThreshold) || !SkScalarIsFinite(outerThreshold)) {
         return nullptr;
     }

diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index f50311b..9b38cb5 100644 (file)
--- a/src/effects/gradients/SkGradientShader.cpp
+++ b/src/effects/gradients/SkGradientShader.cpp
@@ -1754,7 +1754,9 @@ void GrGradientEffect::onComputeInvariantOutput(GrInvariantOutput* inout) const
 
 #if GR_TEST_UTILS
 GrGradientEffect::RandomGradientParams::RandomGradientParams(SkRandom* random) {
-    fColorCount = random->nextRangeU(1, kMaxRandomGradientColors);
+    // Set color count to min of 2 so that we don't trigger the const color optimization and make
+    // a non-gradient processor.
+    fColorCount = random->nextRangeU(2, kMaxRandomGradientColors);
     fUseColors4f = random->nextBool();
 
     // if one color, omit stops, otherwise randomly decide whether or not to

diff --git a/src/effects/gradients/SkGradientShaderPriv.h b/src/effects/gradients/SkGradientShaderPriv.h
index a9eef7d..fe238ff 100644 (file)
--- a/src/effects/gradients/SkGradientShaderPriv.h
+++ b/src/effects/gradients/SkGradientShaderPriv.h
@@ -410,7 +410,7 @@ protected:
         the gradient factory. (The constructor may decide not to use stops, in which case fStops
         will be nullptr). */
     struct RandomGradientParams {
-        static const int kMaxRandomGradientColors = 4;
+        static const int kMaxRandomGradientColors = 5;
 
         RandomGradientParams(SkRandom* r);
 

diff --git a/src/effects/gradients/SkRadialGradient.cpp b/src/effects/gradients/SkRadialGradient.cpp
index 7e56863..f282bab 100644 (file)
--- a/src/effects/gradients/SkRadialGradient.cpp
+++ b/src/effects/gradients/SkRadialGradient.cpp
@@ -303,15 +303,19 @@ GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrRadialGradient);
 
 #if GR_TEST_UTILS
 sk_sp<GrFragmentProcessor> GrRadialGradient::TestCreate(GrProcessorTestData* d) {
-    SkPoint center = {d->fRandom->nextUScalar1(), d->fRandom->nextUScalar1()};
-    SkScalar radius = d->fRandom->nextUScalar1();
-
-    RandomGradientParams params(d->fRandom);
-    auto shader = params.fUseColors4f ?
-        SkGradientShader::MakeRadial(center, radius, params.fColors4f, params.fColorSpace,
-                                     params.fStops, params.fColorCount, params.fTileMode) :
-        SkGradientShader::MakeRadial(center, radius, params.fColors,
-                                     params.fStops, params.fColorCount, params.fTileMode);
+    sk_sp<SkShader> shader;
+    do {
+        RandomGradientParams params(d->fRandom);
+        SkPoint center = {d->fRandom->nextUScalar1(), d->fRandom->nextUScalar1()};
+        SkScalar radius = d->fRandom->nextUScalar1();
+        shader = params.fUseColors4f
+                         ? SkGradientShader::MakeRadial(center, radius, params.fColors4f,
+                                                        params.fColorSpace, params.fStops,
+                                                        params.fColorCount, params.fTileMode)
+                         : SkGradientShader::MakeRadial(center, radius, params.fColors,
+                                                        params.fStops, params.fColorCount,
+                                                        params.fTileMode);
+    } while (!shader);
     GrTest::TestAsFPArgs asFPArgs(d);
     sk_sp<GrFragmentProcessor> fp = shader->asFragmentProcessor(asFPArgs.args());
     GrAlwaysAssert(fp);

diff --git a/src/gpu/effects/GrConfigConversionEffect.cpp b/src/gpu/effects/GrConfigConversionEffect.cpp
index d9a6fef..771eba8 100644 (file)
--- a/src/gpu/effects/GrConfigConversionEffect.cpp
+++ b/src/gpu/effects/GrConfigConversionEffect.cpp
@@ -98,7 +98,7 @@ GrConfigConversionEffect::GrConfigConversionEffect(GrTexture* texture,
                                                    const GrSwizzle& swizzle,
                                                    PMConversion pmConversion,
                                                    const SkMatrix& matrix)
-        : INHERITED(texture, nullptr, matrix, ModulationFlags(texture->config()))
+        : INHERITED(texture, nullptr, matrix, kNone_OptimizationFlags)
         , fSwizzle(swizzle)
         , fPMConversion(pmConversion) {
     this->initClassID<GrConfigConversionEffect>();
@@ -112,13 +112,14 @@ GrConfigConversionEffect::GrConfigConversionEffect(GrTexture* texture,
 }
 
 GrConfigConversionEffect::GrConfigConversionEffect(GrContext* context,
-                                                   sk_sp<GrTextureProxy> proxy,
+                                                   sk_sp<GrTextureProxy>
+                                                           proxy,
                                                    const GrSwizzle& swizzle,
                                                    PMConversion pmConversion,
                                                    const SkMatrix& matrix)
-    : INHERITED(context, ModulationFlags(proxy->config()), proxy, nullptr, matrix)
-    , fSwizzle(swizzle)
-    , fPMConversion(pmConversion) {
+        : INHERITED(context, kNone_OptimizationFlags, proxy, nullptr, matrix)
+        , fSwizzle(swizzle)
+        , fPMConversion(pmConversion) {
     this->initClassID<GrConfigConversionEffect>();
     // We expect to get here with non-BGRA/RGBA only if we're doing not doing a premul/unpremul
     // conversion.

diff --git a/src/gpu/effects/GrXfermodeFragmentProcessor.cpp b/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
index 0ecb4bd..935b294 100644 (file)
--- a/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
+++ b/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
@@ -39,9 +39,10 @@ private:
     static OptimizationFlags OptFlags(const GrFragmentProcessor* src,
                                       const GrFragmentProcessor* dst, SkBlendMode mode) {
         // We only attempt the constant output optimization.
-        // The CPU and GPU implementations differ significantly for the advanced modes.
-        if (mode <= SkBlendMode::kLastSeparableMode && src->hasConstantOutputForConstantInput() &&
-            dst->hasConstantOutputForConstantInput()) {
+        // The CPU and GPU implementations differ significantly for the advanced modes and
+        // softlight.
+        if (mode <= SkBlendMode::kLastSeparableMode && mode != SkBlendMode::kSoftLight &&
+            src->hasConstantOutputForConstantInput() && dst->hasConstantOutputForConstantInput()) {
             return kConstantOutputForConstantInput_OptimizationFlag;
         }
         return kNone_OptimizationFlags;
@@ -197,8 +198,10 @@ public:
 private:
     OptimizationFlags OptFlags(const GrFragmentProcessor* child, SkBlendMode mode) {
         // We only attempt the constant output optimization.
-        // The CPU and GPU implementations differ significantly for the advanced modes.
-        if (mode <= SkBlendMode::kLastSeparableMode && child->hasConstantOutputForConstantInput()) {
+        // The CPU and GPU implementations differ significantly for the advanced modes and
+        // softlight.
+        if (mode <= SkBlendMode::kLastSeparableMode && mode != SkBlendMode::kSoftLight &&
+            child->hasConstantOutputForConstantInput()) {
             return kConstantOutputForConstantInput_OptimizationFlag;
         }
         return kNone_OptimizationFlags;

diff --git a/src/gpu/glsl/GrGLSLBlend.cpp b/src/gpu/glsl/GrGLSLBlend.cpp
index e9a9f50..2f41c4e 100644 (file)
--- a/src/gpu/glsl/GrGLSLBlend.cpp
+++ b/src/gpu/glsl/GrGLSLBlend.cpp
@@ -422,7 +422,13 @@ void GrGLSLBlend::AppendMode(GrGLSLFragmentBuilder* fsBuilder, const char* srcCo
 
     SkXfermode::Coeff srcCoeff, dstCoeff;
     if (SkXfermode::ModeAsCoeff(mode, &srcCoeff, &dstCoeff)) {
+        // The only coeff mode that can go out of range is plus.
+        bool clamp = mode == SkBlendMode::kPlus;
+
         fsBuilder->codeAppendf("%s = ", outColor);
+        if (clamp) {
+            fsBuilder->codeAppend("clamp(");
+        }
         // append src blend
         bool didAppend = append_porterduff_term(fsBuilder, srcCoeff, srcColor, srcColor, dstColor,
                                                 false);
@@ -430,6 +436,9 @@ void GrGLSLBlend::AppendMode(GrGLSLFragmentBuilder* fsBuilder, const char* srcCo
         if(!append_porterduff_term(fsBuilder, dstCoeff, dstColor, srcColor, dstColor, didAppend)) {
             fsBuilder->codeAppend("vec4(0, 0, 0, 0)");
         }
+        if (clamp) {
+            fsBuilder->codeAppend(", 0, 1);");
+        }
         fsBuilder->codeAppend(";");
     } else {
         emit_advanced_xfermode_code(fsBuilder, srcColor, dstColor, outColor, mode);

diff --git a/tests/ProcessorTest.cpp b/tests/ProcessorTest.cpp
index a23b40a..49938c6 100644 (file)
--- a/tests/ProcessorTest.cpp
+++ b/tests/ProcessorTest.cpp
@@ -11,11 +11,13 @@
 #if SK_SUPPORT_GPU
 #include "GrContext.h"
 #include "GrGpuResource.h"
+#include "GrPipelineBuilder.h"
 #include "GrRenderTargetContext.h"
 #include "GrRenderTargetContextPriv.h"
 #include "GrResourceProvider.h"
 #include "glsl/GrGLSLFragmentProcessor.h"
 #include "glsl/GrGLSLFragmentShaderBuilder.h"
+#include "ops/GrNonAAFillRectOp.h"
 #include "ops/GrTestMeshDrawOp.h"
 
 namespace {
@@ -244,13 +246,21 @@ static GrColor texel_color(int i, int j) {
 
 static GrColor4f texel_color4f(int i, int j) { return GrColor4f::FromGrColor(texel_color(i, j)); }
 
+void test_draw_op(GrRenderTargetContext* rtc, sk_sp<GrFragmentProcessor> fp,
+                  GrTexture* inputDataTexture) {
+    GrPaint paint;
+    paint.addColorTextureProcessor(inputDataTexture, nullptr, SkMatrix::I());
+    paint.addColorFragmentProcessor(std::move(fp));
+    paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
+    GrPipelineBuilder pb(std::move(paint), GrAAType::kNone);
+    auto op =
+            GrNonAAFillRectOp::Make(GrColor_WHITE, SkMatrix::I(),
+                                    SkRect::MakeWH(rtc->width(), rtc->height()), nullptr, nullptr);
+    rtc->addDrawOp(pb, GrNoClip(), std::move(op));
+}
+
 #if GR_TEST_UTILS
-DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, reporter, ctxInfo) {
-    // This tests code under development but not used in skia lib. Leaving this disabled until
-    // some platform-specific issues are addressed.
-    if (1) {
-        return;
-    }
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, reporter, ctxInfo) {
     GrContext* context = ctxInfo.grContext();
     using FPFactory = GrProcessorTestFactory<GrFragmentProcessor>;
     SkRandom random;
@@ -267,7 +277,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, reporter
     GrTexture* textures[] = {tex0.get(), tex1.get()};
     GrProcessorTestData testData(&random, context, rtc.get(), textures);
 
-    std::unique_ptr<GrColor> data(new GrColor[256 * 256]);
+    std::unique_ptr<GrColor[]> data(new GrColor[256 * 256]);
     for (int y = 0; y < 256; ++y) {
         for (int x = 0; x < 256; ++x) {
             data.get()[256 * y + x] = texel_color(x, y);
@@ -294,12 +304,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, reporter
                 !fp->modulatesInput()) {
                 continue;
             }
-            GrPaint paint;
-            paint.addColorTextureProcessor(dataTexture.get(), nullptr, SkMatrix::I());
-            paint.addColorFragmentProcessor(fp);
-            paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
-            rtc->drawRect(GrNoClip(), std::move(paint), GrAA::kNo, SkMatrix::I(),
-                          SkRect::MakeWH(256.f, 256.f));
+            test_draw_op(rtc.get(), fp, dataTexture.get());
             memset(data.get(), 0x0, sizeof(GrColor) * 256 * 256);
             rtc->readPixels(
                     SkImageInfo::Make(256, 256, kRGBA_8888_SkColorType, kPremul_SkAlphaType),
@@ -349,12 +354,17 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, reporter
                         float gDiff = fabsf(output4f.fRGBA[1] - expected4f.fRGBA[1]);
                         float bDiff = fabsf(output4f.fRGBA[2] - expected4f.fRGBA[2]);
                         float aDiff = fabsf(output4f.fRGBA[3] - expected4f.fRGBA[3]);
-                        static constexpr float kTol = 3 / 255.f;
+                        static constexpr float kTol = 4 / 255.f;
                         if (rDiff > kTol || gDiff > kTol || bDiff > kTol || aDiff > kTol) {
                             ERRORF(reporter,
                                    "Processor %s claimed output for const input doesn't match "
-                                   "actual output.",
-                                   fp->name());
+                                   "actual output. Error: %f, Tolerance: %f, input: (%f, %f, %f, "
+                                   "%f), actual: (%f, %f, %f, %f), expected(%f, %f, %f, %f)",
+                                   fp->name(), SkTMax(rDiff, SkTMax(gDiff, SkTMax(bDiff, aDiff))),
+                                   kTol, input4f.fRGBA[0], input4f.fRGBA[1], input4f.fRGBA[2],
+                                   input4f.fRGBA[3], output4f.fRGBA[0], output4f.fRGBA[1],
+                                   output4f.fRGBA[2], output4f.fRGBA[3], expected4f.fRGBA[0],
+                                   expected4f.fRGBA[1], expected4f.fRGBA[2], expected4f.fRGBA[3]);
                             passing = false;
                         }
                     }