Skip to content

Commit 088913a

Browse files
brianosmanSkia Commit-Bot
brianosman
authored and
Skia Commit-Bot
committed
Start adding unit tests of SkRuntimeEffect
- Change SkRuntimeEffect::Make so *it* can fail, and returns [Effect, ErrorText]. - Initial tests just test for expected failure conditions. Next steps are to add tests for effects that should work, and to validate results on CPU and GPU. Change-Id: Ibac8c3046104577434034263e9e4a4b177e89129 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261095 Reviewed-by: Brian Salomon <bsalomon@google.com> Commit-Queue: Brian Osman <brianosman@google.com>
1 parent dcac29b commit 088913a

13 files changed

Lines changed: 122 additions & 66 deletions

File tree

‎gn/tests.gni‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ tests_sources = [
242242
"$_tests/SkRasterPipelineTest.cpp",
243243
"$_tests/SkRemoteGlyphCacheTest.cpp",
244244
"$_tests/SkResourceCacheTest.cpp",
245+
"$_tests/SkRuntimeEffectTest.cpp",
245246
"$_tests/SkSLErrorTest.cpp",
246247
"$_tests/SkSLFPTest.cpp",
247248
"$_tests/SkSLGLSLTest.cpp",

‎src/core/SkColorFilter.cpp‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,11 @@ class SkRuntimeColorFilter : public SkColorFilter {
440440

441441
SkAutoMutexExclusive ama(fByteCodeMutex);
442442
if (!fByteCode) {
443-
auto [byteCode, errorCount, errorText] = fEffect->toByteCode();
444-
if (errorCount) {
443+
auto [byteCode, errorText] = fEffect->toByteCode();
444+
if (!byteCode) {
445445
SkDebugf("%s\n", errorText.c_str());
446446
return false;
447447
}
448-
SkASSERT(byteCode);
449448
fByteCode = std::move(byteCode);
450449
}
451450
ctx->byteCode = fByteCode.get();
@@ -491,13 +490,15 @@ sk_sp<SkFlattenable> SkRuntimeColorFilter::CreateProc(SkReadBuffer& buffer) {
491490
SkString sksl;
492491
buffer.readString(&sksl);
493492
sk_sp<SkData> inputs = buffer.readByteArrayAsData();
494-
return sk_sp<SkFlattenable>(new SkRuntimeColorFilter(SkRuntimeEffect::Make(std::move(sksl)),
495-
std::move(inputs), nullptr));
493+
494+
auto effect = std::get<0>(SkRuntimeEffect::Make(std::move(sksl)));
495+
return sk_sp<SkFlattenable>(new SkRuntimeColorFilter(std::move(effect), std::move(inputs),
496+
nullptr));
496497
}
497498

498499
SkRuntimeColorFilterFactory::SkRuntimeColorFilterFactory(SkString sksl,
499500
SkRuntimeColorFilterFn cpuFunc)
500-
: fEffect(SkRuntimeEffect::Make(std::move(sksl)))
501+
: fEffect(std::get<0>(SkRuntimeEffect::Make(std::move(sksl))))
501502
, fCpuFunc(cpuFunc) {}
502503

503504
SkRuntimeColorFilterFactory::~SkRuntimeColorFilterFactory() = default;
@@ -518,7 +519,7 @@ sk_sp<SkColorFilter> SkRuntimeColorFilterFactory::make(sk_sp<SkData> inputs) {
518519
}
519520

520521
bool SkRuntimeColorFilterFactory::testCompile() const {
521-
return fEffect && fEffect->isValid();
522+
return fEffect != nullptr;
522523
}
523524

524525
#endif // SK_SUPPORT_GPU

‎src/core/SkRuntimeEffect.cpp‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,19 @@ static inline int new_sksl_index() {
1515
return nextIndex++;
1616
}
1717

18-
sk_sp<SkRuntimeEffect> SkRuntimeEffect::Make(SkString sksl) {
19-
return sk_sp<SkRuntimeEffect>(new SkRuntimeEffect(std::move(sksl)));
18+
SkRuntimeEffect::EffectResult SkRuntimeEffect::Make(SkString sksl) {
19+
auto compiler = std::make_unique<SkSL::Compiler>();
20+
auto program = compiler->convertProgram(SkSL::Program::kPipelineStage_Kind,
21+
SkSL::String(sksl.c_str(), sksl.size()),
22+
SkSL::Program::Settings());
23+
if (!program) {
24+
return std::make_pair(nullptr, SkString(compiler->errorText().c_str()));
25+
}
26+
SkASSERT(!compiler->errorCount());
27+
28+
sk_sp<SkRuntimeEffect> effect(new SkRuntimeEffect(std::move(sksl), std::move(compiler),
29+
std::move(program)));
30+
return std::make_pair(std::move(effect), SkString());
2031
}
2132

2233
size_t SkRuntimeEffect::Variable::sizeInBytes() const {
@@ -38,17 +49,13 @@ size_t SkRuntimeEffect::Variable::sizeInBytes() const {
3849
return element_size(fType) * fCount;
3950
}
4051

41-
SkRuntimeEffect::SkRuntimeEffect(SkString sksl)
52+
SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler> compiler,
53+
std::unique_ptr<SkSL::Program> baseProgram)
4254
: fIndex(new_sksl_index())
43-
, fSkSL(std::move(sksl)) {
44-
fBaseProgram = fCompiler.convertProgram(SkSL::Program::kPipelineStage_Kind,
45-
SkSL::String(fSkSL.c_str(), fSkSL.size()),
46-
SkSL::Program::Settings());
47-
if (!fBaseProgram) {
48-
SkDebugf("%s\n", fCompiler.errorText().c_str());
49-
return;
50-
}
51-
SkASSERT(!fCompiler.errorCount());
55+
, fSkSL(std::move(sksl))
56+
, fCompiler(std::move(compiler))
57+
, fBaseProgram(std::move(baseProgram)) {
58+
SkASSERT(fCompiler && fBaseProgram);
5259

5360
size_t offset = 0;
5461
auto gather_variables = [this, &offset](SkSL::Modifiers::Flag flag) {
@@ -68,7 +75,7 @@ SkRuntimeEffect::SkRuntimeEffect(SkString sksl)
6875
SkASSERT((var.fModifiers.fLayout.fFlags & SkSL::Layout::kTracked_Flag) == 0);
6976

7077
if (var.fModifiers.fFlags & flag) {
71-
if (&var.fType == fCompiler.context().fFragmentProcessor_Type.get()) {
78+
if (&var.fType == fCompiler->context().fFragmentProcessor_Type.get()) {
7279
fChildren.push_back(var.fName);
7380
continue;
7481
}
@@ -94,7 +101,7 @@ SkRuntimeEffect::SkRuntimeEffect(SkString sksl)
94101
#define SET_TYPES(cpuType, gpuType) do { v.fType = cpuType; } while (false)
95102
#endif
96103

97-
const SkSL::Context& ctx(fCompiler.context());
104+
const SkSL::Context& ctx(fCompiler->context());
98105
if (type == ctx.fBool_Type.get()) {
99106
SET_TYPES(Variable::Type::kBool, kVoid_GrSLType);
100107
} else if (type == ctx.fInt_Type.get()) {
@@ -168,9 +175,9 @@ bool SkRuntimeEffect::toPipelineStage(const void* inputs, const GrShaderCaps* sh
168175
SkSL::Program::Settings settings;
169176
settings.fCaps = shaderCaps;
170177

171-
auto baseProgram = fCompiler.convertProgram(SkSL::Program::kPipelineStage_Kind,
172-
SkSL::String(fSkSL.c_str(), fSkSL.size()),
173-
settings);
178+
auto baseProgram = fCompiler->convertProgram(SkSL::Program::kPipelineStage_Kind,
179+
SkSL::String(fSkSL.c_str(), fSkSL.size()),
180+
settings);
174181
SkASSERT(baseProgram);
175182

176183
std::unordered_map<SkSL::String, SkSL::Program::Settings::Value> inputMap;
@@ -203,16 +210,16 @@ bool SkRuntimeEffect::toPipelineStage(const void* inputs, const GrShaderCaps* sh
203210
}
204211
}
205212

206-
auto specialized = fCompiler.specialize(*baseProgram, inputMap);
207-
bool optimized = fCompiler.optimize(*specialized);
213+
auto specialized = fCompiler->specialize(*baseProgram, inputMap);
214+
bool optimized = fCompiler->optimize(*specialized);
208215
if (!optimized) {
209-
SkDebugf("%s\n", fCompiler.errorText().c_str());
216+
SkDebugf("%s\n", fCompiler->errorText().c_str());
210217
SkASSERT(false);
211218
return false;
212219
}
213220

214-
if (!fCompiler.toPipelineStage(*specialized, outCode, outFormatArgs, outFunctions)) {
215-
SkDebugf("%s\n", fCompiler.errorText().c_str());
221+
if (!fCompiler->toPipelineStage(*specialized, outCode, outFormatArgs, outFunctions)) {
222+
SkDebugf("%s\n", fCompiler->errorText().c_str());
216223
SkASSERT(false);
217224
return false;
218225
}
@@ -221,8 +228,7 @@ bool SkRuntimeEffect::toPipelineStage(const void* inputs, const GrShaderCaps* sh
221228
}
222229
#endif
223230

224-
std::tuple<std::unique_ptr<SkSL::ByteCode>, int, SkString> SkRuntimeEffect::toByteCode() {
225-
auto byteCode = fCompiler.toByteCode(*fBaseProgram);
226-
return std::make_tuple(std::move(byteCode), fCompiler.errorCount(),
227-
SkString(fCompiler.errorText().c_str()));
231+
SkRuntimeEffect::ByteCodeResult SkRuntimeEffect::toByteCode() {
232+
auto byteCode = fCompiler->toByteCode(*fBaseProgram);
233+
return std::make_tuple(std::move(byteCode), SkString(fCompiler->errorText().c_str()));
228234
}

‎src/core/SkRuntimeEffect.h‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,14 @@ class SkRuntimeEffect : public SkRefCnt {
5959
size_t sizeInBytes() const;
6060
};
6161

62-
static sk_sp<SkRuntimeEffect> Make(SkString sksl);
62+
// [Effect, ErrorText]
63+
// If successful, Effect != nullptr, otherwise, ErrorText contains the reason for failure.
64+
using EffectResult = std::tuple<sk_sp<SkRuntimeEffect>, SkString>;
65+
66+
static EffectResult Make(SkString sksl);
6367

6468
const SkString& source() const { return fSkSL; }
6569
int index() const { return fIndex; }
66-
bool isValid() const { return fBaseProgram != nullptr; }
6770
size_t inputSize() const;
6871
size_t childCount() const { return fChildren.size(); }
6972

@@ -76,16 +79,20 @@ class SkRuntimeEffect : public SkRefCnt {
7679
std::vector<SkSL::Compiler::GLSLFunction>* outFunctions);
7780
#endif
7881

79-
// Returns ByteCode, ErrorCount, ErrorText
80-
std::tuple<std::unique_ptr<SkSL::ByteCode>, int, SkString> toByteCode();
82+
// [ByteCode, ErrorText]
83+
// If successful, ByteCode != nullptr, otherwise, ErrorText contains the reason for failure.
84+
using ByteCodeResult = std::tuple<std::unique_ptr<SkSL::ByteCode>, SkString>;
85+
86+
ByteCodeResult toByteCode();
8187

8288
private:
83-
SkRuntimeEffect(SkString sksl);
89+
SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler> compiler,
90+
std::unique_ptr<SkSL::Program> baseProgram);
8491

8592
int fIndex;
8693
SkString fSkSL;
8794

88-
SkSL::Compiler fCompiler;
95+
std::unique_ptr<SkSL::Compiler> fCompiler;
8996
std::unique_ptr<SkSL::Program> fBaseProgram;
9097
std::vector<Variable> fInAndUniformVars;
9198
std::vector<SkString> fChildren;

‎src/effects/SkOverdrawColorFilter.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,12 @@ void SkOverdrawColorFilter::RegisterFlattenables() {
9090

9191
std::unique_ptr<GrFragmentProcessor> SkOverdrawColorFilter::asFragmentProcessor(
9292
GrRecordingContext* context, const GrColorInfo&) const {
93-
static auto overdrawEffect = SkRuntimeEffect::Make(SkString(SKSL_OVERDRAW_SRC));
93+
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_OVERDRAW_SRC)));
9494
SkColor4f floatColors[kNumColors];
9595
for (int i = 0; i < kNumColors; ++i) {
9696
floatColors[i] = SkColor4f::FromBytes_RGBA(fColors[i]);
9797
}
98-
return GrSkSLFP::Make(context, overdrawEffect, "Overdraw", floatColors, sizeof(floatColors));
98+
return GrSkSLFP::Make(context, effect, "Overdraw", floatColors, sizeof(floatColors));
9999
}
100100

101101
#endif

‎src/effects/imagefilters/SkArithmeticImageFilter.cpp‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -386,16 +386,13 @@ sk_sp<SkSpecialImage> ArithmeticImageFilterImpl::filterImageGPU(
386386
ctx.colorSpace());
387387
paint.addColorFragmentProcessor(std::move(foregroundFP));
388388

389-
static auto arithmeticEffect = SkRuntimeEffect::Make(SkString(SKSL_ARITHMETIC_SRC));
389+
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_ARITHMETIC_SRC)));
390390
ArithmeticFPInputs inputs;
391391
static_assert(sizeof(inputs.k) == sizeof(fK), "struct size mismatch");
392392
memcpy(inputs.k, fK, sizeof(inputs.k));
393393
inputs.enforcePMColor = fEnforcePMColor;
394-
std::unique_ptr<GrFragmentProcessor> xferFP = GrSkSLFP::Make(context,
395-
arithmeticEffect,
396-
"Arithmetic",
397-
&inputs,
398-
sizeof(inputs));
394+
std::unique_ptr<GrFragmentProcessor> xferFP = GrSkSLFP::Make(context, effect, "Arithmetic",
395+
&inputs, sizeof(inputs));
399396
if (xferFP) {
400397
((GrSkSLFP&) *xferFP).addChild(std::move(bgFP));
401398
paint.addColorFragmentProcessor(std::move(xferFP));

‎src/gpu/SkGr.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,8 +469,8 @@ static inline bool skpaint_to_grpaint_impl(GrRecordingContext* context,
469469
grPaint->numColorFragmentProcessors() > 0) {
470470
int32_t ditherRange = dither_range_type_for_config(ct);
471471
if (ditherRange >= 0) {
472-
static auto ditherEffect = SkRuntimeEffect::Make(SkString(SKSL_DITHER_SRC));
473-
auto ditherFP = GrSkSLFP::Make(context, ditherEffect, "Dither",
472+
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_DITHER_SRC)));
473+
auto ditherFP = GrSkSLFP::Make(context, effect, "Dither",
474474
&ditherRange, sizeof(ditherRange));
475475
if (ditherFP) {
476476
grPaint->addColorFragmentProcessor(std::move(ditherFP));

‎src/gpu/effects/GrSkSLFP.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,34 +284,34 @@ std::unique_ptr<GrFragmentProcessor> GrSkSLFP::TestCreate(GrProcessorTestData* d
284284
int type = d->fRandom->nextULessThan(3);
285285
switch (type) {
286286
case 0: {
287-
static auto ditherEffect = SkRuntimeEffect::Make(SkString(SKSL_DITHER_SRC));
287+
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_DITHER_SRC)));
288288
int rangeType = d->fRandom->nextULessThan(3);
289-
auto result = GrSkSLFP::Make(d->context(), ditherEffect, "Dither",
289+
auto result = GrSkSLFP::Make(d->context(), effect, "Dither",
290290
&rangeType, sizeof(rangeType));
291291
return std::unique_ptr<GrFragmentProcessor>(result.release());
292292
}
293293
case 1: {
294-
static auto arithmeticEffect = SkRuntimeEffect::Make(SkString(SKSL_ARITHMETIC_SRC));
294+
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_ARITHMETIC_SRC)));
295295
ArithmeticFPInputs inputs;
296296
inputs.k[0] = d->fRandom->nextF();
297297
inputs.k[1] = d->fRandom->nextF();
298298
inputs.k[2] = d->fRandom->nextF();
299299
inputs.k[3] = d->fRandom->nextF();
300300
inputs.enforcePMColor = d->fRandom->nextBool();
301-
auto result = GrSkSLFP::Make(d->context(), arithmeticEffect, "Arithmetic",
301+
auto result = GrSkSLFP::Make(d->context(), effect, "Arithmetic",
302302
&inputs, sizeof(inputs));
303303
result->addChild(GrConstColorProcessor::Make(
304304
SK_PMColor4fWHITE,
305305
GrConstColorProcessor::InputMode::kIgnore));
306306
return std::unique_ptr<GrFragmentProcessor>(result.release());
307307
}
308308
case 2: {
309-
static auto overdrawEffect = SkRuntimeEffect::Make(SkString(SKSL_OVERDRAW_SRC));
309+
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_OVERDRAW_SRC)));
310310
SkColor4f inputs[6];
311311
for (int i = 0; i < 6; ++i) {
312312
inputs[i] = SkColor4f::FromBytes_RGBA(d->fRandom->nextU());
313313
}
314-
auto result = GrSkSLFP::Make(d->context(), overdrawEffect, "Overdraw",
314+
auto result = GrSkSLFP::Make(d->context(), effect, "Overdraw",
315315
&inputs, sizeof(inputs));
316316
return std::unique_ptr<GrFragmentProcessor>(result.release());
317317
}

‎src/shaders/SkRTShader.cpp‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,11 @@ bool SkRTShader::onAppendStages(const SkStageRec& rec) const {
5050

5151
SkAutoMutexExclusive ama(fByteCodeMutex);
5252
if (!fByteCode) {
53-
auto [byteCode, errorCount, errorText] = fEffect->toByteCode();
54-
if (errorCount) {
53+
auto [byteCode, errorText] = fEffect->toByteCode();
54+
if (!byteCode) {
5555
SkDebugf("%s\n", errorText.c_str());
5656
return false;
5757
}
58-
SkASSERT(byteCode);
5958
fByteCode = std::move(byteCode);
6059
}
6160
ctx->byteCode = fByteCode.get();
@@ -119,8 +118,8 @@ sk_sp<SkFlattenable> SkRTShader::CreateProc(SkReadBuffer& buffer) {
119118
// We don't have a way to ensure that indices are consistent and correct when deserializing.
120119
// Perhaps we should have a hash table to map strings to indices? For now, all shaders get a
121120
// new unique ID after serialization.
122-
return sk_sp<SkFlattenable>(new SkRTShader(SkRuntimeEffect::Make(std::move(sksl)),
123-
std::move(inputs), localMPtr,
121+
auto effect = std::get<0>(SkRuntimeEffect::Make(std::move(sksl)));
122+
return sk_sp<SkFlattenable>(new SkRTShader(std::move(effect), std::move(inputs), localMPtr,
124123
children.data(), children.size(), isOpaque));
125124
}
126125

@@ -145,7 +144,7 @@ std::unique_ptr<GrFragmentProcessor> SkRTShader::asFragmentProcessor(const GrFPA
145144
#endif
146145

147146
SkRuntimeShaderFactory::SkRuntimeShaderFactory(SkString sksl, bool isOpaque)
148-
: fEffect(SkRuntimeEffect::Make(std::move(sksl)))
147+
: fEffect(std::get<0>(SkRuntimeEffect::Make(std::move(sksl))))
149148
, fIsOpaque(isOpaque) {}
150149

151150
SkRuntimeShaderFactory::SkRuntimeShaderFactory(const SkRuntimeShaderFactory&) = default;
@@ -159,7 +158,6 @@ SkRuntimeShaderFactory& SkRuntimeShaderFactory::operator=(SkRuntimeShaderFactory
159158
sk_sp<SkShader> SkRuntimeShaderFactory::make(sk_sp<SkData> inputs, const SkMatrix* localMatrix,
160159
sk_sp<SkShader>* children, size_t childCount) {
161160
return fEffect
162-
&& fEffect->isValid()
163161
&& inputs->size() >= fEffect->inputSize()
164162
&& childCount >= fEffect->childCount()
165163
? sk_sp<SkShader>(new SkRTShader(fEffect, std::move(inputs), localMatrix,

‎src/sksl/SkSLIRGenerator.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTNo
271271
baseType->kind() == Type::Kind::kMatrix_Kind) {
272272
fErrors.error(decls.fOffset, "'in' variables may not have matrix type");
273273
}
274+
if ((modifiers.fFlags & Modifiers::kIn_Flag) &&
275+
(modifiers.fFlags & Modifiers::kUniform_Flag)) {
276+
fErrors.error(decls.fOffset,
277+
"'in uniform' variables only permitted within fragment processors");
278+
}
274279
if (modifiers.fLayout.fWhen.fLength) {
275280
fErrors.error(decls.fOffset, "'when' is only permitted within fragment processors");
276281
}

0 commit comments

Comments
 (0)