SkRuntimeEffect SkSL has a new signature for main()

There is no more 'inout half4 color'. Effects return their output color.
If an effect wants the input color, it must use the (already existing)
approach of sampling a nullptr input shader.

The change is guarded for Chromium (so we can update their runtime color
filters in skia_renderer.cc).

For the GPU backend, FPs can now override usesExplicitReturn to indicate
that their emitCode will generate a return statement. If that's true,
then writeProcessorFunction doesn't inject the automatic return of the
output color, and emitFragProc will *always* wrap that FP in a helper
function, even as a top-level FP. GrSkSLFP opts in to this behavior, so
that the user-supplied return becomes the actual return in the FP's
emitCode.

Adapting the skvm code to this wasn't too bad: It looks fragile (what
happens if there are multiple returns?), but that's not really possible
today, without varying control flow.

Bug: skia:10613

Change-Id: I205b81fd87dd32bab30b6d6d5fc78853485da036
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310756
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>

RELEASE_NOTES.txt

@@ -9,6 +9,11 @@ Milestone 87
 
   * <insert new release notes here>
 
+  * The signature of 'main' used with SkRuntimeEffect SkSL has changed. There is no longer an
+    'inout half4 color' parameter, effects must return their color instead.
+    Valid signatures are now 'half4 main()' or 'half4 main(float2 coord)'.
+    https://review.skia.org/310756
+
   * New YUVA planar interface in SkCodec. Chroma subsampling is specified in more structured way.
     Doesn't assume 8bit planar values.
     https://review.skia.org/309658

bench/ColorFilterBench.cpp

@@ -128,7 +128,8 @@ private:
 };
 
 const char RuntimeNone_GPU_SRC[] = R"(
-    void main(inout half4 c) {}
+    in shader input;
+    half4 main() { return sample(input); }
 )";
 
 const char RuntimeColorMatrix_GPU_SRC[] = R"(
@@ -137,8 +138,9 @@ const char RuntimeColorMatrix_GPU_SRC[] = R"(
                  m5 , m6 , m7 , m8 , m9 ,
                  m10, m11, m12, m13, m14,
                  m15, m16, m17, m18, m19;
-    void main(inout half4 c) {
+    in shader input;
-        c = unpremul(c);
+    half4 main() {
+        half4 c = unpremul(sample(input));
 
         half4x4 m = half4x4(m0, m5, m10, m15,
                             m1, m6, m11, m16,
@@ -148,6 +150,7 @@ const char RuntimeColorMatrix_GPU_SRC[] = R"(
 
         c = saturate(c);
         c.rgb *= c.a;
+        return c;
     }
 )";
 
@@ -203,11 +206,14 @@ DEF_BENCH( return new ColorFilterBench("gaussian", []() {
 DEF_BENCH( return new ColorFilterBench("src_runtime", []() {
         static sk_sp<SkRuntimeEffect> gEffect = std::get<0>(
                 SkRuntimeEffect::Make(SkString(RuntimeNone_GPU_SRC)));
-        return gEffect->makeColorFilter(SkData::MakeEmpty());
+        sk_sp<SkColorFilter> input = nullptr;
+        return gEffect->makeColorFilter(SkData::MakeEmpty(), &input, 1);
     });)
 DEF_BENCH( return new ColorFilterBench("matrix_runtime", []() {
         static sk_sp<SkRuntimeEffect> gEffect = std::get<0>(
                 SkRuntimeEffect::Make(SkString(RuntimeColorMatrix_GPU_SRC)));
-        return gEffect->makeColorFilter(SkData::MakeWithCopy(gColorMatrix, sizeof(gColorMatrix)));
+        sk_sp<SkColorFilter> input = nullptr;
+        return gEffect->makeColorFilter(SkData::MakeWithCopy(gColorMatrix, sizeof(gColorMatrix)),
+                                        &input, 1);
     });)
 #endif

gm/fp_sample_chaining.cpp

@@ -278,18 +278,18 @@ DEF_SIMPLE_GPU_GM(fp_sample_chaining, ctx, rtCtx, canvas, 380, 306) {
 
 const char* gConstantMatrixSkSL = R"(
     in shader child;
-    void main(float2 xy, inout half4 color) {
+    half4 main(float2 xy) {
-        color = sample(child, float3x3(0.5, 0.0, 0.0,
+        return sample(child, float3x3(0.5, 0.0, 0.0,
-                                       0.0, 1.0, 0.0,
+                                      0.0, 1.0, 0.0,
-                                       0.0, 0.0, 1.0));
+                                      0.0, 0.0, 1.0));
     }
 )";
 
 const char* gUniformMatrixSkSL = R"(
     in shader child;
     uniform float3x3 matrix;
-    void main(float2 xy, inout half4 color) {
+    half4 main(float2 xy) {
-        color = sample(child, matrix);
+        return sample(child, matrix);
     }
 )";
 
@@ -299,16 +299,16 @@ const char* gUniformMatrixSkSL = R"(
 const char* gVariableMatrixSkSL = R"(
     in shader child;
     uniform float3x3 matrix;
-    void main(float2 xy, inout half4 color) {
+    half4 main(float2 xy) {
         float3x3 varMatrix = matrix * 0.5;
-        color = sample(child, varMatrix);
+        return sample(child, varMatrix);
     }
 )";
 
 const char* gExplicitCoordSkSL = R"(
     in shader child;
-    void main(float2 xy, inout half4 color) {
+    half4 main(float2 xy) {
-        color = sample(child, xy + float2(0, 8));
+        return sample(child, xy + float2(0, 8));
     }
 )";
 

gm/mixercolorfilter.cpp

@@ -113,8 +113,8 @@ private:
                 in shader cf0;
                 in shader cf1;
                 uniform half t;
-                void main(inout half4 color) {
+                half4 main() {
-                    color = mix(sample(cf0), sample(cf1), t);
+                    return mix(sample(cf0), sample(cf1), t);
                 }
             )";
             effect = std::get<0>(SkRuntimeEffect::Make(SkString(sksl)));

gm/runtimecolorfilter.cpp

@@ -21,20 +21,16 @@
 #include <utility>
 
 const char* gLumaSrc = R"(
-    void main(inout half4 color) {
+    in shader input;
-        color.a = color.r*0.3 + color.g*0.6 + color.b*0.1;
+    half4 main() {
-        color.r = 0;
+        return dot(sample(input).rgb, half3(0.3, 0.6, 0.1)).000r;
-        color.g = 0;
-        color.b = 0;
     }
 )";
 
 const char* gLumaSrcWithCoords = R"(
-    void main(float2 p, inout half4 color) {
+    in shader input;
-        color.a = color.r*0.3 + color.g*0.6 + color.b*0.1;
+    half4 main(float2 p) {
-        color.r = 0;
+        return dot(sample(input).rgb, half3(0.3, 0.6, 0.1)).000r;
-        color.g = 0;
-        color.b = 0;
     }
 )";
 
@@ -46,7 +42,8 @@ DEF_SIMPLE_GM(runtimecolorfilter, canvas, 256 * 3, 256) {
         sk_sp<SkRuntimeEffect> effect = std::get<0>(SkRuntimeEffect::Make(SkString(src)));
         SkASSERT(effect);
         SkPaint p;
-        p.setColorFilter(effect->makeColorFilter(nullptr));
+        sk_sp<SkColorFilter> input = nullptr;
+        p.setColorFilter(effect->makeColorFilter(nullptr, &input, 1));
         canvas->translate(256, 0);
         canvas->drawImage(img, 0, 0, &p);
     }

gm/runtimefunctions.cpp

@@ -25,8 +25,8 @@ static const char* RUNTIME_FUNCTIONS_SRC = R"(
         return half4(half3(value), raw.a);
     }
 
-    void main(float2 p, inout half4 color) {
+    half4 main(float2 p) {
-        color = blackAndWhite(half4(scale(p.x), scale(p.y), gColor.b, 1));
+        return blackAndWhite(half4(scale(p.x), scale(p.y), gColor.b, 1));
     }
 )";
 

gm/runtimeshader.cpp

@@ -60,8 +60,8 @@ public:
     SimpleRT() : RuntimeShaderGM("runtime_shader", {512, 256}, R"(
         uniform half4 gColor;
 
-        void main(float2 p, inout half4 color) {
+        half4 main(float2 p) {
-            color = half4(half2(p)*(1.0/255), gColor.b, 1);
+            return half4(half2(p)*(1.0/255), gColor.b, 1);
         }
     )", kBench_RTFlag) {}
 
@@ -132,12 +132,12 @@ public:
             return clamp(x, 0, 1);
         }
 
-        void main(float2 xy, inout half4 color) {
+        half4 main(float2 xy) {
             half4 before = sample(before_map);
             half4 after = sample(after_map);
 
             float m = smooth_cutoff(sample(threshold_map).a);
-            color = mix(before, after, half(m));
+            return mix(before, after, half(m));
         }
     )", kAnimate_RTFlag | kBench_RTFlag) {}
 
@@ -188,7 +188,7 @@ public:
         layout(srgb_unpremul) uniform float4 in_colors0;
         layout(srgb_unpremul) uniform float4 in_colors1;
 
-        void main(float2 p, inout half4 color) {
+        half4 main(float2 p) {
             float2 pp = p - in_center;
             float radius = length(pp);
             radius = sqrt(radius);
@@ -197,7 +197,7 @@ public:
             t += radius * rad_scale;
             t = fract(t);
             float4 m = in_colors0 * (1-t) + in_colors1 * t;
-            color = half4(m);
+            return half4(m);
         }
     )", kAnimate_RTFlag | kBench_RTFlag) {}
 
@@ -227,7 +227,7 @@ public:
         uniform float b_scale;
         uniform float inv_size;
 
-        void main(float2 xy, inout half4 color) {
+        half4 main(float2 xy) {
             float4 c = float4(unpremul(sample(input)));
 
             // Map to cube coords:
@@ -238,11 +238,13 @@ public:
             float2 coords2 = float2(( ceil(cubeCoords.b) + cubeCoords.r) * inv_size, cubeCoords.g);
 
             // Two bilinear fetches, plus a manual lerp for the third axis:
-            color = mix(sample(color_cube, coords1), sample(color_cube, coords2),
+            half4 color = mix(sample(color_cube, coords1), sample(color_cube, coords2),
-                        half(fract(cubeCoords.b)));
+                              half(fract(cubeCoords.b)));
 
             // Premul again
             color.rgb *= color.a;
+
+            return color;
         }
     )") {}
 
@@ -303,8 +305,8 @@ public:
     // runtime shaders work without them being declared (when they're not used).
     DefaultColorRT() : RuntimeShaderGM("default_color_rt", {512, 256}, R"(
         in shader input;
-        void main(inout half4 color) {
+        half4 main() {
-            color = sample(input);
+            return sample(input);
         }
     )") {}
 

gm/vertices.cpp

@@ -301,8 +301,8 @@ DEF_SIMPLE_GM(vertices_data, canvas, 512, 256) {
         SkPaint paint;
         const char* gProg = R"(
             varying float4 vtx_color;
-            void main(float2 p, inout half4 color) {
+            half4 main(float2 p) {
-                color = half4(vtx_color);
+                return half4(vtx_color);
             }
         )";
         auto[effect, errorText] = SkRuntimeEffect::Make(SkString(gProg));
@@ -380,10 +380,10 @@ DEF_SIMPLE_GM(vertices_data_lerp, canvas, 256, 256) {
         in shader c0;
         in shader c1;
         varying float vtx_lerp;
-        void main(float2 p, inout half4 color) {
+        half4 main(float2 p) {
             half4 col0 = sample(c0, p);
             half4 col1 = sample(c1, p);
-            color = mix(col0, col1, half(vtx_lerp));
+            return mix(col0, col1, half(vtx_lerp));
         }
     )";
     auto [effect, errorText] = SkRuntimeEffect::Make(SkString(gProg));
@@ -461,8 +461,8 @@ DEF_SIMPLE_GM(vertices_custom_colors, canvas, 400, 200) {
 
     const char* gProg = R"(
         varying half4 vtx_color;
-        void main(float2 p, inout half4 color) {
+        half4 main(float2 p) {
-            color = vtx_color;
+            return vtx_color;
         }
     )";
     SkPaint skslPaint;
@@ -583,8 +583,8 @@ DEF_SIMPLE_GM(vertices_custom_matrices, canvas, 400, 400) {
 
     const char* vectorProg = R"(
         varying float3 vtx_vec;
-        void main(float2 p, inout half4 color) {
+        half4 main(float2 p) {
-            color.rgb = half3(vtx_vec) * 0.5 + 0.5;
+            return (half3(vtx_vec) * 0.5 + 0.5).rgb1;
         })";
 
     // raw, local vectors, normals, and positions should all look the same (no real transform)
@@ -613,16 +613,16 @@ DEF_SIMPLE_GM(vertices_custom_matrices, canvas, 400, 400) {
 
     const char* ctmPositionProg250 = R"(
         varying float3 vtx_pos;
-        void main(float2 p, inout half4 color) {
+        half4 main(float2 p) {
-            color.rgb = (half3(vtx_pos) - half3(250, 350, 0)) / 50 + 0.5;
+            return ((half3(vtx_pos) - half3(250, 350, 0)) / 50 + 0.5).rgb1;
         }
     )";
     draw(250, 350, make_cone(Attr::Usage::kPosition, nullptr), ctmPositionProg250, 0.5f);
 
     const char* ctmPositionProg350 = R"(
         varying float3 vtx_pos;
-        void main(float2 p, inout half4 color) {
+        half4 main(float2 p) {
-            color.rgb = (half3(vtx_pos) - half3(350, 350, 0)) / 50 + 0.5;
+            return ((half3(vtx_pos) - half3(350, 350, 0)) / 50 + 0.5).rgb1;
         }
     )";
     canvas->saveLayer({ 300, 300, 400, 400 }, nullptr);

modules/canvaskit/CHANGELOG.md

@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
  - We now compile CanvasKit with emsdk 2.0.0 when testing and deploying to npm.
  - WebGL interface creation is a little leaner in terms of code size and speed.
+ - The signature of `main` used with SkSL passed to `CanvasKit.SkRuntimeEffect.Make` has changed.
+   There is no longer an `inout half4 color` parameter, effects must return their color instead.
+   Valid signatures are now `half4 main()` or `half4 main(float2 coord)`.
 
 ## [0.17.3] - 2020-08-05
 

modules/canvaskit/canvaskit/extra.html

@@ -100,7 +100,7 @@
   uniform float4 in_colors0;
   uniform float4 in_colors1;
 
-  void main(float2 p, inout half4 color) {
+  half4 main(float2 p) {
       float2 pp = p - in_center;
       float radius = sqrt(dot(pp, pp));
       radius = sqrt(radius);
@@ -108,7 +108,7 @@
       float t = (angle + 3.1415926/2) / (3.1415926);
       t += radius * rad_scale;
       t = fract(t);
-      color = half4(mix(in_colors0, in_colors1, t));
+      return half4(mix(in_colors0, in_colors1, t));
   }`;
 
   // Examples which only require canvaskit
@@ -341,12 +341,12 @@
           return clamp(x, 0, 1);
       }
 
-      void main(float2 xy, inout half4 color) {
+      half4 main(float2 xy) {
           half4 before = sample(before_map, xy);
           half4 after = sample(after_map, xy);
 
           float m = smooth_cutoff(sample(threshold_map, xy).r);
-          color = mix(before, after, half(m));
+          return mix(before, after, half(m));
       }`;
 
     const canvas = surface.getCanvas();

modules/canvaskit/tests/rtshader.spec.js

@@ -22,7 +22,7 @@ uniform float2 in_center;
 uniform float4 in_colors0;
 uniform float4 in_colors1;
 
-void main(float2 p, inout half4 color) {
+half4 main(float2 p) {
     float2 pp = p - in_center;
     float radius = sqrt(dot(pp, pp));
     radius = sqrt(radius);
@@ -30,7 +30,7 @@ void main(float2 p, inout half4 color) {
     float t = (angle + 3.1415926/2) / (3.1415926);
     t += radius * rad_scale;
     t = fract(t);
-    color = half4(mix(in_colors0, in_colors1, t));
+    return half4(mix(in_colors0, in_colors1, t));
 }`;
 
     // TODO(kjlubick) rewrite testRTShader and callers to use gm.
@@ -90,12 +90,12 @@ float smooth_cutoff(float x) {
     return clamp(x, 0, 1);
 }
 
-void main(float2 xy, inout half4 color) {
+half4 main(float2 xy) {
     half4 before = sample(before_map, xy);
     half4 after = sample(after_map, xy);
 
     float m = smooth_cutoff(sample(threshold_map, xy).r);
-    color = mix(before, after, half(m));
+    return mix(before, after, half(m));
 }`;
 
     // TODO(kjlubick) rewrite testChildrenShader and callers to use gm.

modules/skottie/src/effects/BrightnessContrastEffect.cpp

@@ -72,10 +72,13 @@ static constexpr char CONTRAST_EFFECT[] = R"(
     uniform half a;
     uniform half b;
     uniform half c;
+    in shader input;
 
-    void main(inout half4 color) {
+    half4 main() {
         // C' = a*C^3 + b*C^2 + c*C
+        half4 color = sample(input);
         color.rgb = ((a*color.rgb + b)*color.rgb + c)*color.rgb;
+        return color;
     }
 )";
 #else
@@ -93,9 +96,12 @@ static sk_sp<SkData> make_contrast_coeffs(float contrast) {
 
 static constexpr char CONTRAST_EFFECT[] = R"(
     uniform half a;
+    in shader input;
 
-    void main(inout half4 color) {
+    half4 main() {
+        half4 color = sample(input);
         color.rgb += a * sin(color.rgb * 6.283185);
+        return color;
     }
 )";
 
@@ -117,9 +123,12 @@ static sk_sp<SkData> make_brightness_coeffs(float brightness) {
 
 static constexpr char BRIGHTNESS_EFFECT[] = R"(
     uniform half a;
+    in shader input;
 
-    void main(inout half4 color) {
+    half4 main() {
+        half4 color = sample(input);
         color.rgb = 1 - pow(1 - color.rgb, half3(a));
+        return color;
     }
 )";
 
@@ -210,13 +219,15 @@ private:
         const auto brightness = SkTPin(fBrightness, -150.0f, 150.0f) / 150, // [-1.0 .. 1]
                      contrast = SkTPin(fContrast  ,  -50.0f, 100.0f) / 100; // [-0.5 .. 1]
 
+        sk_sp<SkColorFilter> input = nullptr;
 
         auto b_eff = SkScalarNearlyZero(brightness)
                    ? nullptr
-                   : fBrightnessEffect->makeColorFilter(make_brightness_coeffs(brightness)),
+                   : fBrightnessEffect->makeColorFilter(make_brightness_coeffs(brightness),
+                                                        &input, 1),
              c_eff = SkScalarNearlyZero(fContrast)
                    ? nullptr
-                   : fContrastEffect->makeColorFilter(make_contrast_coeffs(contrast));
+                   : fContrastEffect->makeColorFilter(make_contrast_coeffs(contrast), &input, 1);
 
         return SkColorFilters::Compose(std::move(c_eff), std::move(b_eff));
     }

samplecode/Sample3D.cpp

@@ -381,7 +381,7 @@ public:
                 return n;
             }
 
-            void main(float2 p, inout half4 color) {
+            half4 main(float2 p) {
                 float3 norm = convert_normal_sample(sample(normal_map, p));
                 float3 plane_norm = normalize(localToWorldAdjInv * float4(norm, 0)).xyz;
 
@@ -392,7 +392,7 @@ public:
                 float dp = dot(plane_norm, light_dir);
                 float scale = min(ambient + max(dp, 0), 1);
 
-                color = sample(color_map, p) * half4(float4(scale, scale, scale, 1));
+                return sample(color_map, p) * half4(float4(scale, scale, scale, 1));
             }
         )";
         auto [effect, error] = SkRuntimeEffect::Make(SkString(code));
@@ -466,7 +466,7 @@ public:
             layout (marker=normals(local_to_world)) uniform float4x4 localToWorldAdjInv;
             uniform float3   lightPos;
 
-            void main(float2 p, inout half4 color) {
+            half4 main(float2 p) {
                 float3 norm = normalize(vtx_normal);
                 float3 plane_norm = normalize(localToWorldAdjInv * float4(norm, 0)).xyz;
 
@@ -477,7 +477,7 @@ public:
                 float dp = dot(plane_norm, light_dir);
                 float scale = min(ambient + max(dp, 0), 1);
 
-                color = half4(0.7, 0.9, 0.3, 1) * half4(float4(scale, scale, scale, 1));
+                return half4(0.7, 0.9, 0.3, 1) * half4(float4(scale, scale, scale, 1));
             }
         )";
         auto [effect, error] = SkRuntimeEffect::Make(SkString(code));

site/user/modules/canvaskit.md

@@ -63,7 +63,7 @@ Samples
   <figure>
     <canvas id=shader1 width=512 height=512></canvas>
     <figcaption>
-      <a href="https://jsfiddle.skia.org/canvaskit/33ff9bed883cd5742b4770169da0b36fb0cbc18fd395ddd9563213e178362d30"
+      <a href="https://jsfiddle.skia.org/canvaskit/8ab89ac8f24840509debec604030b9abded5a73de8f6dbc376433f08ed3fba56"
           target=_blank rel=noopener>
         Shader JSFiddle</a>
     </figcaption>
@@ -71,7 +71,7 @@ Samples
   <figure>
     <canvas id=camera3d width=400 height=400></canvas>
     <figcaption>
-      <a href="https://jsfiddle.skia.org/canvaskit/4b7f2cb6683ad3254ac46e3bab62da9a09e994044b2e7512c93d166abeaa2549"
+      <a href="https://jsfiddle.skia.org/canvaskit/518a1694fed734b4533f6ffb62e47c10a45289d63aa749e6062c372076f11a12"
           target=_blank rel=noopener>
         3D Cube JSFiddle</a>
     </figcaption>
@@ -489,7 +489,7 @@ uniform float2 in_center;
 uniform float4 in_colors0;
 uniform float4 in_colors1;
 
-void main(float2 p, inout half4 color) {
+half4 main(float2 p) {
     float2 pp = p - in_center;
     float radius = sqrt(dot(pp, pp));
     radius = sqrt(radius);
@@ -497,7 +497,7 @@ void main(float2 p, inout half4 color) {
     float t = (angle + 3.1415926/2) / (3.1415926);
     t += radius * rad_scale;
     t = fract(t);
-    color = half4(mix(in_colors0, in_colors1, t));
+    return half4(mix(in_colors0, in_colors1, t));
 }
 `;
 
@@ -590,7 +590,7 @@ void main(float2 p, inout half4 color) {
         return n;
       }
 
-      void main(float2 p, inout half4 color) {
+      half4 main(float2 p) {
         float3 norm = convert_normal_sample(sample(normal_map, p));
         float3 plane_norm = normalize(localToWorldAdjInv * float4(norm, 0)).xyz;
 
@@ -601,7 +601,7 @@ void main(float2 p, inout half4 color) {
         float dp = dot(plane_norm, light_dir);
         float scale = min(ambient + max(dp, 0), 1);
 
-        color = sample(color_map, p) * half4(float4(scale, scale, scale, 1));
+        return sample(color_map, p) * half4(float4(scale, scale, scale, 1));
       }
 `;
 

src/core/SkRuntimeEffect.cpp

@@ -347,6 +347,7 @@ static skvm::Color program_fn(skvm::Builder* p,
     auto push = [&](skvm::F32 x) { stack.push_back(x); };
     auto pop  = [&]{ skvm::F32 x = stack.back(); stack.pop_back(); return x; };
 
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
     // main(inout half4 color) or main(float2 local, inout half4 color)
     SkASSERT(fn.getParameterCount() == 4 || fn.getParameterCount() == 6);
     if (fn.getParameterCount() == 6) {
@@ -357,6 +358,14 @@ static skvm::Color program_fn(skvm::Builder* p,
     push(inColor.g);
     push(inColor.b);
     push(inColor.a);
+#else
+    // half4 main() or half4 main(float2 local)
+    SkASSERT(fn.getParameterCount() == 0 || fn.getParameterCount() == 2);
+    if (fn.getParameterCount() == 2) {
+        push(local.x);
+        push(local.y);
+    }
+#endif
 
     for (int i = 0; i < fn.getLocalCount(); i++) {
         push(p->splat(0.0f));
@@ -591,11 +600,26 @@ static skvm::Color program_fn(skvm::Builder* p,
             } break;
 
             case Inst::kReturn: {
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
                 SkAssertResult(u8() == 0);
                 SkASSERT(ip == end);
+#else
+                SkAssertResult(u8() == 4);
+                // We'd like to assert that (ip == end) -> there is only one return, but ByteCode
+                // always includes a kReturn/0 at the end of each function, as a precaution.
+//                SkASSERT(ip == end);
+                SkASSERT(stack.size() >= 4);
+                skvm::F32 a = pop(),
+                          b = pop(),
+                          g = pop(),
+                          r = pop();
+                return { r, g, b, a };
+#endif
             } break;
         }
     }
+
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
     for (int i = 0; i < fn.getLocalCount(); i++) {
         pop();
     }
@@ -605,6 +629,10 @@ static skvm::Color program_fn(skvm::Builder* p,
               g = pop(),
               r = pop();
     return { r, g, b, a };
+#else
+    SkUNREACHABLE;
+    return {};
+#endif
 }
 
 static sk_sp<SkData> get_xformed_uniforms(const SkRuntimeEffect* effect,

src/effects/SkOverdrawColorFilter.cpp

@@ -11,6 +11,7 @@
 #include "include/private/SkColorData.h"
 
 sk_sp<SkColorFilter> SkOverdrawColorFilter::MakeWithSkColors(const SkColor colors[kNumColors]) {
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
     auto [effect, err] = SkRuntimeEffect::Make(SkString(R"(
         uniform half4 color0;
         uniform half4 color1;
@@ -28,13 +29,40 @@ sk_sp<SkColorFilter> SkOverdrawColorFilter::MakeWithSkColors(const SkColor color
                   : alpha < 4.5 ? color4 : color5;
         }
     )"));
+#else
+    auto [effect, err] = SkRuntimeEffect::Make(SkString(R"(
+        uniform half4 color0;
+        uniform half4 color1;
+        uniform half4 color2;
+        uniform half4 color3;
+        uniform half4 color4;
+        uniform half4 color5;
+        in shader input;
+
+        half4 main() {
+            half4 color = sample(input);
+            half alpha = 255.0 * color.a;
+            color = alpha < 0.5 ? color0
+                  : alpha < 1.5 ? color1
+                  : alpha < 2.5 ? color2
+                  : alpha < 3.5 ? color3
+                  : alpha < 4.5 ? color4 : color5;
+            return color;
+        }
+    )"));
+#endif
     if (effect) {
         auto data = SkData::MakeUninitialized(kNumColors * sizeof(SkPMColor4f));
         SkPMColor4f* premul = (SkPMColor4f*)data->writable_data();
         for (int i = 0; i < kNumColors; ++i) {
             premul[i] = SkColor4f::FromColor(colors[i]).premul();
         }
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
         return effect->makeColorFilter(std::move(data));
+#else
+        sk_sp<SkColorFilter> input = nullptr;
+        return effect->makeColorFilter(std::move(data), &input, 1);
+#endif
     }
     return nullptr;
 }

src/gpu/GrFragmentProcessor.h

@@ -189,6 +189,11 @@ public:
         return SkToBool(fFlags & kNetTransformHasPerspective_Flag);
     }
 
+    // True if emitted code returns the output color, rather than assigning it to sk_OutColor.
+    virtual bool usesExplicitReturn() const {
+        return false;
+    }
+
     // The SampleUsage describing how this FP is invoked by its parent using 'sample(matrix)'
     // This only reflects the immediate sampling from parent to this FP
     const SkSL::SampleUsage& sampleUsage() const {

src/gpu/effects/GrSkSLFP.h

@@ -45,6 +45,14 @@ public:
 
     std::unique_ptr<GrFragmentProcessor> clone() const override;
 
+    bool usesExplicitReturn() const override {
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
+        return false;
+#else
+        return true;
+#endif
+    }
+
 private:
     using ShaderErrorHandler = GrContextOptions::ShaderErrorHandler;
 

src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp

@@ -189,7 +189,13 @@ SkString GrGLSLFPFragmentBuilder::writeProcessorFunction(GrGLSLFragmentProcessor
 
     this->codeAppendf("half4 %s;\n", args.fOutputColor);
     fp->emitCode(args);
-    this->codeAppendf("return %s;\n", args.fOutputColor);
+    if (args.fFp.usesExplicitReturn()) {
+        // Some FPs explicitly return their output, so no need to do anything further
+        SkASSERT(SkStrContains(this->code().c_str(), "return"));
+    } else {
+        // Most FPs still just write their output to fOutputColor, so we need to inject the return
+        this->codeAppendf("return %s;\n", args.fOutputColor);
+    }
 
     SkString result;
     this->emitFunction(kHalf4_GrSLType, args.fFp.name(), paramCount, params,

src/gpu/glsl/GrGLSLProgramBuilder.cpp

@@ -147,8 +147,6 @@ void GrGLSLProgramBuilder::emitAndInstallFragProcs(SkString* color, SkString* co
     }
 }
 
-// TODO Processors cannot output zeros because an empty string is all 1s
-// the fix is to allow effects to take the SkString directly
 SkString GrGLSLProgramBuilder::emitFragProc(const GrFragmentProcessor& fp,
                                             GrGLSLFragmentProcessor& glslFP,
                                             int transformedCoordVarsIdx,
@@ -159,11 +157,6 @@ SkString GrGLSLProgramBuilder::emitFragProc(const GrFragmentProcessor& fp,
     AutoStageAdvance adv(this);
     this->nameExpression(&output, "output");
 
-    // Enclose custom code in a block to avoid namespace conflicts
-    SkString openBrace;
-    openBrace.printf("{ // Stage %d, %s\n", fStageIndex, fp.name());
-    fFS.codeAppend(openBrace.c_str());
-
     int samplerIdx = 0;
     for (auto [subFP, subGLSLFP] : GrGLSLFragmentProcessor::ParallelRange(fp, glslFP)) {
         if (auto* te = subFP.asTextureEffect()) {
@@ -188,37 +181,49 @@ SkString GrGLSLProgramBuilder::emitFragProc(const GrFragmentProcessor& fp,
                                            "_coords",
                                            coords);
 
-    if (fp.referencesSampleCoords()) {
+    if (fp.usesExplicitReturn()) {
-        // The fp's generated code expects a _coords variable, but we're at the root so _coords
+        // FPs that explicitly return their output color must be in a helper function
-        // is just the local coordinates produced by the primitive processor.
+        args.fInputColor = "_input";
-        SkASSERT(fp.usesVaryingCoordsDirectly());
+        args.fOutputColor = "_output";
+        auto name = fFS.writeProcessorFunction(&glslFP, args);
+        fFS.codeAppendf("%s = %s(%s);", output.c_str(), name.c_str(), input.c_str());
+    } else {
+        // Enclose custom code in a block to avoid namespace conflicts
+        fFS.codeAppendf("{ // Stage %d, %s\n", fStageIndex, fp.name());
 
-        const GrShaderVar& varying = coordVars[0];
+        if (fp.referencesSampleCoords()) {
-        switch(varying.getType()) {
+            // The fp's generated code expects a _coords variable, but we're at the root so _coords
-            case kFloat2_GrSLType:
+            // is just the local coordinates produced by the primitive processor.
-                fFS.codeAppendf("float2 %s = %s.xy;\n",
+            SkASSERT(fp.usesVaryingCoordsDirectly());
-                                args.fSampleCoord, varying.getName().c_str());
+
-                break;
+            const GrShaderVar& varying = coordVars[0];
-            case kFloat3_GrSLType:
+            switch(varying.getType()) {
-                fFS.codeAppendf("float2 %s = %s.xy / %s.z;\n",
+                case kFloat2_GrSLType:
-                                args.fSampleCoord,
+                    fFS.codeAppendf("float2 %s = %s.xy;\n",
-                                varying.getName().c_str(),
+                                    args.fSampleCoord, varying.getName().c_str());
-                                varying.getName().c_str());
+                    break;
-                break;
+                case kFloat3_GrSLType:
-            default:
+                    fFS.codeAppendf("float2 %s = %s.xy / %s.z;\n",
-                SkDEBUGFAILF("Unexpected type for varying: %d named %s\n",
+                                    args.fSampleCoord,
-                             (int) varying.getType(), varying.getName().c_str());
+                                    varying.getName().c_str(),
-                break;
+                                    varying.getName().c_str());
+                    break;
+                default:
+                    SkDEBUGFAILF("Unexpected type for varying: %d named %s\n",
+                                (int) varying.getType(), varying.getName().c_str());
+                    break;
+            }
         }
-    }
 
-    glslFP.emitCode(args);
+        glslFP.emitCode(args);
+
+        fFS.codeAppend("}");
+    }
 
     // We have to check that effects and the code they emit are consistent, ie if an effect
     // asks for dst color, then the emit code needs to follow suit
     SkDEBUGCODE(verify(fp);)
 
-    fFS.codeAppend("}");
     return output;
 }
 

src/sksl/SkSLIRGenerator.cpp

@@ -932,33 +932,53 @@ void IRGenerator::convertFunction(const ASTNode& f) {
         parameters.push_back(var);
     }
 
+    auto paramIsCoords = [&](int idx) {
+        return parameters[idx]->fType == *fContext.fFloat2_Type &&
+               parameters[idx]->fModifiers.fFlags == 0;
+    };
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
+    auto paramIsColor = [&](int idx) {
+        return parameters[idx]->fType == *fContext.fHalf4_Type &&
+               parameters[idx]->fModifiers.fFlags == (Modifiers::kIn_Flag | Modifiers::kOut_Flag);
+    };
+#endif
+
     if (funcData.fName == "main") {
         switch (fKind) {
             case Program::kPipelineStage_Kind: {
-                bool valid;
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
+                // void main(inout half4)  -or-  void main(float2, inout half4)
+                bool valid = (*returnType == *fContext.fVoid_Type);
                 switch (parameters.size()) {
                     case 2:
-                        valid = parameters[0]->fType == *fContext.fFloat2_Type &&
+                        valid &= paramIsCoords(0) && paramIsColor(1);
-                                parameters[0]->fModifiers.fFlags == 0 &&
-                                parameters[1]->fType == *fContext.fHalf4_Type &&
-                                parameters[1]->fModifiers.fFlags == (Modifiers::kIn_Flag |
-                                                                     Modifiers::kOut_Flag);
                         break;
                     case 1:
-                        valid = parameters[0]->fType == *fContext.fHalf4_Type &&
+                        valid &= paramIsColor(0);
-                                parameters[0]->fModifiers.fFlags == (Modifiers::kIn_Flag |
-                                                                     Modifiers::kOut_Flag);
                         break;
                     default:
-                        valid = false;
+                        valid &= false;
                 }
                 if (!valid) {
-                    fErrors.error(f.fOffset, "pipeline stage 'main' must be declared main(float2, "
+                    fErrors.error(f.fOffset, "pipeline stage 'main' must be declared "
-                                             "inout half4) or main(inout half4)");
+                                             "void main(inout half4) or "
+                                             "void main(float2, inout half4)");
                     return;
                 }
                 break;
-            }
+#else
+                // half4 main()  -or-  half4 main(float2)
+                bool valid = (*returnType == *fContext.fHalf4_Type) &&
+                             ((parameters.size() == 0) ||
+                              (parameters.size() == 1 && paramIsCoords(0)));
+                if (!valid) {
+                    fErrors.error(f.fOffset, "pipeline stage 'main' must be declared "
+                                             "half4 main() or half4 main(float2)");
+                    return;
+                }
+                break;
+#endif
+           }
             case Program::kFragmentProcessor_Kind: {
                 bool valid = parameters.size() <= 1;
                 if (parameters.size() == 1) {
@@ -1056,6 +1076,7 @@ void IRGenerator::convertFunction(const ASTNode& f) {
         std::shared_ptr<SymbolTable> old = fSymbolTable;
         AutoSymbolTable table(this);
         if (funcData.fName == "main" && fKind == Program::kPipelineStage_Kind) {
+#ifdef SK_USE_LEGACY_RUNTIME_EFFECT_SIGNATURE
             if (parameters.size() == 2) {
                 parameters[0]->fModifiers.fLayout.fBuiltin = SK_MAIN_COORDS_BUILTIN;
                 parameters[1]->fModifiers.fLayout.fBuiltin = SK_OUTCOLOR_BUILTIN;
@@ -1063,6 +1084,12 @@ void IRGenerator::convertFunction(const ASTNode& f) {
                 SkASSERT(parameters.size() == 1);
                 parameters[0]->fModifiers.fLayout.fBuiltin = SK_OUTCOLOR_BUILTIN;
             }
+#else
+            if (parameters.size() == 1) {
+                SkASSERT(paramIsCoords(0));
+                parameters[0]->fModifiers.fLayout.fBuiltin = SK_MAIN_COORDS_BUILTIN;
+            }
+#endif
         } else if (funcData.fName == "main" && fKind == Program::kFragmentProcessor_Kind) {
             if (parameters.size() == 1) {
                 parameters[0]->fModifiers.fLayout.fBuiltin = SK_MAIN_COORDS_BUILTIN;

tests/SkRuntimeEffectTest.cpp

@@ -21,7 +21,7 @@
 
 DEF_TEST(SkRuntimeEffectInvalid, r) {
     auto test = [r](const char* hdr, const char* body, const char* expected) {
-        SkString src = SkStringPrintf("%s void main(float2 p, inout half4 color) { %s }",
+        SkString src = SkStringPrintf("%s half4 main(float2 p) { %s return half4(0); }",
                                       hdr, body);
         auto[effect, errorText] = SkRuntimeEffect::Make(src);
         REPORTER_ASSERT(r, !effect);
@@ -52,7 +52,7 @@ DEF_TEST(SkRuntimeEffectInvalid, r) {
     // 'marker' is only permitted on float4x4 uniforms
     test("layout(marker=local_to_world) uniform float3x3 localToWorld;", "", "float4x4");
 
-    test("half missing();", "color.r = missing();", "undefined function");
+    test("float missing();", "p.x = missing();", "undefined function");
 
     // Shouldn't be possible to create an SkRuntimeEffect without "main"
     test("//", "", "main");
@@ -62,20 +62,20 @@ DEF_TEST(SkRuntimeEffectInvalid, r) {
          "shader child;",
          "must be global");
     test("in shader child; half4 helper(shader fp) { return sample(fp); }",
-         "color = helper(child);",
+         "half4 color = helper(child);",
          "parameter");
     test("in shader child; shader get_child() { return child; }",
-         "color = sample(get_child());",
+         "half4 color = sample(get_child());",
          "return");
     test("in shader child;",
-         "color = sample(shader(child));",
+         "half4 color = sample(shader(child));",
          "construct");
     test("in shader child1; in shader child2;",
-         "color = sample(p.x > 10 ? child1 : child2);",
+         "half4 color = sample(p.x > 10 ? child1 : child2);",
          "expression");
 
     // Errors that aren't caught until later in the compilation process (during optimize())
-    test("", "return; color.r = color.g;", "unreachable");
+    test("", "return half4(1);", "unreachable");
     test("half badFunc() { }", "", "without returning");
 }
 
@@ -92,12 +92,12 @@ DEF_TEST(SkRuntimeEffectInvalidColorFilters, r) {
 
     // Runtime effects that use sample coords or sk_FragCoord are valid shaders,
     // but not valid color filters
-    test("void main(float2 p, inout half4 color) { color.rg = half2(p); }");
+    test("half4 main(float2 p) { return half2(p).xy01; }");
-    test("void main(float2 p, inout half4 color) { color.rg = half2(sk_FragCoord.xy); }");
+    test("half4 main(float2 p) { return half2(sk_FragCoord.xy).xy01; }");
 
     // We also can't use layout(marker), which would give the runtime color filter CTM information
     test("layout(marker=ctm) uniform float4x4 ctm;"
-         "void main(float2 p, inout half4 color) { color.r = half(ctm[0][0]); }");
+         "half4 main(float2 p) { return half4(half(ctm[0][0]), 0, 0, 1); }");
 }
 
 class TestEffect {
@@ -106,7 +106,7 @@ public:
             : fReporter(r), fSurface(std::move(surface)) {}
 
     void build(const char* header, const char* body) {
-        SkString src = SkStringPrintf("%s void main(float2 p, inout half4 color) { %s }",
+        SkString src = SkStringPrintf("%s half4 main(float2 p) { %s }",
                                       header, body);
         auto[effect, errorText] = SkRuntimeEffect::Make(src);
         if (!effect) {
@@ -200,12 +200,12 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext
     using float4 = std::array<float, 4>;
 
     // Local coords
-    effect.build("", "color = half4(half2(p - 0.5), 0, 1);");
+    effect.build("", "return half4(half2(p - 0.5), 0, 1);");
     effect.test(0xFF000000, 0xFF0000FF, 0xFF00FF00, 0xFF00FFFF);
 
     // Use of a simple uniform. (Draw twice with two values to ensure it's updated).
     effect.build("uniform float4 gColor;",
-                 "color = half4(gColor);");
+                 "return half4(gColor);");
     effect.uniform("gColor") = float4{ 0.0f, 0.25f, 0.75f, 1.0f };
     effect.test(0xFFBF4000);
     effect.uniform("gColor") = float4{ 1.0f, 0.0f, 0.0f, 0.498f };
@@ -214,7 +214,7 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext
     // Test sk_FragCoord (device coords). Rotate the canvas to be sure we're seeing device coords.
     // Since the surface is 2x2, we should see (0,0), (1,0), (0,1), (1,1). Multiply by 0.498 to
     // make sure we're not saturating unexpectedly.
-    effect.build("", "color = half4(0.498 * (half2(sk_FragCoord.xy) - 0.5), 0, 1);");
+    effect.build("", "return half4(0.498 * (half2(sk_FragCoord.xy) - 0.5), 0, 1);");
     effect.test(0xFF000000, 0xFF00007F, 0xFF007F00, 0xFF007F7F,
                 [](SkCanvas* canvas, SkPaint*) { canvas->rotate(45.0f); });
 
@@ -224,7 +224,7 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext
 
     // Sampling a null child should return the paint color
     effect.build("in shader child;",
-                 "color = sample(child);");
+                 "return sample(child);");
     effect.child("child") = nullptr;
     effect.test(0xFF00FFFF,
                 [](SkCanvas*, SkPaint* paint) { paint->setColor4f({1.0f, 1.0f, 0.0f, 1.0f}); });
@@ -233,19 +233,19 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext
 
     // Sampling a simple child at our coordinates (implicitly)
     effect.build("in shader child;",
-                 "color = sample(child);");
+                 "return sample(child);");
     effect.child("child") = rgbwShader;
     effect.test(0xFF0000FF, 0xFF00FF00, 0xFFFF0000, 0xFFFFFFFF);
 
     // Sampling with explicit coordinates (reflecting about the diagonal)
     effect.build("in shader child;",
-                 "color = sample(child, p.yx);");
+                 "return sample(child, p.yx);");
     effect.child("child") = rgbwShader;
     effect.test(0xFF0000FF, 0xFFFF0000, 0xFF00FF00, 0xFFFFFFFF);
 
     // Sampling with a matrix (again, reflecting about the diagonal)
     effect.build("in shader child;",
-                 "color = sample(child, float3x3(0, 1, 0, 1, 0, 0, 0, 0, 1));");
+                 "return sample(child, float3x3(0, 1, 0, 1, 0, 0, 0, 0, 1));");
     effect.child("child") = rgbwShader;
     effect.test(0xFF0000FF, 0xFFFF0000, 0xFF00FF00, 0xFFFFFFFF);
 
@@ -256,7 +256,7 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext
     // Test case for inlining in the pipeline-stage and fragment-shader passes (skbug.com/10526):
     effect.build("float2 helper(float2 x) { return x + 1; }",
                  "float2 v = helper(p);"
-                 "color = half4(half2(v), 0, 1);");
+                 "return half4(half2(v), 0, 1);");
     effect.test(0xFF00FFFF);
 }
 

tools/viewer/SkSLSlide.cpp

@@ -40,8 +40,8 @@ SkSLSlide::SkSLSlide() {
 
         "in shader child;\n"
         "\n"
-        "void main(float2 p, inout half4 color) {\n"
+        "half4 main(float2 p) {\n"
-        "    color = sample(child, p);\n"
+        "    return sample(child, p);\n"
         "}\n";
 
     fCodeIsDirty = true;