Loosen ES3 restrictions in Runtime Effects for debugging.
This CL adds a RuntimeEffect option flag which skips over the various
`strictES2Mode` checks sprinkled throughout IR generation.
Runtime Effects still won't allow a lot of ES3 things (the Pipeline
stage will reject unsupported statement types, SkVM doesn't support most
non-ES2 constructs, etc). However, this change will give us the ability
to test many more features involving arrays and structs that previously
were off-limits due to ES2 restrictions, and will shore up some
legitimate gaps in our testing. This is a useful starting point to allow
for improved test coverage.
Change-Id: I4a5bc43914e65fc7e59f1cecb76a0ec5a7f05f2f
Bug: skia:11209
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402157
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/include/effects/SkRuntimeEffect.h b/include/effects/SkRuntimeEffect.h
index 43d2868..caa38f5 100644
--- a/include/effects/SkRuntimeEffect.h
+++ b/include/effects/SkRuntimeEffect.h
@@ -76,6 +76,11 @@
// For testing purposes, completely disable the inliner. (Normally, Runtime Effects don't
// run the inliner directly, but they still get an inlining pass once they are painted.)
bool forceNoInline = false;
+ // For testing purposes only; only honored when GR_TEST_UTILS is enabled. This flag lifts
+ // the ES2 restrictions on Runtime Effects that are gated by the `strictES2Mode` check.
+ // Be aware that the software renderer and pipeline-stage effect are still largely
+ // ES3-unaware and can still fail or crash if post-ES2 features are used.
+ bool enforceES2Restrictions = true;
};
// If the effect is compiled successfully, `effect` will be non-null.
diff --git a/resources/sksl/inliner/ForInitializerExpressionsCanBeInlined.sksl b/resources/sksl/inliner/ForInitializerExpressionsCanBeInlined.sksl
index 23506fe..f6fc7e9 100644
--- a/resources/sksl/inliner/ForInitializerExpressionsCanBeInlined.sksl
+++ b/resources/sksl/inliner/ForInitializerExpressionsCanBeInlined.sksl
@@ -13,9 +13,7 @@
}
half4 main(float2 coords) {
- for (sk_FragColor = initLoopVar();
- shouldLoop(sk_FragColor);
- sk_FragColor = grow(sk_FragColor)) {
- }
+ for (half4 color = initLoopVar(); shouldLoop(color); color = grow(color)) {}
+
return colorGreen;
}
diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp
index a1c3aab..9ebc442 100644
--- a/src/core/SkRuntimeEffect.cpp
+++ b/src/core/SkRuntimeEffect.cpp
@@ -133,6 +133,9 @@
SkSL::Program::Settings settings;
settings.fInlineThreshold = 0;
settings.fForceNoInline = options.forceNoInline;
+#if GR_TEST_UTILS
+ settings.fEnforceES2Restrictions = options.enforceES2Restrictions;
+#endif
settings.fAllowNarrowingConversions = true;
program = compiler->convertProgram(kind, SkSL::String(sksl.c_str(), sksl.size()), settings);
@@ -380,9 +383,12 @@
// be accounted for in `fHash`. If you've added a new field to Options and caused the static-
// assert below to trigger, please incorporate your field into `fHash` and update KnownOptions
// to match the layout of Options.
- struct KnownOptions { bool b; };
+ struct KnownOptions { bool a, b; };
static_assert(sizeof(Options) == sizeof(KnownOptions));
- fHash = SkOpts::hash_fn(&options.forceNoInline, sizeof(options.forceNoInline), fHash);
+ fHash = SkOpts::hash_fn(&options.forceNoInline,
+ sizeof(options.forceNoInline), fHash);
+ fHash = SkOpts::hash_fn(&options.enforceES2Restrictions,
+ sizeof(options.enforceES2Restrictions), fHash);
}
SkRuntimeEffect::~SkRuntimeEffect() = default;
diff --git a/src/sksl/SkSLProgramSettings.h b/src/sksl/SkSLProgramSettings.h
index bc4aad5..3396ef7 100644
--- a/src/sksl/SkSLProgramSettings.h
+++ b/src/sksl/SkSLProgramSettings.h
@@ -64,6 +64,9 @@
// 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.
bool fPermitInvalidStaticTests = false;
+ // If true, configurations which demand strict ES2 conformance (runtime effects, generic
+ // programs, and SkVM rendering) will fail during compilation if ES2 restrictions are violated.
+ bool fEnforceES2Restrictions = true;
};
/**
@@ -74,10 +77,11 @@
ProgramSettings fSettings;
bool strictES2Mode() const {
- return fKind == ProgramKind::kRuntimeEffect ||
- fKind == ProgramKind::kRuntimeColorFilter ||
- fKind == ProgramKind::kRuntimeShader ||
- fKind == ProgramKind::kGeneric;
+ return fSettings.fEnforceES2Restrictions &&
+ (fKind == ProgramKind::kRuntimeEffect ||
+ fKind == ProgramKind::kRuntimeColorFilter ||
+ fKind == ProgramKind::kRuntimeShader ||
+ fKind == ProgramKind::kGeneric);
}
};
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index 77f66b7..a6ba883 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -75,11 +75,23 @@
"unknown identifier 'sk_Caps'");
}
+DEF_TEST(SkRuntimeEffectCanDisableES2Restrictions, r) {
+ auto test_valid_es3 = [](skiatest::Reporter* r, const char* sksl) {
+ SkRuntimeEffect::Options opt;
+ opt.enforceES2Restrictions = false;
+ auto [effect, errorText] = SkRuntimeEffect::MakeForShader(SkString(sksl), opt);
+ REPORTER_ASSERT(r, effect, "%s", errorText.c_str());
+ };
+
+ test_invalid_effect(r, "float f[2] = float[2](0, 1);" EMPTY_MAIN, "construction of array type");
+ test_valid_es3 (r, "float f[2] = float[2](0, 1);" EMPTY_MAIN);
+}
+
DEF_TEST(SkRuntimeEffectForColorFilter, r) {
// Tests that the color filter factory rejects or accepts certain SkSL constructs
auto test_valid = [r](const char* sksl) {
auto [effect, errorText] = SkRuntimeEffect::MakeForColorFilter(SkString(sksl));
- REPORTER_ASSERT(r, effect, errorText.c_str());
+ REPORTER_ASSERT(r, effect, "%s", errorText.c_str());
};
auto test_invalid = [r](const char* sksl, const char* expected) {
@@ -131,7 +143,7 @@
// Tests that the shader factory rejects or accepts certain SkSL constructs
auto test_valid = [r](const char* sksl) {
auto [effect, errorText] = SkRuntimeEffect::MakeForShader(SkString(sksl));
- REPORTER_ASSERT(r, effect, errorText.c_str());
+ REPORTER_ASSERT(r, effect, "%s", errorText.c_str());
};
auto test_invalid = [r](const char* sksl, const char* expected) {
diff --git a/tests/SkSLTest.cpp b/tests/SkSLTest.cpp
index 20b3a44..20bcff5 100644
--- a/tests/SkSLTest.cpp
+++ b/tests/SkSLTest.cpp
@@ -19,6 +19,8 @@
#include "include/effects/SkRuntimeEffect.h"
#include "include/private/SkSLDefines.h" // for kDefaultInlineThreshold
#include "include/utils/SkRandom.h"
+#include "src/gpu/GrCaps.h"
+#include "src/gpu/GrDirectContextPriv.h"
#include "tests/Test.h"
#include "tools/Resources.h"
#include "tools/ToolUtils.h"
@@ -92,8 +94,12 @@
SkColorGetA(color), SkColorGetR(color), SkColorGetG(color), SkColorGetB(color));
}
-static void test_permutations(skiatest::Reporter* r, SkSurface* surface, const char* testFile) {
+static void test_permutations(skiatest::Reporter* r,
+ SkSurface* surface,
+ const char* testFile,
+ bool worksInES2) {
SkRuntimeEffect::Options options;
+ options.enforceES2Restrictions = worksInES2;
options.forceNoInline = false;
test_one_permutation(r, surface, testFile, "", options);
@@ -105,14 +111,28 @@
const SkImageInfo info = SkImageInfo::MakeN32Premul(kRect.width(), kRect.height());
sk_sp<SkSurface> surface(SkSurface::MakeRaster(info));
- test_permutations(r, surface.get(), testFile);
+ test_permutations(r, surface.get(), testFile, /*worksInES2=*/true);
}
static void test_gpu(skiatest::Reporter* r, GrDirectContext* ctx, const char* testFile) {
const SkImageInfo info = SkImageInfo::MakeN32Premul(kRect.width(), kRect.height());
sk_sp<SkSurface> surface(SkSurface::MakeRenderTarget(ctx, SkBudgeted::kNo, info));
- test_permutations(r, surface.get(), testFile);
+ test_permutations(r, surface.get(), testFile, /*worksInES2=*/true);
+}
+
+static void test_es3(skiatest::Reporter* r, GrDirectContext* ctx, const char* testFile) {
+ // We don't have an ES2 caps bit, so we check for integer support and derivatives support.
+ // Our ES2 bots should return false for these.
+ if (!ctx->priv().caps()->shaderCaps()->shaderDerivativeSupport() ||
+ !ctx->priv().caps()->shaderCaps()->integerSupport()) {
+ return;
+ }
+ // ES3-only tests never run on the CPU, because SkVM lacks support for many non-ES2 features.
+ const SkImageInfo info = SkImageInfo::MakeN32Premul(kRect.width(), kRect.height());
+ sk_sp<SkSurface> surface(SkSurface::MakeRenderTarget(ctx, SkBudgeted::kNo, info));
+
+ test_permutations(r, surface.get(), testFile, /*worksInES2=*/false);
}
#define SKSL_TEST_CPU(name, path) \
@@ -123,11 +143,16 @@
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(name ## _GPU, r, ctxInfo) { \
test_gpu(r, ctxInfo.directContext(), path); \
}
+#define SKSL_TEST_ES3(name, path) \
+ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(name ## _GPU, r, ctxInfo) { \
+ test_es3(r, ctxInfo.directContext(), path); \
+ }
#define SKSL_TEST(name, path) SKSL_TEST_CPU(name, path) SKSL_TEST_GPU(name, path)
SKSL_TEST(SkSLAssignmentOps, "folding/AssignmentOps.sksl")
SKSL_TEST(SkSLBoolFolding, "folding/BoolFolding.sksl")
SKSL_TEST(SkSLIntFoldingES2, "folding/IntFoldingES2.sksl")
+SKSL_TEST_ES3(SkSLIntFoldingES3, "folding/IntFoldingES3.sksl")
SKSL_TEST(SkSLFloatFolding, "folding/FloatFolding.sksl")
// skbug.com/11919: Fails on Nexus5/7, and Intel GPUs
SKSL_TEST_CPU(SkSLMatrixFoldingES2, "folding/MatrixFoldingES2.sksl")
@@ -137,6 +162,8 @@
SKSL_TEST(SkSLVectorVectorFolding, "folding/VectorVectorFolding.sksl")
SKSL_TEST(SkSLForBodyMustBeInlinedIntoAScope, "inliner/ForBodyMustBeInlinedIntoAScope.sksl")
+SKSL_TEST_ES3(SkSLForInitializerExpressionsCanBeInlined,
+ "inliner/ForInitializerExpressionsCanBeInlined.sksl")
SKSL_TEST(SkSLForWithoutReturnInsideCanBeInlined, "inliner/ForWithoutReturnInsideCanBeInlined.sksl")
SKSL_TEST(SkSLForWithReturnInsideCannotBeInlined, "inliner/ForWithReturnInsideCannotBeInlined.sksl")
SKSL_TEST(SkSLIfBodyMustBeInlinedIntoAScope, "inliner/IfBodyMustBeInlinedIntoAScope.sksl")
@@ -167,6 +194,9 @@
SKSL_TEST(SkSLTernaryResultsCannotBeInlined, "inliner/TernaryResultsCannotBeInlined.sksl")
SKSL_TEST(SkSLTernaryTestCanBeInlined, "inliner/TernaryTestCanBeInlined.sksl")
SKSL_TEST(SkSLTrivialArgumentsInlineDirectly, "inliner/TrivialArgumentsInlineDirectly.sksl")
+SKSL_TEST_ES3(SkSLWhileBodyMustBeInlinedIntoAScope,
+ "inliner/WhileBodyMustBeInlinedIntoAScope.sksl")
+SKSL_TEST_ES3(SkSLWhileTestCannotBeInlined, "inliner/WhileTestCannotBeInlined.sksl")
// TODO(skia:11052): SPIR-V does not yet honor `out` param semantics correctly
SKSL_TEST_CPU(SkSLInlinerHonorsGLSLOutParamSemantics,
@@ -244,17 +274,12 @@
/*
TODO(skia:11209): enable these tests when Runtime Effects have support for ES3
-SKSL_TEST(SkSLIntFoldingES3, "folding/IntFoldingES3.sksl")
SKSL_TEST(SkSLMatrixFoldingES3, "folding/MatrixFoldingES3.sksl")
SKSL_TEST(SkSLDoWhileBodyMustBeInlinedIntoAScope, "inliner/DoWhileBodyMustBeInlinedIntoAScope.sksl")
SKSL_TEST(SkSLDoWhileTestCannotBeInlined, "inliner/DoWhileTestCannotBeInlined.sksl")
-SKSL_TEST(SkSLEnumsCanBeInlinedSafely, "inliner/EnumsCanBeInlinedSafely.sksl")
-SKSL_TEST(SkSLForInitializerExpressionsCanBeInlined,
- "inliner/ForInitializerExpressionsCanBeInlined.sksl")
+SKSL_TEST(SkSLEnumsCanBeInlinedSafely, "inliner/EnumsCanBeInlinedSafely.sksl")
SKSL_TEST(SkSLStaticSwitch, "inliner/StaticSwitch.sksl")
-SKSL_TEST(SkSLWhileBodyMustBeInlinedIntoAScope, "inliner/WhileBodyMustBeInlinedIntoAScope.sksl")
-SKSL_TEST(SkSLWhileTestCannotBeInlined, "inliner/WhileTestCannotBeInlined.sksl")
SKSL_TEST(SkSLIntrinsicAbsInt, "intrinsics/AbsInt.sksl")
SKSL_TEST(SkSLIntrinsicClampInt, "intrinsics/ClampInt.sksl")
diff --git a/tests/sksl/inliner/ForInitializerExpressionsCanBeInlined.glsl b/tests/sksl/inliner/ForInitializerExpressionsCanBeInlined.glsl
index 29c2e35..52339f3 100644
--- a/tests/sksl/inliner/ForInitializerExpressionsCanBeInlined.glsl
+++ b/tests/sksl/inliner/ForInitializerExpressionsCanBeInlined.glsl
@@ -8,7 +8,7 @@
return v + vec4(0.125);
}
vec4 main() {
- for (sk_FragColor = vec4(0.0625);shouldLoop_bh4(sk_FragColor); sk_FragColor = grow_h4h4(sk_FragColor)) {
+ for (vec4 color = vec4(0.0625);shouldLoop_bh4(color); color = grow_h4h4(color)) {
}
return colorGreen;
}