Simplify Q(Declarative)GeoMapParameter

Move the initialPropertyCount to the QGeoMapParameter class, where
previously the 2 was hardcoded. Don't make type/setType virtual, as the
subclass doesn't override those methods anyway. Remove the unused
constructor taking a list of property names and values.

In QDeclarativeGeoMapParameter, optimize the dynamic connections between
property notification signals and the generic forwarder. Connect between
QMetaMethods, which avoids the repeated lookups, and give the
SignalMapper instances the QMetaProperty as a data member so that we
avoid the lookup of the signal for each signal emission, and only need
one connection per property, rather than two. Also, remove the empty
destructor and apply const where appropriate.

QDeclarativeGeoMapParameter is used only by QDeclarativePolyLineMapItem,
where support for the respective parameters, penStyle and penCap, is not
implemented in the rendering code. None of the other map items support
any such dynamic parameters, their support is not documented, and only
used in the mappolyline manual test. So remove all that code. If we want
to support more properties, then the QDeclarativeMapLineProperties type
gives us that infrastructure already.

Change-Id: Iddaac568a7dc09deb0bb5085b5b90c3cca0fa5ca
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

11 files changed, 43 insertions, 151 deletions

diff --git a/src/location/declarativemaps/qdeclarativegeomapparameter.cpp b/src/location/declarativemaps/qdeclarativegeomapparameter.cpp
index ac654572..8cb83fe0 100644
--- a/src/location/declarativemaps/qdeclarativegeomapparameter.cpp
+++ b/src/location/declarativemaps/qdeclarativegeomapparameter.cpp
@@ -50,19 +50,25 @@ namespace {
 class SignalMapper : public QObject
 {
     Q_OBJECT
-
-    int i;
 public:
-    explicit SignalMapper(int i, QObject *parent = nullptr)
-        : QObject(parent), i(i) {}
-
-public Q_SLOTS:
-    void map() { emit mapped(i); }
+    explicit SignalMapper(const QMetaProperty &p, QDeclarativeGeoMapParameter *parent)
+        : QObject(parent), property(p)
+    {}
+
+    inline static QMetaMethod forwarder =
+        staticMetaObject.method(staticMetaObject.indexOfSlot("forward()"));
+
+public slots:
+    void forward()
+    {
+        QDeclarativeGeoMapParameter *that = static_cast<QDeclarativeGeoMapParameter *>(parent());
+        that->propertyUpdated(that, property);
+    }
 
-Q_SIGNALS:
-    void mapped(int);
+private:
+    const QMetaProperty property;
 };
-} // unnamed namespace
+}
 
 /*!
     \qmltype MapParameter
@@ -109,20 +115,11 @@ QDeclarativeGeoMapParameter::QDeclarativeGeoMapParameter(QObject *parent)
 {
 }
 
-QDeclarativeGeoMapParameter::~QDeclarativeGeoMapParameter()
-{
-}
-
 bool QDeclarativeGeoMapParameter::isComponentComplete() const
 {
     return m_complete;
 }
 
-int QDeclarativeGeoMapParameter::initialPropertyCount() const
-{
-    return m_initialPropertyCount;
-}
-
 void QDeclarativeGeoMapParameter::classBegin()
 {
 }
@@ -130,27 +127,18 @@ void QDeclarativeGeoMapParameter::classBegin()
 void QDeclarativeGeoMapParameter::componentComplete()
 {
     for (int i = m_initialPropertyCount; i < metaObject()->propertyCount(); ++i) {
-        QMetaProperty property = metaObject()->property(i);
+        const QMetaProperty property = metaObject()->property(i);
 
-        if (!property.hasNotifySignal()) {
+        if (!property.hasNotifySignal())
             return;
-        }
-
-        SignalMapper *mapper = new SignalMapper(i, this);
 
-        const QByteArray signalName = '2' + property.notifySignal().methodSignature(); // TODO: explain why '2'
-        QObject::connect(this, signalName, mapper, SLOT(map()));
-        QObject::connect(mapper, SIGNAL(mapped(int)), this, SLOT(onPropertyUpdated(int)));
+        SignalMapper *mapper = new SignalMapper(property, this);
+        QObject::connect(this, property.notifySignal(), mapper, SignalMapper::forwarder);
     }
     m_complete = true;
     emit completed(this);
 }
 
-void QDeclarativeGeoMapParameter::onPropertyUpdated(int index)
-{
-    emit propertyUpdated(this, metaObject()->property(index).name());
-}
-
 QT_END_NAMESPACE
 
 #include "qdeclarativegeomapparameter.moc"
diff --git a/src/location/declarativemaps/qdeclarativegeomapparameter_p.h b/src/location/declarativemaps/qdeclarativegeomapparameter_p.h
index 04ca3f64..8fa5bfd1 100644
--- a/src/location/declarativemaps/qdeclarativegeomapparameter_p.h
+++ b/src/location/declarativemaps/qdeclarativegeomapparameter_p.h
@@ -62,10 +62,10 @@ class Q_LOCATION_PRIVATE_EXPORT QDeclarativeGeoMapParameter : public QGeoMapPara
 {
     Q_OBJECT
     Q_INTERFACES(QQmlParserStatus)
+    // this type must not declare any additional properties
 
 public:
     explicit QDeclarativeGeoMapParameter(QObject *parent = nullptr);
-    virtual ~QDeclarativeGeoMapParameter();
 
     bool isComponentComplete() const;
 
@@ -73,16 +73,11 @@ Q_SIGNALS:
     void completed(QDeclarativeGeoMapParameter *);
 
 protected:
-    int initialPropertyCount() const;
     // QQmlParserStatus implementation
     void classBegin() override;
     void componentComplete() override;
 
-private slots:
-    void onPropertyUpdated(int index);
-
 private:
-    const int m_initialPropertyCount = staticMetaObject.propertyCount();
     bool m_complete = false;
 };
 
diff --git a/src/location/maps/qgeomapparameter.cpp b/src/location/maps/qgeomapparameter.cpp
index aef520e0..7b47a541 100644
--- a/src/location/maps/qgeomapparameter.cpp
+++ b/src/location/maps/qgeomapparameter.cpp
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2022 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtLocation module of the Qt Toolkit.
@@ -50,16 +50,6 @@ QGeoMapParameter::QGeoMapParameter(QObject *parent) : QObject(parent)
 
 }
 
-QGeoMapParameter::QGeoMapParameter(const QList<QPair<QLatin1String, QVariant> > &properties, QObject *parent) : QObject(parent)
-{
-    for (const auto &p: properties) {
-        if (p.first == QLatin1String("type"))
-            setType(p.second.toString());
-        else
-            updateProperty(p.first.data(), p.second);
-    }
-}
-
 QGeoMapParameter::~QGeoMapParameter()
 {
 }
@@ -83,18 +73,20 @@ void QGeoMapParameter::setType(const QString &type)
 // DO NOT USE to set "type"
 void QGeoMapParameter::updateProperty(const char *propertyName, QVariant value)
 {
-    setProperty(propertyName, value);
+    const QMetaProperty property = metaObject()->property(metaObject()->indexOfProperty(propertyName));
+    property.write(this, value);
     // This should technically be emitted only for dynamically added properties.
     // Since this object has only type defined as Q_PROPERTY() which is a set-once
     // no check is really needed here.
-    emit propertyUpdated(this, propertyName);
+    emit propertyUpdated(this, property);
 }
 
 QVariantMap QGeoMapParameter::toVariantMap() const
 {
     QVariantMap res;
     const QMetaObject *metaObj = metaObject();
-    for (int i = 2; i < metaObj->propertyCount(); ++i) { // 0 is objectName, 1 is type, we want to skip both of them here.
+    // 0 is objectName, 1 is type, we want to skip both of them here.
+    for (int i = m_initialPropertyCount; i < metaObj->propertyCount(); ++i) {
         const char *propName = metaObj->property(i).name();
         res[QLatin1String(propName)] = property(propName);
     }
diff --git a/src/location/maps/qgeomapparameter_p.h b/src/location/maps/qgeomapparameter_p.h
index b10ab648..5be1b9e0 100644
--- a/src/location/maps/qgeomapparameter_p.h
+++ b/src/location/maps/qgeomapparameter_p.h
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2022 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtLocation module of the Qt Toolkit.
@@ -65,13 +65,12 @@ class Q_LOCATION_PRIVATE_EXPORT QGeoMapParameter : public QObject
     Q_PROPERTY(QString type READ type WRITE setType)
 public:
     explicit QGeoMapParameter(QObject *parent = nullptr);
-    QGeoMapParameter(const QList<QPair<QLatin1String, QVariant>> &properties, QObject *parent = nullptr);
-    virtual ~QGeoMapParameter();
+    ~QGeoMapParameter();
 
     bool operator==(const QGeoMapParameter &other) const;
 
-    virtual QString type() const;
-    virtual void setType(const QString &type);
+    QString type() const;
+    void setType(const QString &type);
 
     void updateProperty(const char *propertyName, QVariant value);
 
@@ -79,10 +78,11 @@ public:
     bool hasProperty(const char *propertyName) const;
 
 Q_SIGNALS:
-    void propertyUpdated(QGeoMapParameter *param, const char *propertyName);
+    void propertyUpdated(QGeoMapParameter *param, const QMetaProperty &property);
 
 protected:
     QString m_type;
+    const int m_initialPropertyCount = staticMetaObject.propertyCount();
 
     Q_DISABLE_COPY(QGeoMapParameter)
     friend class QGeoMap;
diff --git a/src/location/quickmapitems/qdeclarativepolylinemapitem.cpp b/src/location/quickmapitems/qdeclarativepolylinemapitem.cpp
index 3e1f89f4..3fbff186 100644
--- a/src/location/quickmapitems/qdeclarativepolylinemapitem.cpp
+++ b/src/location/quickmapitems/qdeclarativepolylinemapitem.cpp
@@ -1265,64 +1265,6 @@ void QDeclarativePolylineMapItem::updatePolish()
     m_d->updatePolish();
 }
 
-void QDeclarativePolylineMapItem::updateLineStyleParameter(QGeoMapParameter *p,
-                                                           const char *propertyName,
-                                                           bool update)
-{
-    static const QByteArrayList acceptedParameterTypes = QByteArrayList()
-        << QByteArrayLiteral("lineCap")
-        << QByteArrayLiteral("pen");
-    switch (acceptedParameterTypes.indexOf(QByteArray(propertyName))) {
-    case -1:
-        qWarning() << "Invalid property " << QLatin1String(propertyName) << " for parameter lineStyle";
-        break;
-    case 0: // lineCap
-        {
-            const QVariant lineCap = p->property("lineCap");
-            m_d->m_penCapStyle = lineCap.value<Qt::PenCapStyle>(); // if invalid, will return 0 == FlatCap
-            if (update)
-                markSourceDirtyAndUpdate();
-            break;
-        }
-    case 1: // penStyle
-        {
-            const QVariant penStyle = p->property("pen");
-            m_d->m_penStyle = penStyle.value<Qt::PenStyle>();
-            if (m_d->m_penStyle == Qt::NoPen)
-                m_d->m_penStyle = Qt::SolidLine;
-            if (update)
-                markSourceDirtyAndUpdate();
-            break;
-        }
-    }
-}
-
-void QDeclarativePolylineMapItem::updateLineStyleParameter(QGeoMapParameter *p, const char *propertyName)
-{
-    updateLineStyleParameter(p, propertyName, true);
-}
-
-void QDeclarativePolylineMapItem::componentComplete()
-{
-    QQuickItem::componentComplete();
-    // Set up Dynamic Parameters
-    QList<QGeoMapParameter *> dynamicParameters = quickChildren<QGeoMapParameter>();
-    for (QGeoMapParameter *p : qAsConst(dynamicParameters)) {
-        if (p->type() == QLatin1String("lineStyle")) {
-            updateLineStyleParameter(p, "lineCap", false);
-            updateLineStyleParameter(p, "pen", false);
-            connect(p, &QGeoMapParameter::propertyUpdated,
-                    this, static_cast<void (QDeclarativePolylineMapItem::*)(QGeoMapParameter *, const char *)>(&QDeclarativePolylineMapItem::updateLineStyleParameter));
-            markSourceDirtyAndUpdate();
-        }
-    }
-}
-
-void QDeclarativePolylineMapItem::markSourceDirtyAndUpdate()
-{
-    m_d->markSourceDirtyAndUpdate();
-}
-
 /*!
     \internal
 */
diff --git a/src/location/quickmapitems/qdeclarativepolylinemapitem_p.h b/src/location/quickmapitems/qdeclarativepolylinemapitem_p.h
index 0f52aaaa..bd7cc9a2 100644
--- a/src/location/quickmapitems/qdeclarativepolylinemapitem_p.h
+++ b/src/location/quickmapitems/qdeclarativepolylinemapitem_p.h
@@ -139,7 +139,6 @@ Q_SIGNALS:
     void backendChanged();
 
 protected Q_SLOTS:
-    void markSourceDirtyAndUpdate();
     void updateAfterLinePropertiesChanged();
     void afterViewportChanged(const QGeoMapViewportChangeEvent &event) override;
 
@@ -147,9 +146,6 @@ protected:
     void geometryChange(const QRectF &newGeometry, const QRectF &oldGeometry) override;
     void setPathFromGeoList(const QList<QGeoCoordinate> &path);
     void updatePolish() override;
-    void componentComplete() override;
-    void updateLineStyleParameter(QGeoMapParameter *p, const char *propertyName);
-    void updateLineStyleParameter(QGeoMapParameter *p, const char *propertyName, bool update);
 
 #ifdef QT_LOCATION_DEBUG
 public:
diff --git a/src/location/quickmapitems/qdeclarativepolylinemapitem_p_p.h b/src/location/quickmapitems/qdeclarativepolylinemapitem_p_p.h
index 50a1561b..963f7279 100644
--- a/src/location/quickmapitems/qdeclarativepolylinemapitem_p_p.h
+++ b/src/location/quickmapitems/qdeclarativepolylinemapitem_p_p.h
@@ -504,8 +504,6 @@ public:
     virtual bool contains(const QPointF &point) const = 0;
 
     QDeclarativePolylineMapItem &m_poly;
-    Qt::PenStyle m_penStyle = Qt::SolidLine;
-    Qt::PenCapStyle m_penCapStyle = Qt::SquareCap;
 };
 
 class Q_LOCATION_PRIVATE_EXPORT QDeclarativePolylineMapItemPrivateCPU: public QDeclarativePolylineMapItemPrivate
@@ -784,7 +782,7 @@ public:
                          &m_geometry,
                          combinedMatrix,
                          cameraCenter,
-                         m_penCapStyle,
+                         Qt::FlatCap,
                          false,
                          m_poly.zoomForLOD(int(map->cameraData().zoomLevel())));
             m_geometry.setPreserveGeometry(false);
diff --git a/src/plugins/geoservices/mapboxgl/qgeomapmapboxgl.cpp b/src/plugins/geoservices/mapboxgl/qgeomapmapboxgl.cpp
index 9f12c1f5..e00369a7 100644
--- a/src/plugins/geoservices/mapboxgl/qgeomapmapboxgl.cpp
+++ b/src/plugins/geoservices/mapboxgl/qgeomapmapboxgl.cpp
@@ -490,7 +490,7 @@ void QGeoMapMapboxGL::onMapItemGeometryChanged()
     emit sgNodeChanged();
 }
 
-void QGeoMapMapboxGL::onParameterPropertyUpdated(QGeoMapParameter *param, const char *)
+void QGeoMapMapboxGL::onParameterPropertyUpdated(QGeoMapParameter *param)
 {
     Q_D(QGeoMapMapboxGL);
 
diff --git a/tests/auto/geotestplugin/qgeotiledmap_test.cpp b/tests/auto/geotestplugin/qgeotiledmap_test.cpp
index bab66fc1..b4986b08 100644
--- a/tests/auto/geotestplugin/qgeotiledmap_test.cpp
+++ b/tests/auto/geotestplugin/qgeotiledmap_test.cpp
@@ -27,6 +27,7 @@
 ****************************************************************************/
 
 #include "qgeotiledmap_test.h"
+#include <QtCore/QMetaProperty>
 #include <QtPositioning/QGeoCoordinate>
 #include <QtLocation/private/qgeotiledmap_p_p.h>
 #include <QtLocation/private/qgeomapparameter_p.h>
@@ -60,8 +61,8 @@ public:
             cameraData.setCenter(newCenter);
             q->setCameraData(cameraData);
             // Connect for further changes handling
-            q->connect(param, SIGNAL(propertyUpdated(QGeoMapParameter *, const char *)),
-                       q, SLOT(onCameraCenter_testChanged(QGeoMapParameter*, const char*)));
+            q->connect(param, &QGeoMapParameter::propertyUpdated,
+                       q, &QGeoTiledMapTest::onCameraCenter_testChanged);
 
         }
     }
@@ -97,12 +98,12 @@ QGeoTiledMapTest::QGeoTiledMapTest(QGeoTiledMappingManagerEngine *engine,
 {
 }
 
-void QGeoTiledMapTest::onCameraCenter_testChanged(QGeoMapParameter *param, const char *propertyName)
+void QGeoTiledMapTest::onCameraCenter_testChanged(QGeoMapParameter *param, const QMetaProperty &property)
 {
-    if (strcmp(propertyName, "center") == 0) {
+    if (strcmp(property.name(), "center") == 0) {
         QGeoCameraData cameraData = this->cameraData();
         // Not testing for propertyName as this param has only one allowed property
-        QGeoCoordinate newCenter = param->property(propertyName).value<QGeoCoordinate>();
+        QGeoCoordinate newCenter = property.read(param).value<QGeoCoordinate>();
         cameraData.setCenter(newCenter);
         setCameraData(cameraData);
     }
diff --git a/tests/auto/geotestplugin/qgeotiledmap_test.h b/tests/auto/geotestplugin/qgeotiledmap_test.h
index 7bb1ea27..7194e3d6 100644
--- a/tests/auto/geotestplugin/qgeotiledmap_test.h
+++ b/tests/auto/geotestplugin/qgeotiledmap_test.h
@@ -51,7 +51,7 @@ public:
                      QObject *parent = nullptr);
 
 protected slots:
-    void onCameraCenter_testChanged(QGeoMapParameter *param, const char *propertyName);
+    void onCameraCenter_testChanged(QGeoMapParameter *param, const QMetaProperty &property);
 
 public:
     using QGeoTiledMap::setCameraData;
diff --git a/tests/manual/mappolyline_tester/main.qml b/tests/manual/mappolyline_tester/main.qml
index c4e62942..9a657309 100644
--- a/tests/manual/mappolyline_tester/main.qml
+++ b/tests/manual/mappolyline_tester/main.qml
@@ -190,11 +190,6 @@ Window {
                        ? MapPolygon.Software : MapPolygon.OpenGL
             }
 
-            function miterValue()
-            {
-                return (miterSwitch.checked) ? Qt.RoundCap : Qt.FlatCap
-            }
-
             MapPolyline {
                 id: tstPolyLine // to verify the polygon stays where it's supposed to
                 line.color: 'black'
@@ -236,10 +231,6 @@ Window {
                     { latitude: 45, longitude: 174 },
                     { latitude: 43, longitude: -168 }
                 ]
-                DynamicParameter {
-                    type: "lineStyle"
-                    property var lineCap: miterValue()
-                }
 
                 MouseArea {
                     anchors.fill: parent
@@ -298,23 +289,12 @@ Window {
         }
         checked: false
     }
-    C2.Switch {
-        text: qsTr("Miter")
-        id: miterSwitch
-        anchors {
-            top: leftSwitch.bottom
-            left: parent.left
-            leftMargin: 12
-            rightMargin: 12
-        }
-        checked: false
-    }
     C2.Slider {
         id: sliWidth
         orientation: Qt.Vertical
         anchors {
             left: parent.left
-            top: miterSwitch.bottom
+            top: leftSwitch.bottom
             bottom: parent.bottom
             topMargin: 10
             leftMargin: 10