diff --git a/gm/runtimecolorfilter.cpp b/gm/runtimecolorfilter.cpp index 57398f2f3e..35fb6bac42 100644 --- a/gm/runtimecolorfilter.cpp +++ b/gm/runtimecolorfilter.cpp @@ -37,8 +37,7 @@ DEF_SIMPLE_GPU_GM(runtimecolorfilter, context, rtc, canvas, 512, 256) { float b = 0.75; sk_sp data = SkData::MakeWithCopy(&b, sizeof(b)); - static sk_sp effect = std::get<0>( - SkRuntimeEffect::Make(SkString(SKSL_TEST_SRC))); + sk_sp effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_TEST_SRC))); auto cf1 = effect->makeColorFilter(data); SkPaint p; diff --git a/gm/runtimefunctions.cpp b/gm/runtimefunctions.cpp index ffce619052..495e59103a 100644 --- a/gm/runtimefunctions.cpp +++ b/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 gEffect = + sk_sp gEffect = std::get<0>(SkRuntimeEffect::Make(SkString(RUNTIME_FUNCTIONS_SRC))); SkASSERT(gEffect); diff --git a/gm/runtimeshader.cpp b/gm/runtimeshader.cpp index 0c6dbc4c59..c37350f2f1 100644 --- a/gm/runtimeshader.cpp +++ b/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 gEffect = std::get<0>(SkRuntimeEffect::Make(SkString(gProg))); + sk_sp gEffect = std::get<0>(SkRuntimeEffect::Make(SkString(gProg))); SkASSERT(gEffect); SkMatrix localM; diff --git a/include/effects/SkRuntimeEffect.h b/include/effects/SkRuntimeEffect.h index 1495e13a69..71d93a6d9b 100644 --- a/include/effects/SkRuntimeEffect.h +++ b/include/effects/SkRuntimeEffect.h @@ -85,7 +85,7 @@ public: sk_sp makeColorFilter(sk_sp inputs); const SkString& source() const { return fSkSL; } - int index() const { return fIndex; } + uint32_t hash() const { return fHash; } template class ConstIterable { @@ -134,7 +134,7 @@ private: using SpecializeResult = std::tuple, SkString>; SpecializeResult specialize(SkSL::Program& baseProgram, const void* inputs); - int fIndex; + uint32_t fHash; SkString fSkSL; std::unique_ptr fCompiler; diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp index ee28e9895c..efa9d61c6e 100644 --- a/src/core/SkColorFilter.cpp +++ b/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 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 inputs = buffer.readByteArrayAsData(); auto effect = std::get<0>(SkRuntimeEffect::Make(std::move(sksl))); - return sk_sp(new SkRuntimeColorFilter(std::move(effect), std::move(inputs))); + return effect->makeColorFilter(std::move(inputs)); } // Private helper method so SkRuntimeEffect can access SkRuntimeColorFilter diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp index 6ae366bd79..efdb658c45 100644 --- a/src/core/SkRuntimeEffect.cpp +++ b/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 nextIndex{ 0 }; - return nextIndex++; -} - SkRuntimeEffect::EffectResult SkRuntimeEffect::Make(SkString sksl) { auto compiler = std::make_unique(); auto program = compiler->convertProgram(SkSL::Program::kPipelineStage_Kind, @@ -198,7 +194,7 @@ SkRuntimeEffect::SkRuntimeEffect(SkString sksl, std::unique_ptr std::vector&& inAndUniformVars, std::vector&& children, size_t uniformSize) - : fIndex(new_sksl_index()) + : fHash(SkGoodHash()(sksl)) , fSkSL(std::move(sksl)) , fCompiler(std::move(compiler)) , fBaseProgram(std::move(baseProgram)) diff --git a/src/gpu/effects/GrSkSLFP.cpp b/src/gpu/effects/GrSkSLFP.cpp index 7dfb4135cc..9e0d1dad59 100644 --- a/src/gpu/effects/GrSkSLFP.cpp +++ b/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(); - 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 GrSkSLFP::clone() const { diff --git a/src/shaders/SkRTShader.cpp b/src/shaders/SkRTShader.cpp index d368d20df3..f5158cd6a6 100644 --- a/src/shaders/SkRTShader.cpp +++ b/src/shaders/SkRTShader.cpp @@ -116,12 +116,9 @@ sk_sp 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(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