fix(legacy): sync instanced buffer batching optimization to native

cocos/rendering/instanced-buffer.ts

@@ -131,6 +131,11 @@ export class InstancedBuffer {
             stride,
             shader,
             descriptorSet,
+            lightingMap,
+            useReflectionProbeType,
+            reflectionProbeCubemap,
+            reflectionProbePlanarMap,
+            reflectionProbeBlendCubemap,
         );
     }
 
@@ -157,14 +162,20 @@ export class InstancedBuffer {
         stride: number,
         shader: Shader,
         descriptorSet: DescriptorSet,
+        lightingMap: Texture,
+        useReflectionProbeType: number,
+        reflectionProbeCubemap: Texture,
+        reflectionProbePlanarMap: Texture,
+        reflectionProbeBlendCubemap: Texture | null,
     ): void {
+        const newSize = stride * INITIAL_CAPACITY;
         const vb = this._device.createBuffer(new BufferInfo(
             BufferUsageBit.VERTEX | BufferUsageBit.TRANSFER_DST,
             MemoryUsageBit.HOST | MemoryUsageBit.DEVICE,
-            stride * INITIAL_CAPACITY,
+            newSize,
             stride,
         ));
-        const data = new Uint8Array(stride * INITIAL_CAPACITY);
+        const data = new Uint8Array(newSize);
         const vertexBuffers = sourceIA.vertexBuffers.slice();
         const attributes = sourceIA.attributes.slice();
         const indexBuffer = sourceIA.indexBuffer;
@@ -188,6 +199,11 @@ export class InstancedBuffer {
             stride,
             shader,
             descriptorSet,
+            lightingMap,
+            reflectionProbeCubemap,
+            reflectionProbePlanarMap,
+            useReflectionProbeType,
+            reflectionProbeBlendCubemap,
         } as IInstancedItem;
         this.instances.push(instance);
         let mappedInstances = this._instanceMap.get(key);

native/cocos/renderer/pipeline/InstancedBuffer.cpp

@@ -22,13 +22,16 @@
  THE SOFTWARE.
 ****************************************************************************/
 
+#include <algorithm>
+
 #include "InstancedBuffer.h"
 #include "Define.h"
 #include "gfx-base/GFXBuffer.h"
 #include "gfx-base/GFXCommandBuffer.h"
 #include "gfx-base/GFXDescriptorSet.h"
 #include "gfx-base/GFXDevice.h"
 #include "gfx-base/GFXInputAssembler.h"
+#include "base/StringUtil.h"
 
 namespace cc {
 namespace pipeline {
@@ -49,6 +52,7 @@ void InstancedBuffer::destroy() {
         CC_FREE(instance.data);
     }
     _instances.clear();
+    _instancesMap.clear();
 }
 
 void InstancedBuffer::merge(scene::SubModel *subModel, uint32_t passIdx) {
@@ -66,7 +70,7 @@ void InstancedBuffer::merge(scene::SubModel *subModel, uint32_t passIdx, gfx::Sh
     auto *lightingMap = descriptorSet->getTexture(LIGHTMAPTEXTURE::BINDING);
     auto *reflectionProbeCubemap = descriptorSet->getTexture(REFLECTIONPROBECUBEMAP::BINDING);
     auto *reflectionProbePlanarMap = descriptorSet->getTexture(REFLECTIONPROBEPLANARMAP::BINDING);
-    gfx::Texture *reflectionProbeBlendCubemap = ENABLE_PROBE_BLEND
+    auto *reflectionProbeBlendCubemap = ENABLE_PROBE_BLEND
                                                     ? descriptorSet->getTexture(REFLECTIONPROBEBLENDCUBEMAP::BINDING)
                                                     : nullptr;
     const uint32_t reflectionProbeType = subModel->getReflectionProbeType();
@@ -81,91 +85,132 @@ void InstancedBuffer::merge(scene::SubModel *subModel, uint32_t passIdx, gfx::Sh
     _sortRender.hash = hash;
     _sortRender.shaderID = shaderId;
     _sortRender.passIndex = passIdx;
-    for (auto &instance : _instances) {
-        if (instance.ia->getIndexBuffer() != sourceIA->getIndexBuffer() || instance.drawInfo.instanceCount >= MAX_CAPACITY) {
-            continue;
-        }
-
-        // check same binding
-        if (instance.lightingMap != lightingMap) {
-            continue;
+    const ccstd::string key = StringUtil::format("%u/%u/%u/%u/%u/%u/%u",
+                                                 sourceIA->getIndexBuffer() ? sourceIA->getIndexBuffer()->getObjectID() : 0,
+                                                 lightingMap ? lightingMap->getObjectID() : 0,
+                                                 reflectionProbeType,
+                                                 reflectionProbeCubemap ? reflectionProbeCubemap->getObjectID() : 0,
+                                                 reflectionProbePlanarMap ? reflectionProbePlanarMap->getObjectID() : 0,
+                                                 reflectionProbeBlendCubemap ? reflectionProbeBlendCubemap->getObjectID() : 0,
+                                                 stride);
+    const auto iter = _instancesMap.find(key);
+    if (iter != _instancesMap.end()) {
+        for (size_t idx : iter->second) {
+            auto &instance = _instances[idx];
+            if (instance.drawInfo.instanceCount >= MAX_CAPACITY) {
+                continue;
+            }
+            appendInstance(instance, attrs.buffer, shader, descriptorSet);
+            return;
         }
+    }
 
-        if (instance.reflectionProbeType != reflectionProbeType) {
-            continue;
-        }
-        if (instance.reflectionProbeCubemap != reflectionProbeCubemap) {
-            continue;
-        }
-        if (instance.reflectionProbePlanarMap != reflectionProbePlanarMap) {
-            continue;
-        }
-        if (instance.reflectionProbeBlendCubemap != reflectionProbeBlendCubemap) {
-            continue;
-        }
+    createInstance(
+        key,
+        sourceIA,
+        attrs.attributes,
+        attrs.buffer,
+        stride,
+        shader,
+        descriptorSet,
+        lightingMap,
+        reflectionProbeType,
+        reflectionProbeCubemap,
+        reflectionProbePlanarMap,
+        reflectionProbeBlendCubemap
+        );
+}
 
-        if (instance.stride != stride) {
-            continue;
-        }
-        if (instance.drawInfo.instanceCount >= instance.capacity) { // resize buffers
-            instance.capacity <<= 1;
-            const auto newSize = instance.stride * instance.capacity;
-            // NOLINTNEXTLINE(bugprone-suspicious-realloc-usage)
-            instance.data = static_cast<uint8_t *>(CC_REALLOC(instance.data, newSize));
-            instance.vb->resize(newSize);
-        }
-        if (instance.shader != shader) {
-            instance.shader = shader;
-        }
-        if (instance.descriptorSet != descriptorSet) {
-            instance.descriptorSet = descriptorSet;
-        }
-        memcpy(instance.data + static_cast<size_t>(instance.stride) * instance.drawInfo.instanceCount++, attrs.buffer.buffer()->getData(), stride);
-        _hasPendingModels = true;
-        return;
+void InstancedBuffer::appendInstance(InstancedItem &instance,
+                                      Uint8Array& buffer,
+                                      gfx::Shader *shader,
+                                      gfx::DescriptorSet *descriptorSet) {
+    if (instance.drawInfo.instanceCount >= instance.capacity) { // resize buffers
+        instance.capacity = std::min(instance.capacity << 1, MAX_CAPACITY);
+        const auto newSize = instance.stride * instance.capacity;
+        // NOLINTNEXTLINE(bugprone-suspicious-realloc-usage)
+        instance.data = static_cast<uint8_t *>(CC_REALLOC(instance.data, newSize));
+        instance.vb->resize(newSize);
+    }
+    if (instance.shader != shader) {
+        instance.shader = shader;
+    }
+    if (instance.descriptorSet != descriptorSet) {
+        instance.descriptorSet = descriptorSet;
     }
+    auto *destination = instance.data + static_cast<size_t>(instance.stride) * instance.drawInfo.instanceCount;
+    CC_ASSERT(destination);
+    memcpy(destination, buffer.buffer()->getData(), instance.stride);
+    instance.drawInfo.instanceCount++;
+    _hasPendingModels = true;
+}
 
-    // Create a new instance
+void InstancedBuffer::createInstance(const ccstd::string &key,
+                                      gfx::InputAssembler *sourceIA,
+                                      const ccstd::vector<gfx::Attribute> &attributes,
+                                      Uint8Array &buffer,
+                                      uint32_t stride,
+                                      gfx::Shader *shader,
+                                      gfx::DescriptorSet *descriptorSet,
+                                      gfx::Texture *lightingMap,
+                                      uint32_t reflectionProbeType,
+                                      gfx::Texture *reflectionProbeCubemap,
+                                      gfx::Texture *reflectionProbePlanarMap,
+                                      gfx::Texture *reflectionProbeBlendCubemap) {
     const auto newSize = stride * INITIAL_CAPACITY;
     auto *vb = _device->createBuffer({
         gfx::BufferUsageBit::VERTEX | gfx::BufferUsageBit::TRANSFER_DST,
-        gfx::MemoryUsageBit::DEVICE,
+        gfx::MemoryUsageBit::HOST | gfx::MemoryUsageBit::DEVICE,
         static_cast<uint32_t>(newSize),
         static_cast<uint32_t>(stride),
     });
-
+    auto *data = static_cast<uint8_t *>(CC_MALLOC(newSize));
     auto vertexBuffers = sourceIA->getVertexBuffers();
-    auto attributes = sourceIA->getAttributes();
+    auto iaAttributes = sourceIA->getAttributes();
     auto *indexBuffer = sourceIA->getIndexBuffer();
 
-    for (const auto &attribute : attrs.attributes) {
-        attributes.emplace_back(gfx::Attribute{
+    for (const auto &attribute : attributes) {
+        iaAttributes.emplace_back(gfx::Attribute{
             attribute.name,
             attribute.format,
             attribute.isNormalized,
             static_cast<uint32_t>(vertexBuffers.size()), // stream
             true,
             attribute.location});
     }
+    CC_ASSERT(data);
+    memcpy(data, buffer.buffer()->getData(), stride);
 
-    auto *data = static_cast<uint8_t *>(CC_MALLOC(newSize));
-    memcpy(data, attrs.buffer.buffer()->getData(), stride);
     vertexBuffers.emplace_back(vb);
-    const gfx::InputAssemblerInfo iaInfo = {attributes, vertexBuffers, indexBuffer};
+    const gfx::InputAssemblerInfo iaInfo = {iaAttributes, vertexBuffers, indexBuffer};
     auto *ia = _device->createInputAssembler(iaInfo);
     InstancedItem item = {INITIAL_CAPACITY, vb, data, ia, stride, shader, descriptorSet,
-                          lightingMap, reflectionProbeCubemap, reflectionProbePlanarMap, reflectionProbeType, reflectionProbeBlendCubemap,
+                          lightingMap,
+                          reflectionProbeCubemap,
+                          reflectionProbePlanarMap,
+                          reflectionProbeType,
+                          reflectionProbeBlendCubemap,
                           ia->getDrawInfo()};
     item.drawInfo.instanceCount = 1;
     _instances.emplace_back(item);
+    _instancesMap[key].emplace_back(_instances.size() - 1);
     _hasPendingModels = true;
 }
 
 void InstancedBuffer::uploadBuffers(gfx::CommandBuffer *cmdBuff) const {
     for (const auto &instance : _instances) {
         if (!instance.drawInfo.instanceCount) continue;
 
-        cmdBuff->updateBuffer(instance.vb, instance.data, instance.vb->getSize());
+        // `instance.data` is only guaranteed to hold `instance.capacity * instance.stride` bytes.
+        // If `instance.vb` was resized to a larger size than the CPU-side `instance.data`
+        // buffer (e.g. due to a bug in the resize/append logic), copying `instance.vb->getSize()`
+        // bytes from `instance.data` would read out of bounds and crash. Clamp to the smaller
+        // of the two to avoid reading past the end of `instance.data`.
+        const uint32_t dataCapacity = instance.capacity * instance.stride;
+        const uint32_t validSize = instance.drawInfo.instanceCount * instance.stride;
+        CC_ASSERT(validSize <= instance.vb->getSize());
+        CC_ASSERT(validSize <= dataCapacity);
+        cmdBuff->updateBuffer(instance.vb, instance.data, validSize);
         instance.ia->setInstanceCount(instance.drawInfo.instanceCount);
     }
 }

native/cocos/renderer/pipeline/InstancedBuffer.h

@@ -78,14 +78,37 @@ class InstancedBuffer : public RefCounted {
     inline const DynamicOffsetList &dynamicOffsets() const { return _dynamicOffsets; }
 
 private:
-    InstancedItemList _instances;
-    RenderPass _sortRender;
+    void appendInstance(InstancedItem &instance,
+        Uint8Array &buffer,
+        gfx::Shader *shader,
+        gfx::DescriptorSet *descriptorSet);
+    void createInstance(const ccstd::string &key,
+        gfx::InputAssembler *sourceIA,
+        const ccstd::vector<gfx::Attribute> &attributes,
+        Uint8Array &buffer,
+        uint32_t stride,
+        gfx::Shader *shader,
+        gfx::DescriptorSet *descriptorSet,
+        gfx::Texture *lightingMap,
+        uint32_t reflectionProbeType,
+        gfx::Texture *reflectionProbeCubemap,
+        gfx::Texture *reflectionProbePlanarMap,
+        gfx::Texture *reflectionProbeBlendCubemap);
+
     // weak reference
     const scene::Pass *_pass{nullptr};
+    gfx::Device *_device{nullptr};
+
     bool _hasPendingModels{false};
+
+    RenderPass _sortRender;
+
+    InstancedItemList _instances;
     DynamicOffsetList _dynamicOffsets;
-    // weak reference
-    gfx::Device *_device{nullptr};
+    // Maps a merge key to the indices (into `_instances`) of items sharing that key.
+    // Storing indices (not copies or references) keeps this in sync with `_instances`
+    // and remains valid across `_instances` reallocation, since indices don't dangle.
+    ccstd::unordered_map<ccstd::string, ccstd::vector<size_t>> _instancesMap;
 };
 
 } // namespace pipeline