PipelineKey: use Geometry Layout instead of GeometryID

- Allows to create less pipelines
- PipelineUBO: use aligned offsets
- RenderCommand store ShaderResourceBindings (avoids creatings a new one
  every frame) and store last used bindings to see if rebuild is required.

Change-Id: I3a44d340a92f5c48f150896b9aa9912527b3b1ea
Reviewed-by: Mike Krus <mike.krus@kdab.com>

7 files changed, 42 insertions, 25 deletions

diff --git a/src/plugins/renderers/rhi/managers/rhiresourcemanagers_p.h b/src/plugins/renderers/rhi/managers/rhiresourcemanagers_p.h
index c6caa75d1..48e9d8300 100644
--- a/src/plugins/renderers/rhi/managers/rhiresourcemanagers_p.h
+++ b/src/plugins/renderers/rhi/managers/rhiresourcemanagers_p.h
@@ -136,7 +136,7 @@ private:
 
 inline uint qHash(const GraphicsPipelineIdentifier &key, uint seed = 0)
 {
-    const QPair<HGeometry, Qt3DCore::QNodeId> p = { key.geometry, key.shader };
+    const QPair<int, Qt3DCore::QNodeId> p = { key.geometryLayoutKey, key.shader };
     using QT_PREPEND_NAMESPACE(qHash);
     seed = qHash(p, seed);
     seed = qHash(key.renderTarget, seed);
@@ -146,7 +146,7 @@ inline uint qHash(const GraphicsPipelineIdentifier &key, uint seed = 0)
 
 inline bool operator==(const GraphicsPipelineIdentifier &a, const GraphicsPipelineIdentifier &b)
 {
-    return a.geometry == b.geometry &&
+    return a.geometryLayoutKey == b.geometryLayoutKey &&
            a.shader == b.shader &&
            a.renderTarget == b.renderTarget &&
            a.renderViewIndex == b.renderViewIndex;
diff --git a/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp b/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp
index ee80d3cb7..11ba5ea08 100644
--- a/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp
+++ b/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp
@@ -83,7 +83,7 @@ void PipelineUBOSet::setResourceManager(RHIResourceManagers *manager)
     m_resourceManagers = manager;
 }
 
-void PipelineUBOSet::initializeLayout(RHIShader *shader)
+void PipelineUBOSet::initializeLayout(SubmissionContext *ctx, RHIShader *shader)
 {
     // We should only be called with a clean Pipeline
     Q_ASSERT(m_rvUBO.buffer.isNull());
@@ -95,6 +95,7 @@ void PipelineUBOSet::initializeLayout(RHIShader *shader)
 
     m_commandsUBO.binding = 1;
     m_commandsUBO.blockSize = sizeof(CommandUBO);
+    m_commandsUBO.alignedBlockSize = ctx->rhi()->ubufAligned((m_commandsUBO.blockSize));
     m_commandsUBO.buffer = bufferManager->allocateResource();
 
     const std::vector<ShaderUniformBlock> &uniformBlocks = shader->uniformBlocks();
@@ -103,6 +104,7 @@ void PipelineUBOSet::initializeLayout(RHIShader *shader)
             m_materialsUBOs.push_back(
                         { block.m_binding,
                           block.m_size,
+                          size_t(ctx->rhi()->ubufAligned(block.m_size)),
                           bufferManager->allocateResource()});
         }
     }
@@ -136,8 +138,10 @@ bool PipelineUBOSet::allocateUBOs(SubmissionContext *ctx)
     const size_t commandCount = std::max(m_renderCommands.size(), size_t(1));
 
     // RHIBuffer only reallocates if size is < than required
-    m_rvUBO.buffer->allocate(QByteArray(m_rvUBO.blockSize, '\0'), dynamic);
-    m_commandsUBO.buffer->allocate(QByteArray(m_commandsUBO.blockSize * commandCount, '\0'), dynamic);
+    m_rvUBO.buffer->allocate(QByteArray(m_rvUBO.blockSize, '\0'), dynamic);\
+
+    // We need to take into account any minimum alignment requirement for dynamic offsets
+    m_commandsUBO.buffer->allocate(QByteArray(m_commandsUBO.alignedBlockSize * commandCount, '\0'), dynamic);
 
     // Binding buffer ensure underlying RHI resource is created
     m_rvUBO.buffer->bind(ctx, RHIBuffer::UniformBuffer);
@@ -145,7 +149,7 @@ bool PipelineUBOSet::allocateUBOs(SubmissionContext *ctx)
 
     for (const UBOBufferWithBindingAndBlockSize &ubo : m_materialsUBOs) {
         if (ubo.binding > 1) { // Binding 0 and 1 are for RV and Command UBOs
-            ubo.buffer->allocate(QByteArray(ubo.blockSize * commandCount, '\0'), dynamic);
+            ubo.buffer->allocate(QByteArray(ubo.alignedBlockSize * commandCount, '\0'), dynamic);
             ubo.buffer->bind(ctx, RHIBuffer::UniformBuffer);
         }
     }
@@ -172,10 +176,10 @@ std::vector<QRhiCommandBuffer::DynamicOffset> PipelineUBOSet::offsets(const Rend
     // RenderCommand offset
     // binding, offset
     const size_t dToCmd = distanceToCommand(cmd);
-    offsets.push_back({1, dToCmd * sizeof(CommandUBO)});
+    offsets.push_back({1, dToCmd * m_commandsUBO.alignedBlockSize});
 
     for (const UBOBufferWithBindingAndBlockSize &buffer : m_materialsUBOs)
-        offsets.push_back({buffer.binding, dToCmd * buffer.blockSize});
+        offsets.push_back({buffer.binding, dToCmd * buffer.alignedBlockSize});
 
     return offsets;
 }
@@ -359,7 +363,7 @@ void uploadUniform(const PackUniformHash &uniforms,
     QByteArray rawData;
     rawData.resize(member.size);
     memcpy(rawData.data(), value.constData<char>(), std::min(value.byteSize(), member.size));
-    ubo.buffer->update(rawData, ubo.blockSize * distanceToCommand + member.offset + arrayOffset);
+    ubo.buffer->update(rawData, ubo.alignedBlockSize * distanceToCommand + member.offset + arrayOffset);
 
 //    printUpload(value, member);
 }
@@ -376,7 +380,7 @@ void PipelineUBOSet::uploadUBOsForCommand(const RenderCommand &command,
     m_commandsUBO.buffer->update(QByteArray::fromRawData(
                                      reinterpret_cast<const char *>(&command.m_commandUBO),
                                      sizeof(CommandUBO)),
-                                 distanceToCommand * sizeof(CommandUBO));
+                                 distanceToCommand * m_commandsUBO.alignedBlockSize);
 
     const std::vector<RHIShader::UBO_Member> &uboMembers = shader->uboMembers();
     const ShaderParameterPack &parameterPack = command.m_parameterPack;
diff --git a/src/plugins/renderers/rhi/renderer/pipelineuboset_p.h b/src/plugins/renderers/rhi/renderer/pipelineuboset_p.h
index 09173783b..6eae0ccaf 100644
--- a/src/plugins/renderers/rhi/renderer/pipelineuboset_p.h
+++ b/src/plugins/renderers/rhi/renderer/pipelineuboset_p.h
@@ -76,6 +76,7 @@ public:
     {
         int binding = -1;
         int blockSize = -1;
+        size_t alignedBlockSize = 0;
         HRHIBuffer buffer;
     };
 
@@ -93,7 +94,7 @@ public:
     bool allocateUBOs(SubmissionContext *ctx);
     void uploadUBOs(SubmissionContext *ctx, RenderView *rv);
     void setResourceManager(RHIResourceManagers *manager);
-    void initializeLayout(RHIShader *shader);
+    void initializeLayout(SubmissionContext *ctx, RHIShader *shader);
 
 private:
     void releaseResources();
diff --git a/src/plugins/renderers/rhi/renderer/rendercommand_p.h b/src/plugins/renderers/rhi/renderer/rendercommand_p.h
index 42a305122..6f0169acf 100644
--- a/src/plugins/renderers/rhi/renderer/rendercommand_p.h
+++ b/src/plugins/renderers/rhi/renderer/rendercommand_p.h
@@ -175,6 +175,8 @@ public:
 
     CommandUBO m_commandUBO;
     RHIGraphicsPipeline *pipeline {};
+    QRhiShaderResourceBindings *shaderResourceBindings = nullptr;
+    std::vector<QRhiShaderResourceBinding> resourcesBindings;
 };
 
 Q_AUTOTEST_EXPORT bool operator==(const RenderCommand &a, const RenderCommand &b) noexcept;
diff --git a/src/plugins/renderers/rhi/renderer/renderer.cpp b/src/plugins/renderers/rhi/renderer/renderer.cpp
index afc0bc821..e3bda8d6b 100644
--- a/src/plugins/renderers/rhi/renderer/renderer.cpp
+++ b/src/plugins/renderers/rhi/renderer/renderer.cpp
@@ -884,17 +884,17 @@ void Renderer::updateGraphicsPipeline(RenderCommand &cmd, RenderView *rv,
 
     // Try to retrieve existing pipeline
     // TO DO: Make RenderState part of the Key
-    // TO DO: Compute Key based on geometry rather than using geometryId
     // as it is likely many geometrys will have the same layout
     RHIGraphicsPipelineManager *pipelineManager = m_RHIResourceManagers->rhiGraphicsPipelineManager();
-    const GraphicsPipelineIdentifier pipelineKey { cmd.m_geometry, cmd.m_shaderId, rv->renderTargetId(), renderViewIndex };
+    const int geometryLayoutId = pipelineManager->getIdForAttributeVec(cmd.m_attributeInfo);
+    const GraphicsPipelineIdentifier pipelineKey { geometryLayoutId, cmd.m_shaderId, rv->renderTargetId(), renderViewIndex };
     RHIGraphicsPipeline *graphicsPipeline = pipelineManager->lookupResource(pipelineKey);
     if (graphicsPipeline == nullptr) {
         // Init UBOSet the first time we allocate a new pipeline
         graphicsPipeline = pipelineManager->getOrCreateResource(pipelineKey);
         graphicsPipeline->setKey(pipelineKey);
         graphicsPipeline->uboSet()->setResourceManager(m_RHIResourceManagers);
-        graphicsPipeline->uboSet()->initializeLayout(cmd.m_rhiShader);
+        graphicsPipeline->uboSet()->initializeLayout(m_submissionContext.data(), cmd.m_rhiShader);
     }
 
     // Increase score so that we know the pipeline was used for this frame and shouldn't be
@@ -2346,7 +2346,7 @@ bool Renderer::uploadBuffersForCommand(QRhiCommandBuffer *cb, const RenderView *
 }
 
 bool Renderer::performDraw(QRhiCommandBuffer *cb, const QRhiViewport &vp,
-                           const QRhiScissor *scissor, const RenderCommand &command)
+                           const QRhiScissor *scissor, RenderCommand &command)
 {
     RHIGraphicsPipeline *pipeline = command.pipeline;
     if (!pipeline)
@@ -2362,17 +2362,27 @@ bool Renderer::performDraw(QRhiCommandBuffer *cb, const QRhiViewport &vp,
     // have different textures or reference custom UBOs (if using Parameters with UBOs directly).
     // TO DO: We could propably check for texture and use the UBO set default ShaderResourceBindings
     // if we only have UBOs with offsets
-    const std::vector<QRhiShaderResourceBinding> resourcesBindings = pipeline->uboSet()->resourceBindings(command);
-    const std::vector<QRhiCommandBuffer::DynamicOffset> offsets = pipeline->uboSet()->offsets(command);
-    QRhiShaderResourceBindings *shaderResourceBindings = m_submissionContext->rhi()->newShaderResourceBindings();
-    shaderResourceBindings->setBindings(resourcesBindings.cbegin(), resourcesBindings.cend());
-    if (!shaderResourceBindings->create()) {
+    bool needsRecreate = false;
+    if (command.shaderResourceBindings == nullptr) {
+        command.shaderResourceBindings = m_submissionContext->rhi()->newShaderResourceBindings();
+        needsRecreate = true;
+    }
+
+    // TO DO: Improve this to only perform when required
+    const std::vector<QRhiShaderResourceBinding> &resourcesBindings = pipeline->uboSet()->resourceBindings(command);
+    if (command.resourcesBindings != resourcesBindings) {
+        command.resourcesBindings = std::move(resourcesBindings);
+        command.shaderResourceBindings->setBindings(command.resourcesBindings.cbegin(), command.resourcesBindings.cend());
+        needsRecreate = true;
+    }
+
+    if (needsRecreate && !command.shaderResourceBindings->create()) {
         qCWarning(Backend) << "Failed to create ShaderResourceBindings";
         return false;
     }
+    const std::vector<QRhiCommandBuffer::DynamicOffset> offsets = pipeline->uboSet()->offsets(command);
 
-    // TO DO: Use UBO set shaderResourcesBindings
-    cb->setShaderResources(pipeline->pipeline()->shaderResourceBindings(),
+    cb->setShaderResources(command.shaderResourceBindings,
                            offsets.size(),
                            offsets.data());
 
@@ -2506,7 +2516,7 @@ bool Renderer::executeCommandsSubmission(const RHIPassInfo &passInfo)
         }
 
         // Render drawing commands
-        rv->forEachCommand([&] (const RenderCommand &command) {
+        rv->forEachCommand([&] (RenderCommand &command) {
             if (Q_UNLIKELY(!command.isValid()))
                 return;
 
diff --git a/src/plugins/renderers/rhi/renderer/renderer_p.h b/src/plugins/renderers/rhi/renderer/renderer_p.h
index 2f966249a..190fed178 100644
--- a/src/plugins/renderers/rhi/renderer/renderer_p.h
+++ b/src/plugins/renderers/rhi/renderer/renderer_p.h
@@ -432,7 +432,7 @@ private:
     bool uploadBuffersForCommand(QRhiCommandBuffer *cb, const RenderView *rv,
                                  RenderCommand &command);
     bool performDraw(QRhiCommandBuffer *cb, const QRhiViewport &vp, const QRhiScissor *scissor,
-                     const RenderCommand &command);
+                     RenderCommand &command);
 };
 
 } // namespace Rhi
diff --git a/src/plugins/renderers/rhi/renderer/rhigraphicspipeline_p.h b/src/plugins/renderers/rhi/renderer/rhigraphicspipeline_p.h
index 80b724ee4..68242b525 100644
--- a/src/plugins/renderers/rhi/renderer/rhigraphicspipeline_p.h
+++ b/src/plugins/renderers/rhi/renderer/rhigraphicspipeline_p.h
@@ -70,7 +70,7 @@ class RHIBuffer;
 // Geometry | Shader | RenderStateMask
 struct GraphicsPipelineIdentifier
 {
-    HGeometry geometry;
+    int geometryLayoutKey;
     Qt3DCore::QNodeId shader;
     Qt3DCore::QNodeId renderTarget;
     int renderViewIndex = 0;