From 5d4cd9ea8818f65bb4878c050dbdb9dcd48505ad Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Thu, 9 Feb 2017 11:16:46 -0500 Subject: [PATCH] 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 Commit-Queue: Brian Salomon --- include/gpu/GrFragmentProcessor.h | 4 +-- include/gpu/GrRenderTargetContext.h | 2 ++ src/effects/GrAlphaThresholdFragmentProcessor.cpp | 8 +++-- src/effects/SkAlphaThresholdFilter.cpp | 3 ++ src/effects/gradients/SkGradientShader.cpp | 4 ++- src/effects/gradients/SkGradientShaderPriv.h | 2 +- src/effects/gradients/SkRadialGradient.cpp | 22 +++++++----- src/gpu/effects/GrConfigConversionEffect.cpp | 11 +++--- src/gpu/effects/GrXfermodeFragmentProcessor.cpp | 13 ++++--- src/gpu/glsl/GrGLSLBlend.cpp | 9 +++++ tests/ProcessorTest.cpp | 42 ++++++++++++++--------- 11 files changed, 79 insertions(+), 41 deletions(-) diff --git a/include/gpu/GrFragmentProcessor.h b/include/gpu/GrFragmentProcessor.h index 2ed187d..202e7fa 100644 --- 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 --- 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, 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 --- a/src/effects/GrAlphaThresholdFragmentProcessor.cpp +++ b/src/effects/GrAlphaThresholdFragmentProcessor.cpp @@ -174,8 +174,12 @@ GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrAlphaThresholdFragmentProcessor); sk_sp 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 --- a/src/effects/SkAlphaThresholdFilter.cpp +++ b/src/effects/SkAlphaThresholdFilter.cpp @@ -65,6 +65,9 @@ sk_sp 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 --- 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 --- 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 --- 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 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 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 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 --- 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(); @@ -112,13 +112,14 @@ GrConfigConversionEffect::GrConfigConversionEffect(GrTexture* texture, } GrConfigConversionEffect::GrConfigConversionEffect(GrContext* context, - sk_sp proxy, + sk_sp + 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(); // 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 --- 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 --- 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 --- 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 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; 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 data(new GrColor[256 * 256]); + std::unique_ptr 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; } } -- 2.7.4