Better first-class shader & color filter support in runtime effects

This does a few things, because they're all intertwined:

1) SkRuntimeEffect's API now includes details about children (which Skia
   stage was declared, not just the name). The factories verify that the
   declared types in the SkSL match up with the C++ types being passed.
   Today, we still only support adding children of the same type, so the
   checks are simple. Once we allow mixing types, we'll be testing the
   declared type against the actual C++ type supplied for each slot.
2) Adds sample variants that supply the input color to the child. This
   is now the only way to invoke a colorFilter child. Internally, we
   support passing a color when invoking a child shader, but I'm not
   exposing that. It's not clearly part of the semantics of the Skia
   pipeline, and is almost never useful. It also exposes users to
   several inconsistencies (skbug.com/11942).
3) Because of #2, it's possible that we can't compute a reusable program
   to filter individual colors. In that case, we don't set the constant
   output for constant input optimization, and filterColor4f falls back
   to the slower base-class implementation.

Bug: skia:11813 skia:11942
Change-Id: I06c41e1b35056e486f3163a72acf6b9535d7fed4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401917
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/fuzz/oss_fuzz/FuzzSKSL2Pipeline.cpp b/fuzz/oss_fuzz/FuzzSKSL2Pipeline.cpp
index bf79ca6..e96743d 100644
--- a/fuzz/oss_fuzz/FuzzSKSL2Pipeline.cpp
+++ b/fuzz/oss_fuzz/FuzzSKSL2Pipeline.cpp
@@ -37,9 +37,16 @@
         void defineStruct(const char* /*definition*/) override {}
         void declareGlobal(const char* /*declaration*/) override {}
 
-        String sampleChild(int index, String coords) override {
-            return SkSL::String::printf("sample(%d%s%s)", index, coords.empty() ? "" : ", ",
-                                        coords.c_str());
+        String sampleChild(int index, String coords, String color) override {
+            String result = "sample(" + SkSL::to_string(index);
+            if (!coords.empty()) {
+                result += ", " + coords;
+            }
+            if (!color.empty()) {
+                result += ", " + color;
+            }
+            result += ")";
+            return result;
         }
     };
 
diff --git a/gm/composecolorfilter.cpp b/gm/composecolorfilter.cpp
new file mode 100644
index 0000000..50460a8
--- /dev/null
+++ b/gm/composecolorfilter.cpp
@@ -0,0 +1,101 @@
+/*
+ * Copyright 2021 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "gm/gm.h"
+#include "include/core/SkCanvas.h"
+#include "include/core/SkColor.h"
+#include "include/core/SkColorFilter.h"
+#include "include/core/SkImage.h"
+#include "include/core/SkMatrix.h"
+#include "include/core/SkPaint.h"
+#include "include/core/SkPoint.h"
+#include "include/core/SkRect.h"
+#include "include/core/SkRefCnt.h"
+#include "include/core/SkScalar.h"
+#include "include/core/SkShader.h"
+#include "include/core/SkSize.h"
+#include "include/core/SkString.h"
+#include "include/core/SkTileMode.h"
+#include "include/core/SkTypes.h"
+#include "include/effects/SkGradientShader.h"
+#include "include/effects/SkLumaColorFilter.h"
+#include "include/effects/SkRuntimeEffect.h"
+#include "tools/Resources.h"
+
+#include <math.h>
+
+// A tint filter maps colors to a given range (gradient), based on the input luminance:
+//
+//   c' = lerp(lo, hi, luma(c))
+static sk_sp<SkColorFilter> MakeTintColorFilter(SkColor lo, SkColor hi, bool useSkSL) {
+    const auto r_lo = SkColorGetR(lo),
+    g_lo = SkColorGetG(lo),
+    b_lo = SkColorGetB(lo),
+    a_lo = SkColorGetA(lo),
+    r_hi = SkColorGetR(hi),
+    g_hi = SkColorGetG(hi),
+    b_hi = SkColorGetB(hi),
+    a_hi = SkColorGetA(hi);
+
+    // We map component-wise:
+    //
+    //   r' = lo.r + (hi.r - lo.r) * luma
+    //   g' = lo.g + (hi.g - lo.g) * luma
+    //   b' = lo.b + (hi.b - lo.b) * luma
+    //   a' = lo.a + (hi.a - lo.a) * luma
+    //
+    // The input luminance is stored in the alpha channel
+    // (and RGB are cleared -- see SkLumaColorFilter). Thus:
+    const float tint_matrix[] = {
+        0, 0, 0, (r_hi - r_lo) / 255.0f, SkIntToScalar(r_lo) / 255.0f,
+        0, 0, 0, (g_hi - g_lo) / 255.0f, SkIntToScalar(g_lo) / 255.0f,
+        0, 0, 0, (b_hi - b_lo) / 255.0f, SkIntToScalar(b_lo) / 255.0f,
+        0, 0, 0, (a_hi - a_lo) / 255.0f, SkIntToScalar(a_lo) / 255.0f,
+    };
+
+    sk_sp<SkColorFilter> inner = SkLumaColorFilter::Make(),
+                         outer = SkColorFilters::Matrix(tint_matrix);
+
+    // Prove that we can implement compose-color-filter using runtime effects
+    if (useSkSL) {
+        auto [effect, error] = SkRuntimeEffect::MakeForColorFilter(SkString(R"(
+            uniform colorFilter inner;
+            uniform colorFilter outer;
+            half4 main(half4 c) { return sample(outer, sample(inner, c)); }
+        )"));
+        SkASSERT(effect);
+        sk_sp<SkColorFilter> children[] = { inner, outer };
+        return effect->makeColorFilter(nullptr, children, SK_ARRAY_COUNT(children));
+    } else {
+        return outer->makeComposed(inner);
+    }
+}
+
+DEF_SIMPLE_GM(composeCF, canvas, 200, 200) {
+    // This GM draws a simple color-filter network, using the existing "makeComposed" API, and also
+    // using a runtime color filter that does the same thing.
+    SkPaint paint;
+    const SkColor gradient_colors[] = {SK_ColorRED, SK_ColorGREEN, SK_ColorBLUE, SK_ColorRED};
+    paint.setShader(SkGradientShader::MakeSweep(
+            50, 50, gradient_colors, nullptr, SK_ARRAY_COUNT(gradient_colors)));
+
+    canvas->save();
+    for (bool useSkSL : {false, true}) {
+        auto cf0 = MakeTintColorFilter(0xff300000, 0xffa00000, useSkSL);  // red tint
+        auto cf1 = MakeTintColorFilter(0xff003000, 0xff00a000, useSkSL);  // green tint
+
+        paint.setColorFilter(cf0);
+        canvas->drawRect({0, 0, 100, 100}, paint);
+        canvas->translate(100, 0);
+
+        paint.setColorFilter(cf1);
+        canvas->drawRect({0, 0, 100, 100}, paint);
+
+        canvas->restore();
+        canvas->translate(0, 100);
+    }
+}
diff --git a/gn/gm.gni b/gn/gm.gni
index b325b51..9004832 100644
--- a/gn/gm.gni
+++ b/gn/gm.gni
@@ -98,6 +98,7 @@
   "$_gm/complexclip3.cpp",
   "$_gm/complexclip4.cpp",
   "$_gm/complexclip_blur_tiled.cpp",
+  "$_gm/composecolorfilter.cpp",
   "$_gm/composeshader.cpp",
   "$_gm/compositor_quads.cpp",
   "$_gm/compressed_textures.cpp",
diff --git a/include/effects/SkRuntimeEffect.h b/include/effects/SkRuntimeEffect.h
index 9c235da..1be9ad2 100644
--- a/include/effects/SkRuntimeEffect.h
+++ b/include/effects/SkRuntimeEffect.h
@@ -72,6 +72,17 @@
         size_t sizeInBytes() const;
     };
 
+    struct Child {
+        enum class Type {
+            kShader,
+            kColorFilter,
+        };
+
+        SkString name;
+        Type     type;
+        int      index;
+    };
+
     struct Options {
         // 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.)
@@ -161,13 +172,13 @@
     size_t uniformSize() const;
 
     ConstIterable<Uniform> uniforms() const { return ConstIterable<Uniform>(fUniforms); }
-    ConstIterable<SkString> children() const { return ConstIterable<SkString>(fChildren); }
+    ConstIterable<Child> children() const { return ConstIterable<Child>(fChildren); }
 
     // Returns pointer to the named uniform variable's description, or nullptr if not found
     const Uniform* findUniform(const char* name) const;
 
-    // Returns index of the named child, or -1 if not found
-    int findChild(const char* name) const;
+    // Returns pointer to the named child's description, or nullptr if not found
+    const Child* findChild(const char* name) const;
 
     static void RegisterFlattenables();
     ~SkRuntimeEffect() override;
@@ -191,7 +202,7 @@
                     const Options& options,
                     const SkSL::FunctionDefinition& main,
                     std::vector<Uniform>&& uniforms,
-                    std::vector<SkString>&& children,
+                    std::vector<Child>&& children,
                     std::vector<SkSL::SampleUsage>&& sampleUsages,
                     uint32_t flags);
 
@@ -208,9 +219,10 @@
     bool allowColorFilter() const { return (fFlags & kAllowColorFilter_Flag); }
 
     struct FilterColorInfo {
-        const skvm::Program& program;
+        const skvm::Program* program;         // May be nullptr if it's not possible to compute
         bool                 alphaUnchanged;
     };
+    void initFilterColorInfo();
     FilterColorInfo getFilterColorInfo();
 
 #if SK_SUPPORT_GPU
@@ -227,12 +239,11 @@
     std::unique_ptr<SkSL::Program> fBaseProgram;
     const SkSL::FunctionDefinition& fMain;
     std::vector<Uniform> fUniforms;
-    std::vector<SkString> fChildren;
+    std::vector<Child> fChildren;
     std::vector<SkSL::SampleUsage> fSampleUsages;
 
-    SkOnce fColorFilterProgramOnce;
     std::unique_ptr<skvm::Program> fColorFilterProgram;
-    bool fColorFilterProgramLeavesAlphaUnchanged;
+    bool fColorFilterProgramLeavesAlphaUnchanged = false;
 
     uint32_t fFlags;  // Flags
 };
@@ -296,16 +307,18 @@
 
     struct BuilderChild {
         template <typename C> BuilderChild& operator=(C&& val) {
-            if (fIndex < 0) {
+            // TODO(skbug:11813): Validate that the type of val lines up with the type of the child
+            // (SkShader vs. SkColorFilter).
+            if (!fChild) {
                 SkDEBUGFAIL("Assigning to missing child");
             } else {
-                fOwner->fChildren[fIndex] = std::forward<C>(val);
+                fOwner->fChildren[fChild->index] = std::forward<C>(val);
             }
             return *this;
         }
 
-        SkRuntimeEffectBuilder* fOwner;
-        int                     fIndex;  // -1 if the child was not found
+        SkRuntimeEffectBuilder*       fOwner;
+        const SkRuntimeEffect::Child* fChild;  // nullptr if the child was not found
     };
 
     const SkRuntimeEffect* effect() const { return fEffect.get(); }
diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index 3633e09..87f4f27 100644
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -464,12 +464,11 @@
 
     sk_sp<SkRuntimeEffect> effect = SkMakeCachedRuntimeEffect(
         SkRuntimeEffect::MakeForColorFilter,
-        "uniform shader cf0;"
-        "uniform shader cf1;"
+        "uniform colorFilter cf0;"
+        "uniform colorFilter cf1;"
         "uniform half   weight;"
         "half4 main(half4 color) {"
-            "float2 moot_xy;"
-            "return mix(sample(cf0, moot_xy), sample(cf1, moot_xy), weight);"
+            "return mix(sample(cf0, color), sample(cf1, color), weight);"
         "}"
     );
     SkASSERT(effect);
diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp
index 5fd4c88..4a72713 100644
--- a/src/core/SkRuntimeEffect.cpp
+++ b/src/core/SkRuntimeEffect.cpp
@@ -118,6 +118,14 @@
     return false;
 }
 
+static SkRuntimeEffect::Child::Type child_type(const SkSL::Type& type) {
+    switch (type.typeKind()) {
+        case SkSL::Type::TypeKind::kColorFilter: return SkRuntimeEffect::Child::Type::kColorFilter;
+        case SkSL::Type::TypeKind::kShader:      return SkRuntimeEffect::Child::Type::kShader;
+        default: SkUNREACHABLE;
+    }
+}
+
 // TODO: Many errors aren't caught until we process the generated Program here. Catching those
 // in the IR generator would provide better errors messages (with locations).
 #define RETURN_FAILURE(...) return Result{nullptr, SkStringPrintf(__VA_ARGS__)}
@@ -196,7 +204,7 @@
 
     size_t offset = 0;
     std::vector<Uniform> uniforms;
-    std::vector<SkString> children;
+    std::vector<Child> children;
     std::vector<SkSL::SampleUsage> sampleUsages;
     const SkSL::Context& ctx(compiler->context());
 
@@ -212,7 +220,11 @@
 
             // Child effects that can be sampled ('shader' or 'colorFilter')
             if (varType.isEffectChild()) {
-                children.push_back(var.name());
+                Child c;
+                c.name  = var.name();
+                c.type  = child_type(varType);
+                c.index = children.size();
+                children.push_back(c);
                 sampleUsages.push_back(SkSL::Analysis::GetSampleUsage(
                         *program, var, sampleCoordsUsage.fWrite != 0));
             }
@@ -354,7 +366,7 @@
                                  const Options& options,
                                  const SkSL::FunctionDefinition& main,
                                  std::vector<Uniform>&& uniforms,
-                                 std::vector<SkString>&& children,
+                                 std::vector<Child>&& children,
                                  std::vector<SkSL::SampleUsage>&& sampleUsages,
                                  uint32_t flags)
         : fHash(SkGoodHash()(sksl))
@@ -378,6 +390,8 @@
                       sizeof(options.forceNoInline), fHash);
     fHash = SkOpts::hash_fn(&options.enforceES2Restrictions,
                       sizeof(options.enforceES2Restrictions), fHash);
+
+    this->initFilterColorInfo();
 }
 
 SkRuntimeEffect::~SkRuntimeEffect() = default;
@@ -393,70 +407,85 @@
     return iter == fUniforms.end() ? nullptr : &(*iter);
 }
 
-int SkRuntimeEffect::findChild(const char* name) const {
+const SkRuntimeEffect::Child* SkRuntimeEffect::findChild(const char* name) const {
     auto iter = std::find_if(fChildren.begin(), fChildren.end(),
-                             [name](const SkString& s) { return s.equals(name); });
-    return iter == fChildren.end() ? -1 : static_cast<int>(iter - fChildren.begin());
+                             [name](const Child& c) { return c.name.equals(name); });
+    return iter == fChildren.end() ? nullptr : &(*iter);
+}
+
+void SkRuntimeEffect::initFilterColorInfo() {
+    // Runtime effects are often long lived & cached. So: build and save a program that can
+    // filter a single color, without baking in anything tied to a particular instance
+    // (uniforms or children). This isn't possible (or needed) for shaders.
+    if (!this->allowColorFilter()) {
+        return;
+    }
+
+    // We allocate a uniform color for the input color, and for each child in the SkSL. When we run
+    // this program later, these uniform values are replaced with either the results of the child,
+    // or the input color (if the child is nullptr). These Uniform ids are loads from the *first*
+    // arg ptr.
+    //
+    // This scheme only works if every child is sampled using the original input color. If we detect
+    // a sampleChild call where a different color is being supplied, we bail out, and the returned
+    // info will have a null program. (Callers will need to fall back to another implementation.)
+    skvm::Builder p;
+    skvm::Uniforms childColorUniforms{p.uniform(), 0};
+    skvm::Color inputColor = p.uniformColor(/*placeholder*/ SkColors::kWhite, &childColorUniforms);
+    std::vector<skvm::Color> childColors;
+    for (size_t i = 0; i < fChildren.size(); ++i) {
+        childColors.push_back(
+                p.uniformColor(/*placeholder*/ SkColors::kWhite, &childColorUniforms));
+    }
+    bool allSampleCallsPassInputColor = true;
+    auto sampleChild = [&](int ix, skvm::Coord, skvm::Color color) {
+        if (color.r.id != inputColor.r.id ||
+            color.g.id != inputColor.g.id ||
+            color.b.id != inputColor.b.id ||
+            color.a.id != inputColor.a.id) {
+            allSampleCallsPassInputColor = false;
+        }
+        return childColors[ix];
+    };
+
+    // For SkSL uniforms, we reserve space and allocate skvm Uniform ids for each one. When we run
+    // the program, these ids will be loads from the *second* arg ptr, the uniform data of the
+    // specific color filter instance.
+    skvm::Uniforms skslUniforms{p.uniform(), 0};
+    const size_t uniformCount = this->uniformSize() / 4;
+    std::vector<skvm::Val> uniform;
+    uniform.reserve(uniformCount);
+    for (size_t i = 0; i < uniformCount; i++) {
+        uniform.push_back(p.uniform32(skslUniforms.push(/*placeholder*/ 0)).id);
+    }
+
+    // Emit the skvm instructions for the SkSL
+    skvm::Coord zeroCoord = {p.splat(0.0f), p.splat(0.0f)};
+    skvm::Color result = SkSL::ProgramToSkVM(*fBaseProgram,
+                                             fMain,
+                                             &p,
+                                             uniform,
+                                             /*device=*/zeroCoord,
+                                             /*local=*/zeroCoord,
+                                             inputColor,
+                                             sampleChild);
+
+    // Then store the result to the *third* arg ptr
+    p.store({skvm::PixelFormat::FLOAT, 32, 32, 32, 32, 0, 32, 64, 96}, p.arg(16), result);
+
+    // This is conservative. If a filter gets the input color by sampling a null child, we'll
+    // return an (acceptable) false negative. All internal runtime color filters should work.
+    fColorFilterProgramLeavesAlphaUnchanged = (inputColor.a.id == result.a.id);
+
+    // We'll use this program to filter one color at a time, don't bother with jit
+    fColorFilterProgram = allSampleCallsPassInputColor
+                                  ? std::make_unique<skvm::Program>(
+                                            p.done(/*debug_name=*/nullptr, /*allow_jit=*/false))
+                                  : nullptr;
 }
 
 SkRuntimeEffect::FilterColorInfo SkRuntimeEffect::getFilterColorInfo() {
-    SkASSERT(this->allowColorFilter());
-
-    fColorFilterProgramOnce([&] {
-        // Runtime effects are often long lived & cached. So: build and save a program that can
-        // filter a single color, without baking in anything tied to a particular instance
-        // (uniforms or children).
-        skvm::Builder p;
-
-        // We allocate a uniform color for the input color, and for each child in the SkSL.
-        // When we run this program later, these uniform values are replaced with either the
-        // results of the child, or the input color (if the child is nullptr). These Uniform ids
-        // are loads from the *first* arg ptr.
-        skvm::Uniforms childColorUniforms{p.uniform(), 0};
-        skvm::Color inputColor =
-                p.uniformColor(/*placeholder*/ SkColors::kWhite, &childColorUniforms);
-        std::vector<skvm::Color> childColors;
-        for (size_t i = 0; i < fChildren.size(); ++i) {
-            childColors.push_back(
-                    p.uniformColor(/*placeholder*/ SkColors::kWhite, &childColorUniforms));
-        }
-        auto sampleChild = [&](int ix, skvm::Coord) { return childColors[ix]; };
-
-        // For SkSL uniforms, we reserve space and allocate skvm Uniform ids for each one.
-        // When we run the program, these ids will be loads from the *second* arg ptr, the
-        // uniform data of the specific color filter instance.
-        skvm::Uniforms skslUniforms{p.uniform(), 0};
-        const size_t uniformCount = this->uniformSize() / 4;
-        std::vector<skvm::Val> uniform;
-        uniform.reserve(uniformCount);
-        for (size_t i = 0; i < uniformCount; i++) {
-            uniform.push_back(p.uniform32(skslUniforms.push(/*placeholder*/ 0)).id);
-        }
-
-        // Emit the skvm instructions for the SkSL
-        skvm::Coord zeroCoord = { p.splat(0.0f), p.splat(0.0f) };
-        skvm::Color result = SkSL::ProgramToSkVM(*fBaseProgram,
-                                                 fMain,
-                                                 &p,
-                                                 uniform,
-                                                 /*device=*/zeroCoord,
-                                                 /*local=*/zeroCoord,
-                                                 inputColor,
-                                                 sampleChild);
-
-        // Then store the result to the *third* arg ptr
-        p.store({skvm::PixelFormat::FLOAT, 32,32,32,32, 0,32,64,96}, p.arg(16), result);
-
-        // This is conservative. If a filter gets the input color by sampling a null child, we'll
-        // return an (acceptable) false negative. All internal runtime color filters should work.
-        fColorFilterProgramLeavesAlphaUnchanged = (inputColor.a.id == result.a.id);
-
-        // We'll use this program to filter one color at a time, don't bother with jit
-        fColorFilterProgram = std::make_unique<skvm::Program>(
-                p.done(/*debug_name=*/nullptr, /*allow_jit=*/false));
-    });
-
-    return {*fColorFilterProgram, fColorFilterProgramLeavesAlphaUnchanged};
+    return {fColorFilterProgram.get(), fColorFilterProgramLeavesAlphaUnchanged};
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
@@ -561,11 +590,11 @@
         // something. (Uninitialized values can trigger asserts in skvm::Builder).
         skvm::Coord zeroCoord = { p->splat(0.0f), p->splat(0.0f) };
 
-        auto sampleChild = [&](int ix, skvm::Coord /*coord*/) {
+        auto sampleChild = [&](int ix, skvm::Coord /*coord*/, skvm::Color color) {
             if (fChildren[ix]) {
-                return as_CFB(fChildren[ix])->program(p, c, dstCS, uniforms, alloc);
+                return as_CFB(fChildren[ix])->program(p, color, dstCS, uniforms, alloc);
             } else {
-                return c;
+                return color;
             }
         };
 
@@ -584,7 +613,12 @@
 
     SkPMColor4f onFilterColor4f(const SkPMColor4f& color, SkColorSpace* dstCS) const override {
         // Get the generic program for filtering a single color
-        const skvm::Program& program = fEffect->getFilterColorInfo().program;
+        const skvm::Program* program = fEffect->getFilterColorInfo().program;
+        if (!program) {
+            // We were unable to build a cached (per-effect) program. Use the base-class fallback,
+            // which builds a program for the specific filter instance.
+            return SkColorFilterBase::onFilterColor4f(color, dstCS);
+        }
 
         // Get our specific uniform values
         sk_sp<SkData> inputs = get_xformed_uniforms(fEffect.get(), fUniforms, dstCS);
@@ -601,7 +635,7 @@
         }
 
         SkPMColor4f result;
-        program.eval(1, inputColors.begin(), inputs->data(), result.vec());
+        program->eval(1, inputColors.begin(), inputs->data(), result.vec());
         return result;
     }
 
@@ -678,9 +712,14 @@
                 get_xformed_uniforms(fEffect.get(), fUniforms, args.fDstColorInfo->colorSpace());
         SkASSERT(uniforms);
 
+        // If we sample children with explicit colors, this may not be true.
+        // TODO: Determine this via analysis?
+        GrFPArgs childArgs = args;
+        childArgs.fInputColorIsOpaque = false;
+
         auto fp = GrSkSLFP::Make(fEffect, "runtime_shader", std::move(uniforms));
         for (const auto& child : fChildren) {
-            auto childFP = child ? as_SB(child)->asFragmentProcessor(args) : nullptr;
+            auto childFP = child ? as_SB(child)->asFragmentProcessor(childArgs) : nullptr;
             fp->addChild(std::move(childFP));
         }
         std::unique_ptr<GrFragmentProcessor> result = std::move(fp);
@@ -723,14 +762,14 @@
         }
         local = SkShaderBase::ApplyMatrix(p,inv,local,uniforms);
 
-        auto sampleChild = [&](int ix, skvm::Coord coord) {
+        auto sampleChild = [&](int ix, skvm::Coord coord, skvm::Color color) {
             if (fChildren[ix]) {
                 SkOverrideDeviceMatrixProvider mats{matrices, SkMatrix::I()};
-                return as_SB(fChildren[ix])->program(p, device, coord, paint,
+                return as_SB(fChildren[ix])->program(p, device, coord, color,
                                                      mats, nullptr, dst,
                                                      uniforms, alloc);
             } else {
-                return paint;
+                return color;
             }
         };
 
@@ -850,10 +889,22 @@
     if (!uniforms) {
         uniforms = SkData::MakeEmpty();
     }
+    // Verify that all child objects are shaders (to match the C++ types here).
+    // TODO(skia:11813) When we support shader and colorFilter children (with different samplng
+    // semantics), the 'children' parameter will contain both types, so this will be more complex.
+    if (!std::all_of(fChildren.begin(), fChildren.end(), [](const Child& c) {
+            return c.type == Child::Type::kShader;
+        })) {
+        return nullptr;
+    }
     return uniforms->size() == this->uniformSize() && childCount == fChildren.size()
-        ? sk_sp<SkShader>(new SkRTShader(sk_ref_sp(this), std::move(uniforms), localMatrix,
-                                         children, childCount, isOpaque))
-        : nullptr;
+                   ? sk_sp<SkShader>(new SkRTShader(sk_ref_sp(this),
+                                                    std::move(uniforms),
+                                                    localMatrix,
+                                                    children,
+                                                    childCount,
+                                                    isOpaque))
+                   : nullptr;
 }
 
 sk_sp<SkImage> SkRuntimeEffect::makeImage(GrRecordingContext* recordingContext,
@@ -943,10 +994,18 @@
     if (!uniforms) {
         uniforms = SkData::MakeEmpty();
     }
+    // Verify that all child objects are color filters (to match the C++ types here).
+    // TODO(skia:11813) When we support shader and colorFilter children (with different samplng
+    // semantics), the 'children' parameter will contain both types, so this will be more complex.
+    if (!std::all_of(fChildren.begin(), fChildren.end(), [](const Child& c) {
+            return c.type == Child::Type::kColorFilter;
+        })) {
+        return nullptr;
+    }
     return uniforms->size() == this->uniformSize() && childCount == fChildren.size()
-        ? sk_sp<SkColorFilter>(new SkRuntimeColorFilter(sk_ref_sp(this), std::move(uniforms),
-                                                        children, childCount))
-        : nullptr;
+                   ? sk_sp<SkColorFilter>(new SkRuntimeColorFilter(
+                             sk_ref_sp(this), std::move(uniforms), children, childCount))
+                   : nullptr;
 }
 
 sk_sp<SkColorFilter> SkRuntimeEffect::makeColorFilter(sk_sp<SkData> uniforms) const {
diff --git a/src/gpu/effects/GrSkSLFP.cpp b/src/gpu/effects/GrSkSLFP.cpp
index d78ef7e..8a353aa 100644
--- a/src/gpu/effects/GrSkSLFP.cpp
+++ b/src/gpu/effects/GrSkSLFP.cpp
@@ -95,7 +95,7 @@
                 fArgs.fFragBuilder->definitionAppend(declaration);
             }
 
-            String sampleChild(int index, String coords) override {
+            String sampleChild(int index, String coords, String color) override {
                 // If the child was sampled using the coords passed to main (and they are never
                 // modified), then we will have marked the child as PassThrough. The code generator
                 // doesn't know that, and still supplies coords. Inside invokeChild, we assert that
@@ -112,7 +112,11 @@
                 if (child && !child->isSampledWithExplicitCoords()) {
                     coords.clear();
                 }
-                return String(fSelf->invokeChild(index, fInputColor, fArgs, coords).c_str());
+                return String(fSelf->invokeChild(index,
+                                                 color.empty() ? fInputColor : color.c_str(),
+                                                 fArgs,
+                                                 coords)
+                                      .c_str());
             }
 
             GrGLSLSkSLFP*        fSelf;
@@ -186,12 +190,11 @@
     return std::unique_ptr<GrSkSLFP>(new GrSkSLFP(std::move(effect), name, std::move(uniforms)));
 }
 
-GrSkSLFP::GrSkSLFP(sk_sp<SkRuntimeEffect> effect,
-                   const char* name,
-                   sk_sp<SkData> uniforms)
+GrSkSLFP::GrSkSLFP(sk_sp<SkRuntimeEffect> effect, const char* name, sk_sp<SkData> uniforms)
         : INHERITED(kGrSkSLFP_ClassID,
-                    effect->allowColorFilter() ? kConstantOutputForConstantInput_OptimizationFlag
-                                               : kNone_OptimizationFlags)
+                    effect->getFilterColorInfo().program
+                            ? kConstantOutputForConstantInput_OptimizationFlag
+                            : kNone_OptimizationFlags)
         , fEffect(std::move(effect))
         , fName(name)
         , fUniforms(std::move(uniforms)) {
@@ -245,7 +248,8 @@
 }
 
 SkPMColor4f GrSkSLFP::constantOutputForConstantInput(const SkPMColor4f& inputColor) const {
-    const skvm::Program& program = fEffect->getFilterColorInfo().program;
+    const skvm::Program* program = fEffect->getFilterColorInfo().program;
+    SkASSERT(program);
 
     SkSTArray<3, SkPMColor4f, true> childColors;
     childColors.push_back(inputColor);
@@ -254,7 +258,7 @@
     }
 
     SkPMColor4f result;
-    program.eval(1, childColors.begin(), fUniforms->data(), result.vec());
+    program->eval(1, childColors.begin(), fUniforms->data(), result.vec());
     return result;
 }
 
diff --git a/src/sksl/SkSLMain.cpp b/src/sksl/SkSLMain.cpp
index e8b3620..5c71d7b 100644
--- a/src/sksl/SkSLMain.cpp
+++ b/src/sksl/SkSLMain.cpp
@@ -439,14 +439,19 @@
                             fOutput += declaration;
                         }
 
-                        String sampleChild(int index, String coords) override {
-                            return String::printf("sample(child_%d%s%s)",
-                                                  index,
-                                                  coords.empty() ? "" : ", ",
-                                                  coords.c_str());
+                        String sampleChild(int index, String coords, String color) override {
+                            String result = "sample(child_" + SkSL::to_string(index);
+                            if (!coords.empty()) {
+                                result += ", " + coords;
+                            }
+                            if (!color.empty()) {
+                                result += ", " + color;
+                            }
+                            result += ")";
+                            return result;
                         }
 
-                        String              fOutput;
+                        String fOutput;
                     };
                     Callbacks callbacks;
                     SkSL::PipelineStage::ConvertProgram(program, "_coords", "_inColor", &callbacks);
diff --git a/src/sksl/codegen/SkSLPipelineStageCodeGenerator.cpp b/src/sksl/codegen/SkSLPipelineStageCodeGenerator.cpp
index 979d75d..e555c4f 100644
--- a/src/sksl/codegen/SkSLPipelineStageCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLPipelineStageCodeGenerator.cpp
@@ -144,16 +144,17 @@
     const FunctionDeclaration& function = c.function();
     const ExpressionArray& arguments = c.arguments();
     if (function.isBuiltin() && function.name() == "sample") {
-        SkASSERT(arguments.size() <= 2);
-        SkASSERT(arguments[0]->type().isEffectChild());
-        SkASSERT(arguments[0]->is<VariableReference>());
+        SkASSERT(arguments.size() == 2);
+        const Expression* child = arguments[0].get();
+        SkASSERT(child->type().isEffectChild());
+        SkASSERT(child->is<VariableReference>());
         int index = 0;
         bool found = false;
         for (const ProgramElement* p : fProgram.elements()) {
             if (p->is<GlobalVarDeclaration>()) {
                 const GlobalVarDeclaration& global = p->as<GlobalVarDeclaration>();
                 const VarDeclaration& decl = global.declaration()->as<VarDeclaration>();
-                if (&decl.var() == arguments[0]->as<VariableReference>().variable()) {
+                if (&decl.var() == child->as<VariableReference>().variable()) {
                     found = true;
                 } else if (decl.var().type().isEffectChild()) {
                     ++index;
@@ -165,14 +166,25 @@
         }
         SkASSERT(found);
 
+        // Shaders require a coordinate argument. Color filters require a color argument.
+        // When we call sampleChild, the other value remains empty.
+        String color;
         String coords;
-        if (arguments.size() > 1) {
+        {
             AutoOutputBuffer outputToBuffer(this);
-            this->writeExpression(*arguments[1], Precedence::kSequence);
-            coords = outputToBuffer.fBuffer.str();
+            this->writeExpression(*arguments.back(), Precedence::kSequence);
+            if (child->type().typeKind() == Type::TypeKind::kShader) {
+                SkASSERT(arguments[1]->type() == *fProgram.fContext->fTypes.fFloat2);
+                coords = outputToBuffer.fBuffer.str();
+            } else {
+                SkASSERT(child->type().typeKind() == Type::TypeKind::kColorFilter);
+                SkASSERT(arguments[1]->type() == *fProgram.fContext->fTypes.fHalf4 ||
+                         arguments[1]->type() == *fProgram.fContext->fTypes.fFloat4);
+                color = outputToBuffer.fBuffer.str();
+            }
         }
 
-        this->write(fCallbacks->sampleChild(index, std::move(coords)));
+        this->write(fCallbacks->sampleChild(index, std::move(coords), std::move(color)));
         return;
     }
 
diff --git a/src/sksl/codegen/SkSLPipelineStageCodeGenerator.h b/src/sksl/codegen/SkSLPipelineStageCodeGenerator.h
index f00cfed..bf30482 100644
--- a/src/sksl/codegen/SkSLPipelineStageCodeGenerator.h
+++ b/src/sksl/codegen/SkSLPipelineStageCodeGenerator.h
@@ -28,7 +28,7 @@
         virtual void   declareGlobal(const char* declaration) = 0;
 
         virtual String declareUniform(const VarDeclaration*) = 0;
-        virtual String sampleChild(int index, String coords) = 0;
+        virtual String sampleChild(int index, String coords, String color) = 0;
     };
 
     /*
diff --git a/src/sksl/codegen/SkSLVMCodeGenerator.cpp b/src/sksl/codegen/SkSLVMCodeGenerator.cpp
index 7a772d9..d0dc4dc 100644
--- a/src/sksl/codegen/SkSLVMCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLVMCodeGenerator.cpp
@@ -116,6 +116,7 @@
                   SkSpan<skvm::Val> uniforms,
                   skvm::Coord device,
                   skvm::Coord local,
+                  skvm::Color inputColor,
                   SampleChildFn sampleChild);
 
     void writeFunction(const FunctionDefinition& function,
@@ -287,6 +288,7 @@
     skvm::Builder* fBuilder;
 
     const skvm::Coord fLocalCoord;
+    const skvm::Color fInputColor;
     const SampleChildFn fSampleChild;
 
     // [Variable, first slot in fSlots]
@@ -344,10 +346,12 @@
                              SkSpan<skvm::Val> uniforms,
                              skvm::Coord device,
                              skvm::Coord local,
+                             skvm::Color inputColor,
                              SampleChildFn sampleChild)
         : fProgram(program)
         , fBuilder(builder)
         , fLocalCoord(local)
+        , fInputColor(inputColor)
         , fSampleChild(std::move(sampleChild)) {
     fConditionMask = fLoopMask = fBuilder->splat(0xffff'ffff);
 
@@ -996,25 +1000,32 @@
     if (found->second == Intrinsic::kSample) {
         // Sample is very special, the first argument is a child (shader/colorFilter), which can't
         // be evaluated
-        const Context& ctx = *fProgram.fContext;
-        if (nargs > 2 || !c.arguments()[0]->type().isEffectChild() ||
-            (nargs == 2 && (c.arguments()[1]->type() != *ctx.fTypes.fFloat2 &&
-                            c.arguments()[1]->type() != *ctx.fTypes.fFloat3x3))) {
-            SkDEBUGFAIL("Invalid call to sample");
-            return {};
-        }
+        SkASSERT(nargs == 2);
+        const Expression* child = c.arguments()[0].get();
+        SkASSERT(child->type().isEffectChild());
+        SkASSERT(child->is<VariableReference>());
 
-        auto fp_it = fVariableMap.find(c.arguments()[0]->as<VariableReference>().variable());
+        auto fp_it = fVariableMap.find(child->as<VariableReference>().variable());
         SkASSERT(fp_it != fVariableMap.end());
 
+        // Shaders require a coordinate argument. Color filters require a color argument.
+        // When we call sampleChild, the other value remains the incoming default.
+        skvm::Color inColor = fInputColor;
         skvm::Coord coord = fLocalCoord;
-        if (nargs == 2) {
-            Value arg = this->writeExpression(*c.arguments()[1]);
-            SkASSERT(arg.slots() == 2);
-            coord = {f32(arg[0]), f32(arg[1])};
+        const Expression* arg = c.arguments()[1].get();
+        Value argVal = this->writeExpression(*arg);
+
+        if (child->type().typeKind() == Type::TypeKind::kShader) {
+            SkASSERT(arg->type() == *fProgram.fContext->fTypes.fFloat2);
+            coord = {f32(argVal[0]), f32(argVal[1])};
+        } else {
+            SkASSERT(child->type().typeKind() == Type::TypeKind::kColorFilter);
+            SkASSERT(arg->type() == *fProgram.fContext->fTypes.fHalf4 ||
+                     arg->type() == *fProgram.fContext->fTypes.fFloat4);
+            inColor = {f32(argVal[0]), f32(argVal[1]), f32(argVal[2]), f32(argVal[3])};
         }
 
-        skvm::Color color = fSampleChild(fp_it->second, coord);
+        skvm::Color color = fSampleChild(fp_it->second, coord, inColor);
         Value result(4);
         result[0] = color.r;
         result[1] = color.g;
@@ -1694,7 +1705,8 @@
     }
     SkASSERT(argSlots <= SK_ARRAY_COUNT(args));
 
-    SkVMGenerator generator(program, builder, uniforms, device, local, std::move(sampleChild));
+    SkVMGenerator generator(
+            program, builder, uniforms, device, local, inputColor, std::move(sampleChild));
     generator.writeFunction(function, {args, argSlots}, result);
 
     return skvm::Color{{builder, result[0]},
@@ -1732,9 +1744,11 @@
         returnVals.push_back(b->splat(0.0f).id);
     }
 
-    skvm::Coord zeroCoord = {b->splat(0.0f), b->splat(0.0f)};
+    skvm::F32 zero = b->splat(0.0f);
+    skvm::Coord zeroCoord = {zero, zero};
+    skvm::Color zeroColor = {zero, zero, zero, zero};
     SkVMGenerator generator(program, b, uniforms, /*device=*/zeroCoord, /*local=*/zeroCoord,
-                            /*sampleChild=*/{});
+                            /*inputColor=*/zeroColor, /*sampleChild=*/{});
     generator.writeFunction(function, argVals, returnVals);
 
     // generateCode has updated the contents of 'argVals' for any 'out' or 'inout' parameters.
@@ -1851,7 +1865,7 @@
         children.push_back({uniforms.pushPtr(nullptr), builder->uniform32(uniforms.push(0))});
     }
 
-    auto sampleChild = [&](int i, skvm::Coord coord) {
+    auto sampleChild = [&](int i, skvm::Coord coord, skvm::Color) {
         skvm::PixelFormat pixelFormat = skvm::SkColorType_to_PixelFormat(kRGBA_F32_SkColorType);
         skvm::I32 index  = trunc(coord.x);
                   index += trunc(coord.y) * children[i].rowBytesAsPixels;
diff --git a/src/sksl/codegen/SkSLVMCodeGenerator.h b/src/sksl/codegen/SkSLVMCodeGenerator.h
index 3a13d7a..2ef8f9b 100644
--- a/src/sksl/codegen/SkSLVMCodeGenerator.h
+++ b/src/sksl/codegen/SkSLVMCodeGenerator.h
@@ -20,10 +20,7 @@
 class FunctionDefinition;
 struct Program;
 
-using SampleChildFn = std::function<skvm::Color(int, skvm::Coord)>;
-
-// TODO: Have a generic entry point, supporting SkSpan<skvm::Val> for parameters and return values.
-// That would be useful for interpreter use cases like SkParticleEffect.
+using SampleChildFn = std::function<skvm::Color(int, skvm::Coord, skvm::Color)>;
 
 // Convert 'function' to skvm instructions in 'builder', for use by shaders and color filters
 skvm::Color ProgramToSkVM(const Program& program,
diff --git a/src/sksl/generated/sksl_rt_colorfilter.dehydrated.sksl b/src/sksl/generated/sksl_rt_colorfilter.dehydrated.sksl
index 5459f25..7c9be5c 100644
--- a/src/sksl/generated/sksl_rt_colorfilter.dehydrated.sksl
+++ b/src/sksl/generated/sksl_rt_colorfilter.dehydrated.sksl
@@ -1,11 +1,14 @@
-static uint8_t SKSL_INCLUDE_sksl_rt_colorfilter[] = {36,0,
+static uint8_t SKSL_INCLUDE_sksl_rt_colorfilter[] = {56,0,
 1,115,
 6,115,104,97,100,101,114,
 6,99,111,111,114,100,115,
 6,102,108,111,97,116,50,
 6,115,97,109,112,108,101,
 5,104,97,108,102,52,
-49,3,0,
+1,102,
+11,99,111,108,111,114,70,105,108,116,101,114,
+5,99,111,108,111,114,
+49,7,0,
 53,1,0,
 16,2,0,
 50,2,0,4,0,3,
@@ -14,8 +17,20 @@
 50,4,0,18,0,3,
 30,5,0,
 16,25,0,2,1,0,3,0,
-50,6,0,32,0,1,0,
-2,0,
+50,6,0,32,0,
+53,7,0,
+16,38,0,
+50,8,0,40,0,3,
+53,9,0,
+16,52,0,
+47,6,0,3,
+52,10,0,2,
+47,5,0,
+30,11,0,
+16,25,0,2,7,0,9,0,
+47,6,0,
+47,11,0,1,0,
+5,0,
 19,
 20,};
 static constexpr size_t SKSL_INCLUDE_sksl_rt_colorfilter_LENGTH = sizeof(SKSL_INCLUDE_sksl_rt_colorfilter);
diff --git a/src/sksl/generated/sksl_rt_shader.dehydrated.sksl b/src/sksl/generated/sksl_rt_shader.dehydrated.sksl
index ced2f98..f0e0516 100644
--- a/src/sksl/generated/sksl_rt_shader.dehydrated.sksl
+++ b/src/sksl/generated/sksl_rt_shader.dehydrated.sksl
@@ -1,4 +1,4 @@
-static uint8_t SKSL_INCLUDE_sksl_rt_shader[] = {57,0,
+static uint8_t SKSL_INCLUDE_sksl_rt_shader[] = {77,0,
 0,
 12,115,107,95,70,114,97,103,67,111,111,114,100,
 6,102,108,111,97,116,52,
@@ -8,7 +8,10 @@
 6,102,108,111,97,116,50,
 6,115,97,109,112,108,101,
 5,104,97,108,102,52,
-49,4,0,
+1,102,
+11,99,111,108,111,114,70,105,108,116,101,114,
+5,99,111,108,111,114,
+49,8,0,
 53,1,0,
 37,
 36,0,32,0,0,255,255,255,255,255,15,0,255,255,255,255,2,0,0,0,3,0,
@@ -21,8 +24,20 @@
 50,6,0,39,0,3,
 30,7,0,
 16,46,0,2,3,0,5,0,
-50,8,0,53,0,2,0,
-3,0,
+50,8,0,53,0,
+53,9,0,
+16,59,0,
+50,10,0,61,0,3,
+53,11,0,
+16,73,0,
+47,8,0,3,
+52,12,0,2,
+47,7,0,
+30,13,0,
+16,46,0,2,9,0,11,0,
+47,8,0,
+47,13,0,2,0,
+6,0,
 0,0,
 19,
 55,
diff --git a/src/sksl/sksl_rt_colorfilter.sksl b/src/sksl/sksl_rt_colorfilter.sksl
index 1f6183a..06b4a42 100644
--- a/src/sksl/sksl_rt_colorfilter.sksl
+++ b/src/sksl/sksl_rt_colorfilter.sksl
@@ -1,5 +1,2 @@
 half4 sample(shader s, float2 coords);
-
-// TODO(skbug.com/11813): Implement sample() that takes a color
-// half4 sample(colorFilter f, half4 color);
-// half4 sample(shader s, half4 color, float2 coords);
+half4 sample(colorFilter f, half4 color);
diff --git a/src/sksl/sksl_rt_shader.sksl b/src/sksl/sksl_rt_shader.sksl
index e098bad..e259cc3 100644
--- a/src/sksl/sksl_rt_shader.sksl
+++ b/src/sksl/sksl_rt_shader.sksl
@@ -1,7 +1,4 @@
 layout(builtin=15) float4 sk_FragCoord;
 
 half4 sample(shader s, float2 coords);
-
-// TODO(skbug.com/11813): Implement sample() that takes a color
-// half4 sample(colorFilter f, half4 color);
-// half4 sample(shader s, half4 color, float2 coords);
+half4 sample(colorFilter f, half4 color);
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index a6ba883..88fce0b 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -128,15 +128,43 @@
     // Sampling a child shader requires that we pass explicit coords
     test_valid("uniform shader child;"
                "half4 main(half4 c) { return sample(child, c.rg); }");
+    // Trying to pass a color as well. (Works internally with FPs, but not in runtime effects).
+    test_invalid("uniform shader child;"
+                 "half4 main(half4 c) { return sample(child, c.rg, c); }",
+                 "no match for sample(shader, half2, half4)");
 
+    // Shader with just a color
+    test_invalid("uniform shader child;"
+                 "half4 main(half4 c) { return sample(child, c); }",
+                 "no match for sample(shader, half4)");
+    // Coords and color in a differet order
+    test_invalid("uniform shader child;"
+                 "half4 main(half4 c) { return sample(child, c, c.rg); }",
+                 "no match for sample(shader, half4, half2)");
+
+    // Older variants that are no longer allowed
     test_invalid(
             "uniform shader child;"
             "half4 main(half4 c) { return sample(child); }",
-            "expected 2 arguments");
+            "no match for sample(shader)");
     test_invalid(
             "uniform shader child;"
             "half4 main(half4 c) { return sample(child, float3x3(1)); }",
-            "expected 'float2'");
+            "no match for sample(shader, float3x3)");
+
+    // Sampling a colorFilter requires a color. No other signatures are valid.
+    test_valid("uniform colorFilter child;"
+               "half4 main(half4 c) { return sample(child, c); }");
+
+    test_invalid("uniform colorFilter child;"
+                 "half4 main(half4 c) { return sample(child); }",
+                 "sample(colorFilter)");
+    test_invalid("uniform colorFilter child;"
+                 "half4 main(half4 c) { return sample(child, c.rg); }",
+                 "sample(colorFilter, half2)");
+    test_invalid("uniform colorFilter child;"
+                 "half4 main(half4 c) { return sample(child, c.rg, c); }",
+                 "sample(colorFilter, half2, half4)");
 }
 
 DEF_TEST(SkRuntimeEffectForShader, r) {
@@ -185,14 +213,43 @@
     test_valid("uniform shader child;"
                "half4 main(float2 p) { return sample(child, p); }");
 
+    // Trying to pass a color as well. (Works internally with FPs, but not in runtime effects).
+    test_invalid("uniform shader child;"
+                 "half4 main(float2 p, half4 c) { return sample(child, p, c); }",
+                 "no match for sample(shader, float2, half4)");
+
+    // Shader with just a color
+    test_invalid("uniform shader child;"
+                 "half4 main(float2 p, half4 c) { return sample(child, c); }",
+                 "no match for sample(shader, half4)");
+    // Coords and color in a different order
+    test_invalid("uniform shader child;"
+                 "half4 main(float2 p, half4 c) { return sample(child, c, p); }",
+                 "no match for sample(shader, half4, float2)");
+
+    // Older variants that are no longer allowed
     test_invalid(
             "uniform shader child;"
             "half4 main(float2 p) { return sample(child); }",
-            "expected 2 arguments");
+            "no match for sample(shader)");
     test_invalid(
             "uniform shader child;"
             "half4 main(float2 p) { return sample(child, float3x3(1)); }",
-            "expected 'float2'");
+            "no match for sample(shader, float3x3)");
+
+    // Sampling a colorFilter requires a color. No other signatures are valid.
+    test_valid("uniform colorFilter child;"
+               "half4 main(float2 p, half4 c) { return sample(child, c); }");
+
+    test_invalid("uniform colorFilter child;"
+                 "half4 main(float2 p) { return sample(child); }",
+                 "sample(colorFilter)");
+    test_invalid("uniform colorFilter child;"
+                 "half4 main(float2 p) { return sample(child, p); }",
+                 "sample(colorFilter, float2)");
+    test_invalid("uniform colorFilter child;"
+                 "half4 main(float2 p, half4 c) { return sample(child, p, c); }",
+                 "sample(colorFilter, float2, half4)");
 }
 
 class TestEffect {
diff --git a/tests/SkSLDSLTest.cpp b/tests/SkSLDSLTest.cpp
index fd85c5f..7faf2cc 100644
--- a/tests/SkSLDSLTest.cpp
+++ b/tests/SkSLDSLTest.cpp
@@ -1593,7 +1593,7 @@
     EXPECT_EQUAL(Sample(shader, Float2(0, 0)), "sample(shader, float2(0.0, 0.0))");
 
     {
-        ExpectError error(r, "error: expected 'float2', but found 'half4'\n");
+        ExpectError error(r, "error: no match for sample(shader, half4)\n");
         Sample(shader, Half4(1)).release();
     }
 }
diff --git a/tools/viewer/SkSLSlide.cpp b/tools/viewer/SkSLSlide.cpp
index 9d06fce..7df9cc8 100644
--- a/tools/viewer/SkSLSlide.cpp
+++ b/tools/viewer/SkSLSlide.cpp
@@ -217,15 +217,17 @@
         }
     }
 
-    for (const auto& [i, name] : SkMakeEnumerate(fEffect->children())) {
-        auto curShader = std::find_if(fShaders.begin(), fShaders.end(),
-                                      [tgt = fChildren[i]](auto p) { return p.second == tgt; });
-        SkASSERT(curShader!= fShaders.end());
+    for (const auto& c : fEffect->children()) {
+        auto curShader =
+                std::find_if(fShaders.begin(), fShaders.end(), [tgt = fChildren[c.index]](auto p) {
+                    return p.second == tgt;
+                });
+        SkASSERT(curShader != fShaders.end());
 
-        if (ImGui::BeginCombo(name.c_str(), curShader->first)) {
+        if (ImGui::BeginCombo(c.name.c_str(), curShader->first)) {
             for (const auto& namedShader : fShaders) {
                 if (ImGui::Selectable(namedShader.first, curShader->second == namedShader.second)) {
-                    fChildren[i] = namedShader.second;
+                    fChildren[c.index] = namedShader.second;
                 }
             }
             ImGui::EndCombo();