From cc3d2d25c58e7e6d0b1fb9e5a7b8b4da670af06e Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Fri, 16 Apr 2021 14:53:24 -0400 Subject: [PATCH] Runtime effects: Detect passthrough sample calls automatically As explained, this is *very* conservative. It only works when the child is sampled from within main, and using a direct reference to the coords parameter. If that parameter is ever modified (even after being used), the optimization doesn't happen. For most cases, this is fine. Bug: skia:11869 Change-Id: Ia06181730a6d07e2a4fe2de4cc8e8c3402f0dc52 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397320 Reviewed-by: Michael Ludwig Reviewed-by: John Stiles Commit-Queue: Brian Osman --- gm/runtimeeffectimage.cpp | 2 +- gm/runtimeshader.cpp | 8 +++--- src/core/SkRuntimeEffect.cpp | 35 +++++++++++++------------ src/gpu/effects/GrSkSLFP.cpp | 34 +++++++++++++++++++----- src/sksl/SkSLAnalysis.cpp | 24 +++++++++++++---- src/sksl/SkSLAnalysis.h | 9 ++++++- src/sksl/ir/SkSLProgram.h | 2 ++ tests/SkRuntimeEffectTest.cpp | 49 +++++++++++++++++++++++++++++++++++ 8 files changed, 130 insertions(+), 33 deletions(-) diff --git a/gm/runtimeeffectimage.cpp b/gm/runtimeeffectimage.cpp index c9f8940b4d..b9cb61010b 100644 --- a/gm/runtimeeffectimage.cpp +++ b/gm/runtimeeffectimage.cpp @@ -35,7 +35,7 @@ protected: half b = fract((p.x + 5)/10); half a = min(distance(p, vec2(50, 50))/50 + 0.3, 1); half4 result = half4(r, g, b, a); - result *= sample(child); + result *= sample(child, p); if (gAlphaType == 0) { result.rgb *= a; } diff --git a/gm/runtimeshader.cpp b/gm/runtimeshader.cpp index 5ec6cd7e65..578f28a36f 100644 --- a/gm/runtimeshader.cpp +++ b/gm/runtimeshader.cpp @@ -134,10 +134,10 @@ public: } half4 main(float2 xy) { - half4 before = sample(before_map); - half4 after = sample(after_map); + half4 before = sample(before_map, xy); + half4 after = sample(after_map, xy); - float m = smooth_cutoff(sample(threshold_map).a); + float m = smooth_cutoff(sample(threshold_map, xy).a); return mix(before, after, m); } )", kAnimate_RTFlag | kBench_RTFlag) {} @@ -228,7 +228,7 @@ public: uniform float inv_size; half4 main(float2 xy) { - float4 c = unpremul(sample(input)); + float4 c = unpremul(sample(input, xy)); // Map to cube coords: float3 cubeCoords = float3(c.rg * rg_scale + rg_bias, c.b * b_scale); diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp index 755b3d93a0..fb8a614768 100644 --- a/src/core/SkRuntimeEffect.cpp +++ b/src/core/SkRuntimeEffect.cpp @@ -178,7 +178,19 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl, settings.fForceNoInline = options.forceNoInline; settings.fAllowNarrowingConversions = true; - const SkSL::FunctionDefinition* main = nullptr; + // Find 'main', then locate the sample coords parameter. (It might not be present.) + const SkSL::FunctionDefinition* main = SkSL::Program_GetFunction(*program, "main"); + if (!main) { + RETURN_FAILURE("missing 'main' function"); + } + const auto& mainParams = main->declaration().parameters(); + auto iter = std::find_if(mainParams.begin(), mainParams.end(), [](const SkSL::Variable* p) { + return p->modifiers().fLayout.fBuiltin == SK_MAIN_COORDS_BUILTIN; + }); + const SkSL::ProgramUsage::VariableCounts sampleCoordsUsage = + iter != mainParams.end() ? program->usage()->get(**iter) + : SkSL::ProgramUsage::VariableCounts{}; + uint32_t flags = 0; switch (kind) { case SkSL::ProgramKind::kRuntimeColorFilter: flags |= kAllowColorFilter_Flag; break; @@ -187,7 +199,9 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl, kAllowShader_Flag); break; default: SkUNREACHABLE; } - if (SkSL::Analysis::ReferencesSampleCoords(*program)) { + + + if (sampleCoordsUsage.fRead || sampleCoordsUsage.fWrite) { flags |= kUsesSampleCoords_Flag; } @@ -201,7 +215,7 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl, // this can be simpler. There is no way for color filters to refer to sk_FragCoord or sample // coords in that mode. if ((flags & kAllowColorFilter_Flag) && - (SkSL::Analysis::ReferencesFragCoords(*program) || (flags & kUsesSampleCoords_Flag))) { + ((flags & kUsesSampleCoords_Flag) || SkSL::Analysis::ReferencesFragCoords(*program))) { flags &= ~kAllowColorFilter_Flag; } @@ -233,7 +247,8 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl, // Child effects that can be sampled ('shader' or 'colorFilter') else if (varType.isEffectChild()) { children.push_back(var.name()); - sampleUsages.push_back(SkSL::Analysis::GetSampleUsage(*program, var)); + sampleUsages.push_back(SkSL::Analysis::GetSampleUsage( + *program, var, sampleCoordsUsage.fWrite != 0)); } // 'uniform' variables else if (var.modifiers().fFlags & SkSL::Modifiers::kUniform_Flag) { @@ -274,18 +289,6 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl, uniforms.push_back(uni); } } - // Functions - else if (elem->is()) { - const auto& func = elem->as(); - const SkSL::FunctionDeclaration& decl = func.declaration(); - if (decl.isMain()) { - main = &func; - } - } - } - - if (!main) { - RETURN_FAILURE("missing 'main' function"); } #undef RETURN_FAILURE diff --git a/src/gpu/effects/GrSkSLFP.cpp b/src/gpu/effects/GrSkSLFP.cpp index aba6a96491..50eabbc847 100644 --- a/src/gpu/effects/GrSkSLFP.cpp +++ b/src/gpu/effects/GrSkSLFP.cpp @@ -40,8 +40,13 @@ public: FPCallbacks(GrGLSLSkSLFP* self, EmitArgs& args, const char* inputColor, + const std::vector& sampleUsages, const SkSL::Context& context) - : fSelf(self), fArgs(args), fInputColor(inputColor), fContext(context) {} + : fSelf(self) + , fArgs(args) + , fInputColor(inputColor) + , fSampleUsages(sampleUsages) + , fContext(context) {} using String = SkSL::String; @@ -96,6 +101,21 @@ public: } String sampleChild(int index, String coords) override { + // If the child was sampled using the coords passed to main (and they are never + // modified), then we will have marked the child as PassThrough. The code generator + // doesn't know that, and still supplies coords. Inside invokeChild, we assert that + // any coords passed for a PassThrough child match args.fSampleCoords exactly. + // + // Normally, this is valid. Here, we *copied* the sample coords to a local variable + // (so that they're mutable in the runtime effect SkSL). Thus, the coords string we + // get here is the name of the local copy, and fSampleCoords still points to the + // unmodified original (which might be a varying, for example). + // To prevent the assert, we pass the empty string in this case. Note that for + // children sampled like this, invokeChild doesn't even use the coords parameter, + // except for that assert. + if (fSampleUsages[index].fPassThrough) { + coords.clear(); + } return String(fSelf->invokeChild(index, fInputColor, fArgs, coords).c_str()); } @@ -112,10 +132,11 @@ public: .c_str()); } - GrGLSLSkSLFP* fSelf; - EmitArgs& fArgs; - const char* fInputColor; - const SkSL::Context& fContext; + GrGLSLSkSLFP* fSelf; + EmitArgs& fArgs; + const char* fInputColor; + const std::vector& fSampleUsages; + const SkSL::Context& fContext; }; // Snap off a global copy of the input color at the start of main. We need this when @@ -135,7 +156,8 @@ public: args.fFragBuilder->codeAppendf("float2 %s = %s;\n", coords, args.fSampleCoord); } - FPCallbacks callbacks(this, args, inputColorCopy.c_str(), *program.fContext); + FPCallbacks callbacks( + this, args, inputColorCopy.c_str(), fp.fEffect->fSampleUsages, *program.fContext); SkSL::PipelineStage::ConvertProgram(program, coords, args.fInputColor, &callbacks); } diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp index 75987a6e20..52301d2534 100644 --- a/src/sksl/SkSLAnalysis.cpp +++ b/src/sksl/SkSLAnalysis.cpp @@ -74,8 +74,8 @@ static bool is_sample_call_to_fp(const FunctionCall& fc, const Variable& fp) { // Visitor that determines the merged SampleUsage for a given child 'fp' in the program. class MergeSampleUsageVisitor : public ProgramVisitor { public: - MergeSampleUsageVisitor(const Context& context, const Variable& fp) - : fContext(context), fFP(fp) {} + MergeSampleUsageVisitor(const Context& context, const Variable& fp, bool writesToSampleCoords) + : fContext(context), fFP(fp), fWritesToSampleCoords(writesToSampleCoords) {} SampleUsage visit(const Program& program) { fUsage = SampleUsage(); // reset to none @@ -86,6 +86,7 @@ public: protected: const Context& fContext; const Variable& fFP; + const bool fWritesToSampleCoords; SampleUsage fUsage; bool visitExpression(const Expression& e) override { @@ -97,7 +98,18 @@ protected: if (fc.arguments().size() >= 2) { const Expression* coords = fc.arguments()[1].get(); if (coords->type() == *fContext.fTypes.fFloat2) { - fUsage.merge(SampleUsage::Explicit()); + // If the coords are a direct reference to the program's sample-coords, + // and those coords are never modified, we can conservatively turn this + // into PassThrough sampling. In all other cases, we consider it Explicit. + if (!fWritesToSampleCoords && coords->is() && + coords->as() + .variable() + ->modifiers() + .fLayout.fBuiltin == SK_MAIN_COORDS_BUILTIN) { + fUsage.merge(SampleUsage::PassThrough()); + } else { + fUsage.merge(SampleUsage::Explicit()); + } } else if (coords->type() == *fContext.fTypes.fFloat3x3) { // Determine the type of matrix for this call site if (coords->isConstantOrUniform()) { @@ -568,8 +580,10 @@ public: //////////////////////////////////////////////////////////////////////////////// // Analysis -SampleUsage Analysis::GetSampleUsage(const Program& program, const Variable& fp) { - MergeSampleUsageVisitor visitor(*program.fContext, fp); +SampleUsage Analysis::GetSampleUsage(const Program& program, + const Variable& fp, + bool writesToSampleCoords) { + MergeSampleUsageVisitor visitor(*program.fContext, fp, writesToSampleCoords); return visitor.visit(program); } diff --git a/src/sksl/SkSLAnalysis.h b/src/sksl/SkSLAnalysis.h index 945c2879d7..ee61671b65 100644 --- a/src/sksl/SkSLAnalysis.h +++ b/src/sksl/SkSLAnalysis.h @@ -33,7 +33,14 @@ enum class VariableRefKind : int8_t; * Provides utilities for analyzing SkSL statically before it's composed into a full program. */ struct Analysis { - static SampleUsage GetSampleUsage(const Program& program, const Variable& fp); + /** + * Determines how `program` samples `fp`. By default, assumes that the sample coords + * (SK_MAIN_COORDS_BUILTIN) might be modified, so `sample(fp, sampleCoords)` is treated as + * Explicit. If writesToSampleCoords is false, treats that as PassThrough, instead. + */ + static SampleUsage GetSampleUsage(const Program& program, + const Variable& fp, + bool writesToSampleCoords = true); static bool ReferencesBuiltin(const Program& program, int builtin); diff --git a/src/sksl/ir/SkSLProgram.h b/src/sksl/ir/SkSLProgram.h index e44497e7c9..152c6a610e 100644 --- a/src/sksl/ir/SkSLProgram.h +++ b/src/sksl/ir/SkSLProgram.h @@ -199,6 +199,8 @@ struct Program { return result; } + const ProgramUsage* usage() const { return fUsage.get(); } + std::unique_ptr fSource; std::unique_ptr fConfig; std::shared_ptr fContext; diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp index 23f312d5d8..c32ec62730 100644 --- a/tests/SkRuntimeEffectTest.cpp +++ b/tests/SkRuntimeEffectTest.cpp @@ -16,6 +16,7 @@ #include "src/core/SkColorSpacePriv.h" #include "src/core/SkTLazy.h" #include "src/gpu/GrColor.h" +#include "src/gpu/GrFragmentProcessor.h" #include "tests/Test.h" #include @@ -432,3 +433,51 @@ DEF_TEST(SkRuntimeColorFilterFlags, r) { REPORTER_ASSERT(r, filter && !filter->isAlphaUnchanged()); } } + +DEF_TEST(SkRuntimeShaderSampleUsage, r) { + auto test = [&](const char* src, bool expectExplicit) { + auto [effect, err] = + SkRuntimeEffect::MakeForShader(SkStringPrintf("uniform shader child; %s", src)); + REPORTER_ASSERT(r, effect); + + auto child = GrFragmentProcessor::MakeColor({ 1, 1, 1, 1 }); + auto fp = effect->makeFP(nullptr, &child, 1); + REPORTER_ASSERT(r, fp); + + REPORTER_ASSERT(r, fp->childProcessor(0)->isSampledWithExplicitCoords() == expectExplicit); + }; + + // This test verifies that we detect calls to sample where the coords are the same as those + // passed to main. In those cases, it's safe to turn the "explicit" sampling into "passthrough" + // sampling. This optimization is implemented very conservatively. + + // Cases where our optimization is valid, and works: + + // Direct use of passed-in coords + test("half4 main(float2 xy) { return sample(child, xy); }", false); + // Sample with passed-in coords, read (but don't write) sample coords elsewhere + test("half4 main(float2 xy) { return sample(child, xy) + sin(xy.x); }", false); + + // Cases where our optimization is not valid, and does not happen: + + // Sampling with values completely unrelated to passed-in coords + test("half4 main(float2 xy) { return sample(child, float2(0, 0)); }", true); + // Use of expression involving passed in coords + test("half4 main(float2 xy) { return sample(child, xy * 0.5); }", true); + // Use of coords after modification + test("half4 main(float2 xy) { xy *= 2; return sample(child, xy); }", true); + // Use of coords after modification via out-param call + test("void adjust(inout float2 xy) { xy *= 2; }" + "half4 main(float2 xy) { adjust(xy); return sample(child, xy); }", true); + + // There should (must) not be any false-positive cases. There are false-negatives. + // In all of these cases, our optimization would be valid, but does not happen: + + // Direct use of passed-in coords, modified after use + test("half4 main(float2 xy) { half4 c = sample(child, xy); xy *= 2; return c; }", true); + // Passed-in coords copied to a temp variable + test("half4 main(float2 xy) { float2 p = xy; return sample(child, p); }", true); + // Use of coords passed to helper function + test("half4 helper(float2 xy) { return sample(child, xy); }" + "half4 main(float2 xy) { return helper(xy); }", true); +}