fixed sksl static ifs to work for CircleEffect
static ifs (and switches) in .fp files are a bit tricky, because they
aren't necessarily static when the CPP file is being produced. They
become static when the CPP file produces the final SkSL; at this point
the final values of the 'in' variables are known.
This change permits 'deferred' static ifs and switches. The initial
compilation (.fp -> .cpp) passes the @if / @switch through, and then
the final compilation (.cpp's generated SkSL -> GLSL or whatever)
enforces the static test.
Bug: skia:
Change-Id: I0087dfe1725c8fd350507ac77f64db1d82659cdf
Reviewed-on: https://skia-review.googlesource.com/23403
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
diff --git a/src/gpu/effects/GrCircleEffect.cpp b/src/gpu/effects/GrCircleEffect.cpp
index acd2a9b..23d6551 100644
--- a/src/gpu/effects/GrCircleEffect.cpp
+++ b/src/gpu/effects/GrCircleEffect.cpp
@@ -25,7 +25,7 @@
(void) _outer;
prevRadius = -1.0;
fCircleVar = args.fUniformHandler->addUniform(kFragment_GrShaderFlag, kVec4f_GrSLType, kDefault_GrSLPrecision, "circle");
- fragBuilder->codeAppendf("vec2 prevCenter;\nfloat prevRadius = %f;\nfloat d;\nif (%d == 2 || %d == 3) {\n d = (length((%s.xy - sk_FragCoord.xy) * %s.w) - 1.0) * %s.z;\n} else {\n d = (1.0 - length((%s.xy - sk_FragCoord.xy) * %s.w)) * %s.z;\n}\nif ((%d == 1 || %d == 3) || %d == 4) {\n d = clamp(d, 0.0, 1.0);\n} else {\n d = d > 0.5 ? 1.0 : 0.0;\n}\n%s = %s * d;\n", prevRadius, _outer.edgeType(), _outer.edgeType(), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), _outer.edgeType(), _outer.edgeType(), _outer.edgeType(), args.fOutputColor, args.fInputColor ? args.fInputColor : "vec4(1)");
+ fragBuilder->codeAppendf("vec2 prevCenter;\nfloat prevRadius = %f;\nfloat d;\n@if (%d == 2 || %d == 3) {\n d = (length((%s.xy - sk_FragCoord.xy) * %s.w) - 1.0) * %s.z;\n} else {\n d = (1.0 - length((%s.xy - sk_FragCoord.xy) * %s.w)) * %s.z;\n}\n@if ((%d == 1 || %d == 3) || %d == 4) {\n d = clamp(d, 0.0, 1.0);\n} else {\n d = d > 0.5 ? 1.0 : 0.0;\n}\n%s = %s * d;\n", prevRadius, _outer.edgeType(), _outer.edgeType(), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), args.fUniformHandler->getUniformCStr(fCircleVar), _outer.edgeType(), _outer.edgeType(), _outer.edgeType(), args.fOutputColor, args.fInputColor ? args.fInputColor : "vec4(1)");
}
private:
void onSetData(const GrGLSLProgramDataManager& pdman, const GrFragmentProcessor& _proc) override {
diff --git a/src/gpu/effects/GrCircleEffect.fp b/src/gpu/effects/GrCircleEffect.fp
index 3f0b850..74973ca 100644
--- a/src/gpu/effects/GrCircleEffect.fp
+++ b/src/gpu/effects/GrCircleEffect.fp
@@ -37,15 +37,15 @@
// radius and then denormalized. This is to prevent overflow on devices that have a "real"
// mediump. It'd be nice to only do this on mediump devices.
float d;
- if (edgeType == 2 /* kInverseFillBW_GrProcessorEdgeType */ ||
- edgeType == 3 /* kInverseFillAA_GrProcessorEdgeType */) {
+ @if (edgeType == 2 /* kInverseFillBW_GrProcessorEdgeType */ ||
+ edgeType == 3 /* kInverseFillAA_GrProcessorEdgeType */) {
d = (length((circle.xy - sk_FragCoord.xy) * circle.w) - 1.0) * circle.z;
} else {
d = (1.0 - length((circle.xy - sk_FragCoord.xy) * circle.w)) * circle.z;
}
- if (edgeType == 1 /* kFillAA_GrProcessorEdgeType */ ||
- edgeType == 3 /* kInverseFillAA_GrProcessorEdgeType */ ||
- edgeType == 4 /* kHairlineAA_GrProcessorEdgeType */) {
+ @if (edgeType == 1 /* kFillAA_GrProcessorEdgeType */ ||
+ edgeType == 3 /* kInverseFillAA_GrProcessorEdgeType */ ||
+ edgeType == 4 /* kHairlineAA_GrProcessorEdgeType */) {
d = clamp(d, 0.0, 1.0);
} else {
d = d > 0.5 ? 1.0 : 0.0;
diff --git a/src/sksl/SkSLCPPCodeGenerator.cpp b/src/sksl/SkSLCPPCodeGenerator.cpp
index 238615d..3da7e8e 100644
--- a/src/sksl/SkSLCPPCodeGenerator.cpp
+++ b/src/sksl/SkSLCPPCodeGenerator.cpp
@@ -225,6 +225,20 @@
}
}
+void CPPCodeGenerator::writeIfStatement(const IfStatement& s) {
+ if (s.fIsStatic) {
+ this->write("@");
+ }
+ INHERITED::writeIfStatement(s);
+}
+
+void CPPCodeGenerator::writeSwitchStatement(const SwitchStatement& s) {
+ if (s.fIsStatic) {
+ this->write("@");
+ }
+ INHERITED::writeSwitchStatement(s);
+}
+
void CPPCodeGenerator::writeFunctionCall(const FunctionCall& c) {
if (c.fFunction.fBuiltin && c.fFunction.fName == "COLORSPACE") {
String tmpVar = "_tmpVar" + to_string(++fVarCount);
diff --git a/src/sksl/SkSLCPPCodeGenerator.h b/src/sksl/SkSLCPPCodeGenerator.h
index 0f6da5f..7f60563 100644
--- a/src/sksl/SkSLCPPCodeGenerator.h
+++ b/src/sksl/SkSLCPPCodeGenerator.h
@@ -41,6 +41,10 @@
String getSamplerHandle(const Variable& var);
+ void writeIfStatement(const IfStatement& s) override;
+
+ void writeSwitchStatement(const SwitchStatement& s) override;
+
void writeFunctionCall(const FunctionCall& c) override;
void writeFunction(const FunctionDefinition& f) override;
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 593397b..3a037e7 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -54,8 +54,9 @@
namespace SkSL {
-Compiler::Compiler()
-: fErrorCount(0) {
+Compiler::Compiler(Flags flags)
+: fFlags(flags)
+, fErrorCount(0) {
auto types = std::shared_ptr<SymbolTable>(new SymbolTable(this));
auto symbols = std::shared_ptr<SymbolTable>(new SymbolTable(types, this));
fIRGenerator = new IRGenerator(&fContext, symbols, *this);
@@ -931,7 +932,7 @@
(*iter)->setStatement(std::move(newBlock));
break;
} else {
- if (s.fIsStatic) {
+ if (s.fIsStatic && !(fFlags & kPermitInvalidStaticTests_Flag)) {
this->error(s.fPosition,
"static switch contains non-static conditional break");
s.fIsStatic = false;
@@ -947,7 +948,7 @@
if (newBlock) {
(*iter)->setStatement(std::move(newBlock));
} else {
- if (s.fIsStatic) {
+ if (s.fIsStatic && !(fFlags & kPermitInvalidStaticTests_Flag)) {
this->error(s.fPosition,
"static switch contains non-static conditional break");
s.fIsStatic = false;
@@ -1047,13 +1048,15 @@
const Statement& s = **iter->statement();
switch (s.fKind) {
case Statement::kIf_Kind:
- if (((const IfStatement&) s).fIsStatic) {
+ if (((const IfStatement&) s).fIsStatic &&
+ !(fFlags & kPermitInvalidStaticTests_Flag)) {
this->error(s.fPosition, "static if has non-static test");
}
++iter;
break;
case Statement::kSwitch_Kind:
- if (((const SwitchStatement&) s).fIsStatic) {
+ if (((const SwitchStatement&) s).fIsStatic &&
+ !(fFlags & kPermitInvalidStaticTests_Flag)) {
this->error(s.fPosition, "static switch has non-static test");
}
++iter;
diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h
index 2acb559..38f63e5 100644
--- a/src/sksl/SkSLCompiler.h
+++ b/src/sksl/SkSLCompiler.h
@@ -43,7 +43,15 @@
*/
class Compiler : public ErrorReporter {
public:
- Compiler();
+ enum Flags {
+ kNone_Flags = 0,
+ // permits static if/switch statements to be used with non-constant tests. This is used when
+ // producing H and CPP code; the static tests don't have to have constant values *yet*, but
+ // the generated code will contain a static test which then does have to be a constant.
+ kPermitInvalidStaticTests_Flag = 1,
+ };
+
+ Compiler(Flags flags = kNone_Flags);
~Compiler() override;
@@ -109,6 +117,7 @@
std::shared_ptr<SymbolTable> fTypes;
IRGenerator* fIRGenerator;
String fSkiaVertText; // FIXME store parsed version instead
+ int fFlags;
Context fContext;
int fErrorCount;
diff --git a/src/sksl/SkSLGLSLCodeGenerator.h b/src/sksl/SkSLGLSLCodeGenerator.h
index 5716bde..6fe25d3 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.h
+++ b/src/sksl/SkSLGLSLCodeGenerator.h
@@ -162,7 +162,7 @@
void writeBlock(const Block& b);
- void writeIfStatement(const IfStatement& stmt);
+ virtual void writeIfStatement(const IfStatement& stmt);
void writeForStatement(const ForStatement& f);
@@ -170,7 +170,7 @@
void writeDoStatement(const DoStatement& d);
- void writeSwitchStatement(const SwitchStatement& s);
+ virtual void writeSwitchStatement(const SwitchStatement& s);
void writeReturnStatement(const ReturnStatement& r);
diff --git a/src/sksl/SkSLMain.cpp b/src/sksl/SkSLMain.cpp
index 3e3d747..9f1fec1 100644
--- a/src/sksl/SkSLMain.cpp
+++ b/src/sksl/SkSLMain.cpp
@@ -95,7 +95,7 @@
}
} else if (name.endsWith(".h")) {
SkSL::FileOutputStream out(argv[2]);
- SkSL::Compiler compiler;
+ SkSL::Compiler compiler(SkSL::Compiler::kPermitInvalidStaticTests_Flag);
if (!out.isValid()) {
printf("error writing '%s'\n", argv[2]);
exit(4);
@@ -112,7 +112,7 @@
}
} else if (name.endsWith(".cpp")) {
SkSL::FileOutputStream out(argv[2]);
- SkSL::Compiler compiler;
+ SkSL::Compiler compiler(SkSL::Compiler::kPermitInvalidStaticTests_Flag);
if (!out.isValid()) {
printf("error writing '%s'\n", argv[2]);
exit(4);