Improve forwarding of property changes in QNodePrivate.

Instead of using a hash and the slow senderSignalIndex() to map
slot invocations to property indices, we now use a dedicated
PropertyChangeHandler class to connect to the notify signals of
properties. This one uses a trick similar to QSignalSpy and the
SignalHandler in Qt WebChannel for efficient mapping of slot
invocations to any other integer index. Here, we chose the property
index.

Note that for now, we change the QScenePropertyChange to use a
const char* instead of a QByteArray for the property name. Otherwise
we'd incur a conversion penalty for every property change event,
as QMetaProperty stores the name as a const char*. To fix this odd
API, we should eventually refactor the other abuses of
QScenePropertyChange to use dedicated change classes. Then we can
use QMetaProperty directly.

Change-Id: I9257a48e3852f29427006a7f761bca272d5c4f4e
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

7 files changed, 167 insertions, 41 deletions

diff --git a/src/core/nodes/propertychangehandler_p.h b/src/core/nodes/propertychangehandler_p.h
new file mode 100644
index 000000000..42f73c349
--- /dev/null
+++ b/src/core/nodes/propertychangehandler_p.h
@@ -0,0 +1,134 @@
+/****************************************************************************
+**
+** Copyright (C) 2014 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Milian Wolff <milian.wolff@kdab.com>
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the QtWebChannel module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and Digia. For licensing terms and
+** conditions see http://qt.digia.com/licensing. For further information
+** use the contact form at http://qt.digia.com/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights. These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef SIGNALHANDLER_H
+#define SIGNALHANDLER_H
+
+#include <QObject>
+#include <QHash>
+#include <QVector>
+#include <QMetaMethod>
+#include <QDebug>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3D {
+
+/**
+ * The property change handler is similar to QSignalSpy, but geared towards the usecase of Qt3D.
+ *
+ * It allows connecting to any number of property change signals of the receiver object and forwards
+ * the signal invocations to the Receiver by calling its propertyChanged function.
+ */
+template<class Receiver>
+class PropertyChangeHandler : public QObject
+{
+public:
+    PropertyChangeHandler(Receiver *receiver, QObject *parent = Q_NULLPTR);
+
+    /**
+     * Connect to the change signal of @p property in @p object.
+     */
+    void connectToPropertyChange(const QObject *object, int propertyIndex);
+
+    /**
+     * Disconnect from the change signal of @p property in @p object.
+     */
+    void disconnectFromPropertyChange(const QObject *object, int propertyIndex);
+
+    /**
+     * @internal
+     *
+     * Custom implementation of qt_metacall which calls propertyChanged() on the receiver for
+     * connected signals.
+     */
+    int qt_metacall(QMetaObject::Call call, int methodId, void **args) Q_DECL_OVERRIDE;
+
+private:
+    Receiver *m_receiver;
+};
+
+template<class Receiver>
+PropertyChangeHandler<Receiver>::PropertyChangeHandler(Receiver *receiver, QObject *parent)
+    : QObject(parent)
+    , m_receiver(receiver)
+{
+}
+
+template<class Receiver>
+void PropertyChangeHandler<Receiver>::connectToPropertyChange(const QObject *object, int propertyIndex)
+{
+    const QMetaObject *metaObject = object->metaObject();
+    const QMetaProperty property = metaObject->property(propertyIndex);
+    if (!property.hasNotifySignal())
+        return;
+
+    static const int memberOffset = QObject::staticMetaObject.methodCount();
+    QMetaObject::Connection connection = QMetaObject::connect(object, property.notifySignalIndex(),
+                                                              this, memberOffset + propertyIndex,
+                                                              Qt::DirectConnection, 0);
+    Q_ASSERT(connection);
+    Q_UNUSED(connection);
+}
+
+template<class Receiver>
+void PropertyChangeHandler<Receiver>::disconnectFromPropertyChange(const QObject *object, int propertyIndex)
+{
+    const QMetaObject *metaObject = object->metaObject();
+    const QMetaProperty property = metaObject->property(propertyIndex);
+    if (!property.hasNotifySignal())
+        return;
+
+    static const int memberOffset = QObject::staticMetaObject.methodCount();
+    QMetaObject::disconnect(object, property.notifySignalIndex(), this, memberOffset + propertyIndex);
+}
+
+template<class Receiver>
+int PropertyChangeHandler<Receiver>::qt_metacall(QMetaObject::Call call, int methodId, void **args)
+{
+    methodId = QObject::qt_metacall(call, methodId, args);
+    if (methodId < 0)
+        return methodId;
+
+    if (call == QMetaObject::InvokeMetaMethod) {
+        m_receiver->propertyChanged(methodId);
+        return -1;
+    }
+    return methodId;
+}
+
+}
+
+QT_END_NAMESPACE
+
+#endif // SIGNALHANDLER_H
diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp
index 88bf2697d..506a9e775 100644
--- a/src/core/nodes/qnode.cpp
+++ b/src/core/nodes/qnode.cpp
@@ -65,6 +65,8 @@ QNodePrivate::QNodePrivate(QNode *qq)
     , m_scene(Q_NULLPTR)
     , m_uuid(QUuid::createUuid())
     , m_blockNotifications(false)
+    , m_propertyChangesSetup(false)
+    , m_signals(this)
 {
     q_ptr = qq;
 }
@@ -179,64 +181,50 @@ void QNodePrivate::removeAllChildren()
 void QNodePrivate::registerNotifiedProperties()
 {
     Q_Q(QNode);
-    if (!m_notifiedProperties.isEmpty())
+    if (m_propertyChangesSetup)
         return;
 
     const int offset = QNode::staticMetaObject.propertyOffset();
     const int count = q->metaObject()->propertyCount();
 
-    for (int index = offset; index < count; index++) {
-        const QMetaProperty property = q->metaObject()->property(index);
-        if (!property.hasNotifySignal())
-            continue;
-        m_notifiedProperties[property.notifySignalIndex()] = property.name();
-        const QByteArray signal = QByteArray::number(QSIGNAL_CODE)
-                                + property.notifySignal().methodSignature();
-        QObject::connect(q, signal,
-                         q, SLOT(_q_onNodePropertyChanged()));
-    }
+    for (int index = offset; index < count; index++)
+        m_signals.connectToPropertyChange(q, index);
+
+    m_propertyChangesSetup = true;
 }
 
 void QNodePrivate::unregisterNotifiedProperties()
 {
     Q_Q(QNode);
-    if (!m_notifiedProperties.isEmpty())
+    if (!m_propertyChangesSetup)
         return;
 
     const int offset = QNode::staticMetaObject.propertyOffset();
     const int count = q->metaObject()->propertyCount();
 
-    for (int index = offset; index < count; index++) {
-        const QMetaProperty property = q->metaObject()->property(index);
-        if (!property.hasNotifySignal())
-            continue;
-        const QByteArray signal = QByteArray::number(QSIGNAL_CODE)
-                + property.notifySignal().methodSignature();
-        QObject::disconnect(q, signal,
-                            q, SLOT(_q_onNodePropertyChanged()));
-    }
-    m_notifiedProperties.clear();
+    for (int index = offset; index < count; index++)
+        m_signals.disconnectFromPropertyChange(q, index);
+
+    m_propertyChangesSetup = false;
 }
 
-void QNodePrivate::_q_onNodePropertyChanged()
+void QNodePrivate::propertyChanged(int propertyIndex)
 {
     // Bail out early if we can to avoid the cost below
     if (m_blockNotifications)
         return;
 
     Q_Q(QNode);
-    const int signalIndex = q->senderSignalIndex();
-    const QByteArray &name = m_notifiedProperties.value(signalIndex);
-    if (name.isEmpty()) // not contained
-        return;
 
-    const QVariant data = q->property(name);
-    if (data.canConvert<QNode*>()) {
+    const QMetaProperty property = q->metaObject()->property(propertyIndex);
+
+    const QVariant data = property.read(q);
+    if (property.userType() == qMetaTypeId<QNode*>()) {
         const QNode * const node = data.value<QNode*>();
         const QNodeUuid uuid = node ? node->uuid() : QNodeUuid();
-        notifyPropertyChange(name, uuid);
+        notifyPropertyChange(property.name(), uuid);
     } else {
-        notifyPropertyChange(name, data);
+        notifyPropertyChange(property.name(), data);
     }
 }
 
@@ -266,7 +254,7 @@ QSceneInterface *QNodePrivate::scene() const
     return m_scene;
 }
 
-void QNodePrivate::notifyPropertyChange(const QByteArray &name, const QVariant &value)
+void QNodePrivate::notifyPropertyChange(const char *name, const QVariant &value)
 {
     // Bail out early if we can to avoid operator new
     if (m_blockNotifications)
diff --git a/src/core/nodes/qnode.h b/src/core/nodes/qnode.h
index befc1045b..829eb740d 100644
--- a/src/core/nodes/qnode.h
+++ b/src/core/nodes/qnode.h
@@ -88,7 +88,6 @@ protected:
 
 private:
     Q_DECLARE_PRIVATE(QNode)
-    Q_PRIVATE_SLOT(d_func(), void _q_onNodePropertyChanged())
     virtual QNode *doClone() const = 0;
 
     friend class QAspectEngine;
diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h
index 5f97f521f..5aa584c2b 100644
--- a/src/core/nodes/qnode_p.h
+++ b/src/core/nodes/qnode_p.h
@@ -47,6 +47,7 @@
 #include <Qt3DCore/qnode.h>
 #include <Qt3DCore/private/qobservableinterface_p.h>
 #include <Qt3DCore/private/qchangearbiter_p.h>
+#include "propertychangehandler_p.h"
 
 QT_BEGIN_NAMESPACE
 
@@ -68,7 +69,7 @@ public:
 
     void setArbiter(QChangeArbiter *arbiter) Q_DECL_OVERRIDE;
 
-    void notifyPropertyChange(const QByteArray &name, const QVariant &value);
+    void notifyPropertyChange(const char *name, const QVariant &value);
     virtual void notifyObservers(const QSceneChangePtr &change);
 
     void insertTree(QNode *treeRoot, int depth = 0);
@@ -91,9 +92,11 @@ private:
     void removeAllChildren();
     void registerNotifiedProperties();
     void unregisterNotifiedProperties();
-    void _q_onNodePropertyChanged();
+    void propertyChanged(int propertyIndex);
 
-    QHash<int, QByteArray> m_notifiedProperties;
+    friend class PropertyChangeHandler<QNodePrivate>;
+    bool m_propertyChangesSetup;
+    PropertyChangeHandler<QNodePrivate> m_signals;
 
     static QHash<QNodeUuid, QNode *> m_clonesLookupTable;
 };
diff --git a/src/core/qscenepropertychange.cpp b/src/core/qscenepropertychange.cpp
index 3761ca58b..33ac53a62 100644
--- a/src/core/qscenepropertychange.cpp
+++ b/src/core/qscenepropertychange.cpp
@@ -101,7 +101,7 @@ QScenePropertyChange::QScenePropertyChange(QScenePropertyChangePrivate &dd, Chan
 {
 }
 
-QByteArray QScenePropertyChange::propertyName() const
+const char *QScenePropertyChange::propertyName() const
 {
     Q_D(const QScenePropertyChange);
     return d->m_propertyName;
@@ -113,7 +113,7 @@ QVariant QScenePropertyChange::value() const
     return d->m_value;
 }
 
-void QScenePropertyChange::setPropertyName(const QByteArray &name)
+void QScenePropertyChange::setPropertyName(const char *name)
 {
     Q_D(QScenePropertyChange);
     d->m_propertyName = name;
diff --git a/src/core/qscenepropertychange.h b/src/core/qscenepropertychange.h
index eeff2a618..ca0d8963a 100644
--- a/src/core/qscenepropertychange.h
+++ b/src/core/qscenepropertychange.h
@@ -56,10 +56,10 @@ public:
     QScenePropertyChange(ChangeFlag type, QNode *node, Priority priority = Standard);
     virtual ~QScenePropertyChange();
 
-    QByteArray propertyName() const;
+    const char *propertyName() const;
     QVariant value() const;
 
-    void setPropertyName(const QByteArray &name);
+    void setPropertyName(const char *name);
     void setValue(const QVariant &value);
 
     static void *operator new(size_t size);
diff --git a/src/core/qscenepropertychange_p.h b/src/core/qscenepropertychange_p.h
index 114537d58..3116a1bd7 100644
--- a/src/core/qscenepropertychange_p.h
+++ b/src/core/qscenepropertychange_p.h
@@ -64,7 +64,9 @@ public:
 
     Q_DECLARE_PUBLIC(QScenePropertyChange)
 
-    QByteArray m_propertyName;
+    /// FIXME: use QMetaProperty here once the NodeAboutToBeDeleted etc. change events
+    /// get refactored to their own QSceneChange subclass
+    const char *m_propertyName;
     QVariant m_value;
 
     static QFrameAllocator *m_allocator;