GrSkSLFP: Avoid making a global copy of inColor most of the time
We still need *a* copy, to satisfy SkSL that mutates the input color
parameter, but it almost never needs to be global, unless there are
calls to sample() from a function other than main.
With that tweak, we generate much better SkSL/GLSL code from every one
of our effects, avoiding the proliferation of global variables.
Bug: skia:12140
Change-Id: I5c3bd42df34a095969abcbbc60e69c653a08087a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/424299
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/include/effects/SkRuntimeEffect.h b/include/effects/SkRuntimeEffect.h
index 88880f4..71073b6 100644
--- a/include/effects/SkRuntimeEffect.h
+++ b/include/effects/SkRuntimeEffect.h
@@ -220,10 +220,11 @@
private:
enum Flags {
- kUsesSampleCoords_Flag = 0x1,
- kAllowColorFilter_Flag = 0x2,
- kAllowShader_Flag = 0x4,
- kAllowBlender_Flag = 0x8,
+ kUsesSampleCoords_Flag = 0x1,
+ kAllowColorFilter_Flag = 0x2,
+ kAllowShader_Flag = 0x4,
+ kAllowBlender_Flag = 0x8,
+ kSamplesOutsideMain_Flag = 0x10,
};
SkRuntimeEffect(std::unique_ptr<SkSL::Program> baseProgram,
@@ -243,10 +244,11 @@
SkSL::ProgramKind kind);
uint32_t hash() const { return fHash; }
- bool usesSampleCoords() const { return (fFlags & kUsesSampleCoords_Flag); }
- bool allowShader() const { return (fFlags & kAllowShader_Flag); }
- bool allowColorFilter() const { return (fFlags & kAllowColorFilter_Flag); }
- bool allowBlender() const { return (fFlags & kAllowBlender_Flag); }
+ bool usesSampleCoords() const { return (fFlags & kUsesSampleCoords_Flag); }
+ bool allowShader() const { return (fFlags & kAllowShader_Flag); }
+ bool allowColorFilter() const { return (fFlags & kAllowColorFilter_Flag); }
+ bool allowBlender() const { return (fFlags & kAllowBlender_Flag); }
+ bool samplesOutsideMain() const { return (fFlags & kSamplesOutsideMain_Flag); }
const SkFilterColorProgram* getFilterColorProgram();
diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp
index 7d62bc7..b08515f 100644
--- a/src/core/SkRuntimeEffect.cpp
+++ b/src/core/SkRuntimeEffect.cpp
@@ -206,6 +206,10 @@
SkASSERT(!SkSL::Analysis::ReferencesFragCoords(*program));
}
+ if (SkSL::Analysis::CallsSampleOutsideMain(*program)) {
+ flags |= kSamplesOutsideMain_Flag;
+ }
+
size_t offset = 0;
std::vector<Uniform> uniforms;
std::vector<Child> children;
diff --git a/src/gpu/effects/GrSkSLFP.cpp b/src/gpu/effects/GrSkSLFP.cpp
index 9a1694b..1bd7a3d 100644
--- a/src/gpu/effects/GrSkSLFP.cpp
+++ b/src/gpu/effects/GrSkSLFP.cpp
@@ -171,10 +171,18 @@
// we call child processors (particularly from helper functions, which can't "see" the
// parameter to main). Even from within main, if the code mutates the parameter, calls to
// sample should still be passing the original color (by default).
- GrShaderVar inputColorCopy(args.fFragBuilder->getMangledFunctionName("inColor"),
- kHalf4_GrSLType);
- args.fFragBuilder->declareGlobal(inputColorCopy);
- args.fFragBuilder->codeAppendf("%s = %s;\n", inputColorCopy.c_str(), args.fInputColor);
+ SkString inputColorName;
+ if (fp.fEffect->samplesOutsideMain()) {
+ GrShaderVar inputColorCopy(args.fFragBuilder->getMangledFunctionName("inColor"),
+ kHalf4_GrSLType);
+ args.fFragBuilder->declareGlobal(inputColorCopy);
+ inputColorName = inputColorCopy.getName();
+ args.fFragBuilder->codeAppendf("%s = %s;\n", inputColorName.c_str(), args.fInputColor);
+ } else {
+ inputColorName = args.fFragBuilder->newTmpVarName("inColor");
+ args.fFragBuilder->codeAppendf(
+ "half4 %s = %s;\n", inputColorName.c_str(), args.fInputColor);
+ }
// Copy the incoming coords to a local variable. Code in main might modify the coords
// parameter. fSampleCoord could be a varying, so writes to it would be illegal.
@@ -198,7 +206,7 @@
FPCallbacks callbacks(this,
args,
- inputColorCopy.c_str(),
+ inputColorName.c_str(),
*program.fContext,
fp.uniformData(),
fp.uniformFlags());
diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp
index 96dc90f..4865c5d 100644
--- a/src/sksl/SkSLAnalysis.cpp
+++ b/src/sksl/SkSLAnalysis.cpp
@@ -151,6 +151,30 @@
using INHERITED = ProgramVisitor;
};
+// Visitor that searches for calls to sample() from a function other than main()
+class SampleOutsideMainVisitor : public ProgramVisitor {
+public:
+ SampleOutsideMainVisitor() {}
+
+ bool visitExpression(const Expression& e) override {
+ if (e.is<FunctionCall>()) {
+ const FunctionDeclaration& f = e.as<FunctionCall>().function();
+ if (f.isBuiltin() && f.name() == "sample") {
+ return true;
+ }
+ }
+ return INHERITED::visitExpression(e);
+ }
+
+ bool visitProgramElement(const ProgramElement& p) override {
+ return p.is<FunctionDefinition>() &&
+ !p.as<FunctionDefinition>().declaration().isMain() &&
+ INHERITED::visitProgramElement(p);
+ }
+
+ using INHERITED = ProgramVisitor;
+};
+
// Visitor that counts the number of nodes visited
class NodeCountVisitor : public ProgramVisitor {
public:
@@ -592,6 +616,11 @@
return Analysis::ReferencesBuiltin(program, SK_FRAGCOORD_BUILTIN);
}
+bool Analysis::CallsSampleOutsideMain(const Program& program) {
+ SampleOutsideMainVisitor visitor;
+ return visitor.visit(program);
+}
+
int Analysis::NodeCountUpToLimit(const FunctionDefinition& function, int limit) {
return NodeCountVisitor{limit}.visit(*function.body());
}
diff --git a/src/sksl/SkSLAnalysis.h b/src/sksl/SkSLAnalysis.h
index 937bee4..596bfcb 100644
--- a/src/sksl/SkSLAnalysis.h
+++ b/src/sksl/SkSLAnalysis.h
@@ -50,6 +50,8 @@
static bool ReferencesSampleCoords(const Program& program);
static bool ReferencesFragCoords(const Program& program);
+ static bool CallsSampleOutsideMain(const Program& program);
+
static int NodeCountUpToLimit(const FunctionDefinition& function, int limit);
/**
