Remove string concatenations when uploading UBOs

- We instead record nameIds when we first parse the shader and use
those at submission time.

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

4 files changed, 72 insertions, 83 deletions

diff --git a/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp b/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp
index 11ba5ea08..5f896fe1c 100644
--- a/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp
+++ b/src/plugins/renderers/rhi/renderer/pipelineuboset.cpp
@@ -336,34 +336,19 @@ void printUpload(const UniformValue &value, const QShaderDescription::BlockVaria
 }
 
 void uploadUniform(const PackUniformHash &uniforms,
-                   const RHIShader::UBO_Member &uboMember,
-                   const std::vector<PipelineUBOSet::UBOBufferWithBindingAndBlockSize> &uboBuffers,
-                   const QString &uniformName, const QShaderDescription::BlockVariable &member,
+                   const PipelineUBOSet::UBOBufferWithBindingAndBlockSize &ubo,
+                   const RHIShader::UBO_Member &member,
                    size_t distanceToCommand, int arrayOffset = 0)
 {
-    const int uniformNameId = StringToInt::lookupId(uniformName);
-
-    if (!uniforms.contains(uniformNameId))
-        return;
-
-    const UniformValue value = uniforms.value(uniformNameId);
-    const ShaderUniformBlock block = uboMember.block;
-
-    // Update UBO with uniform value
-    auto it = std::find_if(uboBuffers.begin(), uboBuffers.end(), [&block] (const PipelineUBOSet::UBOBufferWithBindingAndBlockSize &buffer) {
-        return buffer.binding == block.m_binding;
-    });
-
-    if (it == uboBuffers.end()) {
+    if (!uniforms.contains(member.nameId))
         return;
-    }
 
-    const PipelineUBOSet::UBOBufferWithBindingAndBlockSize &ubo = *it;
+    const UniformValue value = uniforms.value(member.nameId);
 
     QByteArray rawData;
-    rawData.resize(member.size);
-    memcpy(rawData.data(), value.constData<char>(), std::min(value.byteSize(), member.size));
-    ubo.buffer->update(rawData, ubo.alignedBlockSize * distanceToCommand + member.offset + arrayOffset);
+    rawData.resize(member.blockVariable.size);
+    memcpy(rawData.data(), value.constData<char>(), std::min(value.byteSize(), member.blockVariable.size));
+    ubo.buffer->update(rawData, ubo.alignedBlockSize * distanceToCommand + member.blockVariable.offset + arrayOffset);
 
 //    printUpload(value, member);
 }
@@ -382,40 +367,49 @@ void PipelineUBOSet::uploadUBOsForCommand(const RenderCommand &command,
                                      sizeof(CommandUBO)),
                                  distanceToCommand * m_commandsUBO.alignedBlockSize);
 
-    const std::vector<RHIShader::UBO_Member> &uboMembers = shader->uboMembers();
+    const std::vector<RHIShader::UBO_Block> &uboBlocks = shader->uboBlocks();
     const ShaderParameterPack &parameterPack = command.m_parameterPack;
     const PackUniformHash &uniforms = parameterPack.uniforms();
 
     // Update Buffer CPU side data based on uniforms being set
-    for (const RHIShader::UBO_Member &uboMember : uboMembers) {
+    for (const RHIShader::UBO_Block &uboBlock : uboBlocks) {
 
         // No point in trying to update Bindings 0 or 1 which are reserved
         // for Qt3D default uniforms
-        if (uboMember.block.m_binding <= 1)
+        const ShaderUniformBlock &block = uboBlock.block;
+        if (block.m_binding <= 1)
             continue;
 
-        for (const QShaderDescription::BlockVariable &member : qAsConst(uboMember.members)) {
-            if (!member.arrayDims.empty()) {
-                if (!member.structMembers.empty()) {
-                    const int arr0 = member.arrayDims[0];
+        // Update UBO with uniform value
+        auto it = std::find_if(m_materialsUBOs.begin(), m_materialsUBOs.end(), [&block] (const PipelineUBOSet::UBOBufferWithBindingAndBlockSize &buffer) {
+            return buffer.binding == block.m_binding;
+        });
+
+        if (it == m_materialsUBOs.end())
+            continue;
+
+        const PipelineUBOSet::UBOBufferWithBindingAndBlockSize &ubo = *it;
+
+        for (const RHIShader::UBO_Member &member : qAsConst(uboBlock.members)) {
+            const QShaderDescription::BlockVariable &blockVariable = member.blockVariable;
+            if (!blockVariable.arrayDims.empty()) {
+                if (!blockVariable.structMembers.empty()) {
+                    const int arr0 = blockVariable.arrayDims[0];
                     for (int i = 0; i < arr0; i++) {
-                        for (const QShaderDescription::BlockVariable &structMember :
-                             member.structMembers) {
-                            const QString processedName = member.name + "[" + QString::number(i)
-                                    + "]." + structMember.name;
-                            uploadUniform(uniforms, uboMember, m_materialsUBOs,
-                                          processedName, structMember,
+                        for (const RHIShader::UBO_Member &structMember : member.structMembers) {
+                            uploadUniform(uniforms, ubo,
+                                          structMember,
                                           distanceToCommand,
-                                          i * member.size / arr0);
+                                          i * blockVariable.size / arr0);
                         }
                     }
                 } else {
-                    uploadUniform(uniforms, uboMember, m_materialsUBOs,
-                                  member.name, member, distanceToCommand);
+                    uploadUniform(uniforms, ubo,
+                                  member, distanceToCommand);
                 }
             } else {
-                uploadUniform(uniforms, uboMember, m_materialsUBOs,
-                              member.name, member, distanceToCommand);
+                uploadUniform(uniforms, ubo,
+                              member, distanceToCommand);
             }
         }
     }
diff --git a/src/plugins/renderers/rhi/renderer/renderer.cpp b/src/plugins/renderers/rhi/renderer/renderer.cpp
index e3bda8d6b..2b9c858d0 100644
--- a/src/plugins/renderers/rhi/renderer/renderer.cpp
+++ b/src/plugins/renderers/rhi/renderer/renderer.cpp
@@ -1256,9 +1256,6 @@ Renderer::prepareCommandsSubmission(const std::vector<RenderView *> &renderViews
                 if (rGeometryRenderer->isDirty())
                     rGeometryRenderer->unsetDirty();
 
-                // Prepare the ShaderParameterPack based on the active uniforms of the shader
-                //  shader->prepareUniforms(command.m_parameterPack);
-
                 updateGraphicsPipeline(command, rv, i);
 
             } else if (command.m_type == RenderCommand::Compute) {
diff --git a/src/plugins/renderers/rhi/renderer/rhishader.cpp b/src/plugins/renderers/rhi/renderer/rhishader.cpp
index 0e1c35b97..9d79b3282 100644
--- a/src/plugins/renderers/rhi/renderer/rhishader.cpp
+++ b/src/plugins/renderers/rhi/renderer/rhishader.cpp
@@ -325,9 +325,10 @@ void forEachArrayAccessor(const QList<int> &maxs, F f)
 }
 }
 
-void RHIShader::recordAllUniforms(const QShaderDescription::BlockVariable &member,
+void RHIShader::recordAllUniforms(UBO_Member &uboMember,
                                   QString parentName)
 {
+    const QShaderDescription::BlockVariable &member = uboMember.blockVariable;
     const bool isStruct = !member.structMembers.empty();
     const bool isArray = !member.arrayDims.empty();
 
@@ -340,8 +341,11 @@ void RHIShader::recordAllUniforms(const QShaderDescription::BlockVariable &membe
         m_structNamesIds.push_back(StringToInt::lookupId(fullMemberName));
 
         for (const QShaderDescription::BlockVariable& bv : member.structMembers) {
+            UBO_Member innerMember {StringToInt::lookupId(fullMemberName), bv, {}};
             // recordAllUniforms("baz", "foo.bar.")
-            recordAllUniforms(bv, fullMemberName + QLatin1Char('.'));
+            const QString structMemberNamePrefix = fullMemberName + QLatin1Char('.');
+            recordAllUniforms(innerMember, structMemberNamePrefix);
+            uboMember.structMembers.push_back(innerMember);
         }
     } else if (!isStruct && isArray) {
         // We iterate through all the [l][n][m] by building [0][0][0] and incrementing
@@ -363,9 +367,16 @@ void RHIShader::recordAllUniforms(const QShaderDescription::BlockVariable &membe
         for (const QShaderDescription::BlockVariable& bv : member.structMembers) {
             forEachArrayAccessor(member.arrayDims, [&] (const QString& str) {
                 //recordAllUniforms("baz", "foo.bar[1][2].")
-                recordAllUniforms(bv, fullMemberName + str + QLatin1Char('.'));
+                const QString structMemberNamePrefix = fullMemberName + str + QLatin1Char('.');
+                UBO_Member innerMember {StringToInt::lookupId(structMemberNamePrefix), bv, {}};
+                recordAllUniforms(innerMember, structMemberNamePrefix);
+                uboMember.structMembers.push_back(innerMember);
             });
         }
+    } else {
+        // Final member (not array or struct)
+        // Replace nameId with final nameId name
+        uboMember.nameId = StringToInt::lookupId(fullMemberName);
     }
 }
 
@@ -451,13 +462,20 @@ void RHIShader::introspect()
         // member
         m_uniformsNamesIds.reserve(m_uniformsNamesIds.size() + ubo.members.size());
 
+        std::vector<UBO_Member> uboMembers;
+        uboMembers.reserve(ubo.members.size());
+
         for (const QShaderDescription::BlockVariable &member : qAsConst(ubo.members)) {
-            m_uniformsNamesIds.push_back(StringToInt::lookupId(member.name));
-            if (addUnqualifiedUniforms) {
-                recordAllUniforms(member, QStringLiteral(""));
-            }
+            const int nameId = StringToInt::lookupId(member.name);
+            m_uniformsNamesIds.push_back(nameId);
+
+            UBO_Member uboMember {nameId, member, {}};
+            if (addUnqualifiedUniforms)
+                recordAllUniforms(uboMember, QStringLiteral(""));
+            uboMembers.push_back(uboMember);
         }
-        m_uboMembers.push_back(UBO_Member{ uniformBlocks.back(), ubo.members });
+
+        m_uboBlocks.push_back(UBO_Block{ uniformBlocks.back(), uboMembers });
     }
 
     for (const QShaderDescription::StorageBlock &ssbo : rhiSSBO) {
@@ -563,32 +581,6 @@ void RHIShader::setShaderCode(const std::vector<QByteArray> &shaderCode)
     Qt3DCore::append(m_shaderCode, shaderCode);
 }
 
-void RHIShader::prepareUniforms(ShaderParameterPack &pack)
-{
-
-    const PackUniformHash &values = pack.uniforms();
-
-    auto it = values.keys.cbegin();
-    const auto end = values.keys.cend();
-
-    const int shaderUniformsCount = m_uniforms.size();
-    const auto uIt = m_uniforms.cbegin();
-
-    while (it != end) {
-        // Find if there's a uniform with the same name id
-
-        int i = 0;
-        const int targetNameId = *it;
-        while (i < shaderUniformsCount && (uIt + i)->m_nameId < targetNameId)
-            ++i;
-
-        if (i < shaderUniformsCount && (uIt + i)->m_nameId == targetNameId)
-            pack.setSubmissionUniformIndex(i);
-
-        ++it;
-    }
-}
-
 void RHIShader::setFragOutputs(const QHash<QString, int> &fragOutputs)
 {
     {
diff --git a/src/plugins/renderers/rhi/renderer/rhishader_p.h b/src/plugins/renderers/rhi/renderer/rhishader_p.h
index 8b7fe18e0..05bc5382c 100644
--- a/src/plugins/renderers/rhi/renderer/rhishader_p.h
+++ b/src/plugins/renderers/rhi/renderer/rhishader_p.h
@@ -69,10 +69,18 @@ namespace Rhi {
 class Q_AUTOTEST_EXPORT RHIShader
 {
 public:
+
     struct UBO_Member
     {
+        int nameId;
+        QShaderDescription::BlockVariable blockVariable;
+        std::vector<UBO_Member> structMembers;
+    };
+
+    struct UBO_Block
+    {
         ShaderUniformBlock block;
-        QList<QShaderDescription::BlockVariable> members;
+        std::vector<UBO_Member> members;
     };
 
     RHIShader();
@@ -80,8 +88,6 @@ public:
     bool isLoaded() const { return m_isLoaded; }
     void setLoaded(bool loaded) { m_isLoaded = loaded; }
 
-    void prepareUniforms(ShaderParameterPack &pack);
-
     void setFragOutputs(const QHash<QString, int> &fragOutputs);
     const QHash<QString, int> fragOutputs() const;
 
@@ -125,7 +131,7 @@ public:
     const std::vector<QByteArray> &shaderCode() const;
 
     const QShader &shaderStage(QShader::Stage stage) const noexcept { return m_stages[stage]; }
-    std::vector<UBO_Member> uboMembers() const { return m_uboMembers; }
+    const std::vector<UBO_Block> &uboBlocks() const { return m_uboBlocks; }
 
     const QSet<QString> &unqualifiedUniformNames() const noexcept
     {
@@ -179,9 +185,9 @@ private:
     initializeShaderStorageBlocks(const std::vector<ShaderStorageBlock> &shaderStorageBlockDescription);
     void initializeSamplers(const std::vector<ShaderAttribute> &samplerDescription);
     void initializeImages(const std::vector<ShaderAttribute> &imageDescription);
-    void recordAllUniforms(const QShaderDescription::BlockVariable &ubo, QString parentName);
+    void recordAllUniforms(UBO_Member &uboMember, QString parentName);
 
-    std::vector<UBO_Member> m_uboMembers;
+    std::vector<UBO_Block> m_uboBlocks;
 
     mutable QMutex m_mutex;
     QMetaObject::Connection m_contextConnection;