Hold shader data bindings through to rendering worker

CMakeLists.txt

@@ -230,6 +230,7 @@ add_library(boo
             include/boo/IGraphicsContext.hpp
             include/boo/graphicsdev/IGraphicsDataFactory.hpp
             include/boo/graphicsdev/IGraphicsCommandQueue.hpp
+            lib/graphicsdev/Common.hpp
             include/boo/audiodev/IAudioSubmix.hpp
             include/boo/audiodev/IAudioVoice.hpp
             include/boo/audiodev/IMIDIPort.hpp

include/boo/graphicsdev/GL.hpp

@@ -7,6 +7,7 @@
 #include "GLSLMacros.hpp"
 #include <vector>
 #include <unordered_set>
+#include <unordered_map>
 #include <mutex>
 
 namespace boo

lib/graphicsdev/Common.hpp

@@ -0,0 +1,54 @@
+#ifndef BOO_GRAPHICSDEV_COMMON_HPP
+#define BOO_GRAPHICSDEV_COMMON_HPP
+
+/* Private header for managing shader data
+ * binding lifetimes through rendering cycle */
+
+#include "boo/graphicsdev/IGraphicsDataFactory.hpp"
+
+namespace boo
+{
+
+template <class DataImpl>
+class IGraphicsDataPriv : public IGraphicsData
+{
+    std::atomic_int m_refCount = {1};
+public:
+    void increment() { m_refCount++; }
+    void decrement()
+    {
+        if (m_refCount.fetch_sub(1) == 1)
+            delete static_cast<DataImpl*>(this);
+    }
+};
+
+template <class DataImpl>
+class IShaderDataBindingPriv : public IShaderDataBinding
+{
+    IGraphicsDataPriv<DataImpl>* m_parent;
+
+public:
+    IShaderDataBindingPriv(IGraphicsDataPriv<DataImpl>* p) : m_parent(p) {}
+    class Token
+    {
+        IGraphicsDataPriv<DataImpl>* m_data = nullptr;
+    public:
+        Token() = default;
+        Token(const IShaderDataBindingPriv* p)
+        : m_data(p->m_parent)
+        { m_data->increment(); }
+        Token& operator=(const Token&) = delete;
+        Token(const Token&) = delete;
+        Token& operator=(Token&& other)
+        { m_data = other.m_data; other.m_data = nullptr; return *this; }
+        Token(Token&& other)
+        { m_data = other.m_data; other.m_data = nullptr; }
+        ~Token() { if (m_data) m_data->decrement(); }
+    };
+
+    Token lock() const { return Token(this); }
+};
+
+}
+
+#endif // BOO_GRAPHICSDEV_COMMON_HPP

lib/graphicsdev/D3D11.cpp

@@ -2,6 +2,7 @@
 #include "logvisor/logvisor.hpp"
 #include "boo/graphicsdev/D3D.hpp"
 #include "boo/IGraphicsContext.hpp"
+#include "Common.hpp"
 #include <vector>
 #include <thread>
 #include <mutex>
@@ -32,7 +33,7 @@ static inline void ThrowIfFailed(HRESULT hr)
     }
 }
 
-struct D3D11Data : IGraphicsData
+struct D3D11Data : IGraphicsDataPriv<D3D11Data>
 {
     std::vector<std::shared_ptr<class D3D11ShaderPipeline>> m_SPs;
     std::vector<std::shared_ptr<struct D3D11ShaderDataBinding>> m_SBinds;
@@ -576,9 +577,8 @@ public:
     }
 };
 
-struct D3D11ShaderDataBinding : IShaderDataBinding
+struct D3D11ShaderDataBinding : IShaderDataBindingPriv<D3D11Data>
 {
-    std::weak_ptr<D3D11ShaderDataBinding> m_selfPtr;
     std::shared_ptr<D3D11ShaderPipeline> m_pipeline;
     std::shared_ptr<IGraphicsBuffer> m_vbuf;
     std::shared_ptr<IGraphicsBuffer> m_instVbuf;
@@ -590,13 +590,15 @@ struct D3D11ShaderDataBinding : IShaderDataBinding
     std::vector<std::shared_ptr<ITexture>> m_texs;
     UINT m_baseOffsets[2];
 
-    D3D11ShaderDataBinding(D3D11Context* ctx,
+    D3D11ShaderDataBinding(D3D11Data* d,
+                           D3D11Context* ctx,
                            IShaderPipeline* pipeline,
                            IGraphicsBuffer* vbuf, IGraphicsBuffer* instVbuf, IGraphicsBuffer* ibuf,
                            size_t ubufCount, IGraphicsBuffer** ubufs, const PipelineStage* ubufStages,
                            const size_t* ubufOffs, const size_t* ubufSizes,
                            size_t texCount, ITexture** texs, size_t baseVert, size_t baseInst)
-    : m_pipeline(static_cast<D3D11ShaderPipeline*>(pipeline)->m_selfPtr),
+    : IShaderDataBindingPriv(d),
+      m_pipeline(static_cast<D3D11ShaderPipeline*>(pipeline)->m_selfPtr),
       m_vbuf(D3D11GraphicsBuffer::getPtr(vbuf)),
       m_instVbuf(D3D11GraphicsBuffer::getPtr(instVbuf)),
       m_ibuf(D3D11GraphicsBuffer::getPtr(ibuf))
@@ -849,13 +851,13 @@ struct D3D11CommandQueue : IGraphicsCommandQueue
     struct CommandList
     {
         ComPtr<ID3D11CommandList> list;
-        std::vector<std::shared_ptr<D3D11ShaderDataBinding>> bindings;
+        std::vector<IShaderDataBindingPriv::Token> resTokens;
         std::shared_ptr<D3D11TextureR> workDoPresent;
 
         void reset()
         {
             list.Reset();
-            bindings.clear();
+            resTokens.clear();
             workDoPresent.reset();
         }
     };
@@ -965,7 +967,7 @@ struct D3D11CommandQueue : IGraphicsCommandQueue
     {
         D3D11ShaderDataBinding* cbind = static_cast<D3D11ShaderDataBinding*>(binding);
         cbind->bind(m_deferredCtx.Get(), m_fillBuf);
-        m_cmdLists[m_fillBuf].bindings.push_back(cbind->m_selfPtr.lock());
+        m_cmdLists[m_fillBuf].resTokens.push_back(cbind->lock());
 
         ID3D11SamplerState* samp[] = {m_ctx->m_ss.Get()};
         m_deferredCtx->PSSetSamplers(0, 1, samp);
@@ -1174,14 +1176,14 @@ class D3D11DataFactory : public ID3DDataFactory
         std::unique_lock<std::mutex> lk(m_committedMutex);
         D3D11Data* data = static_cast<D3D11Data*>(d);
         m_committedData.erase(data);
-        delete data;
+        data->decrement();
     }
 
     void destroyAllData()
     {
         std::unique_lock<std::mutex> lk(m_committedMutex);
-        for (IGraphicsData* data : m_committedData)
+        for (D3D11Data* data : m_committedData)
-            delete static_cast<D3D11Data*>(data);
+            data->decrement();
         for (IGraphicsBufferPool* pool : m_committedPools)
             delete static_cast<D3D11Pool*>(pool);
         m_committedData.clear();
@@ -1356,11 +1358,10 @@ public:
         {
             D3D11Data* d = static_cast<D3D11Data*>(m_deferredData);
             D3D11ShaderDataBinding* retval =
-                new D3D11ShaderDataBinding(m_parent.m_ctx, pipeline, vbuf, instVbo, ibuf,
+                new D3D11ShaderDataBinding(d, m_parent.m_ctx, pipeline, vbuf, instVbo, ibuf,
                                            ubufCount, ubufs, ubufStages, ubufOffs, ubufSizes, texCount, texs,
                                            baseVert, baseInst);
             d->m_SBinds.emplace_back(retval);
-            retval->m_selfPtr = d->m_SBinds.back();
             return retval;
         }
     };

lib/graphicsdev/D3D12.cpp

@@ -3,6 +3,7 @@
 #include "logvisor/logvisor.hpp"
 #include "boo/graphicsdev/D3D.hpp"
 #include "boo/IGraphicsContext.hpp"
+#include "Common.hpp"
 #include <vector>
 #include "d3dx12.h"
 #include <d3dcompiler.h>
@@ -40,7 +41,7 @@ static inline UINT64 NextHeapOffset(UINT64 offset, const D3D12_RESOURCE_ALLOCATI
     return (offset + info.Alignment - 1) & ~(info.Alignment - 1);
 }
 
-struct D3D12Data : IGraphicsData
+struct D3D12Data : IGraphicsDataPriv<D3D12Data>
 {
     std::vector<std::unique_ptr<class D3D12ShaderPipeline>> m_SPs;
     std::vector<std::unique_ptr<struct D3D12ShaderDataBinding>> m_SBinds;
@@ -909,7 +910,7 @@ static ID3D12Resource* GetTextureGPUResource(const ITexture* tex, int idx,
     return nullptr;
 }
 
-struct D3D12ShaderDataBinding : IShaderDataBinding
+struct D3D12ShaderDataBinding : IShaderDataBindingPriv<D3D12Data>
 {
     D3D12ShaderPipeline* m_pipeline;
     ComPtr<ID3D12Heap> m_gpuHeap;
@@ -927,14 +928,16 @@ struct D3D12ShaderDataBinding : IShaderDataBinding
     D3D12_INDEX_BUFFER_VIEW m_iboView[2];
     size_t m_vertOffset, m_instOffset;
 
-    D3D12ShaderDataBinding(D3D12Context* ctx,
+    D3D12ShaderDataBinding(D3D12Data* d,
+                           D3D12Context* ctx,
                            IShaderPipeline* pipeline,
                            IGraphicsBuffer* vbuf, IGraphicsBuffer* instVbuf, IGraphicsBuffer* ibuf,
                            size_t ubufCount, IGraphicsBuffer** ubufs,
                            const size_t* ubufOffs, const size_t* ubufSizes,
                            size_t texCount, ITexture** texs,
                            size_t baseVert, size_t baseInst)
-    : m_pipeline(static_cast<D3D12ShaderPipeline*>(pipeline)),
+    : IShaderDataBindingPriv(d),
+      m_pipeline(static_cast<D3D12ShaderPipeline*>(pipeline)),
       m_vbuf(vbuf),
       m_instVbuf(instVbuf),
       m_ibuf(ibuf),
@@ -1541,14 +1544,14 @@ class D3D12DataFactory : public ID3DDataFactory
         std::unique_lock<std::mutex> lk(m_committedMutex);
         D3D12Data* data = static_cast<D3D12Data*>(d);
         m_committedData.erase(data);
-        delete data;
+        data->decrement();
     }
 
     void destroyAllData()
     {
         std::unique_lock<std::mutex> lk(m_committedMutex);
-        for (IGraphicsData* data : m_committedData)
+        for (D3D12Data* data : m_committedData)
-            delete static_cast<D3D12Data*>(data);
+            data->decrement();
         for (IGraphicsBufferPool* pool : m_committedPools)
             delete static_cast<D3D12Pool*>(pool);
         m_committedData.clear();
@@ -1745,7 +1748,7 @@ public:
                 size_t baseVert, size_t baseInst)
         {
             D3D12ShaderDataBinding* retval =
-                new D3D12ShaderDataBinding(m_parent.m_ctx, pipeline, vbuf, instVbuf, ibuf,
+                new D3D12ShaderDataBinding(m_deferredData.get(), m_parent.m_ctx, pipeline, vbuf, instVbuf, ibuf,
                                            ubufCount, ubufs, ubufOffs, ubufSizes, texCount, texs,
                                            baseVert, baseInst);
             static_cast<D3D12Data*>(m_deferredData)->m_SBinds.emplace_back(retval);

lib/graphicsdev/GL.cpp

@@ -1,12 +1,14 @@
 #include "boo/graphicsdev/GL.hpp"
 #include "boo/graphicsdev/glew.h"
 #include "boo/IGraphicsContext.hpp"
+#include "Common.hpp"
 #include <vector>
 #include <thread>
 #include <mutex>
 #include <condition_variable>
 #include <array>
 #include <unordered_map>
+#include <atomic>
 
 #include "logvisor/logvisor.hpp"
 
@@ -18,7 +20,7 @@ namespace boo
 static logvisor::Module Log("boo::GL");
 
 ThreadLocalPtr<struct GLData> GLDataFactory::m_deferredData;
-struct GLData : IGraphicsData
+struct GLData : IGraphicsDataPriv<GLData>
 {
     std::vector<std::unique_ptr<class GLShaderPipeline>> m_SPs;
     std::vector<std::unique_ptr<struct GLShaderDataBinding>> m_SBinds;
@@ -548,7 +550,7 @@ struct GLVertexFormat : IVertexFormat
     void bind(int idx) const {glBindVertexArray(m_vao[idx]);}
 };
 
-struct GLShaderDataBinding : IShaderDataBinding
+struct GLShaderDataBinding : IShaderDataBindingPriv<GLData>
 {
     const GLShaderPipeline* m_pipeline;
     const GLVertexFormat* m_vtxFormat;
@@ -558,12 +560,14 @@ struct GLShaderDataBinding : IShaderDataBinding
     size_t m_texCount;
     std::unique_ptr<ITexture*[]> m_texs;
 
-    GLShaderDataBinding(IShaderPipeline* pipeline,
+    GLShaderDataBinding(GLData* d,
+                        IShaderPipeline* pipeline,
                         IVertexFormat* vtxFormat,
                         size_t ubufCount, IGraphicsBuffer** ubufs,
                         const size_t* ubufOffs, const size_t* ubufSizes,
                         size_t texCount, ITexture** texs)
-    : m_pipeline(static_cast<GLShaderPipeline*>(pipeline)),
+    : IShaderDataBindingPriv(d),
+      m_pipeline(static_cast<GLShaderPipeline*>(pipeline)),
       m_vtxFormat(static_cast<GLVertexFormat*>(vtxFormat)),
       m_ubufCount(ubufCount),
       m_ubufs(new IGraphicsBuffer*[ubufCount]),
@@ -663,7 +667,7 @@ GLDataFactory::Context::newShaderDataBinding(IShaderPipeline* pipeline,
                                              size_t texCount, ITexture** texs, size_t baseVert, size_t baseInst)
 {
     GLShaderDataBinding* retval =
-    new GLShaderDataBinding(pipeline, vtxFormat, ubufCount, ubufs,
+    new GLShaderDataBinding(m_deferredData.get(), pipeline, vtxFormat, ubufCount, ubufs,
                             ubufOffs, ubufSizes, texCount, texs);
     m_deferredData->m_SBinds.emplace_back(retval);
     return retval;
@@ -691,6 +695,7 @@ GraphicsDataToken GLDataFactory::commitTransaction(const FactoryCommitFunc& tran
     GLData* retval = m_deferredData.get();
     m_deferredData.reset();
     m_committedData.insert(retval);
+
     lk.unlock();
     /* Let's go ahead and flush to ensure our data gets to the GPU
        While this isn't strictly required, some drivers might behave
@@ -712,14 +717,14 @@ void GLDataFactory::destroyData(IGraphicsData* d)
     std::unique_lock<std::mutex> lk(m_committedMutex);
     GLData* data = static_cast<GLData*>(d);
     m_committedData.erase(data);
-    delete data;
+    data->decrement();
 }
 
 void GLDataFactory::destroyAllData()
 {
     std::unique_lock<std::mutex> lk(m_committedMutex);
-    for (IGraphicsData* data : m_committedData)
+    for (GLData* data : m_committedData)
-        delete static_cast<GLData*>(data);
+        data->decrement();
     for (IGraphicsBufferPool* pool : m_committedPools)
         delete static_cast<GLPool*>(pool);
     m_committedData.clear();
@@ -836,10 +841,15 @@ struct GLCommandQueue : IGraphicsCommandQueue
                 size_t instCount;
             };
         };
+        IShaderDataBindingPriv<GLData>::Token resToken;
         const ITextureR* resolveTex;
         bool resolveColor : 1;
         bool resolveDepth : 1;
         Command(Op op) : m_op(op) {}
+        Command(const Command&) = delete;
+        Command& operator=(const Command&) = delete;
+        Command(Command&&) = default;
+        Command& operator=(Command&&) = default;
     };
     std::vector<Command> m_cmdBufs[3];
     size_t m_fillBuf = 0;
@@ -1129,6 +1139,7 @@ struct GLCommandQueue : IGraphicsCommandQueue
         std::vector<Command>& cmds = m_cmdBufs[m_fillBuf];
         cmds.emplace_back(Command::Op::SetShaderDataBinding);
         cmds.back().binding = binding;
+        cmds.back().resToken = static_cast<IShaderDataBindingPriv<GLData>*>(binding)->lock();
     }
 
     void setRenderTarget(ITextureR* target)

lib/graphicsdev/Metal.mm

@@ -3,6 +3,7 @@
 #include "logvisor/logvisor.hpp"
 #include "boo/graphicsdev/Metal.hpp"
 #include "boo/IGraphicsContext.hpp"
+#include "Common.hpp"
 #include <vector>
 
 #if !__has_feature(objc_arc)
@@ -18,7 +19,7 @@ static logvisor::Module Log("boo::Metal");
 struct MetalCommandQueue;
 
 ThreadLocalPtr<struct MetalData> MetalDataFactory::m_deferredData;
-struct MetalData : IGraphicsData
+struct MetalData : IGraphicsDataPriv<MetalData>
 {
     std::vector<std::unique_ptr<class MetalShaderPipeline>> m_SPs;
     std::vector<std::unique_ptr<struct MetalShaderDataBinding>> m_SBinds;
@@ -582,7 +583,7 @@ static id<MTLTexture> GetTextureGPUResource(const ITexture* tex, int idx)
     return nullptr;
 }
 
-struct MetalShaderDataBinding : IShaderDataBinding
+struct MetalShaderDataBinding : IShaderDataBindingPriv<MetalData>
 {
     MetalShaderPipeline* m_pipeline;
     IGraphicsBuffer* m_vbuf;
@@ -597,13 +598,15 @@ struct MetalShaderDataBinding : IShaderDataBinding
     size_t m_baseVert;
     size_t m_baseInst;
 
-    MetalShaderDataBinding(MetalContext* ctx,
+    MetalShaderDataBinding(MetalData* d,
+                           MetalContext* ctx,
                            IShaderPipeline* pipeline,
                            IGraphicsBuffer* vbuf, IGraphicsBuffer* instVbo, IGraphicsBuffer* ibuf,
                            size_t ubufCount, IGraphicsBuffer** ubufs, const PipelineStage* ubufStages,
                            const size_t* ubufOffs, const size_t* ubufSizes,
                            size_t texCount, ITexture** texs, size_t baseVert, size_t baseInst)
-    : m_pipeline(static_cast<MetalShaderPipeline*>(pipeline)),
+    : IShaderDataBindingPriv(d),
+    m_pipeline(static_cast<MetalShaderPipeline*>(pipeline)),
     m_vbuf(vbuf),
     m_instVbo(instVbo),
     m_ibuf(ibuf),
@@ -1122,7 +1125,8 @@ MetalDataFactory::Context::newShaderDataBinding(IShaderPipeline* pipeline,
                                                 size_t texCount, ITexture** texs, size_t baseVert, size_t baseInst)
 {
     MetalShaderDataBinding* retval =
-    new MetalShaderDataBinding(m_parent.m_ctx, pipeline, vbuf, instVbo, ibuf,
+    new MetalShaderDataBinding(m_deferredData.get(),
+                               m_parent.m_ctx, pipeline, vbuf, instVbo, ibuf,
                                ubufCount, ubufs, ubufStages, ubufOffs,
                                ubufSizes, texCount, texs, baseVert, baseInst);
     m_deferredData->m_SBinds.emplace_back(retval);
@@ -1163,14 +1167,14 @@ void MetalDataFactory::destroyData(IGraphicsData* d)
     std::unique_lock<std::mutex> lk(m_committedMutex);
     MetalData* data = static_cast<MetalData*>(d);
     m_committedData.erase(data);
-    delete data;
+    data->decrement();
 }
 
 void MetalDataFactory::destroyAllData()
 {
     std::unique_lock<std::mutex> lk(m_committedMutex);
-    for (IGraphicsData* data : m_committedData)
+    for (MetalData* data : m_committedData)
-        delete static_cast<MetalData*>(data);
+        data->decrement();
     for (IGraphicsBufferPool* pool : m_committedPools)
         delete static_cast<MetalPool*>(pool);
     m_committedData.clear();

lib/graphicsdev/Vulkan.cpp

@@ -8,6 +8,7 @@
 #include <SPIRV/GlslangToSpv.h>
 #include <SPIRV/disassemble.h>
 #include "boo/graphicsdev/GLSLMacros.hpp"
+#include "Common.hpp"
 
 #include "logvisor/logvisor.hpp"
 
@@ -652,7 +653,7 @@ void VulkanContext::resizeSwapChain(VulkanContext::Window& windowCtx, VkSurfaceK
     }
 }
 
-struct VulkanData : IGraphicsData
+struct VulkanData : IGraphicsDataPriv<VulkanData>
 {
     VulkanContext* m_ctx;
     VkDeviceMemory m_bufMem = VK_NULL_HANDLE;
@@ -1947,7 +1948,7 @@ static const VkDescriptorImageInfo* GetTextureGPUResource(const ITexture* tex, i
     return nullptr;
 }
 
-struct VulkanShaderDataBinding : IShaderDataBinding
+struct VulkanShaderDataBinding : IShaderDataBindingPriv<VulkanData>
 {
     VulkanContext* m_ctx;
     VulkanShaderPipeline* m_pipeline;
@@ -1977,14 +1978,16 @@ struct VulkanShaderDataBinding : IShaderDataBinding
     bool m_committed = false;
 #endif
 
-    VulkanShaderDataBinding(VulkanContext* ctx,
+    VulkanShaderDataBinding(VulkanData* d,
+                            VulkanContext* ctx,
                             IShaderPipeline* pipeline,
                             IGraphicsBuffer* vbuf, IGraphicsBuffer* instVbuf, IGraphicsBuffer* ibuf,
                             size_t ubufCount, IGraphicsBuffer** ubufs,
                             const size_t* ubufOffs, const size_t* ubufSizes,
                             size_t texCount, ITexture** texs,
                             size_t baseVert, size_t baseInst)
-    : m_ctx(ctx),
+    : IShaderDataBindingPriv(d),
+      m_ctx(ctx),
       m_pipeline(static_cast<VulkanShaderPipeline*>(pipeline)),
       m_vbuf(vbuf),
       m_instVbuf(instVbuf),
@@ -2812,7 +2815,7 @@ void VulkanDataFactory::destroyAllData()
 {
     std::unique_lock<std::mutex> lk(m_committedMutex);
     for (IGraphicsData* data : m_committedData)
-        delete static_cast<VulkanData*>(data);
+        data->decrement();
     for (IGraphicsBufferPool* pool : m_committedPools)
         delete static_cast<VulkanPool*>(pool);
     m_committedData.clear();
@@ -3062,11 +3065,12 @@ IShaderDataBinding* VulkanDataFactory::Context::newShaderDataBinding(IShaderPipe
         size_t texCount, ITexture** texs,
         size_t baseVert, size_t baseInst)
 {
+    VulkanData* d = static_cast<VulkanData*>(m_deferredData.get());
     VulkanShaderDataBinding* retval =
-        new VulkanShaderDataBinding(m_parent.m_ctx, pipeline, vbuf, instVbuf, ibuf,
+        new VulkanShaderDataBinding(d, m_parent.m_ctx, pipeline, vbuf, instVbuf, ibuf,
                                     ubufCount, ubufs, ubufOffs, ubufSizes, texCount, texs,
                                     baseVert, baseInst);
-    static_cast<VulkanData*>(m_deferredData.get())->m_SBinds.emplace_back(retval);
+    d->m_SBinds.emplace_back(retval);
     return retval;
 }
 
@@ -3301,7 +3305,7 @@ void VulkanCommandQueue::execute()
     {
         if ((*it)->m_dead)
         {
-            delete *it;
+            it->decrement();
             it = gfxF->m_committedData.erase(it);
             continue;
         }