Qml: Construct value types by properties from any object-likes

Since we allow any object to be used for property-by-property
construction when encoded as QJSValue, we should do the same when it's
encoded as QVariant.

This allows us to use typed modelData in delegates. The modelData's type
only has to be a structured value. Any matching data from the model will
then be inserted.

Fixes: QTBUG-113752
Pick-to: 6.5 6.6
Change-Id: I200e9acac3c6c302c9dedf8e7e1ccd6c9fdf5eb6
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/qml/jsruntime/qv4engine.cpp

@@ -2736,7 +2736,7 @@ bool ExecutionEngine::metaTypeFromJS(const Value &value, QMetaType metaType, voi
         *reinterpret_cast<QJSPrimitiveValue *>(data) = createPrimitive(&value);
         return true;
     } else if (!isPointer) {
-        if (QQmlValueTypeProvider::createValueType(value, metaType, data))
+        if (QQmlValueTypeProvider::createValueType(metaType, data, value))
             return true;
     }
 

src/qml/qml/qqmlglobal.cpp

@@ -87,36 +87,36 @@ static void *createVariantData(QMetaType type, QVariant *variant)
 }
 
 static void callConstructor(
-        const QMetaObject *mo, int i, void *parameter, void *data)
+        const QMetaObject *targetMetaObject, int i, void *source, void *target)
 {
-    void *p[] = { data, parameter };
-    mo->static_metacall(QMetaObject::ConstructInPlace, i, p);
+    void *p[] = { target, source };
+    targetMetaObject->static_metacall(QMetaObject::ConstructInPlace, i, p);
 }
 
 template<typename Allocate>
 static void fromVerifiedType(
-        const QMetaObject *mo, int ctorIndex, void *sData, Allocate &&allocate)
+        const QMetaObject *targetMetaObject, int ctorIndex, void *source, Allocate &&allocate)
 {
-    const QMetaMethod ctor = mo->constructor(ctorIndex);
+    const QMetaMethod ctor = targetMetaObject->constructor(ctorIndex);
     Q_ASSERT_X(ctor.parameterCount() == 1, "fromVerifiedType",
                "Value type constructor must take exactly one argument");
-    callConstructor(mo, ctorIndex, sData, allocate());
+    callConstructor(targetMetaObject, ctorIndex, source, allocate());
 }
 
 
 template<typename Allocate>
 static bool fromMatchingType(
-        const QMetaObject *mo, const QV4::Value &s, Allocate &&allocate)
+        const QMetaObject *targetMetaObject, const QV4::Value &source, Allocate &&allocate)
 {
-    for (int i = 0, end = mo->constructorCount(); i < end; ++i) {
-        const QMetaMethod ctor = mo->constructor(i);
+    for (int i = 0, end = targetMetaObject->constructorCount(); i < end; ++i) {
+        const QMetaMethod ctor = targetMetaObject->constructor(i);
         if (ctor.parameterCount() != 1)
             continue;
 
         const QMetaType parameterType = ctor.parameterMetaType(0);
-        QVariant parameter = QV4::ExecutionEngine::toVariant(s, parameterType);
+        QVariant parameter = QV4::ExecutionEngine::toVariant(source, parameterType);
         if (parameter.metaType() == parameterType) {
-            callConstructor(mo, i, parameter.data(), allocate());
+            callConstructor(targetMetaObject, i, parameter.data(), allocate());
             return true;
         }
 
@@ -127,7 +127,7 @@ static bool fromMatchingType(
         QVariant converted(parameterType);
         if (QMetaType::convert(parameter.metaType(), parameter.constData(),
                                parameterType, converted.data())) {
-            callConstructor(mo, i, converted.data(), allocate());
+            callConstructor(targetMetaObject, i, converted.data(), allocate());
             return true;
         }
     }
@@ -136,23 +136,25 @@ static bool fromMatchingType(
 }
 
 template<typename Allocate>
-static bool fromMatchingType(const QMetaObject *mo, QVariant s, Allocate &&allocate)
+static bool fromMatchingType(
+    const QMetaObject *targetMetaObject, QVariant source, Allocate &&allocate)
 {
-    const QMetaType sourceMetaType = s.metaType();
+    const QMetaType sourceMetaType = source.metaType();
     if (sourceMetaType == QMetaType::fromType<QJSValue>()) {
-        QJSValue val = s.value<QJSValue>();
-        return fromMatchingType(mo, QV4::Value(QJSValuePrivate::asReturnedValue(&val)),
-                                std::forward<Allocate>(allocate));
+        QJSValue val = source.value<QJSValue>();
+        return fromMatchingType(
+            targetMetaObject, QV4::Value(QJSValuePrivate::asReturnedValue(&val)),
+            std::forward<Allocate>(allocate));
     }
 
-    for (int i = 0, end = mo->constructorCount(); i < end; ++i) {
-        const QMetaMethod ctor = mo->constructor(i);
+    for (int i = 0, end = targetMetaObject->constructorCount(); i < end; ++i) {
+        const QMetaMethod ctor = targetMetaObject->constructor(i);
         if (ctor.parameterCount() != 1)
             continue;
 
         const QMetaType parameterType = ctor.parameterMetaType(0);
         if (sourceMetaType == parameterType) {
-            callConstructor(mo, i, s.data(), allocate());
+            callConstructor(targetMetaObject, i, source.data(), allocate());
             return true;
         }
 
@@ -161,8 +163,9 @@ static bool fromMatchingType(const QMetaObject *mo, QVariant s, Allocate &&alloc
         // At this point, s should be a builtin type. For builtin types
         // the QMetaType converters are good enough.
         QVariant parameter(parameterType);
-        if (QMetaType::convert(sourceMetaType, s.constData(), parameterType, parameter.data())) {
-            callConstructor(mo, i, parameter.data(), allocate());
+        if (QMetaType::convert(
+                sourceMetaType, source.constData(), parameterType, parameter.data())) {
+            callConstructor(targetMetaObject, i, parameter.data(), allocate());
             return true;
         }
     }
@@ -187,14 +190,42 @@ static bool fromString(const QMetaObject *mo, QString s, Allocate &&allocate)
     return false;
 }
 
-static void doWriteProperties(const QMetaObject *mo, const QV4::Value &s, void *target)
+template<typename Get, typename Convert>
+static bool doWriteProperty(const QMetaProperty &metaProperty, void *target,
+                            Get &&get, Convert &&convert)
 {
-    const QV4::Object *o = static_cast<const QV4::Object *>(&s);
+    const QMetaType propertyType = metaProperty.metaType();
+    QVariant property = get(propertyType);
+    if (property.metaType() == propertyType) {
+        metaProperty.writeOnGadget(target, std::move(property));
+        return true;
+    }
+
+    QVariant converted = convert(propertyType);
+    if (converted.isValid()) {
+        metaProperty.writeOnGadget(target, std::move(converted));
+        return true;
+    }
+
+    converted = QVariant(propertyType);
+    if (QMetaType::convert(property.metaType(), property.constData(),
+                           propertyType, converted.data())) {
+        metaProperty.writeOnGadget(target, std::move(converted));
+        return true;
+    }
+
+    return false;
+}
+
+static void doWriteProperties(
+    const QMetaObject *targetMetaObject, void *target, const QV4::Value &source)
+{
+    const QV4::Object *o = static_cast<const QV4::Object *>(&source);
     QV4::Scope scope(o->engine());
     QV4::ScopedObject object(scope, o);
 
-    for (int i = 0; i < mo->propertyCount(); ++i) {
-        const QMetaProperty metaProperty = mo->property(i);
+    for (int i = 0; i < targetMetaObject->propertyCount(); ++i) {
+        const QMetaProperty metaProperty = targetMetaObject->property(i);
         const QString propertyName = QString::fromUtf8(metaProperty.name());
 
         QV4::ScopedString v4PropName(scope, scope.engine->newString(propertyName));
@@ -205,6 +236,14 @@ static void doWriteProperties(const QMetaObject *mo, const QV4::Value &s, void *
         if (v4PropValue->isUndefined())
             continue;
 
+        if (doWriteProperty(metaProperty, target, [&](const QMetaType &propertyType) {
+            return QV4::ExecutionEngine::toVariant(v4PropValue, propertyType);
+        }, [&](const QMetaType &propertyType) {
+            return QQmlValueTypeProvider::createValueType(v4PropValue, propertyType);
+        })) {
+            continue;
+        }
+
         const QMetaType propertyType = metaProperty.metaType();
         QVariant property = QV4::ExecutionEngine::toVariant(v4PropValue, propertyType);
         if (property.metaType() == propertyType) {
@@ -232,23 +271,175 @@ static void doWriteProperties(const QMetaObject *mo, const QV4::Value &s, void *
     }
 }
 
-static QVariant byProperties(const QMetaObject *mo, QMetaType metaType, const QV4::Value &s)
+static QVariant byProperties(
+    const QMetaObject *targetMetaObject, QMetaType metaType, const QV4::Value &source)
 {
-    if (!s.isObject() || !mo)
+    if (!source.isObject() || !targetMetaObject)
         return QVariant();
 
     QVariant result(metaType);
-    doWriteProperties(mo, s, result.data());
+    doWriteProperties(targetMetaObject, result.data(), source);
     return result;
 }
 
-static QVariant byProperties(const QMetaObject *mo, QMetaType metaType, const QVariant &s)
+template<typename Read>
+static void doWriteProperties(
+    const QMetaObject *targetMetaObject, void *target,
+    const QMetaObject *sourceMetaObject, Read &&read)
 {
-    if (!mo || s.metaType() != QMetaType::fromType<QJSValue>())
+    for (int i = 0; i < targetMetaObject->propertyCount(); ++i) {
+        const QMetaProperty metaProperty = targetMetaObject->property(i);
+
+        const int sourceProperty = sourceMetaObject->indexOfProperty(metaProperty.name());
+
+        // We assume that data is freshly constructed.
+        // There is no point in reset()'ing properties of a freshly created object.
+        if (sourceProperty == -1)
+            continue;
+
+        const QMetaType propertyType = metaProperty.metaType();
+        QVariant property = read(sourceMetaObject, sourceProperty);
+        if (property.metaType() == propertyType) {
+            metaProperty.writeOnGadget(target, std::move(property));
+            continue;
+        }
+
+        QVariant converted = QQmlValueTypeProvider::createValueType(property, propertyType);
+        if (converted.isValid()) {
+            metaProperty.writeOnGadget(target, std::move(converted));
+            continue;
+        }
+
+        converted = QVariant(propertyType);
+        if (QMetaType::convert(property.metaType(), property.constData(),
+                               propertyType, converted.data())) {
+            metaProperty.writeOnGadget(target, std::move(converted));
+            continue;
+        }
+
+        qWarning().noquote()
+            << QLatin1String("Could not convert %1 to %2 for property %3")
+                   .arg(property.toString(), QString::fromUtf8(propertyType.name()),
+                        QString::fromUtf8(metaProperty.name()));
+    }
+}
+
+
+static void doWriteProperties(const QMetaObject *targetMeta, void *target, QObject *source)
+{
+    doWriteProperties(
+            targetMeta, target, source->metaObject(),
+            [source](const QMetaObject *sourceMetaObject, int sourceProperty) {
+        return sourceMetaObject->property(sourceProperty).read(source);
+    });
+}
+
+static QVariant byProperties(
+    const QMetaObject *targetMetaObject, QMetaType targetMetaType, QObject *source)
+{
+    if (!source || !targetMetaObject)
         return QVariant();
 
-    QJSValue val = s.value<QJSValue>();
-    return byProperties(mo, metaType, QV4::Value(QJSValuePrivate::asReturnedValue(&val)));
+    QVariant result(targetMetaType);
+    doWriteProperties(targetMetaObject, result.data(), source);
+    return result;
+}
+
+static QVariant byProperties(
+    const QMetaObject *targetMetaObject, QMetaType targetMetaType,
+    const QMetaObject *sourceMetaObject, const void *source)
+{
+    if (!source || !sourceMetaObject || !targetMetaObject)
+        return QVariant();
+
+    QVariant result(targetMetaType);
+    doWriteProperties(
+            targetMetaObject, result.data(), sourceMetaObject,
+            [source](const QMetaObject *sourceMetaObject, int sourceProperty) {
+        return sourceMetaObject->property(sourceProperty).readOnGadget(source);
+    });
+    return result;
+}
+
+template<typename Map>
+void doWriteProperties(const QMetaObject *targetMetaObject, void *target, const Map &source)
+{
+    for (int i = 0; i < targetMetaObject->propertyCount(); ++i) {
+        const QMetaProperty metaProperty = targetMetaObject->property(i);
+
+        // We assume that data is freshly constructed.
+        // There is no point in reset()'ing properties of a freshly created object.
+        const auto it = source.constFind(QString::fromUtf8(metaProperty.name()));
+        if (it == source.constEnd())
+            continue;
+
+        const QMetaType propertyType = metaProperty.metaType();
+        QVariant property = *it;
+        if (property.metaType() == propertyType) {
+            metaProperty.writeOnGadget(target, std::move(property));
+            continue;
+        }
+
+        QVariant converted = QQmlValueTypeProvider::createValueType(property, propertyType);
+        if (converted.isValid()) {
+            metaProperty.writeOnGadget(target, std::move(converted));
+            continue;
+        }
+
+        converted = QVariant(propertyType);
+        if (QMetaType::convert(property.metaType(), property.constData(),
+                               propertyType, converted.data())) {
+            metaProperty.writeOnGadget(target, std::move(converted));
+            continue;
+        }
+
+        qWarning().noquote()
+            << QLatin1String("Could not convert %1 to %2 for property %3")
+                   .arg(property.toString(), QString::fromUtf8(propertyType.name()),
+                        QString::fromUtf8(metaProperty.name()));
+    }
+}
+
+template<typename Map>
+QVariant byProperties(
+    const QMetaObject *targetMetaObject, QMetaType targetMetaType, const Map &source)
+{
+    QVariant result(targetMetaType);
+    doWriteProperties(targetMetaObject, result.data(), source);
+    return result;
+}
+
+static QVariant byProperties(
+    const QMetaObject *targetMetaObject, QMetaType targetMetaType, const QVariant &source)
+{
+    if (!targetMetaObject)
+        return QVariant();
+
+    if (source.metaType() == QMetaType::fromType<QJSValue>()) {
+        QJSValue val = source.value<QJSValue>();
+        return byProperties(
+            targetMetaObject, targetMetaType, QV4::Value(QJSValuePrivate::asReturnedValue(&val)));
+    }
+
+    if (source.metaType() == QMetaType::fromType<QVariantMap>()) {
+        return byProperties(
+            targetMetaObject, targetMetaType,
+            *static_cast<const QVariantMap *>(source.constData()));
+    }
+
+    if (source.metaType() == QMetaType::fromType<QVariantHash>()) {
+        return byProperties(
+            targetMetaObject, targetMetaType,
+            *static_cast<const QVariantHash *>(source.constData()));
+    }
+
+    if (source.metaType().flags() & QMetaType::PointerToQObject)
+        return byProperties(targetMetaObject, targetMetaType, source.value<QObject *>());
+
+    if (const QMetaObject *sourceMeta = QQmlMetaType::metaObjectForValueType(source.metaType()))
+        return byProperties(targetMetaObject, targetMetaType, sourceMeta, source.constData());
+
+    return QVariant();
 }
 
 /*!
@@ -258,37 +449,38 @@ static QVariant byProperties(const QMetaObject *mo, QMetaType metaType, const QV
  * pre-constructed object. It also avoids the creation of a QVariant in most cases. It is less
  * efficient if you're going to create a QVariant anyway.
  */
-bool QQmlValueTypeProvider::createValueType(const QV4::Value &s, QMetaType metaType, void *target)
+bool QQmlValueTypeProvider::createValueType(
+    QMetaType targetMetaType, void *target, const QV4::Value &source)
 {
-    if (!isConstructibleMetaType(metaType))
+    if (!isConstructibleMetaType(targetMetaType))
         return false;
 
-    auto destruct = [metaType, target]() {
-        metaType.destruct(target);
+    auto destruct = [targetMetaType, target]() {
+        targetMetaType.destruct(target);
         return target;
     };
 
-    const QQmlType type = QQmlMetaType::qmlType(metaType);
-    if (const QMetaObject *mo = QQmlMetaType::metaObjectForValueType(type)) {
+    const QQmlType type = QQmlMetaType::qmlType(targetMetaType);
+    if (const QMetaObject *targetMeta = QQmlMetaType::metaObjectForValueType(type)) {
         const auto warn = [&]() {
             qWarning().noquote()
                     << "Could not find any constructor for value type"
-                    << mo->className() << "to call with value" << s.toQStringNoThrow();
+                    << targetMeta->className() << "to call with value" << source.toQStringNoThrow();
         };
 
         if (type.canPopulateValueType()) {
-            if (s.isObject() && mo) {
-                doWriteProperties(mo, s, target);
+            if (source.isObject() && targetMeta) {
+                doWriteProperties(targetMeta, target, source);
                 return true;
             }
             if (type.canConstructValueType()) {
-                if (fromMatchingType(mo, s, destruct))
+                if (fromMatchingType(targetMeta, source, destruct))
                     return true;
                 warn();
 
             }
         } else if (type.canConstructValueType()) {
-            if (fromMatchingType(mo, s, destruct))
+            if (fromMatchingType(targetMeta, source, destruct))
                 return true;
             warn();
         }
@@ -296,9 +488,9 @@ bool QQmlValueTypeProvider::createValueType(const QV4::Value &s, QMetaType metaT
 
     if (const auto valueTypeFunction = type.createValueTypeFunction()) {
         const QVariant result
-                = valueTypeFunction(QJSValuePrivate::fromReturnedValue(s.asReturnedValue()));
-        if (result.metaType() == metaType) {
-            metaType.construct(destruct(), result.constData());
+                = valueTypeFunction(QJSValuePrivate::fromReturnedValue(source.asReturnedValue()));
+        if (result.metaType() == targetMetaType) {
+            targetMetaType.construct(destruct(), result.constData());
             return true;
         }
     }

src/qml/qml/qqmlglobal_p.h

@@ -204,9 +204,9 @@ inline void QQml_setParent_noEvent(QObject *object, QObject *parent)
 class QQmlValueTypeProvider
 {
 public:
-    static bool createValueType(const QV4::Value &, QMetaType, void *);
+    static bool createValueType(QMetaType targetMetaType, void *target, const QV4::Value &source);
     static QVariant constructValueType(
-            QMetaType resultMetaType, const QMetaObject *resultMetaObject,
+            QMetaType targetMetaType, const QMetaObject *targetMetaObject,
             int ctorIndex, void *ctorArg);
 
     static QVariant createValueType(const QJSValue &, QMetaType);

tests/auto/qml/qqmldelegatemodel/data/typedModelData.qml

@@ -0,0 +1,64 @@
+import QtQml
+
+DelegateModel {
+    id: root
+
+    // useful object as model, int as modelData
+    property ListModel singularModel: ListModel {
+        ListElement {
+            x: 11
+        }
+        ListElement {
+            x: 12
+        }
+    }
+
+    // same, useful, object as model and modelData
+    property ListModel listModel: ListModel {
+        ListElement {
+            x: 13
+            y: 14
+        }
+        ListElement {
+            x: 15
+            y: 16
+        }
+    }
+
+    // useful but different objects as modelData and model
+    // This is how the array accessor works. We can live with it.
+    property var array: [
+        {x: 17, y: 18}, {x: 19, y: 20}
+    ]
+
+    // useful but different objects as modelData and model
+    // This is how the object accessor works. We can live with it.
+    property QtObject object: QtObject {
+        property int x: 21
+        property int y: 22
+    }
+
+    property int n: -1
+
+    model: {
+        switch (n) {
+        case 0: return singularModel
+        case 1: return listModel
+        case 2: return array
+        case 3: return object
+        }
+        return undefined;
+    }
+
+    delegate: QtObject {
+        required property point modelData
+        required property QtObject model
+
+        property real modelX: model.x
+        property real modelDataX: modelData.x
+        property point modelSelf: model
+        property point modelDataSelf: modelData
+        property point modelModelData: model.modelData
+        property point modelAnonymous: model[""]
+    }
+}

tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp

@@ -29,6 +29,7 @@ private slots:
     void redrawUponColumnChange();
     void nestedDelegates();
     void universalModelData();
+    void typedModelData();
     void deleteRace();
 };
 
@@ -359,6 +360,92 @@ void tst_QQmlDelegateModel::universalModelData()
 
 }
 
+void tst_QQmlDelegateModel::typedModelData()
+{
+    QQmlEngine engine;
+    const QUrl url = testFileUrl("typedModelData.qml");
+    QQmlComponent c(&engine, url);
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+
+    QQmlDelegateModel *delegateModel = qobject_cast<QQmlDelegateModel *>(o.data());
+    QVERIFY(delegateModel);
+
+    for (int i = 0; i < 4; ++i) {
+        if (i == 0) {
+            for (int j = 0; j < 3; ++j) {
+                QTest::ignoreMessage(
+                    QtWarningMsg,
+                    "Could not find any constructor for value type QQmlPointFValueType "
+                    "to call with value QVariant(double, 11)");
+            }
+
+            QTest::ignoreMessage(
+                QtWarningMsg,
+                qPrintable(url.toString() + ":62:9: Unable to assign double to QPointF"));
+            QTest::ignoreMessage(
+                QtWarningMsg,
+                qPrintable(url.toString() + ":61:9: Unable to assign double to QPointF"));
+        }
+
+        delegateModel->setProperty("n", i);
+        QObject *delegate = delegateModel->object(0);
+        QVERIFY(delegate);
+        const QPointF modelItem = delegate->property("modelSelf").value<QPointF>();
+        switch (i) {
+        case 0: {
+            // list model with 1 role.
+            // Does not work, for the most part, because the model is singular
+            QCOMPARE(delegate->property("modelX"), 11.0);
+            QCOMPARE(delegate->property("modelDataX"), 0.0);
+            QCOMPARE(delegate->property("modelSelf"), QPointF(11.0, 0.0));
+            QCOMPARE(delegate->property("modelDataSelf"), QPointF());
+            QCOMPARE(delegate->property("modelModelData"), QPointF());
+            QCOMPARE(delegate->property("modelAnonymous"), QPointF());
+            break;
+        }
+        case 1: {
+            // list model with 2 roles
+            QCOMPARE(delegate->property("modelX"), 13.0);
+            QCOMPARE(delegate->property("modelDataX"), 13.0);
+            QCOMPARE(delegate->property("modelSelf"), QVariant::fromValue(modelItem));
+            QCOMPARE(delegate->property("modelDataSelf"), QVariant::fromValue(modelItem));
+            QCOMPARE(delegate->property("modelModelData"), QVariant::fromValue(modelItem));
+            QCOMPARE(delegate->property("modelAnonymous"), QVariant::fromValue(modelItem));
+            break;
+        }
+        case 2: {
+            // JS array of objects
+            QCOMPARE(delegate->property("modelX"), 17.0);
+            QCOMPARE(delegate->property("modelDataX"), 17.0);
+
+            const QPointF modelData = delegate->property("modelDataSelf").value<QPointF>();
+            QCOMPARE(modelData, QPointF(17, 18));
+            QCOMPARE(delegate->property("modelSelf"), QVariant::fromValue(modelData));
+            QCOMPARE(delegate->property("modelModelData").value<QPointF>(), modelData);
+            QCOMPARE(delegate->property("modelAnonymous").value<QPointF>(), modelData);
+            break;
+        }
+        case 3: {
+            // single object
+            QCOMPARE(delegate->property("modelX"), 21);
+            QCOMPARE(delegate->property("modelDataX"), 21);
+            const QPointF modelData = delegate->property("modelDataSelf").value<QPointF>();
+            QCOMPARE(modelData, QPointF(21, 22));
+            QCOMPARE(delegate->property("modelSelf"), QVariant::fromValue(modelData));
+            QCOMPARE(delegate->property("modelModelData"), QVariant::fromValue(modelData));
+            QCOMPARE(delegate->property("modelAnonymous"), QVariant::fromValue(modelData));
+            break;
+        }
+        default:
+            QFAIL("wrong model number");
+            break;
+        }
+    }
+
+}
+
+
 void tst_QQmlDelegateModel::deleteRace()
 {
     QQmlEngine engine;