QParameter: only copy QNodeId when parameter value is a QNodeSubclass

This avoid doing useless clones and will be further improved to support
UBO/SSBO when Buffer are used as a Parameter's value.

QNode subclasses that can be used as the value of an attribute are:
QAbstractTextureProvider -> default uniform block
QShaderData -> UBO (could be made to work with SSBO as well)
Buffer -> UBO / SSBO

Change-Id: I4928b95107656afc64db1ef989dde5dde8a92df8
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

5 files changed, 39 insertions, 58 deletions

diff --git a/src/render/backend/renderview.cpp b/src/render/backend/renderview.cpp
index 523cc9248..5c423167c 100644
--- a/src/render/backend/renderview.cpp
+++ b/src/render/backend/renderview.cpp
@@ -80,6 +80,8 @@ static const int MAX_LIGHTS = 8;
 
 namespace  {
 
+const int qNodeIdTypeId = qMetaTypeId<Qt3DCore::QNodeId>();
+
 // TODO: Should we treat lack of layer data as implicitly meaning that an
 // entity is in all layers?
 bool isEntityInLayers(const Entity *entity, const QStringList &layers)
@@ -545,12 +547,18 @@ void RenderView::setUniformValue(QUniformPack &uniformPack, const QString &name,
         destroyUniformValue(val, m_allocator);
 
     Texture *tex = Q_NULLPTR;
-
-    if ((tex = value.value<Texture *>()) != Q_NULLPTR) {
-        uniformPack.setTexture(name, tex->peerUuid());
-        TextureUniform *texUniform = m_allocator->allocate<TextureUniform>();
-        texUniform->setTextureId(tex->peerUuid());
-        uniformPack.setUniform(name, texUniform);
+    // At this point a uniform value can only be a scalar type
+    // or a Qt3DCore::QNodeId corresponding to a Texture
+    // ShaderData/Buffers would be handled as UBO/SSBO and would therefore
+    // not be in the default uniform block
+    if (static_cast<QMetaType::Type>(value.type()) == qNodeIdTypeId) {
+        if ((tex = m_manager->textureManager()->lookupResource(value.value<Qt3DCore::QNodeId>()))
+                != Q_NULLPTR) {
+            uniformPack.setTexture(name, tex->peerUuid());
+            TextureUniform *texUniform = m_allocator->allocate<TextureUniform>();
+            texUniform->setTextureId(tex->peerUuid());
+            uniformPack.setUniform(name, texUniform);
+        }
     } else {
         uniformPack.setUniform(name, QUniformValue::fromVariant(value, m_allocator));
     }
@@ -567,7 +575,9 @@ void RenderView::setStandardUniformValue(QUniformPack &uniformPack, const QStrin
 void RenderView::setUniformBlockValue(QUniformPack &uniformPack, Shader *shader, const ShaderUniformBlock &block, const QVariant &value)
 {
     ShaderData *shaderData = Q_NULLPTR;
-    if ((shaderData = value.value<ShaderData *>())) {
+
+    if (static_cast<QMetaType::Type>(value.type()) == qNodeIdTypeId &&
+        (shaderData = m_manager->shaderDataManager()->lookupResource(value.value<Qt3DCore::QNodeId>())) != Q_NULLPTR) {
         // UBO are indexed by <ShaderId, ShaderDataId> so that a same QShaderData can be used among different shaders
         // while still making sure that if they have a different layout everything will still work
         // If two shaders define the same block with the exact same layout, in that case the UBO could be shared
@@ -754,10 +764,11 @@ void RenderView::setShaderAndUniforms(RenderCommand *command, RenderPass *rPass,
                             const ShaderUniformBlock &block = shader->uniformBlock(it->name);
                             setUniformBlockValue(command->m_uniforms, shader, block, it->value);
                             it = parameters.erase(it);
-                        } else {
+                        } else { // Parameter is a struct
                             const QVariant &v = it->value;
                             ShaderData *shaderData = Q_NULLPTR;
-                            if ((shaderData = v.value<ShaderData *>()) != Q_NULLPTR) {
+                            if (static_cast<QMetaType::Type>(v.type()) == qNodeIdTypeId &&
+                                (shaderData = m_manager->shaderDataManager()->lookupResource(v.value<Qt3DCore::QNodeId>())) != Q_NULLPTR) {
                                 // Try to check if we have a struct or array matching a QShaderData parameter
                                 setDefaultUniformBlockShaderDataValue(command->m_uniforms, shader, shaderData, it->name);
                                 it = parameters.erase(it);
diff --git a/src/render/materialsystem/parameter.cpp b/src/render/materialsystem/parameter.cpp
index fea400e0b..d7147e5a4 100644
--- a/src/render/materialsystem/parameter.cpp
+++ b/src/render/materialsystem/parameter.cpp
@@ -37,8 +37,11 @@
 #include "parameter_p.h"
 #include <Qt3DCore/qscenepropertychange.h>
 #include <Qt3DRender/qparameter.h>
+#include <Qt3DRender/private/qparameter_p.h>
 #include <Qt3DRender/qtexture.h>
 #include <Qt3DRender/qshaderdata.h>
+#include <Qt3DRender/qbuffer.h>
+#include <Qt3DRender/private/buffer_p.h>
 
 #include <Qt3DRender/private/managers_p.h>
 
@@ -51,16 +54,14 @@ namespace Render {
 
 Parameter::Parameter()
     : QBackendNode()
-    , m_shaderDataManager(Q_NULLPTR)
-    , m_textureManager(Q_NULLPTR)
 {
 }
 
-void Parameter::updateFromPeer(Qt3DCore::QNode *mat)
+void Parameter::updateFromPeer(Qt3DCore::QNode *peer)
 {
-    QParameter *param = static_cast<QParameter *>(mat);
+    QParameter *param = static_cast<QParameter *>(peer);
     m_name = param->name();
-    m_value = toBackendValue(param->value());
+    m_value = static_cast<QParameterPrivate *>(QNodePrivate::get(param))->m_backendValue;
 }
 
 void Parameter::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
@@ -71,7 +72,7 @@ void Parameter::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
         if (propertyChange->propertyName() == QByteArrayLiteral("name"))
             m_name = propertyChange->value().toString();
         else if (propertyChange->propertyName() == QByteArrayLiteral("value"))
-            m_value = toBackendValue(propertyChange->value());
+            m_value = propertyChange->value();
     }
 }
 
@@ -85,32 +86,6 @@ QVariant Parameter::value() const
     return m_value;
 }
 
-void Parameter::setShaderDataManager(ShaderDataManager *shaderDataManager)
-{
-    m_shaderDataManager = shaderDataManager;
-}
-
-void Parameter::setTextureManager(TextureManager *textureManager)
-{
-    m_textureManager = textureManager;
-}
-
-QVariant Parameter::toBackendValue(const QVariant &value)
-{
-    QNode *node = value.value<QNode *>();
-
-    if (node == Q_NULLPTR) {
-        return value;
-    } else if (qobject_cast<QAbstractTextureProvider*>(node)) {
-        return QVariant::fromValue(static_cast<Texture*>(createBackendNode(node)));
-    } else if (qobject_cast<QShaderData*>(node)) {
-        return QVariant::fromValue(static_cast<ShaderData*>(createBackendNode(node)));
-    } else {
-        qFatal("Texture and ShaderData are the only types of Node allowed as parameters");
-        return QVariant();
-    }
-}
-
 } // namespace Render
 } // namespace Qt3DRender
 
diff --git a/src/render/materialsystem/parameter_p.h b/src/render/materialsystem/parameter_p.h
index a40af65ee..a1175311c 100644
--- a/src/render/materialsystem/parameter_p.h
+++ b/src/render/materialsystem/parameter_p.h
@@ -72,15 +72,7 @@ public:
     QString name() const;
     QVariant value() const;
 
-    void setShaderDataManager(ShaderDataManager *shaderDataManager);
-    void setTextureManager(TextureManager *textureManager);
-
 private:
-    QVariant toBackendValue(const QVariant &value);
-
-    ShaderDataManager *m_shaderDataManager;
-    TextureManager *m_textureManager;
-
     QString m_name;
     QVariant m_value;
 };
diff --git a/src/render/materialsystem/qparameter.cpp b/src/render/materialsystem/qparameter.cpp
index 62e38e3b3..b2c32733a 100644
--- a/src/render/materialsystem/qparameter.cpp
+++ b/src/render/materialsystem/qparameter.cpp
@@ -57,6 +57,11 @@ QParameterPrivate::QParameterPrivate()
 
 void QParameterPrivate::setValue(const QVariant &v)
 {
+    Qt3DCore::QNode *nodeValue = v.value<Qt3DCore::QNode *>();
+    if (nodeValue != Q_NULLPTR)
+        m_backendValue = QVariant::fromValue(nodeValue->id());
+    else
+        m_backendValue = v;
     m_value = v;
 }
 
@@ -66,6 +71,7 @@ void QParameter::copy(const QNode *ref)
     const QParameter *param = static_cast<const QParameter*>(ref);
     d_func()->m_name = param->d_func()->m_name;
     d_func()->m_value = param->d_func()->m_value;
+    d_func()->m_backendValue = param->d_func()->m_backendValue;
 }
 
 /*! \internal */
@@ -123,17 +129,14 @@ void QParameter::setValue(const QVariant &dv)
         d->setValue(dv);
         emit valueChanged(dv);
 
-        // In case texture are declared inline
-        QNode *txt = dv.value<QNode *>();
-        if (txt != Q_NULLPTR && !txt->parent())
-           txt->setParent(this);
+        // In case node values are declared inline
+        QNode *nodeValue = dv.value<QNode *>();
+        if (nodeValue != Q_NULLPTR && !nodeValue->parent())
+            nodeValue->setParent(this);
 
         QScenePropertyChangePtr change(new QScenePropertyChange(NodeUpdated, QSceneChange::Node, id()));
         change->setPropertyName(d->m_name.toUtf8().data());
-        if (txt != Q_NULLPTR)
-            change->setValue(QVariant::fromValue(QNode::clone(txt)));
-        else
-            change->setValue(d->m_value);
+        change->setValue(d->m_backendValue);
 
         d->notifyObservers(change);
     }
diff --git a/src/render/materialsystem/qparameter_p.h b/src/render/materialsystem/qparameter_p.h
index 96db66b7a..bc57a7ca0 100644
--- a/src/render/materialsystem/qparameter_p.h
+++ b/src/render/materialsystem/qparameter_p.h
@@ -68,7 +68,7 @@ public:
 
     QString m_name;
     QVariant m_value;
-    QString m_meshName;
+    QVariant m_backendValue;
 };
 
 } // namespace Qt3DRender