QmlDesigner: Avoid reflection when setting values from puppet

When editing values in the puppet we did not take reflection into account.
This means that any changes we did from the puppet for notified back
from Qt Creator. Since those notifications are asynchronous this leads
to various issues especially when more than one axis (property) was modified
at once.

This patch avoids reflection. The notifications are 'flagged' and then
ignored in the Qt5InformationNodeInstanceServer.

While a node is moved we ignore any changes to that specific node.

Task-number: QDS-1191
Change-Id: Ic74e22ea71832ce12321f9085a7296c2a7d9893d
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

8 files changed, 69 insertions, 2 deletions

diff --git a/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.cpp b/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.cpp
index 19265df035..c460bbc712 100644
--- a/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.cpp
+++ b/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.cpp
@@ -74,12 +74,29 @@ TypeName PropertyValueContainer::dynamicTypeName() const
     return m_dynamicTypeName;
 }
 
+// The reflection flag indicates that a property change notification
+// is reflected. This means that the notification is the reaction to a
+// property change original done by the puppet itself.
+// In the Qt5InformationNodeInstanceServer such notification are
+// therefore ignored.
+
+void PropertyValueContainer::setReflectionFlag(bool b)
+{
+    m_isReflected = b;
+}
+
+bool PropertyValueContainer::isReflected() const
+{
+    return m_isReflected;
+}
+
 QDataStream &operator<<(QDataStream &out, const PropertyValueContainer &container)
 {
     out << container.instanceId();
     out << container.name();
     out << container.value();
     out << container.dynamicTypeName();
+    out << container.isReflected();
 
     return out;
 }
@@ -90,6 +107,7 @@ QDataStream &operator>>(QDataStream &in, PropertyValueContainer &container)
     in >> container.m_name;
     in >> container.m_value;
     in >> container.m_dynamicTypeName;
+    in >> container.m_isReflected;
 
     return in;
 }
@@ -99,7 +117,8 @@ bool operator ==(const PropertyValueContainer &first, const PropertyValueContain
     return first.m_instanceId == second.m_instanceId
             && first.m_name == second.m_name
             && first.m_value == second.m_value
-            && first.m_dynamicTypeName == second.m_dynamicTypeName;
+            && first.m_dynamicTypeName == second.m_dynamicTypeName
+            && first.m_isReflected == second.m_isReflected;
 }
 
 bool operator <(const PropertyValueContainer &first, const PropertyValueContainer &second)
diff --git a/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.h b/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.h
index 4d30f3fb7e..8770355cfd 100644
--- a/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.h
+++ b/share/qtcreator/qml/qmlpuppet/container/propertyvaluecontainer.h
@@ -50,12 +50,15 @@ public:
     QVariant value() const;
     bool isDynamic() const;
     TypeName dynamicTypeName() const;
+    void setReflectionFlag(bool b);
+    bool isReflected() const;
 
 private:
     qint32 m_instanceId;
     PropertyName m_name;
     QVariant m_value;
     TypeName m_dynamicTypeName;
+    bool m_isReflected = false;
 };
 
 QDataStream &operator<<(QDataStream &out, const PropertyValueContainer &container);
diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp
index 2f0244ddeb..83382fb198 100644
--- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp
+++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp
@@ -396,6 +396,11 @@ void ObjectNodeInstance::setHideInEditor(bool)
 {
 }
 
+void ObjectNodeInstance::setModifiedFlag(bool b)
+{
+    m_isModified = b;
+}
+
 QVariant ObjectNodeInstance::convertEnumToValue(const QVariant &value, const PropertyName &name)
 {
     Q_ASSERT(value.canConvert<Enumeration>());
@@ -420,6 +425,9 @@ void ObjectNodeInstance::setPropertyVariant(const PropertyName &name, const QVar
     if (ignoredProperties().contains(name))
         return;
 
+    if (m_isModified)
+        return;
+
     QQmlProperty property(object(), QString::fromUtf8(name), context());
 
     if (!property.isValid())
diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h
index 8aa7ce4ad4..ced76dee58 100644
--- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h
+++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h
@@ -195,6 +195,8 @@ public:
 
     void virtual setHideInEditor(bool b);
 
+    void setModifiedFlag(bool b);
+
 protected:
     explicit ObjectNodeInstance(QObject *object);
     void doResetProperty(const PropertyName &propertyName);
@@ -220,6 +222,7 @@ private:
     qint32 m_instanceId;
     bool m_deleteHeldInstance;
     bool m_isInLayoutable;
+    bool m_isModified = false;
     static QHash<EnumerationName, QVariant> m_enumationValueHash;
 };
 
diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp
index 879918e95f..d97cc2d503 100644
--- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp
+++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp
@@ -56,6 +56,7 @@
 #include "tokencommand.h"
 #include "removesharedmemorycommand.h"
 #include "changeselectioncommand.h"
+#include "objectnodeinstance.h"
 
 #include "dummycontextobject.h"
 #include "../editor3d/cameracontrolhelper.h"
@@ -166,10 +167,18 @@ void Qt5InformationNodeInstanceServer::modifyVariantValue(
         targetPopertyName = propertyName;
 
     auto *obj = node.value<QObject *>();
+
     if (obj) {
+        ServerNodeInstance instance = instanceForObject(obj);
+
+        if (option == ValuesModifiedCommand::TransactionOption::Start)
+            instance.setModifiedFlag(true);
+        else if (option == ValuesModifiedCommand::TransactionOption::End)
+            instance.setModifiedFlag(false);
+
         // We do have to split position into position.x, position.y, position.z
         ValuesModifiedCommand command = createValuesModifiedCommand(vectorToPropertyValue(
-            instanceForObject(obj),
+            instance,
             targetPopertyName,
             obj->property(propertyName)));
 
@@ -539,4 +548,21 @@ void Qt5InformationNodeInstanceServer::changeSelection(const ChangeSelectionComm
     }
 }
 
+void Qt5InformationNodeInstanceServer::changePropertyValues(const ChangeValuesCommand &command)
+{
+    bool hasDynamicProperties = false;
+    const QVector<PropertyValueContainer> values = command.valueChanges();
+    for (const PropertyValueContainer &container : values) {
+        if (!container.isReflected()) {
+            hasDynamicProperties |= container.isDynamic();
+            setInstancePropertyVariant(container);
+        }
+    }
+
+    if (hasDynamicProperties)
+        refreshBindings();
+
+    startRenderTimer();
+}
+
 } // namespace QmlDesigner
diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.h b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.h
index 83a37a4b52..e8a0291a9f 100644
--- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.h
+++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.h
@@ -47,6 +47,7 @@ public:
     void token(const TokenCommand &command) override;
     void removeSharedMemory(const RemoveSharedMemoryCommand &command) override;
     void changeSelection(const ChangeSelectionCommand &command) override;
+    void changePropertyValues(const ChangeValuesCommand &command) override;
 
 private slots:
     void objectClicked(const QVariant &object);
diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.cpp
index 376ca29524..b128161aee 100644
--- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.cpp
+++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.cpp
@@ -129,6 +129,11 @@ bool ServerNodeInstance::isSubclassOf(QObject *object, const QByteArray &superTy
     return  Internal::QmlPrivateGate::isSubclassOf(object, superTypeName);
 }
 
+void ServerNodeInstance::setModifiedFlag(bool b)
+{
+    m_nodeInstance->setModifiedFlag(b);
+}
+
 void ServerNodeInstance::setNodeSource(const QString &source)
 {
     m_nodeInstance->setNodeSource(source);
diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.h b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.h
index 7e72878dca..86003b51b6 100644
--- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.h
+++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/servernodeinstance.h
@@ -165,6 +165,8 @@ public:
 
     static bool isSubclassOf(QObject *object, const QByteArray &superTypeName);
 
+    void setModifiedFlag(bool b);
+
 private: // functions
     ServerNodeInstance(const QSharedPointer<Internal::ObjectNodeInstance> &abstractInstance);