In SkRuntimeEffect, just use hash of SkSL as the ID

Change-Id: I2ce210889452756d5d100fbb15f578bb72e2af2a
Bug: skia:9813
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268687
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

gm/runtimecolorfilter.cpp

@@ -37,8 +37,7 @@ DEF_SIMPLE_GPU_GM(runtimecolorfilter, context, rtc, canvas, 512, 256) {
 
     float b = 0.75;
     sk_sp<SkData> data = SkData::MakeWithCopy(&b, sizeof(b));
-    static sk_sp<SkRuntimeEffect> effect = std::get<0>(
-            SkRuntimeEffect::Make(SkString(SKSL_TEST_SRC)));
+    sk_sp<SkRuntimeEffect> effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_TEST_SRC)));
 
     auto cf1 = effect->makeColorFilter(data);
     SkPaint p;

gm/runtimefunctions.cpp

@@ -38,8 +38,7 @@ class RuntimeFunctions : public skiagm::GM {
     SkISize onISize() override { return {256, 256}; }
 
     void onDraw(SkCanvas* canvas) override {
-        // static to pass gl persistent cache test in dm
-        static sk_sp<SkRuntimeEffect> gEffect =
+        sk_sp<SkRuntimeEffect> gEffect =
                 std::get<0>(SkRuntimeEffect::Make(SkString(RUNTIME_FUNCTIONS_SRC)));
         SkASSERT(gEffect);
 

gm/runtimeshader.cpp

@@ -29,8 +29,7 @@ class RuntimeShader : public skiagm::GM {
     SkISize onISize() override { return {512, 256}; }
 
     void onDraw(SkCanvas* canvas) override {
-        // static to pass gl persistent cache test in dm
-        static sk_sp<SkRuntimeEffect> gEffect = std::get<0>(SkRuntimeEffect::Make(SkString(gProg)));
+        sk_sp<SkRuntimeEffect> gEffect = std::get<0>(SkRuntimeEffect::Make(SkString(gProg)));
         SkASSERT(gEffect);
 
         SkMatrix localM;

include/effects/SkRuntimeEffect.h

@@ -85,7 +85,7 @@ public:
     sk_sp<SkColorFilter> makeColorFilter(sk_sp<SkData> inputs);
 
     const SkString& source() const { return fSkSL; }
-    int index() const { return fIndex; }
+    uint32_t hash() const { return fHash; }
 
     template <typename T>
     class ConstIterable {
@@ -134,7 +134,7 @@ private:
     using SpecializeResult = std::tuple<std::unique_ptr<SkSL::Program>, SkString>;
     SpecializeResult specialize(SkSL::Program& baseProgram, const void* inputs);
 
-    int fIndex;
+    uint32_t fHash;
     SkString fSkSL;
 
     std::unique_ptr<SkSL::Compiler> fCompiler;

src/core/SkColorFilter.cpp

@@ -439,8 +439,6 @@ public:
 
 protected:
     void flatten(SkWriteBuffer& buffer) const override {
-        // See comment in CreateProc about this index
-        buffer.writeInt(fEffect->index());
         buffer.writeString(fEffect->source().c_str());
         if (fInputs) {
             buffer.writeDataAsByteArray(fInputs.get());
@@ -466,17 +464,12 @@ private:
 };
 
 sk_sp<SkFlattenable> SkRuntimeColorFilter::CreateProc(SkReadBuffer& buffer) {
-    // We don't have a way to ensure that indices are consistent and correct when deserializing.
-    // For now, all shaders get a new unique ID after serialization.
-    int index = buffer.readInt();
-    (void)index;
-
     SkString sksl;
     buffer.readString(&sksl);
     sk_sp<SkData> inputs = buffer.readByteArrayAsData();
 
     auto effect = std::get<0>(SkRuntimeEffect::Make(std::move(sksl)));
-    return sk_sp<SkFlattenable>(new SkRuntimeColorFilter(std::move(effect), std::move(inputs)));
+    return effect->makeColorFilter(std::move(inputs));
 }
 
 // Private helper method so SkRuntimeEffect can access SkRuntimeColorFilter

src/core/SkRuntimeEffect.cpp

@@ -8,16 +8,12 @@
 #include "include/core/SkColorFilter.h"
 #include "include/core/SkData.h"
 #include "include/effects/SkRuntimeEffect.h"
+#include "include/private/SkChecksum.h"
 #include "src/shaders/SkRTShader.h"
 #include "src/sksl/SkSLByteCode.h"
 #include "src/sksl/SkSLCompiler.h"
 #include "src/sksl/ir/SkSLVarDeclarations.h"
 
-static inline int new_sksl_index() {
-    static std::atomic<int> nextIndex{ 0 };
-    return nextIndex++;
-}
-
 SkRuntimeEffect::EffectResult SkRuntimeEffect::Make(SkString sksl) {
     auto compiler = std::make_unique<SkSL::Compiler>();
     auto program = compiler->convertProgram(SkSL::Program::kPipelineStage_Kind,
@@ -198,7 +194,7 @@ SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr<SkSL::Compiler>
                                  std::vector<Variable>&& inAndUniformVars,
                                  std::vector<SkString>&& children,
                                  size_t uniformSize)
-        : fIndex(new_sksl_index())
+        : fHash(SkGoodHash()(sksl))
         , fSkSL(std::move(sksl))
         , fCompiler(std::move(compiler))
         , fBaseProgram(std::move(baseProgram))

src/gpu/effects/GrSkSLFP.cpp

@@ -211,7 +211,11 @@ GrGLSLFragmentProcessor* GrSkSLFP::onCreateGLSLInstance() const {
 }
 
 void GrSkSLFP::onGetGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorKeyBuilder* b) const {
-    b->add32(fEffect->index());
+    // In the unlikely event of a hash collision, we also include the input size in the key.
+    // That ensures that we will (at worst) use the wrong program, but one that expects the same
+    // amount of input data.
+    b->add32(fEffect->hash());
+    b->add32(SkToU32(fInputs->size()));
     const uint8_t* inputs = fInputs->bytes();
     for (const auto& v : fEffect->inputs()) {
         if (v.fQualifier != SkRuntimeEffect::Variable::Qualifier::kIn) {
@@ -236,7 +240,7 @@ void GrSkSLFP::onGetGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorKeyBui
 
 bool GrSkSLFP::onIsEqual(const GrFragmentProcessor& other) const {
     const GrSkSLFP& sk = other.cast<GrSkSLFP>();
-    return fEffect->index() == sk.fEffect->index() && fInputs->equals(sk.fInputs.get());
+    return fEffect->hash() == sk.fEffect->hash() && fInputs->equals(sk.fInputs.get());
 }
 
 std::unique_ptr<GrFragmentProcessor> GrSkSLFP::clone() const {

src/shaders/SkRTShader.cpp

@@ -116,12 +116,9 @@ sk_sp<SkFlattenable> SkRTShader::CreateProc(SkReadBuffer& buffer) {
         children[i] = buffer.readShader();
     }
 
-    // We don't have a way to ensure that indices are consistent and correct when deserializing.
-    // Perhaps we should have a hash table to map strings to indices? For now, all shaders get a
-    // new unique ID after serialization.
     auto effect = std::get<0>(SkRuntimeEffect::Make(std::move(sksl)));
-    return sk_sp<SkFlattenable>(new SkRTShader(std::move(effect), std::move(inputs), localMPtr,
-                                               children.data(), children.size(), isOpaque));
+    return effect->makeShader(std::move(inputs), children.data(), children.size(), localMPtr,
+                              isOpaque);
 }
 
 #if SK_SUPPORT_GPU