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

Commit e64ae86

Browse files
brianosmanSkia Commit-Bot
brianosman
authored and
Skia Commit-Bot
committed
Rearrange logic in SkRuntimeEffect::Make
To extract metadata and validate the shader, we've added several iterations over all program elements. This CL rearranges things to iterate once (*). Variable conversion is moved to a separate loop later, to help with nesting and readability. Removes hard-to-read asserts. These were validating things enforced by both the IR generator and unit tests. *: Technically, there are additional implicit iterations when we call SkSL::Analysis functions. Folding all of this into a single pass would be even better, but much more complicated. Change-Id: I4f5aa649e74094e94c365ad20ef2ac96082285cd Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303924 Commit-Queue: Brian Osman <[email protected]> Reviewed-by: John Stiles <[email protected]> Reviewed-by: Michael Ludwig <[email protected]>
1 parent c86c523 commit e64ae86

File tree

2 files changed

+97
-104
lines changed

2 files changed

+97
-104
lines changed

‎src/core/SkRuntimeEffect.cpp

Lines changed: 92 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -122,130 +122,121 @@ SkRuntimeEffect::EffectResult SkRuntimeEffect::Make(SkString sksl) {
122122

123123
bool mainHasSampleCoords = SkSL::Analysis::ReferencesSampleCoords(*program);
124124

125-
size_t offset = 0, uniformSize = 0;
126-
std::vector<Variable> inAndUniformVars;
125+
std::vector<const SkSL::Variable*> inVars;
126+
std::vector<const SkSL::Variable*> uniformVars;
127127
std::vector<SkString> children;
128128
std::vector<SkSL::SampleUsage> sampleUsages;
129129
std::vector<Varying> varyings;
130130
const SkSL::Context& ctx(compiler->context());
131131

132-
// Scrape the varyings
133-
for (const auto& e : *program) {
134-
if (e.fKind == SkSL::ProgramElement::kVar_Kind) {
135-
SkSL::VarDeclarations& v = (SkSL::VarDeclarations&) e;
136-
for (const auto& varStatement : v.fVars) {
137-
const SkSL::Variable& var = *((SkSL::VarDeclaration&) *varStatement).fVar;
132+
// Go through program elements, pulling out information that we need
133+
for (const auto& elem : *program) {
134+
// Variables (in, uniform, varying, etc.)
135+
if (elem.fKind == SkSL::ProgramElement::kVar_Kind) {
136+
const SkSL::VarDeclarations& varDecls = static_cast<const SkSL::VarDeclarations&>(elem);
137+
for (const auto& varDecl : varDecls.fVars) {
138+
const SkSL::Variable& var =
139+
*(static_cast<const SkSL::VarDeclaration&>(*varDecl).fVar);
138140

141+
// Varyings (only used in conjunction with drawVertices)
139142
if (var.fModifiers.fFlags & SkSL::Modifiers::kVarying_Flag) {
140143
varyings.push_back({var.fName, var.fType.kind() == SkSL::Type::kVector_Kind
141144
? var.fType.columns()
142145
: 1});
143146
}
147+
// Fragment Processors (aka 'shader'): These are child effects
148+
else if (&var.fType == ctx.fFragmentProcessor_Type.get()) {
149+
children.push_back(var.fName);
150+
sampleUsages.push_back(SkSL::Analysis::GetSampleUsage(*program, var));
151+
}
152+
// 'in' variables (other than fragment processors)
153+
else if (var.fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) {
154+
inVars.push_back(&var);
155+
}
156+
// 'uniform' variables
157+
else if (var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag) {
158+
uniformVars.push_back(&var);
159+
}
144160
}
145161
}
146162
}
147163

148-
// Gather the inputs in two passes, to de-interleave them in our input layout.
149-
// We put the uniforms *first*, so that the CPU backend can alias the combined input block as
150-
// the uniform block when calling the interpreter.
151-
for (auto flag : { SkSL::Modifiers::kUniform_Flag, SkSL::Modifiers::kIn_Flag }) {
152-
if (flag == SkSL::Modifiers::kIn_Flag) {
164+
size_t offset = 0, uniformSize = 0;
165+
std::vector<Variable> inAndUniformVars;
166+
inAndUniformVars.reserve(inVars.size() + uniformVars.size());
167+
168+
// We've gathered the 'in' and 'uniform' variables in separate lists. We build a single list of
169+
// both, in our own structure. We put the uniforms *first* in our input layout, so that the CPU
170+
// backend can alias the combined input block as the uniform block when calling the interpreter.
171+
for (bool uniform : {true, false}) {
172+
if (!uniform) {
153173
uniformSize = offset;
154174
}
155-
for (const auto& e : *program) {
156-
if (e.fKind == SkSL::ProgramElement::kVar_Kind) {
157-
SkSL::VarDeclarations& varDecls = (SkSL::VarDeclarations&) e;
158-
for (const auto& varStatement : varDecls.fVars) {
159-
const SkSL::Variable& var = *((SkSL::VarDeclaration&) *varStatement).fVar;
160-
161-
// Sanity check some rules that should be enforced by the IR generator.
162-
// These are all layout options that only make sense in .fp files.
163-
SkASSERT(!var.fModifiers.fLayout.fKey);
164-
SkASSERT((var.fModifiers.fFlags & SkSL::Modifiers::kIn_Flag) == 0 ||
165-
(var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag) == 0);
166-
SkASSERT(var.fModifiers.fLayout.fCType == SkSL::Layout::CType::kDefault);
167-
SkASSERT(var.fModifiers.fLayout.fWhen.fLength == 0);
168-
SkASSERT((var.fModifiers.fLayout.fFlags & SkSL::Layout::kTracked_Flag) == 0);
169-
170-
if (var.fModifiers.fFlags & flag) {
171-
if (&var.fType == ctx.fFragmentProcessor_Type.get()) {
172-
children.push_back(var.fName);
173-
sampleUsages.push_back(SkSL::Analysis::GetSampleUsage(*program, var));
174-
continue;
175-
}
176-
177-
Variable v;
178-
v.fName = var.fName;
179-
v.fQualifier = (var.fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag)
180-
? Variable::Qualifier::kUniform
181-
: Variable::Qualifier::kIn;
182-
v.fFlags = 0;
183-
v.fCount = 1;
184-
185-
const SkSL::Type* type = &var.fType;
186-
if (type->kind() == SkSL::Type::kArray_Kind) {
187-
v.fFlags |= Variable::kArray_Flag;
188-
v.fCount = type->columns();
189-
type = &type->componentType();
190-
}
175+
for (const SkSL::Variable* var : (uniform ? uniformVars : inVars)) {
176+
Variable v;
177+
v.fName = var->fName;
178+
v.fFlags = 0;
179+
v.fQualifier = (var->fModifiers.fFlags & SkSL::Modifiers::kUniform_Flag)
180+
? Variable::Qualifier::kUniform
181+
: Variable::Qualifier::kIn;
182+
v.fCount = 1;
183+
184+
const SkSL::Type* type = &var->fType;
185+
if (type->kind() == SkSL::Type::kArray_Kind) {
186+
v.fFlags |= Variable::kArray_Flag;
187+
v.fCount = type->columns();
188+
type = &type->componentType();
189+
}
191190

192-
if (!init_variable_type(ctx, type, &v)) {
193-
RETURN_FAILURE("Invalid input/uniform type: '%s'",
194-
type->displayName().c_str());
195-
}
191+
if (!init_variable_type(ctx, type, &v)) {
192+
RETURN_FAILURE("Invalid input/uniform type: '%s'", type->displayName().c_str());
193+
}
196194

197-
switch (v.fType) {
198-
case Variable::Type::kBool:
199-
case Variable::Type::kInt:
200-
if (v.fQualifier == Variable::Qualifier::kUniform) {
201-
RETURN_FAILURE("'uniform' variables may not have '%s' type",
202-
type->displayName().c_str());
203-
}
204-
break;
205-
206-
case Variable::Type::kFloat:
207-
// Floats can be 'in' or 'uniform'
208-
break;
209-
210-
case Variable::Type::kFloat2:
211-
case Variable::Type::kFloat3:
212-
case Variable::Type::kFloat4:
213-
case Variable::Type::kFloat2x2:
214-
case Variable::Type::kFloat3x3:
215-
case Variable::Type::kFloat4x4:
216-
if (v.fQualifier == Variable::Qualifier::kIn) {
217-
RETURN_FAILURE("'in' variables may not have '%s' type",
218-
type->displayName().c_str());
219-
}
220-
break;
221-
}
195+
switch (v.fType) {
196+
case Variable::Type::kBool:
197+
case Variable::Type::kInt:
198+
if (v.fQualifier == Variable::Qualifier::kUniform) {
199+
RETURN_FAILURE("'uniform' variables may not have '%s' type",
200+
type->displayName().c_str());
201+
}
202+
break;
203+
204+
case Variable::Type::kFloat:
205+
// Floats can be 'in' or 'uniform'
206+
break;
207+
208+
case Variable::Type::kFloat2:
209+
case Variable::Type::kFloat3:
210+
case Variable::Type::kFloat4:
211+
case Variable::Type::kFloat2x2:
212+
case Variable::Type::kFloat3x3:
213+
case Variable::Type::kFloat4x4:
214+
if (v.fQualifier == Variable::Qualifier::kIn) {
215+
RETURN_FAILURE("'in' variables may not have '%s' type",
216+
type->displayName().c_str());
217+
}
218+
break;
219+
}
222220

223-
const SkSL::StringFragment& marker(var.fModifiers.fLayout.fMarker);
224-
if (marker.fLength) {
225-
// Rules that should be enforced by the IR generator:
226-
SkASSERT(v.fQualifier == Variable::Qualifier::kUniform);
227-
SkASSERT(v.fType == Variable::Type::kFloat4x4);
228-
v.fFlags |= Variable::kMarker_Flag;
229-
if (!parse_marker(marker, &v.fMarker, &v.fFlags)) {
230-
RETURN_FAILURE("Invalid 'marker' string: '%.*s'",
231-
(int)marker.fLength, marker.fChars);
232-
}
233-
}
221+
const SkSL::StringFragment& marker(var->fModifiers.fLayout.fMarker);
222+
if (marker.fLength) {
223+
v.fFlags |= Variable::kMarker_Flag;
224+
if (!parse_marker(marker, &v.fMarker, &v.fFlags)) {
225+
RETURN_FAILURE("Invalid 'marker' string: '%.*s'", (int)marker.fLength,
226+
marker.fChars);
227+
}
228+
}
234229

235-
if (var.fModifiers.fLayout.fFlags &
236-
SkSL::Layout::Flag::kSRGBUnpremul_Flag) {
237-
v.fFlags |= Variable::kSRGBUnpremul_Flag;
238-
}
230+
if (var->fModifiers.fLayout.fFlags & SkSL::Layout::Flag::kSRGBUnpremul_Flag) {
231+
v.fFlags |= Variable::kSRGBUnpremul_Flag;
232+
}
239233

240-
if (v.fType != Variable::Type::kBool) {
241-
offset = SkAlign4(offset);
242-
}
243-
v.fOffset = offset;
244-
offset += v.sizeInBytes();
245-
inAndUniformVars.push_back(v);
246-
}
247-
}
234+
if (v.fType != Variable::Type::kBool) {
235+
offset = SkAlign4(offset);
248236
}
237+
v.fOffset = offset;
238+
offset += v.sizeInBytes();
239+
inAndUniformVars.push_back(v);
249240
}
250241
}
251242

‎tests/SkRuntimeEffectTest.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,14 @@ DEF_TEST(SkRuntimeEffectInvalid, r) {
5050
test("in float2 Input;", "", "'in'");
5151
test("in half3x3 Input;", "", "'in'");
5252

53+
// 'marker' is only permitted on 'uniform' variables
54+
test("layout(marker=local_to_world) in float4x4 localToWorld;", "", "'uniform'");
55+
// 'marker' is only permitted on float4x4 variables
56+
test("layout(marker=local_to_world) uniform float3x3 localToWorld;", "", "float4x4");
57+
5358
test("half missing();", "color.r = missing();", "undefined function");
5459
}
5560

56-
// Our packing rules and unit test code here relies on this:
57-
static_assert(sizeof(bool) == 1);
58-
5961
class TestEffect {
6062
public:
6163
TestEffect(skiatest::Reporter* r, const char* hdr, const char* body) {

0 commit comments

Comments
 (0)