Revert "Make GrRingBuffer more generic so it can be used over all backends"
This reverts commit b6d4ad92adcac186c3bc52feef85de38752a1f1d.
Reason for revert: Breaking the bots
Original change's description:
> Make GrRingBuffer more generic so it can be used over all backends
>
> * Removes the spinlock (should no longer be necessary)
> * Uses GrGpuBuffer and creation through GrResourceProvider
> instead of internal native creation
> * Changes the SubmitData to pass up all buffers used over a given submit
> so the backend can track that better
>
> Bug: skia:10530
> Change-Id: I0d3a686b950dd5cb5f720f827b573238386b9524
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305567
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
TBR=egdaniel@google.com,jvanverth@google.com
Change-Id: Ief7df521f66e85d9bfb85508e5a5f8223d7d6725
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10530
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305720
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
diff --git a/src/gpu/GrRingBuffer.cpp b/src/gpu/GrRingBuffer.cpp
index a911c20..145c88a 100644
--- a/src/gpu/GrRingBuffer.cpp
+++ b/src/gpu/GrRingBuffer.cpp
@@ -7,15 +7,12 @@
#include "src/gpu/GrRingBuffer.h"
-#include "src/gpu/GrContextPriv.h"
-#include "src/gpu/GrGpu.h"
-#include "src/gpu/GrResourceProvider.h"
-
// Get offset into buffer that has enough space for size
// Returns fTotalSize if no space
size_t GrRingBuffer::getAllocationOffset(size_t size) {
// capture current state locally (because fTail could be overwritten by the completion handler)
size_t head, tail;
+ SkAutoSpinlock lock(fMutex);
head = fHead;
tail = fTail;
@@ -55,40 +52,39 @@
}
GrRingBuffer::Slice GrRingBuffer::suballocate(size_t size) {
- if (fCurrentBuffer) {
- size_t offset = this->getAllocationOffset(size);
- if (offset < fTotalSize) {
- return { fCurrentBuffer.get(), offset };
- }
-
- // Try to grow allocation (old allocation will age out).
- fTotalSize *= 2;
+ size_t offset = this->getAllocationOffset(size);
+ if (offset < fTotalSize) {
+ return { fBuffer, offset };
}
- GrResourceProvider* resourceProvider = fGpu->getContext()->priv().resourceProvider();
- fCurrentBuffer = resourceProvider->createBuffer(fTotalSize, fType, kDynamic_GrAccessPattern);
-
- SkASSERT(fCurrentBuffer);
- fTrackedBuffers.push_back(fCurrentBuffer);
- fHead = 0;
- fTail = 0;
- fGenID++;
- size_t offset = this->getAllocationOffset(size);
+ // Try to grow allocation (old allocation will age out).
+ fTotalSize *= 2;
+ fBuffer = this->createBuffer(fTotalSize);
+ SkASSERT(fBuffer);
+ {
+ SkAutoSpinlock lock(fMutex);
+ fHead = 0;
+ fTail = 0;
+ fGenID++;
+ }
+ offset = this->getAllocationOffset(size);
SkASSERT(offset < fTotalSize);
- return { fCurrentBuffer.get(), offset };
+ return { fBuffer, offset };
}
// used when current command buffer/command list is submitted
-void GrRingBuffer::startSubmit(GrRingBuffer::SubmitData* submitData) {
- submitData->fTrackedBuffers = std::move(fTrackedBuffers);
- submitData->fLastHead = fHead;
- submitData->fGenID = fGenID;
- // add current buffer to be tracked for next submit
- fTrackedBuffers.push_back(fCurrentBuffer);
+GrRingBuffer::SubmitData GrRingBuffer::startSubmit() {
+ SubmitData submitData;
+ SkAutoSpinlock lock(fMutex);
+ submitData.fBuffer = fBuffer;
+ submitData.fLastHead = fHead;
+ submitData.fGenID = fGenID;
+ return submitData;
}
// used when current command buffer/command list is completed
void GrRingBuffer::finishSubmit(const GrRingBuffer::SubmitData& submitData) {
+ SkAutoSpinlock lock(fMutex);
if (submitData.fGenID == fGenID) {
fTail = submitData.fLastHead;
}
diff --git a/src/gpu/GrRingBuffer.h b/src/gpu/GrRingBuffer.h
index caa2311..e46afb7 100644
--- a/src/gpu/GrRingBuffer.h
+++ b/src/gpu/GrRingBuffer.h
@@ -10,9 +10,7 @@
#include "src/gpu/GrGpuBuffer.h"
-#include <vector>
-
-class GrGpu;
+#include "include/private/SkSpinlock.h"
/**
* A wrapper for a GPU buffer that allocates slices in a continuous ring.
@@ -20,13 +18,12 @@
* It's assumed that suballocate and startSubmit are always called in the same thread,
* and that finishSubmit could be called in a separate thread.
*/
-class GrRingBuffer {
+class GrRingBuffer : public SkRefCnt {
public:
- GrRingBuffer(GrGpu* gpu, size_t size, size_t alignment, GrGpuBufferType intendedType)
- : fGpu(gpu)
+ GrRingBuffer(sk_sp<GrGpuBuffer> buffer, size_t size, size_t alignment)
+ : fBuffer(std::move(buffer))
, fTotalSize(size)
, fAlignment(alignment)
- , fType(intendedType)
, fHead(0)
, fTail(0)
, fGenID(0) {
@@ -36,39 +33,39 @@
}
struct Slice {
- GrGpuBuffer* fBuffer;
+ sk_sp<GrGpuBuffer> fBuffer;
size_t fOffset;
};
+
Slice suballocate(size_t size);
class SubmitData {
public:
- const GrRingBuffer* fRingBuffer;
- std::vector<sk_sp<GrGpuBuffer>> fTrackedBuffers;
+ GrGpuBuffer* buffer() const { return fBuffer.get(); }
private:
friend class GrRingBuffer;
+ sk_sp<GrGpuBuffer> fBuffer;
size_t fLastHead;
size_t fGenID;
};
// Backends should call startSubmit() at submit time, and finishSubmit() when the
// command buffer/list finishes.
- void startSubmit(SubmitData*);
+ SubmitData startSubmit();
void finishSubmit(const SubmitData&);
size_t size() const { return fTotalSize; }
private:
+ virtual sk_sp<GrGpuBuffer> createBuffer(size_t size) = 0;
size_t getAllocationOffset(size_t size);
- GrGpu* fGpu;
- sk_sp<GrGpuBuffer> fCurrentBuffer;
- std::vector<sk_sp<GrGpuBuffer>> fTrackedBuffers; // all buffers we've used in this submit
+ sk_sp<GrGpuBuffer> fBuffer;
size_t fTotalSize;
size_t fAlignment;
- GrGpuBufferType fType;
- size_t fHead; // where we start allocating
- size_t fTail; // where we start deallocating
- uint64_t fGenID; // incremented when createBuffer is called
+ size_t fHead SK_GUARDED_BY(fMutex); // where we start allocating
+ size_t fTail SK_GUARDED_BY(fMutex); // where we start deallocating
+ uint64_t fGenID SK_GUARDED_BY(fMutex); // incremented when createBuffer is called
+ SkSpinlock fMutex;
};
#endif
diff --git a/src/gpu/d3d/GrD3DCommandList.cpp b/src/gpu/d3d/GrD3DCommandList.cpp
index 19e64a5..e2e8019 100644
--- a/src/gpu/d3d/GrD3DCommandList.cpp
+++ b/src/gpu/d3d/GrD3DCommandList.cpp
@@ -245,15 +245,13 @@
}
}
-void GrD3DDirectCommandList::setCurrentConstantBuffer(GrRingBuffer* constantsRingBuffer) {
- fCurrentConstantRingBuffer = constantsRingBuffer;
+void GrD3DDirectCommandList::setCurrentConstantBuffer(
+ const sk_sp<GrD3DConstantRingBuffer>& constantBuffer) {
+ fCurrentConstantRingBuffer = constantBuffer.get();
if (fCurrentConstantRingBuffer) {
- constantsRingBuffer->startSubmit(&fConstantRingBufferSubmitData);
- for (unsigned int i = 0; i < fConstantRingBufferSubmitData.fTrackedBuffers.size(); ++i) {
- this->addGrBuffer(std::move(fConstantRingBufferSubmitData.fTrackedBuffers[i]));
- }
- // we don't need these any more so clear this copy out
- fConstantRingBufferSubmitData.fTrackedBuffers.clear();
+ fConstantRingBufferSubmitData = constantBuffer->startSubmit();
+ this->addResource(
+ static_cast<GrD3DBuffer*>(fConstantRingBufferSubmitData.buffer())->resource());
}
}
diff --git a/src/gpu/d3d/GrD3DCommandList.h b/src/gpu/d3d/GrD3DCommandList.h
index ea9ccf6..2e6af9a 100644
--- a/src/gpu/d3d/GrD3DCommandList.h
+++ b/src/gpu/d3d/GrD3DCommandList.h
@@ -12,7 +12,7 @@
#include "include/gpu/d3d/GrD3DTypes.h"
#include "include/private/SkColorData.h"
#include "src/gpu/GrManagedResource.h"
-#include "src/gpu/GrRingBuffer.h"
+#include "src/gpu/d3d/GrD3DConstantRingBuffer.h"
#include "src/gpu/d3d/GrD3DRootSignature.h"
#include <memory>
@@ -140,7 +140,7 @@
void setPipelineState(sk_sp<GrD3DPipelineState> pipelineState);
- void setCurrentConstantBuffer(GrRingBuffer* constantsRingBuffer);
+ void setCurrentConstantBuffer(const sk_sp<GrD3DConstantRingBuffer>& constantBuffer);
void setStencilRef(unsigned int stencilRef);
void setBlendFactor(const float blendFactor[4]);
@@ -195,8 +195,8 @@
size_t fCurrentInstanceStride;
const GrBuffer* fCurrentIndexBuffer;
- GrRingBuffer* fCurrentConstantRingBuffer;
- GrRingBuffer::SubmitData fConstantRingBufferSubmitData;
+ GrD3DConstantRingBuffer* fCurrentConstantRingBuffer;
+ GrD3DConstantRingBuffer::SubmitData fConstantRingBufferSubmitData;
D3D12_GPU_VIRTUAL_ADDRESS fCurrentConstantBufferAddress;
D3D12_GPU_DESCRIPTOR_HANDLE fCurrentRootDescriptorTable[GrD3DRootSignature::kParamIndexCount];
diff --git a/src/gpu/d3d/GrD3DConstantRingBuffer.cpp b/src/gpu/d3d/GrD3DConstantRingBuffer.cpp
new file mode 100644
index 0000000..cf719c7
--- /dev/null
+++ b/src/gpu/d3d/GrD3DConstantRingBuffer.cpp
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2020 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/d3d/GrD3DConstantRingBuffer.h"
+
+#include "src/gpu/d3d/GrD3DBuffer.h"
+#include "src/gpu/d3d/GrD3DGpu.h"
+
+sk_sp<GrD3DConstantRingBuffer> GrD3DConstantRingBuffer::Make(GrD3DGpu* gpu, size_t size,
+ size_t alignment) {
+ sk_sp<GrGpuBuffer> buffer = GrD3DBuffer::Make(gpu, size, GrGpuBufferType::kVertex,
+ kDynamic_GrAccessPattern);
+ if (!buffer) {
+ return nullptr;
+ }
+
+ return sk_sp<GrD3DConstantRingBuffer>(new GrD3DConstantRingBuffer(std::move(buffer), size,
+ alignment, gpu));
+}
+
+sk_sp<GrGpuBuffer> GrD3DConstantRingBuffer::createBuffer(size_t size) {
+ // Make sure the old buffer is added to the current command list
+ fGpu->resourceProvider().prepForSubmit();
+
+ return GrD3DBuffer::Make(fGpu, size, GrGpuBufferType::kVertex, kDynamic_GrAccessPattern);
+}
diff --git a/src/gpu/d3d/GrD3DConstantRingBuffer.h b/src/gpu/d3d/GrD3DConstantRingBuffer.h
new file mode 100644
index 0000000..0a4bbf6
--- /dev/null
+++ b/src/gpu/d3d/GrD3DConstantRingBuffer.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2020 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrD3DConstantRingBuffer_DEFINED
+
+#define GrD3DConstantRingBuffer_DEFINED
+
+#include "src/gpu/GrRingBuffer.h"
+
+class GrD3DGpu;
+
+class GrD3DConstantRingBuffer : public GrRingBuffer {
+public:
+ static sk_sp<GrD3DConstantRingBuffer> Make(GrD3DGpu* gpu, size_t size, size_t alignment);
+
+private:
+ GrD3DConstantRingBuffer(sk_sp<GrGpuBuffer> buffer, size_t size, size_t alignment, GrD3DGpu* gpu)
+ : INHERITED(std::move(buffer), size, alignment)
+ , fGpu(gpu) {}
+ ~GrD3DConstantRingBuffer() override = default;
+
+ sk_sp<GrGpuBuffer> createBuffer(size_t size) override;
+
+ GrD3DGpu* fGpu;
+
+ typedef GrRingBuffer INHERITED;
+};
+
+#endif
diff --git a/src/gpu/d3d/GrD3DGpu.cpp b/src/gpu/d3d/GrD3DGpu.cpp
index 74d91f5..e852e17 100644
--- a/src/gpu/d3d/GrD3DGpu.cpp
+++ b/src/gpu/d3d/GrD3DGpu.cpp
@@ -39,17 +39,14 @@
// command lists we expect to see.
static const int kDefaultOutstandingAllocCnt = 8;
-// constants have to be aligned to 256
-constexpr int kConstantAlignment = 256;
-
GrD3DGpu::GrD3DGpu(GrDirectContext* direct, const GrContextOptions& contextOptions,
const GrD3DBackendContext& backendContext)
: INHERITED(direct)
, fDevice(backendContext.fDevice)
+
, fQueue(backendContext.fQueue)
, fResourceProvider(this)
, fStagingBufferManager(this)
- , fConstantsRingBuffer(this, 128 * 1024, kConstantAlignment, GrGpuBufferType::kVertex)
, fOutstandingCommandLists(sizeof(OutstandingCommandList), kDefaultOutstandingAllocCnt)
, fCompiler(new SkSL::Compiler()) {
fCaps.reset(new GrD3DCaps(contextOptions,
@@ -120,9 +117,6 @@
bool GrD3DGpu::submitDirectCommandList(SyncQueue sync) {
SkASSERT(fCurrentDirectCommandList);
- // set up constant data
- fCurrentDirectCommandList->setCurrentConstantBuffer(&fConstantsRingBuffer);
-
fResourceProvider.prepForSubmit();
GrD3DDirectCommandList::SubmitResult result = fCurrentDirectCommandList->submit(fQueue.get());
diff --git a/src/gpu/d3d/GrD3DGpu.h b/src/gpu/d3d/GrD3DGpu.h
index 67beb60..c7b8361 100644
--- a/src/gpu/d3d/GrD3DGpu.h
+++ b/src/gpu/d3d/GrD3DGpu.h
@@ -48,9 +48,6 @@
GrStagingBufferManager* stagingBufferManager() override { return &fStagingBufferManager; }
void takeOwnershipOfStagingBuffer(sk_sp<GrGpuBuffer>) override;
- // TODO: hoist up to GrGpu
- GrRingBuffer* constantsRingBuffer() { return &fConstantsRingBuffer; }
-
bool protectedContext() const { return false; }
void querySampleLocations(GrRenderTarget*, SkTArray<SkPoint>* sampleLocations) override;
@@ -259,7 +256,6 @@
GrD3DResourceProvider fResourceProvider;
GrStagingBufferManager fStagingBufferManager;
- GrRingBuffer fConstantsRingBuffer;
gr_cp<ID3D12Fence> fFence;
uint64_t fCurrentFenceValue = 0;
diff --git a/src/gpu/d3d/GrD3DResourceProvider.cpp b/src/gpu/d3d/GrD3DResourceProvider.cpp
index 9e7db62..4c65d1a 100644
--- a/src/gpu/d3d/GrD3DResourceProvider.cpp
+++ b/src/gpu/d3d/GrD3DResourceProvider.cpp
@@ -195,19 +195,27 @@
// constant size has to be aligned to 256
constexpr int kConstantAlignment = 256;
+ // Due to dependency on the resource cache we can't initialize this in the constructor, so
+ // we do so it here.
+ if (!fConstantBuffer) {
+ fConstantBuffer = GrD3DConstantRingBuffer::Make(fGpu, 128 * 1024, kConstantAlignment);
+ SkASSERT(fConstantBuffer);
+ }
+
// upload the data
size_t paddedSize = GrAlignTo(size, kConstantAlignment);
- GrRingBuffer::Slice slice = fGpu->constantsRingBuffer()->suballocate(paddedSize);
+ GrRingBuffer::Slice slice = fConstantBuffer->suballocate(paddedSize);
char* destPtr = static_cast<char*>(slice.fBuffer->map()) + slice.fOffset;
memcpy(destPtr, data, size);
// create the associated constant buffer view descriptor
- GrD3DBuffer* d3dBuffer = static_cast<GrD3DBuffer*>(slice.fBuffer);
+ GrD3DBuffer* d3dBuffer = static_cast<GrD3DBuffer*>(slice.fBuffer.get());
D3D12_GPU_VIRTUAL_ADDRESS gpuAddress = d3dBuffer->d3dResource()->GetGPUVirtualAddress();
return gpuAddress + slice.fOffset;
}
void GrD3DResourceProvider::prepForSubmit() {
+ fGpu->currentCommandList()->setCurrentConstantBuffer(fConstantBuffer);
fDescriptorTableManager.prepForSubmit(fGpu);
// Any heap memory used for these will be returned when the command buffer finishes,
// so we have to invalidate all entries.
diff --git a/src/gpu/d3d/GrD3DResourceProvider.h b/src/gpu/d3d/GrD3DResourceProvider.h
index f989cf4..1b963b7 100644
--- a/src/gpu/d3d/GrD3DResourceProvider.h
+++ b/src/gpu/d3d/GrD3DResourceProvider.h
@@ -13,8 +13,8 @@
#include "include/private/SkTHash.h"
#include "src/core/SkLRUCache.h"
#include "src/gpu/GrProgramDesc.h"
-#include "src/gpu/GrRingBuffer.h"
#include "src/gpu/d3d/GrD3DCommandSignature.h"
+#include "src/gpu/d3d/GrD3DConstantRingBuffer.h"
#include "src/gpu/d3d/GrD3DCpuDescriptorManager.h"
#include "src/gpu/d3d/GrD3DDescriptorTableManager.h"
#include "src/gpu/d3d/GrD3DRootSignature.h"
@@ -153,6 +153,8 @@
GrD3DCpuDescriptorManager fCpuDescriptorManager;
GrD3DDescriptorTableManager fDescriptorTableManager;
+ sk_sp<GrD3DConstantRingBuffer> fConstantBuffer;
+
std::unique_ptr<PipelineStateCache> fPipelineStateCache;
SkTHashMap<uint32_t, D3D12_CPU_DESCRIPTOR_HANDLE> fSamplers;