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 <michaelludwig@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/gm/runtimeeffectimage.cpp b/gm/runtimeeffectimage.cpp
index c9f8940..b9cb610 100644
--- a/gm/runtimeeffectimage.cpp
+++ b/gm/runtimeeffectimage.cpp
@@ -35,7 +35,7 @@
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 5ec6cd7..578f28a 100644
--- a/gm/runtimeshader.cpp
+++ b/gm/runtimeshader.cpp
@@ -134,10 +134,10 @@
}
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 @@
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 755b3d9..fb8a614 100644
--- a/src/core/SkRuntimeEffect.cpp
+++ b/src/core/SkRuntimeEffect.cpp
@@ -178,7 +178,19 @@
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 @@
kAllowShader_Flag); break;
default: SkUNREACHABLE;
}
- if (SkSL::Analysis::ReferencesSampleCoords(*program)) {
+
+
+ if (sampleCoordsUsage.fRead || sampleCoordsUsage.fWrite) {
flags |= kUsesSampleCoords_Flag;
}
@@ -201,7 +215,7 @@
// 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 @@
// 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 @@
uniforms.push_back(uni);
}
}
- // Functions
- else if (elem->is<SkSL::FunctionDefinition>()) {
- const auto& func = elem->as<SkSL::FunctionDefinition>();
- 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 aba6a96..50eabbc 100644
--- a/src/gpu/effects/GrSkSLFP.cpp
+++ b/src/gpu/effects/GrSkSLFP.cpp
@@ -40,8 +40,13 @@
FPCallbacks(GrGLSLSkSLFP* self,
EmitArgs& args,
const char* inputColor,
+ const std::vector<SkSL::SampleUsage>& 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 @@
}
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 @@
.c_str());
}
- GrGLSLSkSLFP* fSelf;
- EmitArgs& fArgs;
- const char* fInputColor;
- const SkSL::Context& fContext;
+ GrGLSLSkSLFP* fSelf;
+ EmitArgs& fArgs;
+ const char* fInputColor;
+ const std::vector<SkSL::SampleUsage>& 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 @@
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 75987a6..52301d2 100644
--- a/src/sksl/SkSLAnalysis.cpp
+++ b/src/sksl/SkSLAnalysis.cpp
@@ -74,8 +74,8 @@
// 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 @@
protected:
const Context& fContext;
const Variable& fFP;
+ const bool fWritesToSampleCoords;
SampleUsage fUsage;
bool visitExpression(const Expression& e) override {
@@ -97,7 +98,18 @@
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<VariableReference>() &&
+ coords->as<VariableReference>()
+ .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 @@
////////////////////////////////////////////////////////////////////////////////
// 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 945c287..ee61671 100644
--- a/src/sksl/SkSLAnalysis.h
+++ b/src/sksl/SkSLAnalysis.h
@@ -33,7 +33,14 @@
* 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 e44497e..152c6a6 100644
--- a/src/sksl/ir/SkSLProgram.h
+++ b/src/sksl/ir/SkSLProgram.h
@@ -199,6 +199,8 @@
return result;
}
+ const ProgramUsage* usage() const { return fUsage.get(); }
+
std::unique_ptr<String> fSource;
std::unique_ptr<ProgramConfig> fConfig;
std::shared_ptr<Context> fContext;
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index 23f312d..c32ec62 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 <algorithm>
@@ -432,3 +433,51 @@
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);
+}
