Fix Datainput undo/redo

Creates proper undo stack when datainput control is set on or off. Also,
removes extra connection pointer and uses existing connection pointer list
for inspector item. Only checks datainput control UI status for properties
that are actually controllable (currently only textstring in Text element.)

Change-Id: Iae35bed53f492a5ec25c66a50b40278830717ce9
Task-Id: QT3DS-1077
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>

7 files changed, 82 insertions, 113 deletions

diff --git a/src/Authoring/Client/Code/Core/Doc/Doc.cpp b/src/Authoring/Client/Code/Core/Doc/Doc.cpp
index cc39c7c1..2972697f 100644
--- a/src/Authoring/Client/Code/Core/Doc/Doc.cpp
+++ b/src/Authoring/Client/Code/Core/Doc/Doc.cpp
@@ -238,6 +238,69 @@ void CDoc::SetInstancePropertyValue(qt3dsdm::Qt3DSDMInstanceHandle inInstance,
         thePropertySystem->SetInstancePropertyValue(inInstance, theProperty, inValue);
 }
 
+// Set a property in an instance to be controlled by datainput.
+void CDoc::SetInstancePropertyControlled(
+    qt3dsdm::Qt3DSDMInstanceHandle instance, Q3DStudio::CString instancepath,
+    qt3dsdm::Qt3DSDMPropertyHandle propName, Q3DStudio::CString controller,
+    bool controlled)
+{
+    qt3dsdm::SComposerObjectDefinitions &theDefinitions(
+        GetStudioSystem()->GetClientDataModelBridge()->GetObjectDefinitions());
+    qt3dsdm::IPropertySystem *thePropertySystem = GetStudioSystem()->GetPropertySystem();
+    // get the name of controlled property
+    auto metadataHandle
+        = GetStudioSystem()->GetActionMetaData()->GetMetaDataProperty(instance, propName);
+    auto metadata
+        = GetStudioSystem()->GetActionMetaData()->GetMetaDataPropertyInfo(metadataHandle);
+
+    qt3dsdm::SValue controlledProperty;
+    qt3dsdm::SValue controllerName;
+
+    // Build controller - controlled property string.
+    if (controlled) {
+        Q3DStudio::CString controlledElemStr = controller;
+        controlledElemStr.append(" ");
+        controlledElemStr.append(metadata->m_Name.c_str());
+        controlledProperty = std::make_shared<qt3dsdm::CDataStr>(controlledElemStr);
+    } else {
+        // TODO: this is Text element-specific at the moment
+        // Get controller - property -pair for this element
+        qt3dsdm::SValue currentControlledProperty;
+        thePropertySystem->GetInstancePropertyValue(
+            instance, theDefinitions.m_Text.m_ControlledProperty, currentControlledProperty);
+
+        Q3DStudio::CString controllerNameStr
+            = qt3dsdm::get<qt3dsdm::TDataStrPtr>(currentControlledProperty)->GetData();
+
+        // Check if this property has a controlling datainput before trying
+        // to delete it from the list of controlled properties
+        if (controllerNameStr.size()) {
+            // Set controller - property -string empty for this element.
+            // We need to explicitly set controlledProperty with an empty string initializer,
+            // otherwise it will have type "None" instead of "String".
+            // TODO: for now only a single textstring property can be controlled. For
+            // control of several properties in a single element, we must only remove
+            // a specific controller - property pair from controlledProperty string, not all.
+            controlledProperty = std::make_shared<qt3dsdm::CDataStr>(Q3DStudio::CString());
+
+            // Set the textstring content to default when disabling datainput control
+            // TODO: restore the previous text content prior to setting it to controlled
+            controllerName = theDefinitions.m_Text.m_TextString.m_DefaultValue.getValue();
+        } else {
+            // We are trying to turn control off for property that had no existing control.
+            // Nothing to do except to make sure that we set the
+            // controlledProperty to an empty string.
+            controlledProperty = std::make_shared<qt3dsdm::CDataStr>(Q3DStudio::CString());
+        }
+    }
+
+    // Set the controlledproperty string in the controlled element
+    Q3DStudio::ScopedDocumentEditor(*this, L"Set controlled", __FILE__, __LINE__)
+        ->SetInstancePropertyValue(instance,
+                                   theDefinitions.m_Text.m_ControlledProperty,
+                                   controlledProperty);
+}
+
 Q3DStudio::IDocumentBufferCache &CDoc::GetBufferCache()
 {
     if (!m_DocumentBufferCache)
diff --git a/src/Authoring/Client/Code/Core/Doc/Doc.h b/src/Authoring/Client/Code/Core/Doc/Doc.h
index f422f2de..c296bbff 100644
--- a/src/Authoring/Client/Code/Core/Doc/Doc.h
+++ b/src/Authoring/Client/Code/Core/Doc/Doc.h
@@ -294,6 +294,11 @@ public:
     void SetInstancePropertyValue(qt3dsdm::Qt3DSDMInstanceHandle inInstance,
                                   const std::wstring &inPropertyName,
                                   const qt3dsdm::SValue &inValue) override;
+    void SetInstancePropertyControlled(qt3dsdm::Qt3DSDMInstanceHandle instance,
+                                       Q3DStudio::CString instancepath,
+                                       qt3dsdm::Qt3DSDMPropertyHandle propName,
+                                       Q3DStudio::CString controller,
+                                       bool controlled) override;
     Q3DStudio::IDocumentBufferCache &GetBufferCache() override;
     Q3DStudio::IDocumentReader &GetDocumentReader() override;
     Q3DStudio::IDocumentEditor &OpenTransaction(const Q3DStudio::CString &inCmdName,
diff --git a/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp b/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp
index 96ad8f57..bd355c7a 100644
--- a/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp
+++ b/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp
@@ -1424,90 +1424,6 @@ public:
         SetName(theMaterial, materialName);
     }
 
-    // Set a property in an instance to be controlled by datainput.
-    void SetInstancePropertyControlled(TInstanceHandle instance, CString instancepath,
-                                       TPropertyHandle propName, CString controller,
-                                       bool controlled) override
-    {
-        SComposerObjectDefinitions &theDefinitions(m_Bridge.GetObjectDefinitions());
-        IObjectReferenceHelper* objReferenceHelper = m_Doc.GetDataModelObjectReferenceHelper();
-
-        // get the name of controlled property
-        auto metadataHandle = m_MetaData.GetMetaDataProperty(instance, propName);
-        auto metadata = m_MetaData.GetMetaDataPropertyInfo(metadataHandle);
-        const wchar_t *propname = metadata->m_Name.wide_str();
-
-        SValue controlledProperty;
-        SValue controllerName;
-
-        // Build controller - controlled property string.
-        if (controlled) {
-            CString controlledElemStr = controller;
-            controlledElemStr.append(" ");
-            controlledElemStr.append(metadata->m_Name.c_str());
-
-            controlledProperty = std::make_shared<CDataStr>(controlledElemStr);
-        } else {
-            // TODO: this is Text element-specific at the moment
-            // Get controller - property -pair for this element
-            Option<SValue> currentControlledProperty = GetInstancePropertyValue(
-                instance, theDefinitions.m_Text.m_ControlledProperty);
-
-            TDataStrPtr theNamePtr(get<TDataStrPtr>(*currentControlledProperty));
-            CString controllerNameStr = theNamePtr->GetData();
-
-            // Check if this property has a controlling datainput before trying
-            // to delete it from the list of controlled properties
-            if (controllerNameStr.size()) {
-                // Set controller - property -string empty for this element.
-                // We need to explicitly set controlledProperty with an empty string initializer,
-                // otherwise it will have type "None" instead of "String".
-                // TODO: for now only a single textstring property can be controlled. For
-                // control of several properties in a single element, we must only remove
-                // a specific controller - property pair from controlledProperty string, not all.
-                controlledProperty = std::make_shared<CDataStr>(CString());
-
-                // Set the textstring content to default when disabling datainput control
-                // TODO: restore the previous text content prior to setting it to controlled
-                controllerName = theDefinitions.m_Text.m_TextString.m_DefaultValue.getValue();
-            } else {
-                // We are trying to turn control off for property that had no existing control.
-                // Nothing to do except to make sure that we set the
-                // controlledProperty to an empty string.
-                controlledProperty = std::make_shared<CDataStr>(CString());
-            }
-        }
-
-        // Set the controlledproperty string in the controlled element
-        // TODO: For the moment this is Text element -specific only
-        m_DataCore.SetInstancePropertyValue(instance,
-                                 theDefinitions.m_Text.m_ControlledProperty,
-                                 controlledProperty);
-    }
-
-    // Remove datainput control of all properties of this instance,
-    // for example when deleting the entire instance
-    void RemoveAllControlForInstance(TInstanceHandle instance)
-    {
-        SComposerObjectDefinitions &theDefinitions(m_Bridge.GetObjectDefinitions());
-        IObjectReferenceHelper* objReferenceHelper = m_Doc.GetDataModelObjectReferenceHelper();
-
-        // TODO: text element-specific. Also, only removes a single controller-property
-        // pair as currently only textstring can be controlled. For generic
-        // implementation we need to iterate through all controller-property pairs
-        // and remove control for each of them separately (a single instance can have
-        // several different datainputs each controlling a single or multiple properties
-        // i.e. one-to-many mapping).
-        SetInstancePropertyControlled(instance,
-                                      objReferenceHelper->GetObjectReferenceString(
-                                          m_Doc.GetSceneInstance(),
-                                          CRelativePathTools::EPATHTYPE_GUID,
-                                          instance),
-                                      theDefinitions.m_Text.m_TextString,
-                                      CString(),
-                                      false);
-    }
-
     // Normal way in to the system.
     void SetInstancePropertyValue(TInstanceHandle instance, TPropertyHandle propName,
                                           const SValue &value, bool inAutoDelete = true) override
@@ -2513,15 +2429,6 @@ public:
         if (inInstances.empty())
             return Qt3DSDMInstanceHandle();
 
-        // TODO: For now, we need to break control relationship for instances that are
-        // included in the newly made component as the binding goes wrong during
-        // instance "copy to mem - delete - insert" sequence needed for component creation.
-        // This means that at the moment, datainput cannot control a property
-        // within a component instance.
-        for (size_t idx = 0; idx < inInstances.size(); idx++) {
-            if (IsControlled(inInstances[idx]))
-                RemoveAllControlForInstance(inInstances[idx]);
-        }
         qt3dsdm::TInstanceHandleList theInstances = ToGraphOrdering(inInstances);
         // Do this in reverse order.
         // first add new component.
diff --git a/src/Authoring/Client/Code/Core/Doc/IDoc.h b/src/Authoring/Client/Code/Core/Doc/IDoc.h
index 42406049..bd56e9ad 100644
--- a/src/Authoring/Client/Code/Core/Doc/IDoc.h
+++ b/src/Authoring/Client/Code/Core/Doc/IDoc.h
@@ -105,6 +105,11 @@ public:
     virtual void SetInstancePropertyValue(qt3dsdm::Qt3DSDMInstanceHandle inInstance,
                                           const std::wstring &inPropertyName,
                                           const qt3dsdm::SValue &inValue) = 0;
+    virtual void SetInstancePropertyControlled(qt3dsdm::Qt3DSDMInstanceHandle instance,
+                                               Q3DStudio::CString instancepath,
+                                               qt3dsdm::Qt3DSDMPropertyHandle propName,
+                                               Q3DStudio::CString controller,
+                                               bool controlled) = 0;
     // Return an editor to editing the scene graph of the document.
     // This editor takes care of putting objects into the property slide
     // as well as updating the world
diff --git a/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h b/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h
index f1d246c6..7dea9b34 100644
--- a/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h
+++ b/src/Authoring/Client/Code/Core/Doc/IDocumentEditor.h
@@ -196,15 +196,6 @@ public:
     SetInstancePropertyValueAsRenderable(TInstanceHandle instance, TPropertyHandle propName,
                                          const Q3DStudio::CString &inSourcePath) = 0;
 
-    // Sets/unsets "instance" property "propName" to be controlled by datainput "controller"
-    // and adds element - property pair to controlling datainput control list. "Instancepath"
-    // must correspond to controlled instance path as it is the data used by controller to
-    // locate element when controlling the property value
-    virtual void SetInstancePropertyControlled(TInstanceHandle instance, CString instancepath,
-                                               TPropertyHandle propName,
-                                               CString controller,
-                                               bool controlled) = 0;
-
     virtual void SetMaterialType(TInstanceHandle instance,
                                  const Q3DStudio::CString &inRelativePathToMaterialFile) = 0;
 
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
index 6693b164..f910a4bb 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
@@ -393,9 +393,9 @@ InspectorControlBase* InspectorControlModel::createItem(Qt3DSDMInspectable *insp
 
         // Update UI icon state and tooltip
         updateControlledToggleState(item);
-        m_controlledToggleConnection = signalProvider->ConnectControlledToggled(
+        item->m_connections.push_back(signalProvider->ConnectControlledToggled(
             std::bind(&InspectorControlModel::updateControlledToggleState,
-                      this, item));
+                      this, item)));
     }
 
     // synchronize the value itself
@@ -423,16 +423,17 @@ void InspectorControlModel::updateControlledToggleState(InspectorControlBase* in
     qt3dsdm::SValue currPropVal = currentPropertyValue(
         inItem->m_instance, studio->GetPropertySystem()->GetAggregateInstancePropertyByName(
             inItem->m_instance, qt3dsdm::TCharStr(L"controlledproperty")));
+    Q3DStudio::CString currPropValStr;
+    if (!currPropVal.empty())
+        currPropValStr = qt3dsdm::get<qt3dsdm::TDataStrPtr>(currPropVal)->GetData();
     // Restore original tool tip from metadata when turning control off
-    if (currPropVal.empty()) {
+    if (!currPropValStr.size()) {
         inItem->m_controlled = false;
         inItem->m_tooltip = Q3DStudio::CString(
             inItem->m_metaProperty.m_Description.c_str()).toQString();
     } else {
        Q3DStudio::CString propName
            = studio->GetPropertySystem()->GetName(inItem->m_property).c_str();
-       Q3DStudio::CString currPropValStr
-           = qt3dsdm::get<qt3dsdm::TDataStrPtr>(currPropVal)->GetData();
        // TODO this only works when controlling a single property,
        // see below
        // Item is not controlled if property name is not found altogether,
@@ -805,7 +806,7 @@ void InspectorControlModel::updatePropertyValue(InspectorControlBase *element) c
     // Controlled state must be manually set after undo operations,
     // as only the "controlledproperty" is restored in undo,
     // not the controlled flag nor the tooltip
-    if (element->m_controlled)
+    if (element->m_controllable)
           updateControlledToggleState(element);
 }
 
@@ -1009,14 +1010,12 @@ void InspectorControlModel::setPropertyControllerInstance(
                                                CRelativePathTools::EPATHTYPE_GUID, instance));
     Q_ASSERT(instancepath.size());
 
-    Q3DStudio::SCOPED_DOCUMENT_EDITOR(*doc, QObject::tr("Set Property Controlled"))
-        ->SetInstancePropertyControlled(instance, instancepath, property,
-                                        controllerInstance, controlled);
+    doc->SetInstancePropertyControlled(instance, instancepath, property,
+                                       controllerInstance, controlled);
 }
 
 void InspectorControlModel::setPropertyControlled(long instance, int property)
 {
-
     const auto signalSender
         = g_StudioApp.GetCore()->GetDoc()->GetStudioSystem()->GetFullSystemSignalSender();
 
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h
index 14060907..bcb2c07f 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.h
@@ -193,7 +193,6 @@ private:
 
     std::shared_ptr<qt3dsdm::ISignalConnection> m_notifier;
     std::shared_ptr<qt3dsdm::ISignalConnection> m_slideNotifier;
-    std::shared_ptr<qt3dsdm::ISignalConnection> m_controlledToggleConnection;
 
     QStringList materialValues() const;
     InspectorControlBase *createMaterialItem(Qt3DSDMInspectable *inspectable, int groupIndex);