Return the correct type from Item::mapToItem/Item::mapFromItem.

Previously we were returning a JavaScript object with x/y/width/height
properties, instead of a point/rect. This meant that the type couldn't
be converted to a point/rect because we don't support duck typing,
where we would deduce the type based on the properties.

One example of a broken use case that this patch fixes is when QML is
unable to convert the return type to a point in a property declaration:

property point p: mouseArea.mapToItem(child, mouseArea.mouseX, mouseArea.mouseY)

Another is using the result of the function to pass to another function:

child.contains(mouseArea.mapToItem(child, mouseArea.mouseX, mouseArea.mouseY))

Change-Id: I3ce82f10175f904dd02c8af6b5e42cee14b2ebb2
Task-number: QTBUG-41452
Reviewed-by: Liang Qi <liang.qi@theqtcompany.com>
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index 7ab2faf052e51ef404da14803958e861f4d82818..4d1bb696fa32bf6ad851e1e1131614e3c1d3f07c 100644 (file)
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -4113,8 +4113,8 @@ void QQuickItem::polish()
     \qmlmethod object QtQuick::Item::mapFromItem(Item item, real x, real y, real width, real height)
 
     Maps the point (\a x, \a y) or rect (\a x, \a y, \a width, \a height), which is in \a
-    item's coordinate system, to this item's coordinate system, and returns an object with \c x and
-    \c y (and optionally \c width and \c height) properties matching the mapped coordinate.
+    item's coordinate system, to this item's coordinate system, and returns a \l point or \rect
+    matching the mapped coordinate.
 
     If \a item is a \c null value, this maps the point or rect from the coordinate system of
     the root QML view.
@@ -4124,50 +4124,43 @@ void QQuickItem::polish()
   */
 void QQuickItem::mapFromItem(QQmlV4Function *args) const
 {
-    if (args->length() != 0) {
-        QV4::ExecutionEngine *v4 = args->v4engine();
-        QV4::Scope scope(v4);
-        QV4::ScopedValue item(scope, (*args)[0]);
-
-        QQuickItem *itemObj = 0;
-        if (!item->isNull()) {
-            QV4::Scoped<QV4::QObjectWrapper> qobjectWrapper(scope, item->as<QV4::QObjectWrapper>());
-            if (qobjectWrapper)
-                itemObj = qobject_cast<QQuickItem*>(qobjectWrapper->object());
-        }
-
-        if (!itemObj && !item->isNull()) {
-            qmlInfo(this) << "mapFromItem() given argument \"" << item->toQStringNoThrow()
-                          << "\" which is neither null nor an Item";
-            return;
-        }
+    if (args->length() == 0)
+        return;
 
-        QV4::ScopedObject rv(scope, v4->newObject());
-        args->setReturnValue(rv.asReturnedValue());
+    QV4::ExecutionEngine *v4 = args->v4engine();
+    QV4::Scope scope(v4);
+    QV4::ScopedValue item(scope, (*args)[0]);
 
-        QV4::ScopedString s(scope);
-        QV4::ScopedValue v(scope);
+    QQuickItem *itemObj = 0;
+    if (!item->isNull()) {
+        QV4::Scoped<QV4::QObjectWrapper> qobjectWrapper(scope, item->as<QV4::QObjectWrapper>());
+        if (qobjectWrapper)
+            itemObj = qobject_cast<QQuickItem*>(qobjectWrapper->object());
+    }
 
-        qreal x = (args->length() > 1) ? (v = (*args)[1])->asDouble() : 0;
-        qreal y = (args->length() > 2) ? (v = (*args)[2])->asDouble() : 0;
+    if (!itemObj && !item->isNull()) {
+        qmlInfo(this) << "mapFromItem() given argument \"" << item->toQStringNoThrow()
+                      << "\" which is neither null nor an Item";
+        return;
+    }
 
-        if (args->length() > 3) {
-            qreal w = (v = (*args)[3])->asDouble();
-            qreal h = (args->length() > 4) ? (v = (*args)[4])->asDouble() : 0;
+    QV4::ScopedValue v(scope);
 
-            QRectF r = mapRectFromItem(itemObj, QRectF(x, y, w, h));
+    qreal x = (args->length() > 1) ? (v = (*args)[1])->asDouble() : 0;
+    qreal y = (args->length() > 2) ? (v = (*args)[2])->asDouble() : 0;
 
-            rv->put((s = v4->newString(QStringLiteral("x"))), (v = QV4::Primitive::fromDouble(r.x())));
-            rv->put((s = v4->newString(QStringLiteral("y"))), (v = QV4::Primitive::fromDouble(r.y())));
-            rv->put((s = v4->newString(QStringLiteral("width"))), (v = QV4::Primitive::fromDouble(r.width())));
-            rv->put((s = v4->newString(QStringLiteral("height"))), (v = QV4::Primitive::fromDouble(r.height())));
-        } else {
-            QPointF p = mapFromItem(itemObj, QPointF(x, y));
+    QVariant result;
 
-            rv->put((s = v4->newString(QStringLiteral("x"))), (v = QV4::Primitive::fromDouble(p.x())));
-            rv->put((s = v4->newString(QStringLiteral("y"))), (v = QV4::Primitive::fromDouble(p.y())));
-        }
+    if (args->length() > 3) {
+        qreal w = (v = (*args)[3])->asDouble();
+        qreal h = (args->length() > 4) ? (v = (*args)[4])->asDouble() : 0;
+        result = mapRectFromItem(itemObj, QRectF(x, y, w, h));
+    } else {
+        result = mapFromItem(itemObj, QPointF(x, y));
     }
+
+    QV4::ScopedObject rv(scope, v4->fromVariant(result));
+    args->setReturnValue(rv.asReturnedValue());
 }
 
 /*!
@@ -4192,8 +4185,8 @@ QTransform QQuickItem::itemTransform(QQuickItem *other, bool *ok) const
     \qmlmethod object QtQuick::Item::mapToItem(Item item, real x, real y, real width, real height)
 
     Maps the point (\a x, \a y) or rect (\a x, \a y, \a width, \a height), which is in this
-    item's coordinate system, to \a item's coordinate system, and returns an object with \c x and
-    \c y (and optionally \c width and \c height) properties matching the mapped coordinate.
+    item's coordinate system, to \a item's coordinate system, and returns a \l point or \l rect
+    matching the mapped coordinate.
 
     If \a item is a \c null value, this maps the point or rect to the coordinate system of the
     root QML view.
@@ -4203,51 +4196,43 @@ QTransform QQuickItem::itemTransform(QQuickItem *other, bool *ok) const
   */
 void QQuickItem::mapToItem(QQmlV4Function *args) const
 {
-    if (args->length() != 0) {
-        QV4::ExecutionEngine *v4 = args->v4engine();
-        QV4::Scope scope(v4);
-        QV4::ScopedValue item(scope, (*args)[0]);
-
-        QQuickItem *itemObj = 0;
-        if (!item->isNull()) {
-            QV4::Scoped<QV4::QObjectWrapper> qobjectWrapper(scope, item->as<QV4::QObjectWrapper>());
-            if (qobjectWrapper)
-                itemObj = qobject_cast<QQuickItem*>(qobjectWrapper->object());
-        }
-
-        if (!itemObj && !item->isNull()) {
-            qmlInfo(this) << "mapToItem() given argument \"" << item->toQStringNoThrow()
-                          << "\" which is neither null nor an Item";
-            return;
-        }
-
-        QV4::ScopedObject rv(scope, v4->newObject());
-        args->setReturnValue(rv.asReturnedValue());
+    if (args->length() == 0)
+        return;
 
-        QV4::ScopedValue v(scope);
+    QV4::ExecutionEngine *v4 = args->v4engine();
+    QV4::Scope scope(v4);
+    QV4::ScopedValue item(scope, (*args)[0]);
 
-        qreal x = (args->length() > 1) ? (v = (*args)[1])->asDouble() : 0;
-        qreal y = (args->length() > 2) ? (v = (*args)[2])->asDouble() : 0;
+    QQuickItem *itemObj = 0;
+    if (!item->isNull()) {
+        QV4::Scoped<QV4::QObjectWrapper> qobjectWrapper(scope, item->as<QV4::QObjectWrapper>());
+        if (qobjectWrapper)
+            itemObj = qobject_cast<QQuickItem*>(qobjectWrapper->object());
+    }
 
-        QV4::ScopedString s(scope);
+    if (!itemObj && !item->isNull()) {
+        qmlInfo(this) << "mapToItem() given argument \"" << item->toQStringNoThrow()
+                      << "\" which is neither null nor an Item";
+        return;
+    }
 
-        if (args->length() > 3) {
-            qreal w = (v = (*args)[3])->asDouble();
-            qreal h = (args->length() > 4) ? (v = (*args)[4])->asDouble() : 0;
+    QV4::ScopedValue v(scope);
+    QVariant result;
 
-            QRectF r = mapRectToItem(itemObj, QRectF(x, y, w, h));
+    qreal x = (args->length() > 1) ? (v = (*args)[1])->asDouble() : 0;
+    qreal y = (args->length() > 2) ? (v = (*args)[2])->asDouble() : 0;
 
-            rv->put((s = v4->newString(QStringLiteral("x"))), (v = QV4::Primitive::fromDouble(r.x())));
-            rv->put((s = v4->newString(QStringLiteral("y"))), (v = QV4::Primitive::fromDouble(r.y())));
-            rv->put((s = v4->newString(QStringLiteral("width"))), (v = QV4::Primitive::fromDouble(r.width())));
-            rv->put((s = v4->newString(QStringLiteral("height"))), (v = QV4::Primitive::fromDouble(r.height())));
-        } else {
-            QPointF p = mapToItem(itemObj, QPointF(x, y));
+    if (args->length() > 3) {
+        qreal w = (v = (*args)[3])->asDouble();
+        qreal h = (args->length() > 4) ? (v = (*args)[4])->asDouble() : 0;
 
-            rv->put((s = v4->newString(QStringLiteral("x"))), (v = QV4::Primitive::fromDouble(p.x())));
-            rv->put((s = v4->newString(QStringLiteral("y"))), (v = QV4::Primitive::fromDouble(p.y())));
-        }
+        result = mapRectToItem(itemObj, QRectF(x, y, w, h));
+    } else {
+        result = mapToItem(itemObj, QPointF(x, y));
     }
+
+    QV4::ScopedObject rv(scope, v4->fromVariant(result));
+    args->setReturnValue(rv.asReturnedValue());
 }
 
 /*!

diff --git a/tests/auto/quick/qquickitem/data/contains.qml b/tests/auto/quick/qquickitem/data/contains.qml
new file mode 100644 (file)
index 0000000..72fd8bd
--- /dev/null
+++ b/tests/auto/quick/qquickitem/data/contains.qml
@@ -0,0 +1,26 @@
+import QtQuick 2.3
+
+Item {
+    id: root
+    width: 300
+    height: 300
+
+    function childContainsViaMapToItem(x, y) {
+        var childLocalPos = root.mapToItem(child, x, y);
+        return child.contains(childLocalPos);
+    }
+
+    function childContainsViaMapFromItem(x, y) {
+        var childLocalPos = child.mapFromItem(root, x, y);
+        return child.contains(childLocalPos);
+    }
+
+    Item {
+        id: child
+        x: 50
+        y: 50
+        width: 50
+        height: 50
+    }
+}
+

diff --git a/tests/auto/quick/qquickitem/tst_qquickitem.cpp b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
index 832cbab97111340678487398a0bc5825e2983dab..c79af91747dca33f72771bcf3eea0a4eb58b4316 100644 (file)
--- a/tests/auto/quick/qquickitem/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
@@ -169,6 +169,9 @@ private slots:
 
     void objectChildTransform();
 
+    void contains_data();
+    void contains();
+
 private:
 
     enum PaintOrderOp {
@@ -1927,6 +1930,48 @@ void tst_qquickitem::objectChildTransform()
     // Shouldn't crash.
 }
 
+void tst_qquickitem::contains_data()
+{
+    QTest::addColumn<int>("x");
+    QTest::addColumn<int>("y");
+    QTest::addColumn<bool>("contains");
+
+    QTest::newRow("(0, 0) = false") << 0 << 0 << false;
+    QTest::newRow("(50, 0) = false") << 50 << 0 << false;
+    QTest::newRow("(0, 50) = false") << 0 << 50 << false;
+    QTest::newRow("(50, 50) = true") << 50 << 50 << true;
+    QTest::newRow("(100, 100) = true") << 100 << 100 << true;
+    QTest::newRow("(150, 150) = false") << 150 << 150 << false;
+}
+
+void tst_qquickitem::contains()
+{
+    // Tests that contains works, but also checks that mapToItem/mapFromItem
+    // return the correct type (point or rect, not a JS object with those properties),
+    // as this is a common combination of calls.
+
+    QFETCH(int, x);
+    QFETCH(int, y);
+    QFETCH(bool, contains);
+
+    QQuickView view;
+    view.setSource(testFileUrl("contains.qml"));
+
+    QQuickItem *root = qobject_cast<QQuickItem*>(view.rootObject());
+    QVERIFY(root);
+
+    QVariant result = false;
+    QVERIFY(QMetaObject::invokeMethod(root, "childContainsViaMapToItem",
+        Q_RETURN_ARG(QVariant, result), Q_ARG(QVariant, qreal(x)), Q_ARG(QVariant, qreal(y))));
+    QCOMPARE(result.toBool(), contains);
+
+    result = false;
+    QVERIFY(QMetaObject::invokeMethod(root, "childContainsViaMapFromItem",
+        Q_RETURN_ARG(QVariant, result), Q_ARG(QVariant, qreal(x)), Q_ARG(QVariant, qreal(y))));
+    QCOMPARE(result.toBool(), contains);
+}
+
+
 QTEST_MAIN(tst_qquickitem)
 
 #include "tst_qquickitem.moc"