#5943: Try to copy less data around. Also use a std::set to prevent t…

…he same slot from being synced multiple times.
codereader · Apr 19, 2022 · 36f4977 · 36f4977
1 parent 33f9091
commit 36f4977
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 13 deletions.
diff --git a/libs/render/ContinuousBuffer.h b/libs/render/ContinuousBuffer.h
@@ -80,7 +80,7 @@ class ContinuousBuffer
     std::size_t _lastSyncedBufferSize;
 
     // The slots that have been modified in between syncs
-    std::vector<Handle> _unsyncedSlots;
+    std::set<Handle> _unsyncedSlots;
 
     std::size_t _allocatedElements;
 
@@ -164,7 +164,7 @@ class ContinuousBuffer
         std::copy(elements.begin(), elements.end(), _buffer.begin() + slot.Offset);
         slot.Used = numElements;
 
-        _unsyncedSlots.push_back(handle);
+        _unsyncedSlots.insert(handle);
     }
 
     void setSubData(Handle handle, std::size_t elementOffset, const std::vector<ElementType>& elements)
@@ -180,7 +180,7 @@ class ContinuousBuffer
         std::copy(elements.begin(), elements.end(), _buffer.begin() + slot.Offset + elementOffset);
         slot.Used = std::max(slot.Used, elementOffset + numElements);
 
-        _unsyncedSlots.push_back(handle);
+        _unsyncedSlots.insert(handle);
     }
 
     void resizeData(Handle handle, std::size_t elementCount)
@@ -194,7 +194,7 @@ class ContinuousBuffer
 
         slot.Used = elementCount;
 
-        _unsyncedSlots.push_back(handle);
+        _unsyncedSlots.insert(handle);
     }
 
     void deallocate(Handle handle)
@@ -248,7 +248,7 @@ class ContinuousBuffer
                 // Only the updated slots will actually have altered any data
                 if (transaction.type == detail::BufferTransaction::Type::Update)
                 {
-                    _unsyncedSlots.push_back(getHandle(transaction.slot));
+                    _unsyncedSlots.insert(getHandle(transaction.slot));
                 }
             }
 
@@ -274,7 +274,7 @@ class ContinuousBuffer
                 memcpy(_buffer.data() + otherSlot.Offset, other._buffer.data() + otherSlot.Offset, otherSlot.Size * sizeof(ElementType));
 
                 // Remember this slot to be synced to the GPU
-                _unsyncedSlots.push_back(handle);
+                _unsyncedSlots.insert(handle);
             }
         }
 
@@ -299,14 +299,18 @@ class ContinuousBuffer
             _lastSyncedBufferSize = currentBufferSize;
 
             // Re-upload everything
+            buffer->bind();
             buffer->setData(0, reinterpret_cast<unsigned char*>(_buffer.data()),
                 _buffer.size() * sizeof(ElementType));
+            buffer->unbind();
         }
         else
         {
             std::size_t minimumOffset = std::numeric_limits<std::size_t>::max();
             std::size_t maximumOffset = 0;
 
+            std::size_t elementsToCopy = 0;
+
             // Size is the same, apply the updates to the GPU buffer
             // Determine the modified memory range
             for (auto handle : _unsyncedSlots)
@@ -315,14 +319,34 @@ class ContinuousBuffer
 
                 minimumOffset = std::min(slot.Offset, minimumOffset);
                 maximumOffset = std::max(slot.Offset + slot.Used, maximumOffset);
+                elementsToCopy += slot.Used;
             }
 
-            // Copy the data in one single operation
-            if (!_unsyncedSlots.empty())
+            // Copy the data in one single operation or in multiple, depending on the effort
+            if (elementsToCopy > 0)
             {
-                buffer->setData(minimumOffset * sizeof(ElementType),
-                    reinterpret_cast<unsigned char*>(_buffer.data() + minimumOffset),
-                    (maximumOffset - minimumOffset) * sizeof(ElementType));
+                buffer->bind();
+
+                // Less than a couple of operations will be copied piece by piece
+                if (_unsyncedSlots.size() < 20)
+                {
+                    for (auto handle : _unsyncedSlots)
+                    {
+                        auto& slot = _slots[handle];
+
+                        buffer->setData(slot.Offset * sizeof(ElementType),
+                            reinterpret_cast<unsigned char*>(_buffer.data() + slot.Offset),
+                            slot.Used * sizeof(ElementType));
+                    }
+                }
+                else // copy everything in between minimum and maximum in one operation
+                {
+                    buffer->setData(minimumOffset * sizeof(ElementType),
+                        reinterpret_cast<unsigned char*>(_buffer.data() + minimumOffset),
+                        (maximumOffset - minimumOffset) * sizeof(ElementType));
+                }
+
+                buffer->unbind();
             }
         }
 

diff --git a/radiantcore/rendersystem/backend/BufferObjectProvider.h b/radiantcore/rendersystem/backend/BufferObjectProvider.h
@@ -48,9 +48,7 @@ class BufferObjectProvider final :
 
         void setData(std::size_t offset, const unsigned char* firstElement, std::size_t numBytes) override
         {
-            glBindBuffer(_target, _buffer);
             glBufferSubData(_target, static_cast<GLintptr>(offset), static_cast<GLsizeiptr>(numBytes), firstElement);
-            glBindBuffer(_target, 0);
         }
 
         // Re-allocates the memory of this buffer, does not transfer the data