From 00ce96456b0c72fe84557af5619338810a894e7d Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Thu, 24 Mar 2022 12:01:50 -0400 Subject: [PATCH] [graphite] Add support for finished callbacks. This also reworks a little bit about what we send to insertRecording and what we store on Context. Bug: skia:12974 Change-Id: I747a1cdd1559d4d5fbe928e689a23a734142557b Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524012 Reviewed-by: Jim Van Verth Commit-Queue: Greg Daniel --- bench/nanobench.cpp | 8 +++++-- dm/DMSrcSink.cpp | 5 +++-- experimental/graphite/include/Context.h | 6 +++-- experimental/graphite/include/GraphiteTypes.h | 13 +++++++++++ experimental/graphite/src/CommandBuffer.cpp | 14 +++++++++--- experimental/graphite/src/CommandBuffer.h | 5 +++++ experimental/graphite/src/Context.cpp | 21 +++++++++++------- experimental/graphite/src/Gpu.cpp | 12 +++++++--- experimental/graphite/src/Gpu.h | 5 +++-- .../graphite/src/GpuWorkSubmission.cpp | 22 +++++++++++++++++++ experimental/graphite/src/GpuWorkSubmission.h | 10 +++++++-- experimental/graphite/src/TextureUtils.cpp | 5 ++++- experimental/graphite/src/mtl/MtlGpu.h | 2 +- experimental/graphite/src/mtl/MtlGpu.mm | 16 ++++++-------- gn/graphite.gni | 1 + tools/sk_app/GraphiteMetalWindowContext.mm | 4 +++- 16 files changed, 113 insertions(+), 36 deletions(-) create mode 100644 experimental/graphite/src/GpuWorkSubmission.cpp diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp index d2868c3901..47b53ab848 100644 --- a/bench/nanobench.cpp +++ b/bench/nanobench.cpp @@ -317,7 +317,9 @@ struct GraphiteTarget : public Target { void endTiming() override { if (context && recorder) { std::unique_ptr recording = this->recorder->snap(); - this->context->insertRecording(std::move(recording)); + skgpu::InsertRecordingInfo info; + info.fRecording = recording.get(); + this->context->insertRecording(info); context->submit(skgpu::SyncToCpu::kNo); } } @@ -326,7 +328,9 @@ struct GraphiteTarget : public Target { // TODO: have a way to sync work with out submitting a Recording which is currently // required. std::unique_ptr recording = this->recorder->snap(); - this->context->insertRecording(std::move(recording)); + skgpu::InsertRecordingInfo info; + info.fRecording = recording.get(); + this->context->insertRecording(info); this->context->submit(skgpu::SyncToCpu::kYes); } } diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index b2304e3006..ef79c9b6b8 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -2148,8 +2148,9 @@ Result GraphiteSink::draw(const Src& src, } std::unique_ptr recording = recorder->snap(); - - context->insertRecording(std::move(recording)); + skgpu::InsertRecordingInfo info; + info.fRecording = recording.get(); + context->insertRecording(info); context->submit(skgpu::SyncToCpu::kYes); return Result::Ok(); diff --git a/experimental/graphite/include/Context.h b/experimental/graphite/include/Context.h index ba3cc3762a..97ab6cff17 100644 --- a/experimental/graphite/include/Context.h +++ b/experimental/graphite/include/Context.h @@ -20,6 +20,7 @@ namespace skgpu { class BackendTexture; +class CommandBuffer; class ContextPriv; class GlobalCache; class Gpu; @@ -70,7 +71,7 @@ public: std::unique_ptr makeRecorder(); - void insertRecording(std::unique_ptr); + void insertRecording(const InsertRecordingInfo&); void submit(SyncToCpu = SyncToCpu::kNo); void preCompile(const PaintCombo&); @@ -106,7 +107,8 @@ protected: private: friend class ContextPriv; - std::vector> fRecordings; + sk_sp fCurrentCommandBuffer; + sk_sp fGpu; sk_sp fGlobalCache; BackendApi fBackend; diff --git a/experimental/graphite/include/GraphiteTypes.h b/experimental/graphite/include/GraphiteTypes.h index 5d8bb3e763..16c9a3f0f4 100644 --- a/experimental/graphite/include/GraphiteTypes.h +++ b/experimental/graphite/include/GraphiteTypes.h @@ -10,8 +10,21 @@ #include "include/core/SkTypes.h" +#include + namespace skgpu { +class Recording; + +using GpuFinishedContext = void*; +using GpuFinishedProc = void (*)(GpuFinishedContext finishedContext); + +struct InsertRecordingInfo { + Recording* fRecording = nullptr; + GpuFinishedContext fFinishedContext = nullptr; + GpuFinishedProc fFinishedProc = nullptr; +}; + /** * Actually submit work to the GPU and track its completion */ diff --git a/experimental/graphite/src/CommandBuffer.cpp b/experimental/graphite/src/CommandBuffer.cpp index a5e7ef218c..1950ca34a9 100644 --- a/experimental/graphite/src/CommandBuffer.cpp +++ b/experimental/graphite/src/CommandBuffer.cpp @@ -7,13 +7,13 @@ #include "experimental/graphite/src/CommandBuffer.h" -#include "experimental/graphite/src/GraphicsPipeline.h" -#include "src/core/SkTraceEvent.h" - #include "experimental/graphite/src/Buffer.h" +#include "experimental/graphite/src/GraphicsPipeline.h" #include "experimental/graphite/src/Sampler.h" #include "experimental/graphite/src/Texture.h" #include "experimental/graphite/src/TextureProxy.h" +#include "src/core/SkTraceEvent.h" +#include "src/gpu/RefCntedCallback.h" namespace skgpu { @@ -33,6 +33,14 @@ void CommandBuffer::trackResource(sk_sp resource) { fTrackedResources.push_back(std::move(resource)); } +void CommandBuffer::addFinishedProc(sk_sp finishedProc) { + fFinishedProcs.push_back(std::move(finishedProc)); +} + +void CommandBuffer::callFinishedProcs() { + fFinishedProcs.reset(); +} + bool CommandBuffer::beginRenderPass(const RenderPassDesc& renderPassDesc, sk_sp colorTexture, sk_sp resolveTexture, diff --git a/experimental/graphite/src/CommandBuffer.h b/experimental/graphite/src/CommandBuffer.h index 1c661fb789..a3be958796 100644 --- a/experimental/graphite/src/CommandBuffer.h +++ b/experimental/graphite/src/CommandBuffer.h @@ -22,6 +22,7 @@ namespace skgpu { class Buffer; class Gpu; class GraphicsPipeline; +class RefCntedCallback; class Resource; class Sampler; class Texture; @@ -75,6 +76,9 @@ public: void trackResource(sk_sp resource); + void addFinishedProc(sk_sp finishedProc); + void callFinishedProcs(); + bool beginRenderPass(const RenderPassDesc&, sk_sp colorTexture, sk_sp resolveTexture, @@ -207,6 +211,7 @@ private: inline static constexpr int kInitialTrackedResourcesCount = 32; SkSTArray> fTrackedResources; + SkTArray> fFinishedProcs; }; } // namespace skgpu diff --git a/experimental/graphite/src/Context.cpp b/experimental/graphite/src/Context.cpp index 2c953c1942..c227f3bc03 100644 --- a/experimental/graphite/src/Context.cpp +++ b/experimental/graphite/src/Context.cpp @@ -22,6 +22,7 @@ #include "src/core/SkKeyContext.h" #include "src/core/SkKeyHelpers.h" #include "src/core/SkShaderCodeDictionary.h" +#include "src/gpu/RefCntedCallback.h" #ifdef SK_METAL #include "experimental/graphite/src/mtl/MtlTrampoline.h" @@ -51,19 +52,23 @@ std::unique_ptr Context::makeRecorder() { return std::unique_ptr(new Recorder(fGpu, fGlobalCache)); } -void Context::insertRecording(std::unique_ptr recording) { - fRecordings.emplace_back(std::move(recording)); +void Context::insertRecording(const InsertRecordingInfo& info) { + SkASSERT(!fCurrentCommandBuffer); + // For now we only allow one CommandBuffer. So we just ref it off the InsertRecordingInfo and + // hold onto it until we submit. + fCurrentCommandBuffer = info.fRecording->fCommandBuffer; + if (info.fFinishedProc) { + fCurrentCommandBuffer->addFinishedProc(RefCntedCallback::Make(info.fFinishedProc, + info.fFinishedContext)); + } } void Context::submit(SyncToCpu syncToCpu) { - // TODO: we want Gpu::submit to take an array of command buffers but, for now, it just takes - // one. Once we have more than one recording queued up we will need to extract the - // command buffers and submit them as a block. - SkASSERT(fRecordings.size() == 1); - fGpu->submit(fRecordings[0]->fCommandBuffer); + SkASSERT(fCurrentCommandBuffer); + + fGpu->submit(std::move(fCurrentCommandBuffer)); fGpu->checkForFinishedWork(syncToCpu); - fRecordings.clear(); } void Context::preCompile(const PaintCombo& paintCombo) { diff --git a/experimental/graphite/src/Gpu.cpp b/experimental/graphite/src/Gpu.cpp index 6a1ebd3b7c..0f42c7a78e 100644 --- a/experimental/graphite/src/Gpu.cpp +++ b/experimental/graphite/src/Gpu.cpp @@ -25,8 +25,8 @@ namespace skgpu { static constexpr int kDefaultOutstandingAllocCnt = 8; Gpu::Gpu(sk_sp caps) - : fOutstandingSubmissions(sizeof(OutstandingSubmission), kDefaultOutstandingAllocCnt) - , fCaps(std::move(caps)) { + : fCaps(std::move(caps)) + , fOutstandingSubmissions(sizeof(OutstandingSubmission), kDefaultOutstandingAllocCnt) { // subclasses create their own subclassed resource provider } @@ -58,7 +58,13 @@ bool Gpu::submit(sk_sp commandBuffer) { } #endif - return this->onSubmit(std::move(commandBuffer)); + auto submission = this->onSubmit(std::move(commandBuffer)); + if (!submission) { + return false; + } + + new (fOutstandingSubmissions.push_back()) OutstandingSubmission(std::move(submission)); + return true; } void Gpu::checkForFinishedWork(SyncToCpu sync) { diff --git a/experimental/graphite/src/Gpu.h b/experimental/graphite/src/Gpu.h index 004be6716e..b0ebbf9b26 100644 --- a/experimental/graphite/src/Gpu.h +++ b/experimental/graphite/src/Gpu.h @@ -66,10 +66,9 @@ protected: void initCompiler(); using OutstandingSubmission = std::unique_ptr; - SkDeque fOutstandingSubmissions; private: - virtual bool onSubmit(sk_sp) = 0; + virtual OutstandingSubmission onSubmit(sk_sp) = 0; virtual BackendTexture onCreateBackendTexture(SkISize dimensions, const TextureInfo&) = 0; virtual void onDeleteBackendTexture(BackendTexture&) = 0; @@ -78,6 +77,8 @@ private: // Compiler used for compiling SkSL into backend shader code. We only want to create the // compiler once, as there is significant overhead to the first compile. std::unique_ptr fCompiler; + + SkDeque fOutstandingSubmissions; }; } // namespace skgpu diff --git a/experimental/graphite/src/GpuWorkSubmission.cpp b/experimental/graphite/src/GpuWorkSubmission.cpp new file mode 100644 index 0000000000..1d0467d82b --- /dev/null +++ b/experimental/graphite/src/GpuWorkSubmission.cpp @@ -0,0 +1,22 @@ +/* + * Copyright 2022 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "experimental/graphite/src/GpuWorkSubmission.h" + +#include "experimental/graphite/src/CommandBuffer.h" + +namespace skgpu { + +GpuWorkSubmission::GpuWorkSubmission(sk_sp cmdBuffer) + : fCommandBuffer(std::move(cmdBuffer)) {} + +GpuWorkSubmission::~GpuWorkSubmission() { + fCommandBuffer->callFinishedProcs(); +} + +} // namespace skgpu + diff --git a/experimental/graphite/src/GpuWorkSubmission.h b/experimental/graphite/src/GpuWorkSubmission.h index a4b5a7647c..31e43ccc4c 100644 --- a/experimental/graphite/src/GpuWorkSubmission.h +++ b/experimental/graphite/src/GpuWorkSubmission.h @@ -8,20 +8,26 @@ #ifndef skgpu_GpuWorkSubmission_DEFINED #define skgpu_GpuWorkSubmission_DEFINED +#include "include/core/SkRefCnt.h" + namespace skgpu { +class CommandBuffer; class Gpu; class GpuWorkSubmission { public: - virtual ~GpuWorkSubmission() = default; + virtual ~GpuWorkSubmission(); virtual bool isFinished() = 0; virtual void waitUntilFinished(const Gpu*) = 0; protected: - GpuWorkSubmission() = default; + CommandBuffer* commandBuffer() { return fCommandBuffer.get(); } + + GpuWorkSubmission(sk_sp cmdBuffer); private: + sk_sp fCommandBuffer; }; } // namespace skgpu diff --git a/experimental/graphite/src/TextureUtils.cpp b/experimental/graphite/src/TextureUtils.cpp index dab299bdea..392d1da406 100644 --- a/experimental/graphite/src/TextureUtils.cpp +++ b/experimental/graphite/src/TextureUtils.cpp @@ -154,9 +154,12 @@ bool ReadPixelsHelper(FlushPendingWorkCallback&& flushPendingWork, flushPendingWork(); recorder->priv().add(std::move(task)); + auto recording = recorder->snap(); // TODO: Can snapping ever fail? - context->insertRecording(recorder->snap()); + skgpu::InsertRecordingInfo info; + info.fRecording = recording.get(); + context->insertRecording(info); context->submit(SyncToCpu::kYes); void* mappedMemory = dstBuffer->map(); diff --git a/experimental/graphite/src/mtl/MtlGpu.h b/experimental/graphite/src/mtl/MtlGpu.h index 4fe93146f7..a9aacdaddc 100644 --- a/experimental/graphite/src/mtl/MtlGpu.h +++ b/experimental/graphite/src/mtl/MtlGpu.h @@ -35,7 +35,7 @@ public: private: Gpu(sk_cfp>, sk_cfp>, sk_sp); - bool onSubmit(sk_sp) override; + skgpu::Gpu::OutstandingSubmission onSubmit(sk_sp) override; BackendTexture onCreateBackendTexture(SkISize dimensions, const skgpu::TextureInfo&) override; void onDeleteBackendTexture(BackendTexture&) override; diff --git a/experimental/graphite/src/mtl/MtlGpu.mm b/experimental/graphite/src/mtl/MtlGpu.mm index 24e14eebe8..db8b0e35b5 100644 --- a/experimental/graphite/src/mtl/MtlGpu.mm +++ b/experimental/graphite/src/mtl/MtlGpu.mm @@ -60,31 +60,29 @@ std::unique_ptr Gpu::makeResourceProvider( class WorkSubmission final : public skgpu::GpuWorkSubmission { public: WorkSubmission(sk_sp cmdBuffer) - : fCommandBuffer(std::move(cmdBuffer)) {} + : GpuWorkSubmission(std::move(cmdBuffer)) {} ~WorkSubmission() override {} bool isFinished() override { - return fCommandBuffer->isFinished(); + return static_cast(this->commandBuffer())->isFinished(); } void waitUntilFinished(const skgpu::Gpu*) override { - return fCommandBuffer->waitUntilFinished(); + return static_cast(this->commandBuffer())->waitUntilFinished(); } private: sk_sp fCommandBuffer; }; -bool Gpu::onSubmit(sk_sp commandBuffer) { +skgpu::Gpu::OutstandingSubmission Gpu::onSubmit(sk_sp commandBuffer) { SkASSERT(commandBuffer); sk_sp& mtlCmdBuffer = (sk_sp&)(commandBuffer); if (!mtlCmdBuffer->commit()) { - return false; + return nullptr; } - std::unique_ptr submission(new WorkSubmission(mtlCmdBuffer)); - new (fOutstandingSubmissions.push_back()) OutstandingSubmission(std::move(submission)); - - return true; + std::unique_ptr submission(new WorkSubmission(mtlCmdBuffer)); + return submission; } BackendTexture Gpu::onCreateBackendTexture(SkISize dimensions, const skgpu::TextureInfo& info) { diff --git a/gn/graphite.gni b/gn/graphite.gni index b657deea09..5a8b852512 100644 --- a/gn/graphite.gni +++ b/gn/graphite.gni @@ -52,6 +52,7 @@ skia_graphite_sources = [ "$_src/GlobalCache.h", "$_src/Gpu.cpp", "$_src/Gpu.h", + "$_src/GpuWorkSubmission.cpp", "$_src/GpuWorkSubmission.h", "$_src/GraphicsPipeline.cpp", "$_src/GraphicsPipeline.h", diff --git a/tools/sk_app/GraphiteMetalWindowContext.mm b/tools/sk_app/GraphiteMetalWindowContext.mm index d2309beb95..7f3e336a43 100644 --- a/tools/sk_app/GraphiteMetalWindowContext.mm +++ b/tools/sk_app/GraphiteMetalWindowContext.mm @@ -100,7 +100,9 @@ void GraphiteMetalWindowContext::swapBuffers() { // This chunk of code should not be in this class but higher up either in Window or // WindowContext std::unique_ptr recording = fGraphiteRecorder->snap(); - fGraphiteContext->insertRecording(std::move(recording)); + skgpu::InsertRecordingInfo info; + info.fRecording = recording.get(); + fGraphiteContext->insertRecording(info); fGraphiteContext->submit(skgpu::SyncToCpu::kNo); id currentDrawable = (id)fDrawableHandle;