ShaderParameterPack: check we only store a single UBO/SSBO per BlockBinding

Since we cache ShaderParameterPacks over multiple frames, we need to
ensure that we check that when setting a UBO/SSBO we don't already
have an entry matching the block index.
If that's the case we should simply replace the entry rather than
append a new entry. Failing to do so results in appending a growing
number of UBO entries for the same block.

Change-Id: Ie7535a80b9426ce2fd5ab6ebe16ea71bd7447750
Reviewed-by: Mike Krus <mike.krus@kdab.com>
(cherry picked from commit f672753556cf28d44f864457dc6338a55e2e0ed4)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 80 insertions, 2 deletions

diff --git a/src/plugins/renderers/opengl/renderer/shaderparameterpack.cpp b/src/plugins/renderers/opengl/renderer/shaderparameterpack.cpp
index 6ca2155b5..683d04d8d 100644
--- a/src/plugins/renderers/opengl/renderer/shaderparameterpack.cpp
+++ b/src/plugins/renderers/opengl/renderer/shaderparameterpack.cpp
@@ -62,12 +62,28 @@ void ShaderParameterPack::setImage(const int glslNameId, int uniformArrayIndex,
 // Contains Uniform Block Index and QNodeId of the ShaderData (UBO)
 void ShaderParameterPack::setUniformBuffer(BlockToUBO blockToUBO)
 {
-    m_uniformBuffers.push_back(std::move(blockToUBO));
+    const auto uEnd = m_uniformBuffers.end();
+    auto it = std::find_if(m_uniformBuffers.begin(), uEnd, [&] (const BlockToUBO &block) {
+        return blockToUBO.m_blockIndex == block.m_blockIndex;
+    });
+
+    if (it == uEnd)
+        m_uniformBuffers.push_back(std::move(blockToUBO));
+    else
+        *it = std::move(blockToUBO);
 }
 
 void ShaderParameterPack::setShaderStorageBuffer(BlockToSSBO blockToSSBO)
 {
-    m_shaderStorageBuffers.push_back(std::move(blockToSSBO));
+    const auto uEnd = m_shaderStorageBuffers.end();
+    auto it = std::find_if(m_shaderStorageBuffers.begin(), uEnd, [&] (const BlockToSSBO &block) {
+        return blockToSSBO.m_blockIndex == block.m_blockIndex;
+    });
+
+    if (it == uEnd)
+        m_shaderStorageBuffers.push_back(std::move(blockToSSBO));
+    else
+        *it = std::move(blockToSSBO);
 }
 
 void ShaderParameterPack::setSubmissionUniformIndex(const int uniformIdx)
diff --git a/tests/auto/render/opengl/renderviews/tst_renderviews.cpp b/tests/auto/render/opengl/renderviews/tst_renderviews.cpp
index eba3e9721..69d151dfc 100644
--- a/tests/auto/render/opengl/renderviews/tst_renderviews.cpp
+++ b/tests/auto/render/opengl/renderviews/tst_renderviews.cpp
@@ -269,6 +269,68 @@ private Q_SLOTS:
                 << QList<ShaderParameterPack> { pack1, minifiedPack1, minifiedPack1, pack1, minifiedPack1, minifiedPack1 };
     }
 
+    void checkShaderParameterPackStoresSingleUBOPerBlockIndex()
+    {
+        // GIVEN
+        ShaderParameterPack pack;
+        auto nodeId1 = Qt3DCore::QNodeId::createId();
+        auto nodeId2 = Qt3DCore::QNodeId::createId();
+
+        BlockToUBO ubo1 { 1, nodeId1, false, {}};
+        BlockToUBO ubo2 { 1, nodeId2, false, {}};
+
+        // WHEN
+        pack.setUniformBuffer(ubo1);
+        pack.setUniformBuffer(ubo2);
+
+        // THEN
+        QCOMPARE(pack.uniformBuffers().size(), 1);
+        QCOMPARE(pack.uniformBuffers().front().m_blockIndex, 1);
+        QCOMPARE(pack.uniformBuffers().front().m_bufferID, nodeId2);
+
+        // WHEN
+        BlockToUBO ubo3 { 2, nodeId2, false, {}};
+        pack.setUniformBuffer(ubo3);
+
+        // THEN
+        QCOMPARE(pack.uniformBuffers().size(), 2);
+        QCOMPARE(pack.uniformBuffers().front().m_blockIndex, 1);
+        QCOMPARE(pack.uniformBuffers().front().m_bufferID, nodeId2);
+        QCOMPARE(pack.uniformBuffers().back().m_blockIndex, 2);
+        QCOMPARE(pack.uniformBuffers().back().m_bufferID, nodeId2);
+    }
+
+    void checkShaderParameterPackStoresSingleSSBOPerBlockIndex()
+    {
+        // GIVEN
+        ShaderParameterPack pack;
+        auto nodeId1 = Qt3DCore::QNodeId::createId();
+        auto nodeId2 = Qt3DCore::QNodeId::createId();
+
+        BlockToSSBO ssbo1 { 1, 0, nodeId1};
+        BlockToSSBO ssbo2 { 1, 0, nodeId2};
+
+        // WHEN
+        pack.setShaderStorageBuffer(ssbo1);
+        pack.setShaderStorageBuffer(ssbo2);
+
+        // THEN
+        QCOMPARE(pack.shaderStorageBuffers().size(), 1);
+        QCOMPARE(pack.shaderStorageBuffers().front().m_blockIndex, 1);
+        QCOMPARE(pack.shaderStorageBuffers().front().m_bufferID, nodeId2);
+
+        // WHEN
+        BlockToSSBO ssbo3 { 2, 1, nodeId2};
+        pack.setShaderStorageBuffer(ssbo3);
+
+        // THEN
+        QCOMPARE(pack.shaderStorageBuffers().size(), 2);
+        QCOMPARE(pack.shaderStorageBuffers().front().m_blockIndex, 1);
+        QCOMPARE(pack.shaderStorageBuffers().front().m_bufferID, nodeId2);
+        QCOMPARE(pack.shaderStorageBuffers().back().m_blockIndex, 2);
+        QCOMPARE(pack.shaderStorageBuffers().back().m_bufferID, nodeId2);
+    }
+
     void checkRenderViewUniformMinification()
     {
         QFETCH(QList<QShaderProgram*>, shaders);