#5912: Change IGeometryStore interface. Allocation now reserves a cer…

…tain amount of memory only, data is committed through IGeometryStore::updateData() only. It's now allowed to submit data that is not fully exhausting the available space in a slot.

include/igeometrystore.h

@@ -28,9 +28,20 @@ class IGeometryStore
     // Slot ID handed out to client code
     using Slot = std::uint64_t;
 
-    virtual Slot allocateSlot(const std::vector<ArbitraryMeshVertex>& vertices,
-        const std::vector<unsigned int>& indices) = 0;
+    /**
+     * Allocate memory blocks, one for vertices and one for indices, of the given size.
+     * The block can be populated using updateData(), where it's possible to
+     * fill the entire block or just a portion of it.
+     * Returns a handle as reference to the block for use in later calls.
+     * The allocated block cannot be resized later.
+     */
+    virtual Slot allocateSlot(std::size_t numVertices, std::size_t numIndices) = 0;
 
+    /**
+     * Load vertex and index data into the specified block. The given vertex and
+     * index arrays must not be larger than what has been allocated earlier, 
+     * but they're allowed to be smaller.
+     */
     virtual void updateData(Slot slot, const std::vector<ArbitraryMeshVertex>& vertices,
         const std::vector<unsigned int>& indices) = 0;
 

libs/render/ContinuousBuffer.h

@@ -54,17 +54,20 @@ class ContinuousBuffer
         bool Occupied;      // whether this slot is free
         std::size_t Offset; // The index to the first element within the buffer
         std::size_t Size;   // Number of allocated elements
+        std::size_t Used;   // Number of used elements
 
         SlotInfo() :
             Occupied(false),
             Offset(0),
-            Size(0)
+            Size(0),
+            Used(0)
         {}
 
         SlotInfo(std::size_t offset, std::size_t size, bool occupied) :
             Occupied(occupied),
             Offset(offset),
-            Size(size)
+            Size(size),
+            Used(0)
         {}
     };
 
@@ -117,27 +120,35 @@ class ContinuousBuffer
         return _slots[handle].Size;
     }
 
+    std::size_t getNumUsedElements(Handle handle) const
+    {
+        return _slots[handle].Used;
+    }
+
     std::size_t getOffset(Handle handle) const
     {
         return _slots[handle].Offset;
     }
 
     void setData(Handle handle, const std::vector<ElementType>& elements)
     {
-        const auto& slot = _slots[handle];
+        auto& slot = _slots[handle];
 
-        if (elements.size() != slot.Size)
+        auto numElements = elements.size();
+        if (numElements > slot.Size)
         {
-            throw std::logic_error("Allocation size mismatch in GeometryStore::Buffer::setData");
+            throw std::logic_error("Cannot store more data than allocated in GeometryStore::Buffer::setData");
         }
 
         std::copy(elements.begin(), elements.end(), _buffer.begin() + slot.Offset);
+        slot.Used = numElements;
     }
 
     void deallocate(Handle handle)
     {
         auto& releasedSlot = _slots[handle];
         releasedSlot.Occupied = false;
+        releasedSlot.Used = 0;
 
         // Check if the slot can merge with an adjacent one
         Handle slotIndexToMerge = std::numeric_limits<Handle>::max();
@@ -150,6 +161,7 @@ class ContinuousBuffer
 
             // The merged handle goes to recycling, block it against future use
             slotToMerge.Size = 0;
+            slotToMerge.Used = 0;
             slotToMerge.Occupied = true;
             _emptySlots.push(slotIndexToMerge);
         }
@@ -163,6 +175,7 @@ class ContinuousBuffer
 
             // The merged handle goes to recycling, block it against future use
             slotToMerge.Size = 0;
+            slotToMerge.Used = 0;
             slotToMerge.Occupied = true;
             _emptySlots.push(slotIndexToMerge);
         }
@@ -312,6 +325,7 @@ class ContinuousBuffer
         slot.Occupied = occupied;
         slot.Offset = offset;
         slot.Size = size;
+        slot.Used = 0;
 
         return slot;
     }

libs/render/WindingRenderer.h

@@ -207,7 +207,8 @@ class WindingRenderer :
                     }
                     else
                     {
-                        _geometrySlot = _owner._geometryStore.allocateSlot(vertices, indices);
+                        _geometrySlot = _owner._geometryStore.allocateSlot(vertices.size(), indices.size());
+                        _owner._geometryStore.updateData(_geometrySlot, vertices, indices);
                         _pushedVertexCount = vertices.size();
                         _pushedIndexCount = indices.size();
                     }

radiantcore/rendersystem/backend/GeometryRenderer.h

@@ -81,7 +81,8 @@ class GeometryRenderer :
         auto& slot = _slots.at(newSlotIndex);
 
         // Save the data into the backend storage
-        slot.storageHandle = _store.allocateSlot(vertices, indices);
+        slot.storageHandle = _store.allocateSlot(vertices.size(), indices.size());
+        _store.updateData(slot.storageHandle, vertices, indices);
         group.storageHandles.insert(slot.storageHandle);
 
         slot.groupIndex = groupIndex;

radiantcore/rendersystem/backend/GeometryStore.h

@@ -91,19 +91,15 @@ class GeometryStore :
         current.syncObject = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
     }
 
-    Slot allocateSlot(const std::vector<ArbitraryMeshVertex>& vertices,
-        const std::vector<unsigned int>& indices) override
+    Slot allocateSlot(std::size_t numVertices, std::size_t numIndices) override
     {
-        assert(!vertices.empty());
-        assert(!indices.empty());
+        assert(numVertices > 0);
+        assert(numIndices > 0);
 
         auto& current = getCurrentBuffer();
 
-        auto vertexSlot = current.vertices.allocate(vertices.size());
-        current.vertices.setData(vertexSlot, vertices);
-
-        auto indexSlot = current.indices.allocate(indices.size());
-        current.indices.setData(indexSlot, indices);
+        auto vertexSlot = current.vertices.allocate(numVertices);
+        auto indexSlot = current.indices.allocate(numIndices);
 
         auto slot = GetSlot(vertexSlot, indexSlot);
 

radiantcore/rendersystem/backend/SurfaceRenderer.h

@@ -49,7 +49,12 @@ class SurfaceRenderer :
         // Find a free slot
         auto newSlotIndex = getNextFreeSlotIndex();
 
-        auto slot = _store.allocateSlot(surface.getVertices(), surface.getIndices());
+        const auto& vertices = surface.getVertices();
+        const auto& indices = surface.getIndices();
+
+        auto slot = _store.allocateSlot(vertices.size(), indices.size());
+        _store.updateData(slot, vertices, indices);
+
         _surfaces.emplace(newSlotIndex, SurfaceInfo(surface, slot));
 
         return newSlotIndex;

test/ContinuousBuffer.cpp

@@ -62,7 +62,9 @@ TEST(ContinuousBufferTest, AllocateAndDeallocate)
     render::ContinuousBuffer<int> buffer(24);
 
     auto handle = buffer.allocate(sixteen.size());
+    EXPECT_EQ(buffer.getNumUsedElements(handle), 0) << "Allocated storage should be unused";
     buffer.setData(handle, sixteen);
+    EXPECT_EQ(buffer.getNumUsedElements(handle), sixteen.size()) << "Used element count should be 16 now";
 
     EXPECT_EQ(buffer.getOffset(handle), 0) << "Data should be located at offset 0";
     EXPECT_TRUE(checkData(buffer, handle, sixteen));
@@ -71,7 +73,9 @@ TEST(ContinuousBufferTest, AllocateAndDeallocate)
 
     // Re-allocate
     handle = buffer.allocate(sixteen.size());
+    EXPECT_EQ(buffer.getNumUsedElements(handle), 0) << "Allocated storage should be unused";
     buffer.setData(handle, sixteen);
+    EXPECT_EQ(buffer.getNumUsedElements(handle), sixteen.size()) << "Used element count should be 16 now";
 
     EXPECT_EQ(buffer.getOffset(handle), 0) << "Data should be located at offset 0";
     EXPECT_TRUE(checkData(buffer, handle, sixteen));
@@ -88,9 +92,44 @@ TEST(ContinuousBufferTest, ReplaceData)
     buffer.setData(handle, eight);
 
     EXPECT_TRUE(checkData(buffer, handle, eight));
+    EXPECT_EQ(buffer.getSize(handle), eight.size());
+    EXPECT_EQ(buffer.getNumUsedElements(handle), eight.size());
 
     buffer.setData(handle, eightAlt);
     EXPECT_TRUE(checkData(buffer, handle, eightAlt));
+    EXPECT_EQ(buffer.getNumUsedElements(handle), eightAlt.size());
+}
+
+TEST(ContinuousBufferTest, ReplaceDataPartially)
+{
+    auto eight = std::vector<int>({ 0,1,2,3,4,5,6,7 });
+    auto ten = std::vector<int>({ 8,9,10,11,12,13,14,15,16,17 });
+
+    render::ContinuousBuffer<int> buffer(24);
+
+    auto allocatedSize = eight.size() + 6; // 6 extra elements
+    auto handle = buffer.allocate(allocatedSize);
+    buffer.setData(handle, eight);
+
+    EXPECT_TRUE(checkData(buffer, handle, eight));
+    EXPECT_EQ(buffer.getSize(handle), allocatedSize);
+    EXPECT_EQ(buffer.getNumUsedElements(handle), eight.size());
+
+    buffer.setData(handle, ten);
+    EXPECT_TRUE(checkData(buffer, handle, ten));
+    EXPECT_EQ(buffer.getSize(handle), allocatedSize);
+    EXPECT_EQ(buffer.getNumUsedElements(handle), ten.size());
+}
+
+TEST(ContinuousBufferTest, ReplaceDataOverflow)
+{
+    auto eight = std::vector<int>({ 0,1,2,3,4,5,6,7 });
+
+    render::ContinuousBuffer<int> buffer(24);
+
+    auto handle = buffer.allocate(eight.size() - 3); // too little space
+
+    EXPECT_THROW(buffer.setData(handle, eight), std::logic_error);
 }
 
 TEST(ContinuousBufferTest, BufferGrowth)
@@ -153,7 +192,9 @@ TEST(ContinuousBufferTest, BlockReuse)
     buffer.deallocate(handle2);
 
     handle2 = buffer.allocate(eightFrom6.size());
+    EXPECT_EQ(buffer.getNumUsedElements(handle2), 0);
     buffer.setData(handle2, eightFrom6);
+    EXPECT_EQ(buffer.getNumUsedElements(handle2), eightFrom6.size());
 
     EXPECT_TRUE(checkContinuousData(buffer, handle1, { sixteen, eightFrom6, eight }));
 
@@ -173,9 +214,15 @@ TEST(ContinuousBufferTest, BlockReuseTightlyFit)
     auto handle1 = buffer.allocate(eight.size());
     auto handle2 = buffer.allocate(sixteen.size());
 
+    EXPECT_EQ(buffer.getNumUsedElements(handle1), 0);
+    EXPECT_EQ(buffer.getNumUsedElements(handle2), 0);
+
     buffer.setData(handle1, eight);
     buffer.setData(handle2, sixteen);
 
+    EXPECT_EQ(buffer.getNumUsedElements(handle1), eight.size());
+    EXPECT_EQ(buffer.getNumUsedElements(handle2), sixteen.size());
+
     EXPECT_TRUE(checkContinuousData(buffer, handle1, { eight, sixteen }));
 
     // Release the 8-length buffers and replace the data with 2x 4