From bd472a01edb2db9737ff39b0851aca6cb847868c Mon Sep 17 00:00:00 2001 From: John Stiles Date: Fri, 8 Jul 2022 15:31:23 -0400 Subject: [PATCH] Move runtime-effect dictionary into Graphite ResourceProvider. Putting the dictionary in the Recorder itself was not very useful; the recorder is inaccessible throughout the call chain of pipeline setup. The ResourceProvider, on the other hand, is accessible everywhere we need it, hangs directly off the Recorder, and has the right lifetime for our purposes. Change-Id: I0f494e5890845d73343a71359900598d63b66764 Bug: skia:13405 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556917 Commit-Queue: Greg Daniel Reviewed-by: Greg Daniel Reviewed-by: Robert Phillips Auto-Submit: John Stiles --- gn/core.gni | 1 + include/gpu/graphite/Recorder.h | 5 ---- public.bzl | 1 + src/core/BUILD.bazel | 1 + src/core/SkKeyHelpers.cpp | 5 ++-- src/core/SkRuntimeEffectDictionary.h | 36 +++++++++++++++++++++++++++ src/gpu/graphite/Recorder.cpp | 4 +-- src/gpu/graphite/RecorderPriv.cpp | 9 ------- src/gpu/graphite/RecorderPriv.h | 1 - src/gpu/graphite/ResourceProvider.cpp | 3 +++ src/gpu/graphite/ResourceProvider.h | 7 ++++++ 11 files changed, 54 insertions(+), 19 deletions(-) create mode 100644 src/core/SkRuntimeEffectDictionary.h diff --git a/gn/core.gni b/gn/core.gni index 1188a542bd..fa7521c62d 100644 --- a/gn/core.gni +++ b/gn/core.gni @@ -342,6 +342,7 @@ skia_core_sources = [ "$_src/core/SkRegion_path.cpp", "$_src/core/SkResourceCache.cpp", "$_src/core/SkRuntimeEffect.cpp", + "$_src/core/SkRuntimeEffectDictionary.h", "$_src/core/SkRuntimeEffectPriv.h", "$_src/core/SkSLTypeShared.cpp", "$_src/core/SkSLTypeShared.h", diff --git a/include/gpu/graphite/Recorder.h b/include/gpu/graphite/Recorder.h index 1b06ee0ddb..af1f79b27a 100644 --- a/include/gpu/graphite/Recorder.h +++ b/include/gpu/graphite/Recorder.h @@ -109,11 +109,6 @@ private: std::unique_ptr fStrikeCache; std::unique_ptr fTextBlobCache; - // We keep track of all SkRuntimeEffects that are connected to a Recorder, along with their code - // snippet ID. This ensures that we have a live reference to every effect that we're going to - // paint, and gives us a way to retrieve their shader text when we see an their code-snippet ID. - SkTHashMap> fRuntimeEffectMap; - // In debug builds we guard against improper thread handling // This guard is passed to the ResourceCache. // TODO: Should we also pass this to Device, DrawContext, and similar classes? diff --git a/public.bzl b/public.bzl index 687baeb503..906e84adbf 100644 --- a/public.bzl +++ b/public.bzl @@ -625,6 +625,7 @@ BASE_SRCS_ALL = [ "src/core/SkResourceCache.cpp", "src/core/SkResourceCache.h", "src/core/SkRuntimeEffect.cpp", + "src/core/SkRuntimeEffectDictionary.h", "src/core/SkRuntimeEffectPriv.h", "src/core/SkSLTypeShared.cpp", "src/core/SkSLTypeShared.h", diff --git a/src/core/BUILD.bazel b/src/core/BUILD.bazel index 917ce4a67d..8457359427 100644 --- a/src/core/BUILD.bazel +++ b/src/core/BUILD.bazel @@ -291,6 +291,7 @@ CORE_FILES = [ "SkRegion_path.cpp", "SkResourceCache.cpp", "SkResourceCache.h", + "SkRuntimeEffectDictionary.h", "SkRuntimeEffectPriv.h", "SkSafeMath.h", "SkSafeRange.h", diff --git a/src/core/SkKeyHelpers.cpp b/src/core/SkKeyHelpers.cpp index 99c4cc056e..83065b9533 100644 --- a/src/core/SkKeyHelpers.cpp +++ b/src/core/SkKeyHelpers.cpp @@ -20,6 +20,7 @@ #ifdef SK_GRAPHITE_ENABLED #include "src/gpu/Blend.h" #include "src/gpu/graphite/RecorderPriv.h" +#include "src/gpu/graphite/ResourceProvider.h" #include "src/gpu/graphite/Texture.h" #include "src/gpu/graphite/TextureProxy.h" #include "src/gpu/graphite/UniformManager.h" @@ -588,8 +589,8 @@ void RuntimeShaderBlock::BeginBlock(const SkKeyContext& keyContext, int codeSnippetID = dict->findOrCreateRuntimeEffectSnippet(shaderData.fEffect.get()); skgpu::graphite::Recorder* recorder = keyContext.recorder(); - recorder->priv().addRuntimeEffect(codeSnippetID, shaderData.fEffect); - + recorder->priv().resourceProvider()->runtimeEffectDictionary()->set(codeSnippetID, + shaderData.fEffect); if (gatherer) { const SkShaderSnippet* entry = dict->getEntry(codeSnippetID); SkASSERT(entry); diff --git a/src/core/SkRuntimeEffectDictionary.h b/src/core/SkRuntimeEffectDictionary.h new file mode 100644 index 0000000000..7c73389dfc --- /dev/null +++ b/src/core/SkRuntimeEffectDictionary.h @@ -0,0 +1,36 @@ +/* + * Copyright 2022 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkRuntimeEffectDictionary_DEFINED +#define SkRuntimeEffectDictionary_DEFINED + +#include "include/core/SkRefCnt.h" +#include "include/effects/SkRuntimeEffect.h" +#include "include/private/SkTHash.h" +#include "src/core/SkRuntimeEffectPriv.h" + +class SkRuntimeEffect; + +// We keep track of all SkRuntimeEffects that are used by a recording, along with their code +// snippet ID. This ensures that we have a live reference to every effect that we're going to +// paint, and gives us a way to retrieve their shader text when we see an their code-snippet ID. +class SkRuntimeEffectDictionary { +public: + void set(int codeSnippetID, sk_sp effect) { + // The same code-snippet ID should never refer to two different effects. + SkASSERT(!fDict.find(codeSnippetID) || (SkRuntimeEffectPriv::Hash(*fDict[codeSnippetID]) == + SkRuntimeEffectPriv::Hash(*effect))); + fDict.set(codeSnippetID, std::move(effect)); + } + + void reset() { fDict.reset(); } + +private: + SkTHashMap> fDict; +}; + +#endif // SkRuntimeEffectDictionary_DEFINED diff --git a/src/gpu/graphite/Recorder.cpp b/src/gpu/graphite/Recorder.cpp index c88e748b8e..470c267e4a 100644 --- a/src/gpu/graphite/Recorder.cpp +++ b/src/gpu/graphite/Recorder.cpp @@ -84,7 +84,7 @@ std::unique_ptr Recorder::snap() { fTextureDataCache = std::make_unique(); // We leave the UniformDataCache alone fGraph->reset(); - fRuntimeEffectMap.reset(); + fResourceProvider->resetAfterSnap(); return nullptr; } @@ -93,7 +93,7 @@ std::unique_ptr Recorder::snap() { fUploadBufferManager->transferToRecording(recording.get()); fGraph = std::make_unique(); - fRuntimeEffectMap.reset(); + fResourceProvider->resetAfterSnap(); fTextureDataCache = std::make_unique(); return recording; } diff --git a/src/gpu/graphite/RecorderPriv.cpp b/src/gpu/graphite/RecorderPriv.cpp index 7a4a0b27df..f6e19dedbe 100644 --- a/src/gpu/graphite/RecorderPriv.cpp +++ b/src/gpu/graphite/RecorderPriv.cpp @@ -76,15 +76,6 @@ void RecorderPriv::add(sk_sp task) { fRecorder->fGraph->add(std::move(task)); } -void RecorderPriv::addRuntimeEffect(int codeSnippetID, sk_sp effect) { - ASSERT_SINGLE_OWNER - // The same code-snippet ID should never refer to two different effects. - SkASSERT(!fRecorder->fRuntimeEffectMap.find(codeSnippetID) || - (SkRuntimeEffectPriv::Hash(*fRecorder->fRuntimeEffectMap[codeSnippetID]) == - SkRuntimeEffectPriv::Hash(*effect))); - fRecorder->fRuntimeEffectMap.set(codeSnippetID, std::move(effect)); -} - void RecorderPriv::flushTrackedDevices() { ASSERT_SINGLE_OWNER for (Device* device : fRecorder->fTrackedDevices) { diff --git a/src/gpu/graphite/RecorderPriv.h b/src/gpu/graphite/RecorderPriv.h index e1cc0c8b82..e8c4a07aba 100644 --- a/src/gpu/graphite/RecorderPriv.h +++ b/src/gpu/graphite/RecorderPriv.h @@ -16,7 +16,6 @@ namespace skgpu::graphite { class RecorderPriv { public: void add(sk_sp); - void addRuntimeEffect(int codeSnippetID, sk_sp); ResourceProvider* resourceProvider() const; UniformDataCache* uniformDataCache() const; diff --git a/src/gpu/graphite/ResourceProvider.cpp b/src/gpu/graphite/ResourceProvider.cpp index 030d40eaab..2a09ceb03f 100644 --- a/src/gpu/graphite/ResourceProvider.cpp +++ b/src/gpu/graphite/ResourceProvider.cpp @@ -197,5 +197,8 @@ sk_sp ResourceProvider::findOrCreateBuffer(size_t size, return buffer; } +void ResourceProvider::resetAfterSnap() { + fRuntimeEffectDictionary.reset(); +} } // namespace skgpu::graphite diff --git a/src/gpu/graphite/ResourceProvider.h b/src/gpu/graphite/ResourceProvider.h index 962680407f..042dac01fd 100644 --- a/src/gpu/graphite/ResourceProvider.h +++ b/src/gpu/graphite/ResourceProvider.h @@ -11,6 +11,7 @@ #include "include/core/SkSize.h" #include "include/core/SkTileMode.h" #include "src/core/SkLRUCache.h" +#include "src/core/SkRuntimeEffectDictionary.h" #include "src/gpu/ResourceKey.h" #include "src/gpu/graphite/CommandBuffer.h" #include "src/gpu/graphite/GraphicsPipelineDesc.h" @@ -63,6 +64,10 @@ public: SkShaderCodeDictionary* shaderCodeDictionary() const; + SkRuntimeEffectDictionary* runtimeEffectDictionary() { return &fRuntimeEffectDictionary; } + + void resetAfterSnap(); + #if GRAPHITE_TEST_UTILS ResourceCache* resourceCache() { return fResourceCache.get(); } const Gpu* gpu() { return fGpu; } @@ -116,6 +121,8 @@ private: // Cache of GraphicsPipelines // TODO: Move this onto GlobalCache std::unique_ptr fGraphicsPipelineCache; + + SkRuntimeEffectDictionary fRuntimeEffectDictionary; }; } // namespace skgpu::graphite