GL backend: reworked VAO cache key to use object unique IDs instead of pointers

2 files changed, 41 insertions, 46 deletions

diff --git a/Graphics/GraphicsEngineOpenGL/include/VAOCache.h b/Graphics/GraphicsEngineOpenGL/include/VAOCache.h
index f55ef323..de251651 100644
--- a/Graphics/GraphicsEngineOpenGL/include/VAOCache.h
+++ b/Graphics/GraphicsEngineOpenGL/include/VAOCache.h
@@ -62,35 +62,36 @@ private:
     // This structure is used as the key to find VAO
     struct VAOCacheKey
     {
-        VAOCacheKey(const IPipelineState* pso, const IBuffer* indBuffer) : 
-            pPSO(pso),
-            pIndexBuffer(indBuffer),
-            NumUsedSlots(0)
+        VAOCacheKey(UniqueIdentifier pso_id, UniqueIdentifier ib_id) : 
+            PSOUId         (pso_id),
+            IndexBufferUId (ib_id),
+            NumUsedSlots   (0)
         {}
 
-        // Note that the the pointers are used for ordering only
-        // They are not used to access the objects
+        // Note that using pointers is unsafe as they may (and will) be reused:
+        // pBuffer->Release();
+        // pDevice->CreateBuffer(&pBuffer); // Returns same pointer
 
-        // VAO encapsulates both input layout and all bound buffers
+        // VAO encapsulates both input layout and all bound buffers.
         // PSO uniqly defines the layout (attrib pointers, divisors, etc.),
-        // so we do not need to add individual layout elements to the key
-        // The keey needs to contain all bound buffers
-        const IPipelineState* const pPSO;
-        const IBuffer* const pIndexBuffer;
-        Uint32 NumUsedSlots;
+        // so we do not need to add individual layout elements to the key.
+        // The key needs to contain all bound buffers.
+        const UniqueIdentifier PSOUId;
+        const UniqueIdentifier IndexBufferUId;
+        Uint32                 NumUsedSlots;
         struct StreamAttribs
         {
-            const IBuffer* pBuffer;
-            Uint32 Stride;
-            Uint32 Offset;
+            UniqueIdentifier BufferUId;
+            Uint32           Stride;
+            Uint32           Offset;
         }Streams[MaxBufferSlots];
         
         mutable size_t Hash = 0;
 
         bool operator == (const VAOCacheKey &Key)const
         {
-            return pPSO            == Key.pPSO            &&
-                   pIndexBuffer    == Key.pIndexBuffer    &&
+            return PSOUId          == Key.PSOUId          &&
+                   IndexBufferUId  == Key.IndexBufferUId  &&
                    NumUsedSlots    == Key.NumUsedSlots    &&
                    std::memcmp(Streams, Key.Streams, sizeof(StreamAttribs) * NumUsedSlots) == 0;
         }
@@ -103,18 +104,13 @@ private:
             if (Key.Hash == 0)
             {
                 std::size_t Seed = 0;
-                HashCombine(Seed, Key.pPSO, Key.NumUsedSlots);
-                if (Key.pIndexBuffer)
-                    HashCombine(Seed, Key.pIndexBuffer);
+                HashCombine(Seed, Key.PSOUId, Key.IndexBufferUId, Key.NumUsedSlots);
                 for (Uint32 slot = 0; slot < Key.NumUsedSlots; ++slot)
                 {
                     auto &CurrStream = Key.Streams[slot];
-                    if (CurrStream.pBuffer)
-                    {
-                        HashCombine(Seed, CurrStream.pBuffer);
-                        HashCombine(Seed, CurrStream.Offset);
-                        HashCombine(Seed, CurrStream.Stride);
-                    }
+                    HashCombine(Seed, CurrStream.BufferUId);
+                    HashCombine(Seed, CurrStream.Offset);
+                    HashCombine(Seed, CurrStream.Stride);
                 }
                 Key.Hash = Seed;
             }
diff --git a/Graphics/GraphicsEngineOpenGL/src/VAOCache.cpp b/Graphics/GraphicsEngineOpenGL/src/VAOCache.cpp
index eac99ec8..dcb700ac 100644
--- a/Graphics/GraphicsEngineOpenGL/src/VAOCache.cpp
+++ b/Graphics/GraphicsEngineOpenGL/src/VAOCache.cpp
@@ -71,27 +71,27 @@ void VAOCache::OnDestroyPSO(IPipelineState *pPSO)
     m_PSOToKey.erase(EqualRange.first, EqualRange.second);
 }
 
-const GLObjectWrappers::GLVertexArrayObj& VAOCache::GetVAO( IPipelineState *pPSO,
-                                                            IBuffer *pIndexBuffer,
+const GLObjectWrappers::GLVertexArrayObj& VAOCache::GetVAO( IPipelineState*                pPSO,
+                                                            IBuffer*                       pIndexBuffer,
                                                             VertexStreamInfo<BufferGLImpl> VertexStreams[],
-                                                            Uint32 NumVertexStreams,
-                                                            GLContextState &GLContextState )
+                                                            Uint32                         NumVertexStreams,
+                                                            GLContextState&                GLContextState )
 {
     // Lock the cache
     ThreadingTools::LockHelper CacheLock(m_CacheLockFlag);
 
-    IBuffer* VertexBuffers[MaxBufferSlots];
+    BufferGLImpl* VertexBuffers[MaxBufferSlots];
     for (Uint32 s = 0; s < NumVertexStreams; ++s)
         VertexBuffers[s] = nullptr;
     
     // Get layout
-    auto *pPSOGL = ValidatedCast<PipelineStateGLImpl>(pPSO);
-
+    auto* pPSOGL         = ValidatedCast<PipelineStateGLImpl>(pPSO);
+    auto* pIndexBufferGL = ValidatedCast<BufferGLImpl>(pIndexBuffer);
     const auto &InputLayout = pPSOGL->GetDesc().GraphicsPipeline.InputLayout;
     const LayoutElement *LayoutElems =  InputLayout.LayoutElements;
     Uint32 NumElems = InputLayout.NumElements;
     // Construct the key
-    VAOCacheKey Key(pPSO, pIndexBuffer);
+    VAOCacheKey Key(pPSOGL->GetUniqueID(), pIndexBufferGL ? pIndexBufferGL->GetUniqueID() : 0);
     
     {
         auto LayoutIt = LayoutElems;
@@ -113,10 +113,10 @@ const GLObjectWrappers::GLVertexArrayObj& VAOCache::GetVAO( IPipelineState *pPSO
                 Key.Streams[s] = VAOCacheKey::StreamAttribs{};
             Key.NumUsedSlots = MaxUsedSlot;
 
-            auto &CurrStream = VertexStreams[BuffSlot];
-            auto Stride = pPSOGL->GetBufferStride(BuffSlot);
-            auto &pCurrBuf = VertexBuffers[BuffSlot];
-            auto &CurrStreamKey = Key.Streams[BuffSlot];
+            auto& CurrStream    = VertexStreams[BuffSlot];
+            auto  Stride        = pPSOGL->GetBufferStride(BuffSlot);
+            auto& pCurrBuf      = VertexBuffers[BuffSlot];
+            auto& CurrStreamKey = Key.Streams[BuffSlot];
             if (pCurrBuf == nullptr)
             {
                 pCurrBuf = CurrStream.pBuffer;
@@ -129,23 +129,22 @@ const GLObjectWrappers::GLVertexArrayObj& VAOCache::GetVAO( IPipelineState *pPSO
                                                        // from the GL_VERTEX_ARRAY_BUFFER_BINDING bindings
                     GLContextState);
 
-                CurrStreamKey.pBuffer = pCurrBuf;
-                CurrStreamKey.Stride = Stride;
-                CurrStreamKey.Offset = CurrStream.Offset;
+                CurrStreamKey.BufferUId = pCurrBuf ? pCurrBuf->GetUniqueID() : 0;
+                CurrStreamKey.Stride    = Stride;
+                CurrStreamKey.Offset    = CurrStream.Offset;
             }
             else
             {
                 VERIFY(pCurrBuf == CurrStream.pBuffer, "Buffer no longer exists");
-                VERIFY(CurrStreamKey.pBuffer == pCurrBuf, "Unexpected buffer");
                 VERIFY(CurrStreamKey.Stride == Stride, "Unexpected buffer stride");
                 VERIFY(CurrStreamKey.Offset == CurrStream.Offset, "Unexpected buffer offset");
             }
         }
     }
 
-    if( pIndexBuffer )
+    if (pIndexBuffer)
     {
-        ValidatedCast<BufferGLImpl>(pIndexBuffer)->BufferMemoryBarrier(
+        pIndexBufferGL->BufferMemoryBarrier(
             GL_ELEMENT_ARRAY_BARRIER_BIT,// Vertex array indices sourced from buffer objects after the barrier 
                                          // will reflect data written by shaders prior to the barrier.
                                          // The buffer objects affected by this bit are derived from the
@@ -215,10 +214,10 @@ const GLObjectWrappers::GLVertexArrayObj& VAOCache::GetVAO( IPipelineState *pPSO
         auto NewElems = m_Cache.emplace( std::make_pair(Key, std::move(NewVAO)) );
         // New element must be actually inserted
         VERIFY( NewElems.second, "New element was not inserted into the cache" ); 
-        m_PSOToKey.insert( std::make_pair(Key.pPSO, Key) );
+        m_PSOToKey.insert( std::make_pair(pPSO, Key) );
         for(Uint32 Slot = 0; Slot < Key.NumUsedSlots; ++Slot)
         {
-            auto *pCurrBuff = Key.Streams[Slot].pBuffer;
+            auto* pCurrBuff = VertexBuffers[Slot];
             if( pCurrBuff )
                 m_BuffToKey.insert( std::make_pair(pCurrBuff, Key) );
         }