Move management of QDeclarativeGeoMap camera data into QGeoMap

This patch makes QDeclarativeGeoMap always refer to QGeoMap
when it comes to setting and getting camera data properties.
The signal emission for each of these properties is also deferred
to the reception of QGeoMap::cameraDataChanged.
This patch also enable plugins to enforce tilt ranges per zoom,
updating the tilt value (as well as any other camera data value)
on their own.

Change-Id: Icc16645ea53fa2b53b33530f802ce390a2479d39
Reviewed-by: BogDan Vatra <bogdan@kdab.com>

6 files changed, 109 insertions, 132 deletions

diff --git a/src/location/declarativemaps/qdeclarativegeomap.cpp b/src/location/declarativemaps/qdeclarativegeomap.cpp
index 00643ea1..7d1bcea9 100644
--- a/src/location/declarativemaps/qdeclarativegeomap.cpp
+++ b/src/location/declarativemaps/qdeclarativegeomap.cpp
@@ -325,10 +325,6 @@ void QDeclarativeGeoMap::setError(QGeoServiceProvider::Error error, const QStrin
 void QDeclarativeGeoMap::initialize()
 {
     // try to keep change signals in the end
-    bool centerHasChanged = false;
-    bool bearingHasChanged = false;
-    bool tiltHasChanged = false;
-    bool fovHasChanged = false;
     bool visibleAreaHasChanged = false;
 
     QGeoCoordinate center = m_cameraData.center();
@@ -341,56 +337,38 @@ void QDeclarativeGeoMap::initialize()
     double bearing = m_cameraData.bearing();
     double tilt = m_cameraData.tilt();
     double fov = m_cameraData.fieldOfView(); // Must be 45.0
+    QGeoCameraData cameraData = m_cameraData;
 
-    if (!m_cameraCapabilities.supportsBearing() && bearing != 0.0) {
-        m_cameraData.setBearing(0);
-        bearingHasChanged = true;
-    }
-    if (!m_cameraCapabilities.supportsTilting() && tilt != 0.0) {
-        m_cameraData.setTilt(0);
-        tiltHasChanged = true;
-    }
+    if (!m_cameraCapabilities.supportsBearing() && bearing != 0.0)
+        cameraData.setBearing(0);
+
+    if (!m_cameraCapabilities.supportsTilting() && tilt != 0.0)
+        cameraData.setTilt(0);
 
     m_map->setVisibleArea(m_visibleArea);
     if (m_map->visibleArea() != m_visibleArea)
         visibleAreaHasChanged = true;
 
-    m_cameraData.setFieldOfView(qBound(m_cameraCapabilities.minimumFieldOfView(),
+    cameraData.setFieldOfView(qBound(m_cameraCapabilities.minimumFieldOfView(),
                                        fov,
                                        m_cameraCapabilities.maximumFieldOfView()));
-    if (fov != m_cameraData.fieldOfView())
-        fovHasChanged  = true;
 
     // set latitude boundary check
-    m_maximumViewportLatitude = m_map->maximumCenterLatitudeAtZoom(m_cameraData);
-    m_minimumViewportLatitude = m_map->minimumCenterLatitudeAtZoom(m_cameraData);
+    m_maximumViewportLatitude = m_map->maximumCenterLatitudeAtZoom(cameraData);
+    m_minimumViewportLatitude = m_map->minimumCenterLatitudeAtZoom(cameraData);
 
     center.setLatitude(qBound(m_minimumViewportLatitude, center.latitude(), m_maximumViewportLatitude));
+    cameraData.setCenter(center);
 
-    if (center != m_cameraData.center()) {
-        centerHasChanged = true;
-        m_cameraData.setCenter(center);
-    }
-
-    m_map->setCameraData(m_cameraData);
+    connect(m_map, &QGeoMap::cameraDataChanged,
+            this,  &QDeclarativeGeoMap::onCameraDataChanged);
+    m_map->setCameraData(cameraData);
 
     for (auto obj : qAsConst(m_pendingMapObjects))
         obj->setMap(m_map);
 
     m_initialized = true;
 
-    if (centerHasChanged)
-        emit centerChanged(m_cameraData.center());
-
-    if (bearingHasChanged)
-        emit bearingChanged(m_cameraData.bearing());
-
-    if (tiltHasChanged)
-        emit tiltChanged(m_cameraData.tilt());
-
-    if (fovHasChanged)
-        emit fieldOfViewChanged(m_cameraData.fieldOfView());
-
     if (visibleAreaHasChanged)
         emit visibleAreaChanged();
     connect(m_map, &QGeoMap::visibleAreaChanged, this, &QDeclarativeGeoMap::visibleAreaChanged);
@@ -882,34 +860,26 @@ void QDeclarativeGeoMap::setZoomLevel(qreal zoomLevel)
 */
 void QDeclarativeGeoMap::setZoomLevel(qreal zoomLevel, bool overzoom)
 {
-    const qreal oldZoom = m_cameraData.zoomLevel();
-    if (oldZoom == zoomLevel || zoomLevel < 0)
+    if (zoomLevel < 0)
         return;
 
-    //small optimization to avoid double setCameraData
-    bool centerHasChanged = false;
-
     if (m_initialized) {
-        m_cameraData.setZoomLevel(qBound<qreal>(overzoom ? m_map->minimumZoom() : effectiveMinimumZoomLevel(),
+        QGeoCameraData cameraData = m_map->cameraData();
+        if (cameraData.zoomLevel() == zoomLevel)
+            return;
+
+        cameraData.setZoomLevel(qBound<qreal>(overzoom ? m_map->minimumZoom() : effectiveMinimumZoomLevel(),
                                                 zoomLevel,
                                                 overzoom ? 30 : maximumZoomLevel()));
-        m_maximumViewportLatitude = m_map->maximumCenterLatitudeAtZoom(m_cameraData);
-        m_minimumViewportLatitude = m_map->minimumCenterLatitudeAtZoom(m_cameraData);
-        QGeoCoordinate coord = m_cameraData.center();
+        m_maximumViewportLatitude = m_map->maximumCenterLatitudeAtZoom(cameraData);
+        m_minimumViewportLatitude = m_map->minimumCenterLatitudeAtZoom(cameraData);
+        QGeoCoordinate coord = cameraData.center();
         coord.setLatitude(qBound(m_minimumViewportLatitude, coord.latitude(), m_maximumViewportLatitude));
-        if (coord != m_cameraData.center()) {
-            centerHasChanged = true;
-            m_cameraData.setCenter(coord);
-        }
-        m_map->setCameraData(m_cameraData);
+        cameraData.setCenter(coord);
+        m_map->setCameraData(cameraData);
     } else {
         m_cameraData.setZoomLevel(zoomLevel);
     }
-
-    if (centerHasChanged)
-        emit centerChanged(m_cameraData.center());
-    if (oldZoom != m_cameraData.zoomLevel())
-        emit zoomLevelChanged(m_cameraData.zoomLevel());
 }
 
 bool QDeclarativeGeoMap::addMapChild(QObject *child)
@@ -966,6 +936,8 @@ bool QDeclarativeGeoMap::isGroupNested(QDeclarativeGeoMapItemGroup *group)
 
 qreal QDeclarativeGeoMap::zoomLevel() const
 {
+    if (m_initialized)
+        return m_map->cameraData().zoomLevel();
     return m_cameraData.zoomLevel();
 }
 
@@ -982,13 +954,13 @@ qreal QDeclarativeGeoMap::zoomLevel() const
 void QDeclarativeGeoMap::setBearing(qreal bearing)
 {
     bearing = sanitizeBearing(bearing);
-    if (m_cameraData.bearing() == bearing)
-        return;
-
-    m_cameraData.setBearing(bearing);
-    if (m_map)
-        m_map->setCameraData(m_cameraData);
-    emit bearingChanged(bearing);
+    if (m_initialized) {
+        QGeoCameraData cameraData = m_map->cameraData();
+        cameraData.setBearing(bearing);
+        m_map->setCameraData(cameraData);
+    } else {
+        m_cameraData.setBearing(bearing);
+    }
 }
 
 /*!
@@ -1005,7 +977,7 @@ void QDeclarativeGeoMap::setBearing(qreal bearing)
 */
 void QDeclarativeGeoMap::setBearing(qreal bearing, const QGeoCoordinate &coordinate)
 {
-    if (!m_map)
+    if (!m_initialized)
         return;
 
     const QGeoCoordinate currentCenter = center();
@@ -1017,21 +989,14 @@ void QDeclarativeGeoMap::setBearing(qreal bearing, const QGeoCoordinate &coordin
             || (coordinate == currentCenter && bearing == currentBearing))
         return;
 
-    if (m_map->capabilities() & QGeoMap::SupportsSetBearing) {
-        if (!m_map->setBearing(bearing, coordinate))
-            return;
-
-        m_cameraData = m_map->cameraData();
-
-        if (m_cameraData.center() != currentCenter)
-            emit centerChanged(m_cameraData.center());
-        if (m_cameraData.bearing() != currentBearing)
-            emit bearingChanged(bearing);
-    }
+    if (m_map->capabilities() & QGeoMap::SupportsSetBearing)
+        m_map->setBearing(bearing, coordinate);
 }
 
 qreal QDeclarativeGeoMap::bearing() const
 {
+    if (m_initialized)
+        return m_map->cameraData().bearing();
     return m_cameraData.bearing();
 }
 
@@ -1050,17 +1015,20 @@ qreal QDeclarativeGeoMap::bearing() const
 void QDeclarativeGeoMap::setTilt(qreal tilt)
 {
     tilt = qBound(minimumTilt(), tilt, maximumTilt());
-    if (m_cameraData.tilt() == tilt)
-        return;
 
-    m_cameraData.setTilt(tilt);
-    if (m_map)
-        m_map->setCameraData(m_cameraData);
-    emit tiltChanged(tilt);
+    if (m_initialized) {
+        QGeoCameraData cameraData = m_map->cameraData();
+        cameraData.setTilt(tilt);
+        m_map->setCameraData(cameraData);
+    } else {
+        m_cameraData.setTilt(tilt);
+    }
 }
 
 qreal QDeclarativeGeoMap::tilt() const
 {
+    if (m_initialized)
+        return m_map->cameraData().tilt();
     return m_cameraData.tilt();
 }
 
@@ -1102,17 +1070,20 @@ void QDeclarativeGeoMap::setMinimumTilt(qreal minimumTilt, bool userSet)
 void QDeclarativeGeoMap::setFieldOfView(qreal fieldOfView)
 {
     fieldOfView = qBound(minimumFieldOfView(), fieldOfView, maximumFieldOfView());
-    if (m_cameraData.fieldOfView() == fieldOfView)
-        return;
 
-    m_cameraData.setFieldOfView(fieldOfView);
-    if (m_map)
-        m_map->setCameraData(m_cameraData);
-    emit fieldOfViewChanged(fieldOfView);
+    if (m_initialized) {
+        QGeoCameraData cameraData = m_map->cameraData();
+        cameraData.setFieldOfView(fieldOfView);
+        m_map->setCameraData(cameraData);
+    } else {
+        m_cameraData.setFieldOfView(fieldOfView);
+    }
 }
 
 qreal QDeclarativeGeoMap::fieldOfView() const
 {
+    if (m_initialized)
+        return m_map->cameraData().fieldOfView();
     return m_cameraData.fieldOfView();
 }
 
@@ -1199,6 +1170,8 @@ qreal QDeclarativeGeoMap::maximumFieldOfView() const
     this property.
     If the \l plugin property is not set or the plugin does not support mapping, this property is \c 0.
 
+    Since QtLocation 5.12, plugins can additionally restrict this value depending on the current zoom level.
+
     \sa tilt, maximumTilt
 
     \since QtLocation 5.9
@@ -1236,6 +1209,8 @@ void QDeclarativeGeoMap::setMaximumTilt(qreal maximumTilt, bool userSet)
     this property.
     If the \l plugin property is not set or the plugin does not support mapping, this property is \c 89.5.
 
+    Since QtLocation 5.12, plugins can additionally restrict this value depending on the current zoom level.
+
     \sa tilt, minimumTilt
 
     \since QtLocation 5.9
@@ -1255,26 +1230,24 @@ qreal QDeclarativeGeoMap::maximumTilt() const
 */
 void QDeclarativeGeoMap::setCenter(const QGeoCoordinate &center)
 {
-    if (center == m_cameraData.center())
-        return;
-
     if (!center.isValid())
         return;
 
     if (m_initialized) {
         QGeoCoordinate coord(center);
         coord.setLatitude(qBound(m_minimumViewportLatitude, center.latitude(), m_maximumViewportLatitude));
-        m_cameraData.setCenter(coord);
-        m_map->setCameraData(m_cameraData);
+        QGeoCameraData cameraData = m_map->cameraData();
+        cameraData.setCenter(coord);
+        m_map->setCameraData(cameraData);
     } else {
         m_cameraData.setCenter(center);
     }
-
-    emit centerChanged(m_cameraData.center());
 }
 
 QGeoCoordinate QDeclarativeGeoMap::center() const
 {
+    if (m_initialized)
+        return m_map->cameraData().center();
     return m_cameraData.center();
 }
 
@@ -1484,18 +1457,8 @@ void QDeclarativeGeoMap::fitViewportToGeoShape()
         double newZoom = qMax<double>(minimumZoomLevel(), fitData.second);
         setProperty("zoomLevel", QVariant::fromValue(newZoom)); // not using setZoomLevel(newZoom)  to honor a possible animation set on the zoomLevel property
     } else if (m_map->capabilities() & QGeoMap::SupportsFittingViewportToGeoRectangle) {
-        // Animations cannot be honored in this case, as m_map act as a black box
-        const QGeoCoordinate currentCenter = center();
-        const qreal currentZoom = zoomLevel();
-
-        if (!m_map->fitViewportToGeoRectangle(m_visibleRegion))
-            return;
-
-        m_cameraData = m_map->cameraData();
-        if (m_cameraData.center() != currentCenter)
-            emit centerChanged(m_cameraData.center());
-        if (m_cameraData.zoomLevel() != currentZoom)
-            emit zoomLevelChanged(m_cameraData.zoomLevel());
+        // Animations cannot be honored in this case, as m_map acts as a black box
+        m_map->fitViewportToGeoRectangle(m_visibleRegion);
     }
 }
 
@@ -1535,20 +1498,12 @@ void QDeclarativeGeoMap::alignCoordinateToPoint(const QGeoCoordinate &coordinate
     if (!m_map || !(m_map->capabilities() & QGeoMap::SupportsAnchoringCoordinate))
         return;
 
-    const QGeoCoordinate currentCenter = center();
-
     if (!coordinate.isValid()
             || !qIsFinite(point.x())
             || !qIsFinite(point.y()))
         return;
 
-    if (!m_map->anchorCoordinateToPoint(coordinate, point))
-        return;
-
-    m_cameraData = m_map->cameraData();
-
-    if (m_cameraData.center() != currentCenter)
-        emit centerChanged(m_cameraData.center());
+    m_map->anchorCoordinateToPoint(coordinate, point);
 }
 
 /*!
@@ -1728,6 +1683,28 @@ void QDeclarativeGeoMap::onAttachedCopyrightNoticeVisibilityChanged()
         m_map->setCopyrightVisible(m_copyNoticesVisible > 0);
 }
 
+void QDeclarativeGeoMap::onCameraDataChanged(const QGeoCameraData &cameraData)
+{
+    bool centerHasChanged = cameraData.center() != m_cameraData.center();
+    bool bearingHasChanged = cameraData.bearing() != m_cameraData.bearing();
+    bool tiltHasChanged = cameraData.tilt() != m_cameraData.tilt();
+    bool fovHasChanged = cameraData.fieldOfView() != m_cameraData.fieldOfView();
+    bool zoomHasChanged = cameraData.zoomLevel() != m_cameraData.zoomLevel();
+
+    m_cameraData = cameraData;
+
+    if (centerHasChanged)
+        emit centerChanged(m_cameraData.center());
+    if (zoomHasChanged)
+        emit zoomLevelChanged(m_cameraData.zoomLevel());
+    if (bearingHasChanged)
+        emit bearingChanged(m_cameraData.bearing());
+    if (tiltHasChanged)
+        emit tiltChanged(m_cameraData.tilt());
+    if (fovHasChanged)
+        emit fieldOfViewChanged(m_cameraData.fieldOfView());
+}
+
 /*!
     \qmlmethod void QtLocation::Map::addMapParameter(MapParameter parameter)
 
@@ -2190,20 +2167,17 @@ void QDeclarativeGeoMap::geometryChanged(const QRectF &newGeometry, const QRectF
         setMinimumZoomLevel(m_map->minimumZoom(), false);
 
         // Update the center latitudinal threshold
-        const double maximumCenterLatitudeAtZoom = m_map->maximumCenterLatitudeAtZoom(m_cameraData);
-        const double minimumCenterLatitudeAtZoom = m_map->minimumCenterLatitudeAtZoom(m_cameraData);
+        QGeoCameraData cameraData = m_map->cameraData();
+        const double maximumCenterLatitudeAtZoom = m_map->maximumCenterLatitudeAtZoom(cameraData);
+        const double minimumCenterLatitudeAtZoom = m_map->minimumCenterLatitudeAtZoom(cameraData);
         if (maximumCenterLatitudeAtZoom != m_maximumViewportLatitude
                 || minimumCenterLatitudeAtZoom != m_minimumViewportLatitude) {
             m_maximumViewportLatitude = maximumCenterLatitudeAtZoom;
             m_minimumViewportLatitude = minimumCenterLatitudeAtZoom;
-            QGeoCoordinate coord = m_cameraData.center();
+            QGeoCoordinate coord = cameraData.center();
             coord.setLatitude(qBound(m_minimumViewportLatitude, coord.latitude(), m_maximumViewportLatitude));
-
-            if (coord != m_cameraData.center()) {
-                m_cameraData.setCenter(coord);
-                m_map->setCameraData(m_cameraData);
-                emit centerChanged(m_cameraData.center());
-            }
+            cameraData.setCenter(coord);
+            m_map->setCameraData(cameraData);
         }
     }
 
diff --git a/src/location/declarativemaps/qdeclarativegeomap_p.h b/src/location/declarativemaps/qdeclarativegeomap_p.h
index 02379411..f59f6f54 100644
--- a/src/location/declarativemaps/qdeclarativegeomap_p.h
+++ b/src/location/declarativemaps/qdeclarativegeomap_p.h
@@ -271,6 +271,7 @@ private Q_SLOTS:
     void onSupportedMapTypesChanged();
     void onCameraCapabilitiesChanged(const QGeoCameraCapabilities &oldCameraCapabilities);
     void onAttachedCopyrightNoticeVisibilityChanged();
+    void onCameraDataChanged(const QGeoCameraData &cameraData);
 
 private:
     void setupMapView(QDeclarativeGeoMapItemView *view);
diff --git a/src/location/maps/qgeomap.cpp b/src/location/maps/qgeomap.cpp
index db0df702..963ef6bd 100644
--- a/src/location/maps/qgeomap.cpp
+++ b/src/location/maps/qgeomap.cpp
@@ -146,7 +146,7 @@ QGeoShape QGeoMap::visibleRegion() const
     return geoProjection().visibleRegion();
 }
 
-QGeoCameraData QGeoMap::cameraData() const
+const QGeoCameraData &QGeoMap::cameraData() const
 {
     Q_D(const QGeoMap);
     return d->m_cameraData;
diff --git a/src/location/maps/qgeomap_p.h b/src/location/maps/qgeomap_p.h
index 0c396e16..f19f847a 100644
--- a/src/location/maps/qgeomap_p.h
+++ b/src/location/maps/qgeomap_p.h
@@ -108,7 +108,7 @@ public:
     int viewportHeight() const;
 
 
-    QGeoCameraData cameraData() const;
+    const QGeoCameraData &cameraData() const;
     QGeoCameraCapabilities cameraCapabilities() const;
     virtual Capabilities capabilities() const;
 
diff --git a/tests/auto/declarative_ui/tst_map.qml b/tests/auto/declarative_ui/tst_map.qml
index 2d35fce0..6051a48e 100644
--- a/tests/auto/declarative_ui/tst_map.qml
+++ b/tests/auto/declarative_ui/tst_map.qml
@@ -239,6 +239,7 @@ Item {
 
             compare(mapPar.mapParameters.length, 1)
 
+            // Using toCoordinate, below, to verify the actual value of the center, and not what is in the map.center property
             center = mapPar.toCoordinate(Qt.point((mapPar.width - 1) / 2.0, (mapPar.height - 1) / 2.0))
             fuzzyCompare(center.latitude, -33, 0.1)
             fuzzyCompare(center.longitude, -47, 0.1)
@@ -253,19 +254,21 @@ Item {
             fuzzyCompare(center.latitude, -33, 0.1)
             fuzzyCompare(center.longitude, -47, 0.1)
 
-            testParameter.center = mapPar.center  // map.center has not been affected as it lives in the Declarative Map
+            testParameter.center = mapPar.center  // map.center has been affected as the Declarative Map has received the QGeoMap::cameraDataChanged signal
             mapPar.addMapParameter(testParameter)
             compare(mapPar.mapParameters.length, 1)
 
             center = mapPar.toCoordinate(Qt.point((mapPar.width - 1) / 2.0, (mapPar.height - 1) / 2.0))
-            fuzzyCompare(center.latitude, 10, 0.1)
-            fuzzyCompare(center.longitude, 11, 0.1)
+            fuzzyCompare(center.latitude, -33, 0.1)
+            fuzzyCompare(center.longitude, -47, 0.1)
 
-            testParameter.center = QtPositioning.coordinate(-33.0, -47.0)
+            testParameter.center = QtPositioning.coordinate(-30.0, -40.0)
 
             center = mapPar.toCoordinate(Qt.point((mapPar.width - 1) / 2.0, (mapPar.height - 1) / 2.0))
-            fuzzyCompare(center.latitude, -33, 0.1)
-            fuzzyCompare(center.longitude, -47, 0.1)
+            fuzzyCompare(center.latitude, -30, 0.1)
+            fuzzyCompare(center.longitude, -40, 0.1)
+            fuzzyCompare(mapPar.center.latitude, -30, 0.1)
+            fuzzyCompare(mapPar.center.longitude, -40, 0.1)
 
             mapPar.removeMapParameter(testParameter)
             compare(mapPar.mapParameters.length, 0)
diff --git a/tests/auto/geotestplugin/qgeotiledmap_test.cpp b/tests/auto/geotestplugin/qgeotiledmap_test.cpp
index 62abb313..c3d466ea 100644
--- a/tests/auto/geotestplugin/qgeotiledmap_test.cpp
+++ b/tests/auto/geotestplugin/qgeotiledmap_test.cpp
@@ -98,9 +98,8 @@ QGeoTiledMapTest::QGeoTiledMapTest(QGeoTiledMappingManagerEngine *engine,
 
 void QGeoTiledMapTest::onCameraCenter_testChanged(QGeoMapParameter *param, const char *propertyName)
 {
-    Q_D(QGeoTiledMapTest);
     if (strcmp(propertyName, "center") == 0) {
-        QGeoCameraData cameraData = d->m_cameraData;
+        QGeoCameraData cameraData = this->cameraData();
         // Not testing for propertyName as this param has only one allowed property
         QGeoCoordinate newCenter = param->property(propertyName).value<QGeoCoordinate>();
         cameraData.setCenter(newCenter);