From f9c89a443dc8a5ec924b38513e600e1454039389 Mon Sep 17 00:00:00 2001 From: James Godfrey-Kittle Date: Thu, 28 Apr 2022 15:40:44 -0400 Subject: [PATCH] [graphite] Add UploadBufferManager class. This class manages and suballocates buffers used to upload textures. It maintains a reusable, suballocated buffer for small allocations, and creates dedicated buffers for any allocation larger than its set reusable buffer size. Change-Id: If7877faed870afbc85635ae47553000fa3487aba Reviewed-on: https://skia-review.googlesource.com/c/skia/+/534941 Reviewed-by: Jim Van Verth Commit-Queue: James Godfrey-Kittle Reviewed-by: Greg Daniel --- gn/graphite.gni | 2 + include/gpu/graphite/Recorder.h | 2 + src/gpu/BUILD.bazel | 1 + src/gpu/BufferWriter.h | 29 ++++++++- src/gpu/graphite/BUILD.bazel | 26 +++++++- src/gpu/graphite/CommandBuffer.cpp | 5 +- src/gpu/graphite/CommandBuffer.h | 2 +- src/gpu/graphite/Recorder.cpp | 3 + src/gpu/graphite/RecorderPriv.cpp | 4 ++ src/gpu/graphite/RecorderPriv.h | 1 + src/gpu/graphite/UploadBufferManager.cpp | 82 ++++++++++++++++++++++++ src/gpu/graphite/UploadBufferManager.h | 45 +++++++++++++ src/gpu/graphite/UploadTask.cpp | 39 +++++------ src/gpu/graphite/UploadTask.h | 4 +- 14 files changed, 213 insertions(+), 32 deletions(-) create mode 100644 src/gpu/graphite/UploadBufferManager.cpp create mode 100644 src/gpu/graphite/UploadBufferManager.h diff --git a/gn/graphite.gni b/gn/graphite.gni index 0976c2f8a6..dd5836d21f 100644 --- a/gn/graphite.gni +++ b/gn/graphite.gni @@ -101,6 +101,8 @@ skia_graphite_sources = [ "$_src/TextureUtils.h", "$_src/UniformManager.cpp", "$_src/UniformManager.h", + "$_src/UploadBufferManager.cpp", + "$_src/UploadBufferManager.h", "$_src/UploadTask.cpp", "$_src/UploadTask.h", "$_src/geom/BoundsManager.h", diff --git a/include/gpu/graphite/Recorder.h b/include/gpu/graphite/Recorder.h index a910316abe..bf7ada1bd5 100644 --- a/include/gpu/graphite/Recorder.h +++ b/include/gpu/graphite/Recorder.h @@ -29,6 +29,7 @@ class Recording; class ResourceProvider; class Task; class TaskGraph; +class UploadBufferManager; template class PipelineDataCache; using UniformDataCache = PipelineDataCache; @@ -89,6 +90,7 @@ private: std::unique_ptr fUniformDataCache; std::unique_ptr fTextureDataCache; std::unique_ptr fDrawBufferManager; + std::unique_ptr fUploadBufferManager; std::vector fTrackedDevices; // In debug builds we guard against improper thread handling diff --git a/src/gpu/BUILD.bazel b/src/gpu/BUILD.bazel index 5986b694cb..9e662fb3d6 100644 --- a/src/gpu/BUILD.bazel +++ b/src/gpu/BUILD.bazel @@ -42,6 +42,7 @@ generated_cc_atom( "//include/private:SkColorData_hdr", "//include/private:SkNx_hdr", "//include/private:SkTemplates_hdr", + "//src/core:SkConvertPixels_hdr", ], ) diff --git a/src/gpu/BufferWriter.h b/src/gpu/BufferWriter.h index 9c7f81c19c..8662f7cefd 100644 --- a/src/gpu/BufferWriter.h +++ b/src/gpu/BufferWriter.h @@ -8,11 +8,12 @@ #ifndef skgpu_BufferWriter_DEFINED #define skgpu_BufferWriter_DEFINED +#include #include "include/core/SkRect.h" #include "include/private/SkColorData.h" #include "include/private/SkNx.h" #include "include/private/SkTemplates.h" -#include +#include "src/core/SkConvertPixels.h" namespace skgpu { @@ -430,6 +431,32 @@ struct UniformWriter : public BufferWriter { } }; +/////////////////////////////////////////////////////////////////////////////////////////////////// + +struct UploadWriter : public BufferWriter { + UploadWriter() = default; + + UploadWriter(void* ptr, size_t size) : BufferWriter(ptr, size) {} + + UploadWriter(const UploadWriter&) = delete; + UploadWriter(UploadWriter&& that) { *this = std::move(that); } + + UploadWriter& operator=(const UploadWriter&) = delete; + UploadWriter& operator=(UploadWriter&& that) { + BufferWriter::operator=(std::move(that)); + return *this; + } + + // Writes a block of image data to the upload buffer, starting at `offset`. The source image is + // `srcRowBytes` wide, and the written block is `trimRowBytes` wide and `rowCount` bytes tall. + void write( + size_t offset, const void* src, size_t srcRowBytes, size_t trimRowBytes, int rowCount) { + this->validate(trimRowBytes * rowCount); + void* dst = SkTAddOffset(fPtr, offset); + SkRectMemcpy(dst, trimRowBytes, src, srcRowBytes, trimRowBytes, rowCount); + } +}; + } // namespace skgpu #endif // skgpu_BufferWriter_DEFINED diff --git a/src/gpu/graphite/BUILD.bazel b/src/gpu/graphite/BUILD.bazel index d5686e75a8..092c0c147a 100644 --- a/src/gpu/graphite/BUILD.bazel +++ b/src/gpu/graphite/BUILD.bazel @@ -533,6 +533,7 @@ generated_cc_atom( ":PipelineDataCache_hdr", ":ResourceProvider_hdr", ":TaskGraph_hdr", + ":UploadBufferManager_hdr", "//include/gpu/graphite:Recorder_hdr", "//include/gpu/graphite:Recording_hdr", "//src/core:SkPipelineData_hdr", @@ -933,6 +934,29 @@ generated_cc_atom( ], ) +generated_cc_atom( + name = "UploadBufferManager_hdr", + hdrs = ["UploadBufferManager.h"], + visibility = ["//:__subpackages__"], + deps = [ + ":DrawTypes_hdr", + "//include/core:SkRefCnt_hdr", + "//src/gpu:BufferWriter_hdr", + ], +) + +generated_cc_atom( + name = "UploadBufferManager_src", + srcs = ["UploadBufferManager.cpp"], + visibility = ["//:__subpackages__"], + deps = [ + ":Buffer_hdr", + ":CommandBuffer_hdr", + ":ResourceProvider_hdr", + ":UploadBufferManager_hdr", + ], +) + generated_cc_atom( name = "UploadTask_hdr", hdrs = ["UploadTask.h"], @@ -958,9 +982,9 @@ generated_cc_atom( ":ResourceProvider_hdr", ":TextureProxy_hdr", ":Texture_hdr", + ":UploadBufferManager_hdr", ":UploadTask_hdr", "//include/gpu/graphite:Recorder_hdr", - "//src/core:SkConvertPixels_hdr", "//src/core:SkTraceEvent_hdr", ], ) diff --git a/src/gpu/graphite/CommandBuffer.cpp b/src/gpu/graphite/CommandBuffer.cpp index 14485c8c2b..dbefd46ff2 100644 --- a/src/gpu/graphite/CommandBuffer.cpp +++ b/src/gpu/graphite/CommandBuffer.cpp @@ -143,7 +143,7 @@ bool CommandBuffer::copyTextureToBuffer(sk_sp texture, return true; } -bool CommandBuffer::copyBufferToTexture(sk_sp buffer, +bool CommandBuffer::copyBufferToTexture(const Buffer* buffer, sk_sp texture, const BufferTextureCopyData* copyData, int count) { @@ -151,11 +151,10 @@ bool CommandBuffer::copyBufferToTexture(sk_sp buffer, SkASSERT(texture); SkASSERT(count > 0 && copyData); - if (!this->onCopyBufferToTexture(buffer.get(), texture.get(), copyData, count)) { + if (!this->onCopyBufferToTexture(buffer, texture.get(), copyData, count)) { return false; } - this->trackResource(std::move(buffer)); this->trackResource(std::move(texture)); SkDEBUGCODE(fHasWork = true;) diff --git a/src/gpu/graphite/CommandBuffer.h b/src/gpu/graphite/CommandBuffer.h index 1379fc4b3a..aa1316e36c 100644 --- a/src/gpu/graphite/CommandBuffer.h +++ b/src/gpu/graphite/CommandBuffer.h @@ -149,7 +149,7 @@ public: sk_sp, size_t bufferOffset, size_t bufferRowBytes); - bool copyBufferToTexture(sk_sp, + bool copyBufferToTexture(const Buffer*, sk_sp, const BufferTextureCopyData*, int count); diff --git a/src/gpu/graphite/Recorder.cpp b/src/gpu/graphite/Recorder.cpp index a51c939b7f..550b5a4a01 100644 --- a/src/gpu/graphite/Recorder.cpp +++ b/src/gpu/graphite/Recorder.cpp @@ -19,6 +19,7 @@ #include "src/gpu/graphite/PipelineDataCache.h" #include "src/gpu/graphite/ResourceProvider.h" #include "src/gpu/graphite/TaskGraph.h" +#include "src/gpu/graphite/UploadBufferManager.h" namespace skgpu::graphite { @@ -33,6 +34,7 @@ Recorder::Recorder(sk_sp gpu, sk_sp globalCache) fResourceProvider = fGpu->makeResourceProvider(std::move(globalCache), this->singleOwner()); fDrawBufferManager.reset(new DrawBufferManager(fResourceProvider.get(), fGpu->caps()->requiredUniformBufferAlignment())); + fUploadBufferManager.reset(new UploadBufferManager(fResourceProvider.get())); SkASSERT(fResourceProvider); } @@ -66,6 +68,7 @@ std::unique_ptr Recorder::snap() { } fDrawBufferManager->transferToCommandBuffer(commandBuffer.get()); + fUploadBufferManager->transferToCommandBuffer(commandBuffer.get()); fGraph->reset(); std::unique_ptr recording(new Recording(std::move(commandBuffer), diff --git a/src/gpu/graphite/RecorderPriv.cpp b/src/gpu/graphite/RecorderPriv.cpp index a2d0edf337..9c26f5a3e0 100644 --- a/src/gpu/graphite/RecorderPriv.cpp +++ b/src/gpu/graphite/RecorderPriv.cpp @@ -35,6 +35,10 @@ DrawBufferManager* RecorderPriv::drawBufferManager() const { return fRecorder->fDrawBufferManager.get(); } +UploadBufferManager* RecorderPriv::uploadBufferManager() const { + return fRecorder->fUploadBufferManager.get(); +} + void RecorderPriv::add(sk_sp task) { ASSERT_SINGLE_OWNER fRecorder->fGraph->add(std::move(task)); diff --git a/src/gpu/graphite/RecorderPriv.h b/src/gpu/graphite/RecorderPriv.h index 17f6c69ce8..6b6fc00e63 100644 --- a/src/gpu/graphite/RecorderPriv.h +++ b/src/gpu/graphite/RecorderPriv.h @@ -20,6 +20,7 @@ public: UniformDataCache* uniformDataCache() const; TextureDataCache* textureDataCache() const; DrawBufferManager* drawBufferManager() const; + UploadBufferManager* uploadBufferManager() const; const Caps* caps() const; void flushTrackedDevices(); diff --git a/src/gpu/graphite/UploadBufferManager.cpp b/src/gpu/graphite/UploadBufferManager.cpp new file mode 100644 index 0000000000..12e3c9f66c --- /dev/null +++ b/src/gpu/graphite/UploadBufferManager.cpp @@ -0,0 +1,82 @@ +/* + * 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 "src/gpu/graphite/UploadBufferManager.h" + +#include "src/gpu/graphite/Buffer.h" +#include "src/gpu/graphite/CommandBuffer.h" +#include "src/gpu/graphite/ResourceProvider.h" + +namespace skgpu::graphite { + +static constexpr size_t kReusedBufferSize = 64 << 10; // 64 KB + +UploadBufferManager::UploadBufferManager(ResourceProvider* resourceProvider) + : fResourceProvider(resourceProvider) {} + +UploadBufferManager::~UploadBufferManager() {} + +std::tuple UploadBufferManager::getUploadWriter( + size_t requiredBytes, size_t requiredAlignment) { + if (!requiredBytes) { + return {UploadWriter(), BindBufferInfo()}; + } + + if (requiredBytes > kReusedBufferSize) { + // Create a dedicated buffer for this request. + sk_sp buffer = fResourceProvider->findOrCreateBuffer( + requiredBytes, BufferType::kXferCpuToGpu, PrioritizeGpuReads::kNo); + + BindBufferInfo bindInfo; + bindInfo.fBuffer = buffer.get(); + bindInfo.fOffset = 0; + + void* bufferMapPtr = buffer->map(); + fUsedBuffers.push_back(std::move(buffer)); + return {UploadWriter(bufferMapPtr, requiredBytes), bindInfo}; + } + + // Try to reuse an already-allocated buffer. + fReusedBufferOffset = SkAlignTo(fReusedBufferOffset, requiredAlignment); + if (fReusedBuffer && requiredBytes > fReusedBuffer->size() - fReusedBufferOffset) { + fUsedBuffers.push_back(std::move(fReusedBuffer)); + fReusedBufferOffset = 0; + } + + if (!fReusedBuffer) { + fReusedBuffer = fResourceProvider->findOrCreateBuffer( + kReusedBufferSize, BufferType::kXferCpuToGpu, PrioritizeGpuReads::kNo); + if (!fReusedBuffer) { + return {UploadWriter(), BindBufferInfo()}; + } + } + + BindBufferInfo bindInfo; + bindInfo.fBuffer = fReusedBuffer.get(); + bindInfo.fOffset = fReusedBufferOffset; + + void* bufferMapPtr = fReusedBuffer->map(); + bufferMapPtr = SkTAddOffset(bufferMapPtr, fReusedBufferOffset); + + fReusedBufferOffset += requiredBytes; + return {UploadWriter(bufferMapPtr, requiredBytes), bindInfo}; +} + +void UploadBufferManager::transferToCommandBuffer(CommandBuffer* commandBuffer) { + for (sk_sp& buffer : fUsedBuffers) { + buffer->unmap(); + commandBuffer->trackResource(std::move(buffer)); + } + fUsedBuffers.clear(); + + if (fReusedBuffer) { + fReusedBuffer->unmap(); + commandBuffer->trackResource(std::move(fReusedBuffer)); + } +} + +} // namespace skgpu::graphite diff --git a/src/gpu/graphite/UploadBufferManager.h b/src/gpu/graphite/UploadBufferManager.h new file mode 100644 index 0000000000..5ef48d6b27 --- /dev/null +++ b/src/gpu/graphite/UploadBufferManager.h @@ -0,0 +1,45 @@ +/* + * 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 skgpu_graphite_UploadBufferManager_DEFINED +#define skgpu_graphite_UploadBufferManager_DEFINED + +#include + +#include "include/core/SkRefCnt.h" +#include "src/gpu/BufferWriter.h" +#include "src/gpu/graphite/DrawTypes.h" + +namespace skgpu::graphite { + +class Buffer; +class CommandBuffer; +class ResourceProvider; + +class UploadBufferManager { +public: + UploadBufferManager(ResourceProvider*); + ~UploadBufferManager(); + + std::tuple getUploadWriter(size_t requiredBytes, + size_t requiredAlignment); + + // Finalizes all buffers and transfers ownership of them to the CommandBuffer. + void transferToCommandBuffer(CommandBuffer*); + +private: + ResourceProvider* fResourceProvider; + + sk_sp fReusedBuffer; + size_t fReusedBufferOffset = 0; + + std::vector> fUsedBuffers; +}; + +} // namespace skgpu::graphite + +#endif // skgpu_graphite_UploadBufferManager_DEFINED diff --git a/src/gpu/graphite/UploadTask.cpp b/src/gpu/graphite/UploadTask.cpp index f990581f3b..8b508ae640 100644 --- a/src/gpu/graphite/UploadTask.cpp +++ b/src/gpu/graphite/UploadTask.cpp @@ -8,7 +8,6 @@ #include "src/gpu/graphite/UploadTask.h" #include "include/gpu/graphite/Recorder.h" -#include "src/core/SkConvertPixels.h" #include "src/core/SkTraceEvent.h" #include "src/gpu/graphite/Buffer.h" #include "src/gpu/graphite/Caps.h" @@ -18,15 +17,14 @@ #include "src/gpu/graphite/ResourceProvider.h" #include "src/gpu/graphite/Texture.h" #include "src/gpu/graphite/TextureProxy.h" +#include "src/gpu/graphite/UploadBufferManager.h" namespace skgpu::graphite { -UploadInstance::UploadInstance(sk_sp buffer, +UploadInstance::UploadInstance(const Buffer* buffer, sk_sp textureProxy, std::vector copyData) - : fBuffer(buffer) - , fTextureProxy(textureProxy) - , fCopyData(copyData) {} + : fBuffer(buffer), fTextureProxy(textureProxy), fCopyData(copyData) {} size_t compute_combined_buffer_size(int mipLevelCount, size_t bytesPerPixel, @@ -98,33 +96,28 @@ UploadInstance UploadInstance::Make(Recorder* recorder, dstRect.size(), &individualMipOffsets); SkASSERT(combinedBufferSize); - // TODO: get staging buffer or {void* offset, sk_sp buffer} pair. - ResourceProvider* resourceProvider = recorder->priv().resourceProvider(); - sk_sp buffer = resourceProvider->findOrCreateBuffer(combinedBufferSize, - BufferType::kXferCpuToGpu, - PrioritizeGpuReads::kNo); + UploadBufferManager* bufferMgr = recorder->priv().uploadBufferManager(); + auto [writer, bufferInfo] = bufferMgr->getUploadWriter(combinedBufferSize, minAlignment); std::vector copyData(mipLevelCount); - if (!buffer) { + if (!bufferInfo.fBuffer) { return {}; } - char* bufferData = (char*) buffer->map(); // TODO: get from staging buffer instead - size_t baseOffset = 0; + size_t baseOffset = bufferInfo.fOffset; int currentWidth = dstRect.width(); int currentHeight = dstRect.height(); for (unsigned int currentMipLevel = 0; currentMipLevel < mipLevelCount; currentMipLevel++) { const size_t trimRowBytes = currentWidth * bpp; const size_t rowBytes = levels[currentMipLevel].fRowBytes; + const size_t mipOffset = individualMipOffsets[currentMipLevel]; // copy data into the buffer, skipping any trailing bytes - char* dst = bufferData + individualMipOffsets[currentMipLevel]; const char* src = (const char*)levels[currentMipLevel].fPixels; - SkRectMemcpy(dst, trimRowBytes, src, rowBytes, trimRowBytes, currentHeight); + writer.write(mipOffset, src, rowBytes, trimRowBytes, currentHeight); - copyData[currentMipLevel].fBufferOffset = - baseOffset + individualMipOffsets[currentMipLevel]; + copyData[currentMipLevel].fBufferOffset = baseOffset + mipOffset; copyData[currentMipLevel].fBufferRowBytes = trimRowBytes; copyData[currentMipLevel].fRect = { dstRect.left(), dstRect.top(), // TODO: can we recompute this for mips? @@ -136,13 +129,11 @@ UploadInstance UploadInstance::Make(Recorder* recorder, currentHeight = std::max(1, currentHeight/2); } - buffer->unmap(); - ATRACE_ANDROID_FRAMEWORK("Upload %sTexture [%ux%u]", mipLevelCount > 1 ? "MipMap " : "", dstRect.width(), dstRect.height()); - return {std::move(buffer), std::move(textureProxy), std::move(copyData)}; + return {bufferInfo.fBuffer, std::move(textureProxy), std::move(copyData)}; } void UploadInstance::addCommand(ResourceProvider* resourceProvider, @@ -156,10 +147,10 @@ void UploadInstance::addCommand(ResourceProvider* resourceProvider, return; } - commandBuffer->copyBufferToTexture(std::move(fBuffer), - fTextureProxy->refTexture(), - fCopyData.data(), - fCopyData.size()); + // The CommandBuffer doesn't take ownership of the upload buffer here; it's owned by + // UploadBufferManager, which will transfer ownership in transferToCommandBuffer. + commandBuffer->copyBufferToTexture( + fBuffer, fTextureProxy->refTexture(), fCopyData.data(), fCopyData.size()); } //--------------------------------------------------------------------------- diff --git a/src/gpu/graphite/UploadTask.h b/src/gpu/graphite/UploadTask.h index 6bd64ac0c7..307fd1bdf7 100644 --- a/src/gpu/graphite/UploadTask.h +++ b/src/gpu/graphite/UploadTask.h @@ -49,9 +49,9 @@ public: private: UploadInstance() {} - UploadInstance(sk_sp, sk_sp, std::vector); + UploadInstance(const Buffer*, sk_sp, std::vector); - sk_sp fBuffer; + const Buffer* fBuffer; sk_sp fTextureProxy; std::vector fCopyData; };