Update SkRuntimeEffect::Child to use string_view names.

Change-Id: I294453bb18b9ecd46a84cdfaac831679437a4fe2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/557589
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>

RELEASE_NOTES.txt

@@ -12,6 +12,10 @@ Milestone 104
   * SkRuntimeEffect::Uniform now stores the uniform name as a string_view, rather than a
     SkString. Related methods SkRuntimeEffect::findUniform and SkRuntimeEffectBuilder::uniform
     also take std::string_view instead of const char*.
+  * SkRuntimeEffect::Child now stores the child name as a string_view, rather than a SkString.
+    Related methods SkRuntimeEffect::findChild and SkRuntimeEffectBuilder::child also take
+    std::string_view instead of const char*. Also, SkImageFilters::RuntimeShader now takes the
+    child-name array as a std::string_view[] instead of const char*[].
   * skcms.h has been relocated to //modules/skcms/skcms.h (was //include/third_party/skcms/skcms.h)
   * New functions SkCanvas::getBaseProps and SkCanvas::getTopProps; SkCanvas::getBaseProps is a
     direct replacement for the (now deprecated) SkCanvas::getProps function, while getTopProps is

gm/runtimeimagefilter.cpp

@@ -23,6 +23,8 @@
 #include "tools/Resources.h"
 #include "tools/ToolUtils.h"
 
+#include <string_view>
+
 static sk_sp<SkImageFilter> make_filter() {
     sk_sp<SkRuntimeEffect> effect = SkRuntimeEffect::MakeForShader(SkString(R"(
         uniform shader child;
@@ -89,7 +91,7 @@ DEF_SIMPLE_GM(rtif_unsharp, canvas, 512, 256) {
     auto image = GetResourceAsImage("images/mandrill_256.png");
     auto blurredSrc = SkImageFilters::Blur(1, 1, /*input=*/nullptr);
 
-    const char* childNames[] = { "content", "blurred" };
+    std::string_view childNames[] = { "content", "blurred" };
     sk_sp<SkImageFilter> childNodes[] = { nullptr, blurredSrc };
 
     auto sharpened = SkImageFilters::RuntimeShader(builder, childNames, childNodes, 2);

include/effects/SkImageFilters.h

@@ -344,7 +344,7 @@ public:
      *                         fill the result image
      *  @param childShaderName The name of the child shader defined in the builder that will be
      *                         bound to the input param (or the source image if the input param
-     *                         is null).  If null the builder can have exactly one child shader,
+     *                         is null).  If null, the builder can have exactly one child shader,
      *                         which automatically binds the input param.
      *  @param input           The image filter that will be provided as input to the runtime
      *                         shader. If null the implicit source image is used instead
@@ -369,7 +369,7 @@ public:
      *  @param inputCount       How many entries are present in 'childShaderNames' and 'inputs'.
      */
     static sk_sp<SkImageFilter> RuntimeShader(const SkRuntimeShaderBuilder& builder,
-                                              const char* childShaderNames[],
+                                              std::string_view childShaderNames[],
                                               const sk_sp<SkImageFilter> inputs[],
                                               int inputCount);
 #endif  // SK_ENABLE_SKSL

include/effects/SkRuntimeEffect.h

@@ -110,9 +110,9 @@ public:
     };
 
     struct Child {
-        SkString  name;
+        std::string_view name;
-        ChildType type;
+        ChildType        type;
-        int       index;
+        int              index;
     };
 
     class Options {
@@ -257,7 +257,7 @@ public:
     const Uniform* findUniform(std::string_view name) const;
 
     // Returns pointer to the named child's description, or nullptr if not found
-    const Child* findChild(const char* name) const;
+    const Child* findChild(std::string_view name) const;
 
     static void RegisterFlattenables();
     ~SkRuntimeEffect() override;
@@ -411,10 +411,7 @@ public:
     const SkRuntimeEffect* effect() const { return fEffect.get(); }
 
     BuilderUniform uniform(std::string_view name) { return { this, fEffect->findUniform(name) }; }
-    BuilderChild child(const char* name) {
+    BuilderChild child(std::string_view name) { return { this, fEffect->findChild(name) }; }
-        const SkRuntimeEffect::Child* child = fEffect->findChild(name);
-        return { this, child };
-    }
 
 protected:
     SkRuntimeEffectBuilder() = delete;

src/core/SkRuntimeEffect.cpp

@@ -418,7 +418,7 @@ SkRuntimeEffect::Result SkRuntimeEffect::MakeInternal(std::unique_ptr<SkSL::Prog
             // Child effects that can be sampled ('shader', 'colorFilter', 'blender')
             if (varType.isEffectChild()) {
                 Child c;
-                c.name  = SkString(var.name());
+                c.name  = var.name();
                 c.type  = child_type(varType);
                 c.index = children.size();
                 children.push_back(c);
@@ -642,11 +642,9 @@ const SkRuntimeEffect::Uniform* SkRuntimeEffect::findUniform(std::string_view na
     return iter == fUniforms.end() ? nullptr : &(*iter);
 }
 
-const SkRuntimeEffect::Child* SkRuntimeEffect::findChild(const char* name) const {
+const SkRuntimeEffect::Child* SkRuntimeEffect::findChild(std::string_view name) const {
-    SkASSERT(name);
+    auto iter = std::find_if(fChildren.begin(), fChildren.end(), [name](const Child& c) {
-    size_t len = strlen(name);
+        return c.name == name;
-    auto iter = std::find_if(fChildren.begin(), fChildren.end(), [name, len](const Child& c) {
-        return c.name.equals(name, len);
     });
     return iter == fChildren.end() ? nullptr : &(*iter);
 }

src/effects/imagefilters/SkRuntimeImageFilter.cpp

@@ -26,14 +26,17 @@ public:
                          sk_sp<SkData> uniforms,
                          sk_sp<SkImageFilter> input)
             : INHERITED(&input, 1, /*cropRect=*/nullptr)
-            , fShaderBuilder(std::move(effect), std::move(uniforms))
+            , fShaderBuilder(std::move(effect), std::move(uniforms)) {
-            , fChildShaderNames(&fShaderBuilder.effect()->children().front().name, 1) {}
+        std::string_view childName = fShaderBuilder.effect()->children().front().name;
+        fChildShaderNames.push_back(SkString(childName));
+    }
     SkRuntimeImageFilter(const SkRuntimeShaderBuilder& builder,
-                         const char* childShaderNames[],
+                         std::string_view childShaderNames[],
                          const sk_sp<SkImageFilter> inputs[],
                          int inputCount)
             : INHERITED(inputs, inputCount, /*cropRect=*/nullptr)
             , fShaderBuilder(builder) {
+        fChildShaderNames.reserve_back(inputCount);
         for (int i = 0; i < inputCount; i++) {
             fChildShaderNames.push_back(SkString(childShaderNames[i]));
         }
@@ -99,7 +102,7 @@ sk_sp<SkFlattenable> SkRuntimeImageFilter::CreateProc(SkReadBuffer& buffer) {
     }
 
     // Read the child shader names
-    SkSTArray<4, const char*> childShaderNames;
+    SkSTArray<4, std::string_view> childShaderNames;
     SkSTArray<4, SkString> childShaderNameStrings;
     childShaderNames.resize(common.inputCount());
     childShaderNameStrings.resize(common.inputCount());
@@ -111,8 +114,8 @@ sk_sp<SkFlattenable> SkRuntimeImageFilter::CreateProc(SkReadBuffer& buffer) {
     SkRuntimeShaderBuilder builder(std::move(effect), std::move(uniforms));
 
     // Populate the builder with the corresponding children
-    for (auto& child : builder.effect()->children()) {
+    for (const SkRuntimeEffect::Child& child : builder.effect()->children()) {
-        const char* name = child.name.c_str();
+        std::string_view name = child.name;
         switch (child.type) {
             case SkRuntimeEffect::ChildType::kBlender: {
                 builder.child(name) = buffer.readBlender();
@@ -133,8 +136,8 @@ sk_sp<SkFlattenable> SkRuntimeImageFilter::CreateProc(SkReadBuffer& buffer) {
         return nullptr;
     }
 
-    return SkImageFilters::RuntimeShader(
+    return SkImageFilters::RuntimeShader(builder, childShaderNames.data(),
-            builder, childShaderNames.data(), common.inputs(), common.inputCount());
+                                         common.inputs(), common.inputCount());
 }
 
 void SkRuntimeImageFilter::flatten(SkWriteBuffer& buffer) const {
@@ -223,40 +226,43 @@ static bool child_is_shader(const SkRuntimeEffect::Child* child) {
 sk_sp<SkImageFilter> SkImageFilters::RuntimeShader(const SkRuntimeShaderBuilder& builder,
                                                    const char* childShaderName,
                                                    sk_sp<SkImageFilter> input) {
-    // if no childShaderName is provided check to see if we can implicitly assign it to the only
+    // If no childShaderName is provided, check to see if we can implicitly assign it to the only
-    // child in the effect
+    // child in the effect.
-    if (childShaderName == nullptr) {
+    std::string_view childShaderNameView;
+    if (childShaderName != nullptr) {
+        childShaderNameView = childShaderName;
+    } else {
         auto children = builder.effect()->children();
         if (children.size() != 1) {
             return nullptr;
         }
-        childShaderName = children.front().name.c_str();
+        childShaderNameView = children.front().name;
     }
 
-    return SkImageFilters::RuntimeShader(builder, &childShaderName, &input, 1);
+    return SkImageFilters::RuntimeShader(builder, &childShaderNameView, &input, 1);
 }
 
 sk_sp<SkImageFilter> SkImageFilters::RuntimeShader(const SkRuntimeShaderBuilder& builder,
-                                                   const char* childShaderNames[],
+                                                   std::string_view childShaderNames[],
                                                    const sk_sp<SkImageFilter> inputs[],
                                                    int inputCount) {
     for (int i = 0; i < inputCount; i++) {
-        const char* name = childShaderNames[i];
+        std::string_view name = childShaderNames[i];
-        // All names must be non-null, and present as a child shader in the effect:
+        // All names must be non-empty, and present as a child shader in the effect:
-        if (!name || !child_is_shader(builder.effect()->findChild(name))) {
+        if (name.empty() || !child_is_shader(builder.effect()->findChild(name))) {
             return nullptr;
         }
 
         // We don't allow duplicates, either:
         for (int j = 0; j < i; j++) {
-            if (!strcmp(name, childShaderNames[j])) {
+            if (name == childShaderNames[j]) {
                 return nullptr;
             }
         }
     }
 
-    return sk_sp<SkImageFilter>(
+    return sk_sp<SkImageFilter>(new SkRuntimeImageFilter(builder, childShaderNames,
-            new SkRuntimeImageFilter(builder, childShaderNames, inputs, inputCount));
+                                                         inputs, inputCount));
 }
 
 #endif  // SK_ENABLE_SKSL

src/gpu/ganesh/effects/GrSkSLFP.h

@@ -307,13 +307,14 @@ private:
         SkASSERTF(uIter == uEnd,
                   "Expected more uniforms, starting with '%.*s'",
                   (int)uIter->name.size(), uIter->name.data());
-        SkASSERTF(cIter == cEnd, "Expected more children, starting with '%s'", cIter->name.c_str());
+        SkASSERTF(cIter == cEnd, "Expected more children, starting with '%.*s'",
+                  (int)cIter->name.size(), cIter->name.data());
     }
     static void checkOneChild(child_iterator cIter, child_iterator cEnd, const char* name) {
         SkASSERTF(cIter != cEnd, "Too many children, wasn't expecting '%s'", name);
-        SkASSERTF(cIter->name.equals(name),
+        SkASSERTF(cIter->name == name,
-                  "Expected child '%s', got '%s' instead",
+                  "Expected child '%.*s', got '%s' instead",
-                  cIter->name.c_str(), name);
+                  (int)cIter->name.size(), cIter->name.data(), name);
     }
     template <typename... Args>
     static void checkArgs(uniform_iterator uIter,

tools/viewer/SkSLSlide.cpp

@@ -237,14 +237,16 @@ void SkSLSlide::draw(SkCanvas* canvas) {
         }
     }
 
-    for (const auto& c : fEffect->children()) {
+    for (const SkRuntimeEffect::Child& c : fEffect->children()) {
-        auto curShader =
+        auto curShader = std::find_if(
-                std::find_if(fShaders.begin(), fShaders.end(), [tgt = fChildren[c.index]](auto p) {
+                fShaders.begin(),
+                fShaders.end(),
+                [tgt = fChildren[c.index]](const std::pair<const char*, sk_sp<SkShader>>& p) {
                     return p.second == tgt;
                 });
         SkASSERT(curShader != fShaders.end());
 
-        if (ImGui::BeginCombo(c.name.c_str(), curShader->first)) {
+        if (ImGui::BeginCombo(std::string(c.name).c_str(), curShader->first)) {
             for (const auto& namedShader : fShaders) {
                 if (ImGui::Selectable(namedShader.first, curShader->second == namedShader.second)) {
                     fChildren[c.index] = namedShader.second;