Remove datainput control bindings from map on deleteElement

Update internal datainput control map when dynamically created elements
are deleted. This avoids leaving invalid entries to performance-critical
map lookups. Also fix typo in test code.

Change-Id: I2b6490bf6de607d303fe0f94496b5b0cfff4c463
Task-id: QT3DS-3635
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
 
 

3 files changed, 37 insertions, 1 deletions

diff --git a/src/runtime/Qt3DSApplication.h b/src/runtime/Qt3DSApplication.h
index f38b182..510845e 100644
--- a/src/runtime/Qt3DSApplication.h
+++ b/src/runtime/Qt3DSApplication.h
@@ -89,6 +89,11 @@ struct DataInOutAttribute
     QByteArray elementPath;
     QVector<QByteArray> attributeName;
     Q3DStudio::EAttributeType propertyType = Q3DStudio::ATTRIBUTETYPE_NONE;
+
+    bool operator==(const DataInOutAttribute &inOther) const {
+        return (inOther.elementPath == elementPath && inOther.attributeName == attributeName
+                && inOther.propertyType == propertyType);
+    }
 };
 
 enum DataInOutType {
diff --git a/src/runtime/Qt3DSQmlEngine.cpp b/src/runtime/Qt3DSQmlEngine.cpp
index c828466..46978e0 100644
--- a/src/runtime/Qt3DSQmlEngine.cpp
+++ b/src/runtime/Qt3DSQmlEngine.cpp
@@ -479,6 +479,7 @@ private:
     void initializeDataInputsInPresentation(CPresentation &presentation, bool isPrimary,
                                             QList<TElement *> inElements = QList<TElement *>());
     void initializeDataOutputsInPresentation(CPresentation &presentation, bool isPrimary);
+    void removeDataInputControl(const QVector<TElement *> &inElements);
     // Splits down vector attributes to components as Runtime does not really
     // handle vectors at this level anymore
     bool getAttributeVector3(QVector<QByteArray> &outAttVec, const QByteArray &attName,
@@ -2311,6 +2312,32 @@ void CQmlEngineImpl::initializeDataOutputsInPresentation(CPresentation &presenta
     presentation.AddToDataOutputMap(elementPathToDataOutputDefMap);
 }
 
+// Remove datainput control from listed elements and update internal control map. Should be used
+// when elements having a datainput controller are removed in order to avoid invalid entries.
+void CQmlEngineImpl::removeDataInputControl(const QVector<TElement *> &inElements)
+{
+    const qt3ds::runtime::DataInputMap diMap(m_Application->dataInputMap());
+
+    // Have to do nasty triple-nested search loop as the datainput map is organized around
+    // datainputs for fast lookups in setDataInputValue, not around control targets.
+    QVector<QPair<QString, qt3ds::runtime::DataInOutAttribute>> elemAttrsToRemove;
+    QMapIterator<QString, qt3ds::runtime::DataInputDef> diMapIter(diMap);
+
+    for (const auto &elem : inElements) {
+        while (diMapIter.hasNext()) {
+            diMapIter.next();
+            for (const auto &inOutAttr : qAsConst(diMapIter.value().controlledAttributes)) {
+                if (inOutAttr.elementPath == QByteArray(elem->m_Path))
+                    elemAttrsToRemove.append({diMapIter.key(), inOutAttr});
+            }
+        }
+        diMapIter.toFront();
+    }
+
+    for (const auto &attr : qAsConst(elemAttrsToRemove))
+        m_Application->dataInputMap()[attr.first].controlledAttributes.removeAll(attr.second);
+}
+
 // Bit clumsy way of getting from "position" to "position .x .y .z" and enabling datainput
 // support for vectorized types. UIP parser has already thrown away all vector
 // type attributes and at this point we are operating with scalar components only.
@@ -2686,6 +2713,8 @@ void CQmlEngineImpl::deleteElements(const QVector<TElement *> &elements,
     }
     for (auto parentNode : qAsConst(parentNodes))
         renderer->ChildrenUpdated(*parentNode);
+
+    removeDataInputControl(elements);
 }
 
 int CQmlEngineImpl::getAvailableId()
diff --git a/tests/auto/viewer/tst_qt3dsviewer.cpp b/tests/auto/viewer/tst_qt3dsviewer.cpp
index c9787fd..2273f89 100644
--- a/tests/auto/viewer/tst_qt3dsviewer.cpp
+++ b/tests/auto/viewer/tst_qt3dsviewer.cpp
@@ -226,7 +226,7 @@ void tst_qt3dsviewer::testCreateElement()
     data.insert(QStringLiteral("position"),
                 QVariant::fromValue<QVector3D>(QVector3D(200, 300, 200)));
     data.insert(QStringLiteral("opacity"), 20.0);
-    data.insert(QStringLiteral("controlledproperty"), QStringLiteral("@newDataInput opacity"));
+    data.insert(QStringLiteral("controlledproperty"), QStringLiteral("$newDataInput opacity"));
 
     createElement(QStringLiteral("Scene.Layer"), QStringLiteral("Slide1"), data);
 
@@ -284,6 +284,8 @@ void tst_qt3dsviewer::testCreateElement()
         data.insert(QStringLiteral("endtime"), 10000);
         data.insert(QStringLiteral("position"),
                     QVariant::fromValue<QVector3D>(QVector3D(-100, -100, 0)));
+        // Test that this datainput control entry is removed at element delete
+        data.insert(QStringLiteral("controlledproperty"), QStringLiteral("$newDataInput opacity"));
 
         createElement(QStringLiteral("Scene.Layer"), QStringLiteral("Slide2"), data);