commit 77046a7f0fcf4c105e63c4140e0a54d6be656c38
author Brian Osman <brianosman@google.com> Tue Jul 20 13:16:57 2021 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Wed Jul 21 13:36:56 2021 +0000
tree b01768dee45312488d88ce486818e9710e0ea1af
parent d090aa7feee336b4716edc23a1e99125a5a9c642

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>

tests/SkRuntimeEffectTest.cpp

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);
         });
     }