Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 82d4970

Browse files
brianosmanSkia Commit-Bot
brianosman
authored and
Skia Commit-Bot
committed
More SkRuntimeEffect tests
- 'in' variables can't be arrays (applies to all SkSL) - 'in' and 'uniform' variables are restricted to a fixed list of types. Change-Id: I957ce1ad33aaf6b5970ca7204c568bb533bc86d6 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261436 Reviewed-by: Brian Salomon <bsalomon@google.com> Commit-Queue: Brian Osman <brianosman@google.com>
1 parent c65d006 commit 82d4970

File tree

4 files changed

+79
-69
lines changed

4 files changed

+79
-69
lines changed

‎src/core/SkRuntimeEffect.cpp‎

Lines changed: 61 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -22,71 +22,48 @@ SkRuntimeEffect::EffectResult SkRuntimeEffect::Make(SkString sksl) {
2222
auto program = compiler->convertProgram(SkSL::Program::kPipelineStage_Kind,
2323
SkSL::String(sksl.c_str(), sksl.size()),
2424
SkSL::Program::Settings());
25+
// TODO: Many errors aren't caught until we process the generated Program here. Catching those
26+
// in the IR generator would provide better errors messages (with locations).
27+
#define RETURN_FAILURE(...) return std::make_pair(nullptr, SkStringPrintf(__VA_ARGS__))
28+
2529
if (!program) {
26-
return std::make_pair(nullptr, SkString(compiler->errorText().c_str()));
30+
RETURN_FAILURE("%s", compiler->errorText().c_str());
2731
}
2832
SkASSERT(!compiler->errorCount());
2933

30-
sk_sp<SkRuntimeEffect> effect(new SkRuntimeEffect(std::move(sksl), std::move(compiler),
31-
std::move(program)));
32-
return std::make_pair(std::move(effect), SkString());
33-
}
34-
35-
size_t SkRuntimeEffect::Variable::sizeInBytes() const {
36-
auto element_size = [](Type type) -> size_t {
37-
switch (type) {
38-
case Type::kBool: return 1;
39-
case Type::kInt: return sizeof(int32_t);
40-
case Type::kFloat: return sizeof(float);
41-
case Type::kFloat2: return sizeof(float) * 2;
42-
case Type::kFloat3: return sizeof(float) * 3;
43-
case Type::kFloat4: return sizeof(float) * 4;
44-
45-
case Type::kFloat2x2: return sizeof(float) * 4;
46-
case Type::kFloat3x3: return sizeof(float) * 9;
47-
case Type::kFloat4x4: return sizeof(float) * 16;
48-
default: SkUNREACHABLE;
49-
}
50-
};
51-
return element_size(fType) * fCount;
52-
}
53-
54-
SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler> compiler,
55-
std::unique_ptr<SkSL::Program> baseProgram)
56-
: fIndex(new_sksl_index())
57-
, fSkSL(std::move(sksl))
58-
, fCompiler(std::move(compiler))
59-
, fBaseProgram(std::move(baseProgram)) {
60-
SkASSERT(fCompiler && fBaseProgram);
61-
6234
size_t offset = 0;
63-
auto gather_variables = [this, &offset](SkSL::Modifiers::Flag flag) {
64-
for (const auto& e : *fBaseProgram) {
35+
std::vector<Variable> inAndUniformVars;
36+
std::vector<SkString> children;
37+
const SkSL::Context& ctx(compiler->context());
38+
39+
// Gather the inputs in two passes, to de-interleave them in our input layout
40+
for (auto flag : { SkSL::Modifiers::kIn_Flag, SkSL::Modifiers::kUniform_Flag }) {
41+
for (const auto& e : *program) {
6542
if (e.fKind == SkSL::ProgramElement::kVar_Kind) {
6643
SkSL::VarDeclarations& v = (SkSL::VarDeclarations&) e;
6744
for (const auto& varStatement : v.fVars) {
68-
const SkSL::Variable& var = *((SkSL::VarDeclaration&) * varStatement).fVar;
45+
const SkSL::Variable& var = *((SkSL::VarDeclaration&) *varStatement).fVar;
6946

7047
// Sanity check some rules that should be enforced by the IR generator.
7148
// These are all layout options that only make sense in .fp files.
7249
SkASSERT(!var.fModifiers.fLayout.fKey);
7350
SkASSERT((var.fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) == 0 ||
74-
(var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag) == 0);
51+
(var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag) == 0);
7552
SkASSERT(var.fModifiers.fLayout.fCType == SkSL::Layout::CType::kDefault);
7653
SkASSERT(var.fModifiers.fLayout.fWhen.fLength == 0);
7754
SkASSERT((var.fModifiers.fLayout.fFlags & SkSL::Layout::kTracked_Flag) == 0);
7855

7956
if (var.fModifiers.fFlags & flag) {
80-
if (&var.fType == fCompiler->context().fFragmentProcessor_Type.get()) {
81-
fChildren.push_back(var.fName);
57+
if (&var.fType == ctx.fFragmentProcessor_Type.get()) {
58+
children.push_back(var.fName);
8259
continue;
8360
}
8461

8562
Variable v;
8663
v.fName = var.fName;
8764
v.fQualifier = (var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag)
88-
? Variable::Qualifier::kUniform
89-
: Variable::Qualifier::kIn;
65+
? Variable::Qualifier::kUniform
66+
: Variable::Qualifier::kIn;
9067
v.fFlags = 0;
9168
v.fCount = 1;
9269

@@ -103,7 +80,6 @@ SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler>
10380
#define SET_TYPES(cpuType, gpuType) do { v.fType = cpuType; } while (false)
10481
#endif
10582

106-
const SkSL::Context& ctx(fCompiler->context());
10783
if (type == ctx.fBool_Type.get()) {
10884
SET_TYPES(Variable::Type::kBool, kVoid_GrSLType);
10985
} else if (type == ctx.fInt_Type.get()) {
@@ -137,8 +113,8 @@ SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler>
137113
} else if (type == ctx.fHalf4x4_Type.get()) {
138114
SET_TYPES(Variable::Type::kFloat4x4, kHalf4x4_GrSLType);
139115
} else {
140-
SkDEBUGFAILF("Unsupported input/uniform type: %s\n",
141-
type->description().c_str());
116+
RETURN_FAILURE("Invalid input/uniform type: '%s'",
117+
type->description().c_str());
142118

143119
}
144120

@@ -149,16 +125,53 @@ SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler>
149125
}
150126
v.fOffset = offset;
151127
offset += v.sizeInBytes();
152-
fInAndUniformVars.push_back(v);
128+
inAndUniformVars.push_back(v);
153129
}
154130
}
155131
}
156132
}
133+
}
134+
135+
#undef RETURN_FAILURE
136+
137+
sk_sp<SkRuntimeEffect> effect(new SkRuntimeEffect(std::move(sksl),
138+
std::move(compiler),
139+
std::move(program),
140+
std::move(inAndUniformVars),
141+
std::move(children)));
142+
return std::make_pair(std::move(effect), SkString());
143+
}
144+
145+
size_t SkRuntimeEffect::Variable::sizeInBytes() const {
146+
auto element_size = [](Type type) -> size_t {
147+
switch (type) {
148+
case Type::kBool: return 1;
149+
case Type::kInt: return sizeof(int32_t);
150+
case Type::kFloat: return sizeof(float);
151+
case Type::kFloat2: return sizeof(float) * 2;
152+
case Type::kFloat3: return sizeof(float) * 3;
153+
case Type::kFloat4: return sizeof(float) * 4;
154+
155+
case Type::kFloat2x2: return sizeof(float) * 4;
156+
case Type::kFloat3x3: return sizeof(float) * 9;
157+
case Type::kFloat4x4: return sizeof(float) * 16;
158+
default: SkUNREACHABLE;
159+
}
157160
};
161+
return element_size(fType) * fCount;
162+
}
158163

159-
// Gather the inputs in two passes, to de-interleave them in our input layout
160-
gather_variables(SkSL::Modifiers::kIn_Flag);
161-
gather_variables(SkSL::Modifiers::kUniform_Flag);
164+
SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler> compiler,
165+
std::unique_ptr<SkSL::Program> baseProgram,
166+
std::vector<Variable>&& inAndUniformVars,
167+
std::vector<SkString>&& children)
168+
: fIndex(new_sksl_index())
169+
, fSkSL(std::move(sksl))
170+
, fCompiler(std::move(compiler))
171+
, fBaseProgram(std::move(baseProgram))
172+
, fInAndUniformVars(std::move(inAndUniformVars))
173+
, fChildren(std::move(children)) {
174+
SkASSERT(fCompiler && fBaseProgram);
162175
}
163176

164177
size_t SkRuntimeEffect::inputSize() const {

‎src/core/SkRuntimeEffect.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ class SkRuntimeEffect : public SkRefCnt {
9292

9393
private:
9494
SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler> compiler,
95-
std::unique_ptr<SkSL::Program> baseProgram);
95+
std::unique_ptr<SkSL::Program> baseProgram,
96+
std::vector<Variable>&& inAndUniformVars, std::vector<SkString>&& children);
9697

9798
int fIndex;
9899
SkString fSkSL;

‎src/sksl/SkSLIRGenerator.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,9 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTNo
304304
const Type* type = baseType;
305305
std::vector<std::unique_ptr<Expression>> sizes;
306306
auto iter = varDecl.begin();
307+
if (varData.fSizeCount > 0 && (modifiers.fFlags & Modifiers::kIn_Flag)) {
308+
fErrors.error(varDecl.fOffset, "'in' variables may not have array type");
309+
}
307310
for (size_t i = 0; i < varData.fSizeCount; ++i, ++iter) {
308311
const ASTNode& rawSize = *iter;
309312
if (rawSize) {

‎tests/SkRuntimeEffectTest.cpp‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@
88
#include "src/core/SkRuntimeEffect.h"
99
#include "tests/Test.h"
1010

11-
DEF_TEST(SkRuntimeEffectInvalidPrograms, r) {
12-
auto test = [r](const char* src, const char* expected) {
13-
auto [effect, errorText] = SkRuntimeEffect::Make(SkString(src));
11+
DEF_TEST(SkRuntimeEffectInvalidInputs, r) {
12+
auto test = [r](const char* hdr, const char* expected) {
13+
SkString src = SkStringPrintf("%s void main(float x, float y, inout half4 color) {}", hdr);
14+
auto [effect, errorText] = SkRuntimeEffect::Make(src);
1415
REPORTER_ASSERT(r, !effect);
1516
REPORTER_ASSERT(r, errorText.contains(expected),
1617
"Expected error message to contain \"%s\". Actual message: \"%s\"",
@@ -19,23 +20,15 @@ DEF_TEST(SkRuntimeEffectInvalidPrograms, r) {
1920

2021
// Features that are only allowed in .fp files (key, in uniform, ctype, when, tracked).
2122
// Ensure that these fail, and the error messages contain the relevant keyword.
22-
test("layout(key) in bool Input;"
23-
"void main(float x, float y, inout half4 color) {}",
24-
"key");
23+
test("layout(key) in bool Input;", "key");
24+
test("in uniform float Input;", "in uniform");
25+
test("layout(ctype=SkRect) float4 Input;", "ctype");
26+
test("in bool Flag; layout(when=Flag) uniform float Input;", "when");
27+
test("layout(tracked) uniform float Input;", "tracked");
2528

26-
test("in uniform float Input;"
27-
"void main(float x, float y, inout half4 color) {}",
28-
"in uniform");
29+
// Runtime SkSL supports a limited set of uniform types. No samplers, for example:
30+
test("uniform sampler2D s;", "sampler2D");
2931

30-
test("layout(ctype=SkRect) float4 Input;"
31-
"void main(float x, float y, inout half4 color) {}",
32-
"ctype");
33-
34-
test("in bool Flag; layout(when=Flag) uniform float Input;"
35-
"void main(float x, float y, inout half4 color) {}",
36-
"when");
37-
38-
test("layout(tracked) uniform float Input;"
39-
"void main(float x, float y, inout half4 color) {}",
40-
"tracked");
32+
// 'in' variables can't be arrays
33+
test("in int Input[2];", "array");
4134
}

0 commit comments

Comments
 (0)