Disallow sk_FragCoord in SkRuntimeEffect
It's still accessible internally (and by default when using
SkMakeRuntimeEffect), but we don't want clients using this.
If we can come up with a simple and consistent coordinate
model that doesn't expose implementation details, we'll
revisit this.
Change-Id: I77726830480a286541ae887f9cd9822eb10ea913
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/430422
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/tests/SkDSLRuntimeEffectTest.cpp b/tests/SkDSLRuntimeEffectTest.cpp
index 9e61c56..3f283af 100644
--- a/tests/SkDSLRuntimeEffectTest.cpp
+++ b/tests/SkDSLRuntimeEffectTest.cpp
@@ -14,6 +14,7 @@
#include "include/effects/SkRuntimeEffect.h"
#include "include/gpu/GrDirectContext.h"
#include "include/sksl/DSLRuntimeEffects.h"
+#include "src/core/SkRuntimeEffectPriv.h"
#include "src/core/SkTLazy.h"
#include "src/gpu/GrColor.h"
#include "src/sksl/SkSLCompiler.h"
@@ -37,7 +38,9 @@
}
void end(bool expectSuccess = true) {
- sk_sp<SkRuntimeEffect> effect = EndRuntimeShader();
+ SkRuntimeEffect::Options options;
+ SkRuntimeEffectPriv::EnableFragCoord(&options);
+ sk_sp<SkRuntimeEffect> effect = EndRuntimeShader(options);
REPORTER_ASSERT(fReporter, effect ? expectSuccess : !expectSuccess);
if (effect) {
fBuilder.init(std::move(effect));
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index 6b0d428..79b05e7 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -221,12 +221,14 @@
DEF_TEST(SkRuntimeEffectForShader, r) {
// 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));
+ auto test_valid = [r](const char* sksl, SkRuntimeEffect::Options options = {}) {
+ auto [effect, errorText] = SkRuntimeEffect::MakeForShader(SkString(sksl), options);
REPORTER_ASSERT(r, effect, "%s", errorText.c_str());
};
- auto test_invalid = [r](const char* sksl, const char* expected) {
+ auto test_invalid = [r](const char* sksl,
+ const char* expected,
+ SkRuntimeEffect::Options options = {}) {
auto [effect, errorText] = SkRuntimeEffect::MakeForShader(SkString(sksl));
REPORTER_ASSERT(r, !effect);
REPORTER_ASSERT(r,
@@ -258,8 +260,13 @@
test_invalid("half4 main() { return half4(1); }", "'main' parameter");
test_invalid("half4 main(half4 c) { return c; }", "'main' parameter");
- // sk_FragCoord should be available
- test_valid("half4 main(float2 p) { return sk_FragCoord.xy01; }");
+ // sk_FragCoord should be available, but only if we've enabled it via Options
+ test_invalid("half4 main(float2 p) { return sk_FragCoord.xy01; }",
+ "unknown identifier 'sk_FragCoord'");
+
+ SkRuntimeEffect::Options optionsWithFragCoord;
+ SkRuntimeEffectPriv::EnableFragCoord(&optionsWithFragCoord);
+ test_valid("half4 main(float2 p) { return sk_FragCoord.xy01; }", optionsWithFragCoord);
// Sampling a child shader requires that we pass explicit coords
test_valid("uniform shader child;"
@@ -344,7 +351,9 @@
: fReporter(r), fSurface(std::move(surface)) {}
void build(const char* src) {
- auto [effect, errorText] = SkRuntimeEffect::MakeForShader(SkString(src));
+ SkRuntimeEffect::Options options;
+ SkRuntimeEffectPriv::EnableFragCoord(&options);
+ auto [effect, errorText] = SkRuntimeEffect::MakeForShader(SkString(src), options);
if (!effect) {
REPORT_FAILURE(fReporter, "effect",
SkStringPrintf("Effect didn't compile: %s", errorText.c_str()));
@@ -684,7 +693,9 @@
std::thread threads[16];
for (auto& thread : threads) {
thread = std::thread([r]() {
- auto [effect, error] = SkRuntimeEffect::MakeForShader(SkString(kSource));
+ SkRuntimeEffect::Options options;
+ SkRuntimeEffectPriv::EnableFragCoord(&options);
+ auto [effect, error] = SkRuntimeEffect::MakeForShader(SkString(kSource), options);
REPORTER_ASSERT(r, effect);
});
}