Revert "Make GrRingBuffer more generic so it can be used over all backends"

This reverts commit b6d4ad92ad.

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>

gn/gpu.gni

@@ -770,6 +770,8 @@ skia_direct3d_sources = [
   "$_src/gpu/d3d/GrD3DCommandList.h",
   "$_src/gpu/d3d/GrD3DCommandSignature.cpp",
   "$_src/gpu/d3d/GrD3DCommandSignature.h",
+  "$_src/gpu/d3d/GrD3DConstantRingBuffer.cpp",
+  "$_src/gpu/d3d/GrD3DConstantRingBuffer.h",
   "$_src/gpu/d3d/GrD3DCpuDescriptorManager.cpp",
   "$_src/gpu/d3d/GrD3DCpuDescriptorManager.h",
   "$_src/gpu/d3d/GrD3DDescriptorHeap.cpp",

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 @@ size_t GrRingBuffer::getAllocationOffset(size_t size) {
 }
 
 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;
     }

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 @@ class GrGpu;
  * 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 @@ public:
     }
 
     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

src/gpu/d3d/GrD3DCommandList.cpp

@@ -245,15 +245,13 @@ void GrD3DDirectCommandList::setPipelineState(sk_sp<GrD3DPipelineState> pipeline
     }
 }
 
-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());
     }
 }
 

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 @@ public:
 
     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 @@ private:
     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];

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);
+}

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

src/gpu/d3d/GrD3DGpu.cpp

@@ -39,17 +39,14 @@ sk_sp<GrGpu> GrD3DGpu::Make(const GrD3DBackendContext& backendContext,
 // 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 @@ GrOpsRenderPass* GrD3DGpu::getOpsRenderPass(
 bool GrD3DGpu::submitDirectCommandList(SyncQueue sync) {
     SkASSERT(fCurrentDirectCommandList);
 
-    // set up constant data
-    fCurrentDirectCommandList->setCurrentConstantBuffer(&fConstantsRingBuffer);
-
     fResourceProvider.prepForSubmit();
 
     GrD3DDirectCommandList::SubmitResult result = fCurrentDirectCommandList->submit(fQueue.get());

src/gpu/d3d/GrD3DGpu.h

@@ -48,9 +48,6 @@ public:
     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 @@ private:
 
     GrD3DResourceProvider fResourceProvider;
     GrStagingBufferManager fStagingBufferManager;
-    GrRingBuffer fConstantsRingBuffer;
 
     gr_cp<ID3D12Fence> fFence;
     uint64_t fCurrentFenceValue = 0;

src/gpu/d3d/GrD3DResourceProvider.cpp

@@ -195,19 +195,27 @@ D3D12_GPU_VIRTUAL_ADDRESS GrD3DResourceProvider::uploadConstantData(void* data,
     // 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.

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 @@ private:
     GrD3DCpuDescriptorManager fCpuDescriptorManager;
     GrD3DDescriptorTableManager fDescriptorTableManager;
 
+    sk_sp<GrD3DConstantRingBuffer> fConstantBuffer;
+
     std::unique_ptr<PipelineStateCache> fPipelineStateCache;
 
     SkTHashMap<uint32_t, D3D12_CPU_DESCRIPTOR_HANDLE> fSamplers;