Fix array of textures in shader parameter

Task-number: QTBUG-61759
Change-Id: I2c61f9cf41124816d10da90783c7fb9ff6088fc4
Reviewed-by: Mike Krus <mike.krus@kdab.com>

8 files changed, 94 insertions, 52 deletions

diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp
index 738ce6144..e11ef189d 100644
--- a/src/core/nodes/qnode.cpp
+++ b/src/core/nodes/qnode.cpp
@@ -388,28 +388,43 @@ void QNodePrivate::propertyChanged(int propertyIndex)
     if (m_blockNotifications)
         return;
 
+    const auto toBackendValue = [this](const QVariant &data) -> QVariant
+    {
+        if (data.canConvert<QNode*>()) {
+            QNode *node = data.value<QNode*>();
+
+            // Ensure the node has issued a node creation change. We can end
+            // up here if a newly created node with a parent is immediately set
+            // as a property on another node. In this case the deferred call to
+            // _q_postConstructorInit() will not have happened yet as the event
+            // loop will still be blocked. So force it here and we catch this
+            // eventuality in the _q_postConstructorInit() function so that we
+            // do not repeat the creation and new child scene change events.
+            if (node)
+                QNodePrivate::get(node)->_q_postConstructorInit();
+
+            const QNodeId id = node ? node->id() : QNodeId();
+            return QVariant::fromValue(id);
+        }
+
+        return data;
+    };
+
     Q_Q(QNode);
 
     const QMetaProperty property = q->metaObject()->property(propertyIndex);
 
     const QVariant data = property.read(q);
-    if (data.canConvert<QNode*>()) {
-        QNode *node = data.value<QNode*>();
-
-        // Ensure the node has issued a node creation change. We can end
-        // up here if a newly created node with a parent is immediately set
-        // as a property on another node. In this case the deferred call to
-        // _q_postConstructorInit() will not have happened yet as the event
-        // loop will still be blocked. So force it here and we catch this
-        // eventuality in the _q_postConstructorInit() function so that we
-        // do not repeat the creation and new child scene change events.
-        if (node)
-            QNodePrivate::get(node)->_q_postConstructorInit();
-
-        const QNodeId id = node ? node->id() : QNodeId();
-        notifyPropertyChange(property.name(), QVariant::fromValue(id));
+
+    if (data.type() == QVariant::List) {
+        QSequentialIterable iterable = data.value<QSequentialIterable>();
+        QVariantList variants;
+        variants.reserve(iterable.size());
+        for (const auto &v : iterable)
+            variants.append(toBackendValue(v));
+        notifyPropertyChange(property.name(), variants);
     } else {
-        notifyPropertyChange(property.name(), data);
+        notifyPropertyChange(property.name(), toBackendValue(data));
     }
 }
 
diff --git a/src/render/backend/renderview.cpp b/src/render/backend/renderview.cpp
index 27bc0cb0d..8c4bcaa96 100644
--- a/src/render/backend/renderview.cpp
+++ b/src/render/backend/renderview.cpp
@@ -741,14 +741,19 @@ void RenderView::setUniformValue(ShaderParameterPack &uniformPack, int nameId, c
     // ShaderData/Buffers would be handled as UBO/SSBO and would therefore
     // not be in the default uniform block
     if (value.valueType() == UniformValue::NodeId) {
-        const Qt3DCore::QNodeId texId = *value.constData<Qt3DCore::QNodeId>();
-        const Texture *tex =  m_manager->textureManager()->lookupResource(texId);
-        if (tex != nullptr) {
-            uniformPack.setTexture(nameId, texId);
-            UniformValue::Texture textureValue;
-            textureValue.nodeId = texId;
-            uniformPack.setUniform(nameId, UniformValue(textureValue));
+        const Qt3DCore::QNodeId *nodeIds = value.constData<Qt3DCore::QNodeId>();
+
+        const int uniformArraySize = value.byteSize() / sizeof(Qt3DCore::QNodeId);
+        for (int i = 0; i < uniformArraySize; ++i) {
+            const Qt3DCore::QNodeId texId = nodeIds[i];
+            const Texture *tex =  m_manager->textureManager()->lookupResource(texId);
+            if (tex != nullptr)
+                uniformPack.setTexture(nameId, i, texId);
         }
+
+        UniformValue textureValue(uniformArraySize * sizeof(int), UniformValue::TextureValue);
+        std::fill(textureValue.data<int>(), textureValue.data<int>() + uniformArraySize, -1);
+        uniformPack.setUniform(nameId, textureValue);
     } else {
         uniformPack.setUniform(nameId, value);
     }
diff --git a/src/render/backend/shaderparameterpack.cpp b/src/render/backend/shaderparameterpack.cpp
index 01a977aee..f78e45a5e 100644
--- a/src/render/backend/shaderparameterpack.cpp
+++ b/src/render/backend/shaderparameterpack.cpp
@@ -65,17 +65,17 @@ void ShaderParameterPack::setUniform(const int glslNameId, const UniformValue &v
     m_uniforms.insert(glslNameId, val);
 }
 
-void ShaderParameterPack::setTexture(const int glslNameId, Qt3DCore::QNodeId texId)
+void ShaderParameterPack::setTexture(const int glslNameId, int uniformArrayIndex, Qt3DCore::QNodeId texId)
 {
     for (int t=0; t<m_textures.size(); ++t) {
-        if (m_textures[t].glslNameId != glslNameId)
+        if (m_textures[t].glslNameId != glslNameId || m_textures[t].uniformArrayIndex != uniformArrayIndex)
             continue;
 
         m_textures[t].texId = texId;
         return;
     }
 
-    m_textures.append(NamedTexture(glslNameId, texId));
+    m_textures.append(NamedTexture(glslNameId, texId, uniformArrayIndex));
 }
 
 // Contains Uniform Block Index and QNodeId of the ShaderData (UBO)
diff --git a/src/render/backend/shaderparameterpack_p.h b/src/render/backend/shaderparameterpack_p.h
index c0ab05e57..abd63a187 100644
--- a/src/render/backend/shaderparameterpack_p.h
+++ b/src/render/backend/shaderparameterpack_p.h
@@ -96,7 +96,7 @@ public:
     ~ShaderParameterPack();
 
     void setUniform(const int glslNameId, const UniformValue &val);
-    void setTexture(const int glslNameId, Qt3DCore::QNodeId id);
+    void setTexture(const int glslNameId, int uniformArrayIndex, Qt3DCore::QNodeId id);
     void setUniformBuffer(BlockToUBO blockToUBO);
     void setShaderStorageBuffer(BlockToSSBO blockToSSBO);
     void setSubmissionUniform(const ShaderUniform &uniform);
@@ -108,13 +108,15 @@ public:
     struct NamedTexture
     {
         NamedTexture() {}
-        NamedTexture(const int nm, Qt3DCore::QNodeId t)
-            : glslNameId(nm)
-            , texId(t)
+        NamedTexture(const int glslNameId, Qt3DCore::QNodeId texId, int uniformArrayIndex)
+            : glslNameId(glslNameId)
+            , texId(texId)
+            , uniformArrayIndex(uniformArrayIndex)
         { }
 
         int glslNameId;
         Qt3DCore::QNodeId texId;
+        int uniformArrayIndex;
     };
 
     inline QVector<NamedTexture> textures() const { return m_textures; }
diff --git a/src/render/backend/uniform.cpp b/src/render/backend/uniform.cpp
index c2088b60f..ada83628c 100644
--- a/src/render/backend/uniform.cpp
+++ b/src/render/backend/uniform.cpp
@@ -51,6 +51,9 @@ const int qNodeIdTypeId = qMetaTypeId<Qt3DCore::QNodeId>();
 // glUniform*fv/glUniform*iv/glUniform*uiv -> only handles sizeof(float)/sizeof(int)
 int byteSizeForMetaType(int type)
 {
+    if (type == qNodeIdTypeId)
+        return sizeof(Qt3DCore::QNodeId);
+
     switch (type) {
     case QMetaType::Bool:
     case QMetaType::Int:
@@ -209,6 +212,11 @@ UniformValue UniformValue::fromVariant(const QVariant &variant)
             break;
 
         const int listEntryType = variants.first().userType();
+
+        // array of textures
+        if (listEntryType == qNodeIdTypeId)
+            v.m_valueType = NodeId;
+
         const int stride = byteSizeForMetaType(listEntryType) / sizeof(float);
         // Resize v.m_data
         v.m_data.resize(stride * variants.size());
diff --git a/src/render/backend/uniform_p.h b/src/render/backend/uniform_p.h
index 6b5ae4172..7fb7295d5 100644
--- a/src/render/backend/uniform_p.h
+++ b/src/render/backend/uniform_p.h
@@ -111,11 +111,6 @@ public:
         BufferValue
     };
 
-    struct Texture {
-        int textureId = 0; // Set first so that glUniform1iv will work
-        Qt3DCore::QNodeId nodeId;
-    };
-
     // UniformValue implicitely converts doubles to floats to ensure
     // correct rendering behavior for the cases where Qt3D parameters created from
     // a double or QVariant(double) are used to fill uniform values that in reality
@@ -152,6 +147,13 @@ public:
         memcpy(m_data.data(), mat44.constData(), 16 * sizeof(float));
     }
 
+    // Reserve data to be filled in later
+    UniformValue(int byteSize, ValueType valueType)
+        : m_data(byteSize / sizeof(float))
+        , m_valueType(valueType)
+    {
+    }
+
     // For nodes which will later be replaced by a Texture or Buffer
     UniformValue(Qt3DCore::QNodeId id)
         : UniformValue()
@@ -160,19 +162,13 @@ public:
         memcpy(m_data.data(), &id, sizeof(Qt3DCore::QNodeId));
     }
 
-    // For textures
-    UniformValue(UniformValue::Texture t)
-        : UniformValue()
-    {
-        m_valueType = TextureValue;
-        memcpy(m_data.data(), &t, sizeof(Texture));
-    }
-
     ValueType valueType() const { return m_valueType; }
     UniformType storedType() const { return m_storedType; }
 
     static UniformValue fromVariant(const QVariant &variant);
 
+    int byteSize() const { return m_data.size() * sizeof(float); }
+
     template<typename T>
     const T *constData() const
     {
diff --git a/src/render/graphicshelpers/graphicscontext.cpp b/src/render/graphicshelpers/graphicscontext.cpp
index 0b6870c3a..64e7cffa4 100644
--- a/src/render/graphicshelpers/graphicscontext.cpp
+++ b/src/render/graphicshelpers/graphicscontext.cpp
@@ -1185,7 +1185,7 @@ bool GraphicsContext::setParameters(ShaderParameterPack &parameterPack)
                 UniformValue &texUniform = uniformValues[namedTex.glslNameId];
                 Q_ASSERT(texUniform.valueType() == UniformValue::TextureValue);
                 const int texUnit = activateTexture(TextureScopeMaterial, t);
-                texUniform.data<UniformValue::Texture>()->textureId = texUnit;
+                texUniform.data<int>()[namedTex.uniformArrayIndex] = texUnit;
                 // if the texture data from generators may not be available yet,
                 // make sure that the next frame is rendered
                 if (texUnit == -1)
@@ -1235,11 +1235,10 @@ bool GraphicsContext::setParameters(ShaderParameterPack &parameterPack)
     for (const ShaderUniform &uniform : activeUniforms) {
         // We can use [] as we are sure the the uniform wouldn't
         // be un activeUniforms if there wasn't a matching value
-        const auto &v = values[uniform.m_nameId];
+        const UniformValue &v = values[uniform.m_nameId];
 
         // skip invalid textures
-        if (v.valueType() == UniformValue::TextureValue &&
-                v.constData<UniformValue::Texture>()->textureId == -1)
+        if (v.valueType() == UniformValue::TextureValue && *v.constData<int>() == -1)
             continue;
 
         applyUniform(uniform, v);
diff --git a/src/render/materialsystem/qparameter.cpp b/src/render/materialsystem/qparameter.cpp
index b9bfa44e7..2b2dd29d5 100644
--- a/src/render/materialsystem/qparameter.cpp
+++ b/src/render/materialsystem/qparameter.cpp
@@ -179,13 +179,30 @@ QParameterPrivate::QParameterPrivate()
 {
 }
 
+namespace {
+
+/*! \internal */
+inline QVariant toBackendValue(const QVariant &v)
+{
+    if (auto nodeValue = v.value<Qt3DCore::QNode*>())
+        return QVariant::fromValue(nodeValue->id());
+    return v;
+}
+
+} // anonymous
+
 void QParameterPrivate::setValue(const QVariant &v)
 {
-    Qt3DCore::QNode *nodeValue = v.value<Qt3DCore::QNode *>();
-    if (nodeValue != nullptr)
-        m_backendValue = QVariant::fromValue(nodeValue->id());
-    else
-        m_backendValue = v;
+    if (v.type() == QVariant::List) {
+        QSequentialIterable iterable = v.value<QSequentialIterable>();
+        QVariantList variants;
+        variants.reserve(iterable.size());
+        for (const auto &v : iterable)
+            variants.append(toBackendValue(v));
+        m_backendValue = variants;
+    } else {
+        m_backendValue = toBackendValue(v);
+    }
     m_value = v;
 }