Fixing problems and a bug, review, cleanup

 Reviewed by Jussi Laitinen

 - fixed bug: auto centering was not disabled when centered
   to own or friend item location

 - MapScroller does emit coordinateUpdated() only if the
   animation is in the running state

 - decreased the auto centering disabling distance

diff --git a/src/map/mapcommon.h b/src/map/mapcommon.h
index 86966da..374c850 100644 (file)
--- a/src/map/mapcommon.h
+++ b/src/map/mapcommon.h
@@ -104,7 +104,7 @@ const int GRID_PADDING = 0;  ///< Grid padding used in tile grid calculation
 
 const QString OSM_LICENSE = QString::fromUtf8("© OpenStreetMap contributors, CC-BY-SA");
 
-const int AUTO_CENTERING_DISABLE_DISTANCE = 200; ///< Distance in pixels
+const int AUTO_CENTERING_DISABLE_DISTANCE = 100; ///< Distance in pixels
 
 //String constants for storing map settings:
 const QString MAP_LAST_ZOOMLEVEL = "LAST_MAP_ZOOM_LEVEL";   ///< Maps last zoom level before logout

diff --git a/src/map/mapengine.cpp b/src/map/mapengine.cpp
index 6df3d06..1125fb8 100644 (file)
--- a/src/map/mapengine.cpp
+++ b/src/map/mapengine.cpp
@@ -50,10 +50,12 @@ const int SMOOTH_CENTERING_TIME_MS = 1000;
 MapEngine::MapEngine(QObject *parent)
     : QObject(parent),
       m_autoCenteringEnabled(false),
+      m_scrollStartedByGps(false),
+      m_smoothScrollRunning(false),
       m_zoomedIn(false),
       m_zoomLevel(DEFAULT_ZOOM_LEVEL),
       m_centerTile(QPoint(UNDEFINED, UNDEFINED)),
-      m_lastManualPosition(QPoint(0, 0)),
+      m_lastAutomaticPosition(QPoint(0, 0)),
       m_tilesGridSize(QSize(0, 0)),
       m_viewSize(QSize(DEFAULT_SCREEN_WIDTH, DEFAULT_SCREEN_HEIGHT))
 {
@@ -93,6 +95,9 @@ MapEngine::MapEngine(QObject *parent)
 
     connect(m_scroller, SIGNAL(coordinateUpdated(QPoint)),
             this, SLOT(setCenterPosition(QPoint)));
+
+    connect(m_scroller, SIGNAL(stateChanged(QAbstractAnimation::State, QAbstractAnimation::State)),
+            this, SLOT(scrollerStateChanged(QAbstractAnimation::State)));
 }
 
 MapEngine::~MapEngine()
@@ -129,11 +134,7 @@ void MapEngine::centerToCoordinates(QPointF latLonCoordinate)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
-    QPoint scenePosition = convertLatLonToSceneCoordinate(latLonCoordinate);
-
-    m_lastManualPosition = scenePosition;
-
-    scrollToPosition(scenePosition);
+    scrollToPosition(convertLatLonToSceneCoordinate(latLonCoordinate));
 }
 
 QPoint MapEngine::convertLatLonToSceneCoordinate(QPointF latLonCoordinate)
@@ -196,23 +197,23 @@ QPoint MapEngine::convertTileNumberToSceneCoordinate(int zoomLevel, QPoint tileN
     return QPoint(x, y);
 }
 
-bool MapEngine::disableAutoCentering(QPoint sceneCoordinate)
+void MapEngine::disableAutoCenteringIfRequired(QPoint sceneCoordinate)
 {
     if (isAutoCenteringEnabled()) {
         int zoomFactor = (1 << (MAX_MAP_ZOOM_LEVEL - m_zoomLevel));
 
-        QPoint oldPixelValue = QPoint(m_lastManualPosition.x() / zoomFactor,
-                                      m_lastManualPosition.y() / zoomFactor);
+        QPoint oldPixelValue = QPoint(m_lastAutomaticPosition.x() / zoomFactor,
+                                      m_lastAutomaticPosition.y() / zoomFactor);
 
         QPoint newPixelValue = QPoint(sceneCoordinate.x() / zoomFactor,
                                       sceneCoordinate.y() / zoomFactor);
 
-        if ((abs(oldPixelValue.x() - newPixelValue.x()) > AUTO_CENTERING_DISABLE_DISTANCE) ||
-            (abs(oldPixelValue.y() - newPixelValue.y()) > AUTO_CENTERING_DISABLE_DISTANCE))
-            return true;
-    }
+        if ((abs(oldPixelValue.x() - newPixelValue.x()) > AUTO_CENTERING_DISABLE_DISTANCE)
+            || (abs(oldPixelValue.y() - newPixelValue.y()) > AUTO_CENTERING_DISABLE_DISTANCE)) {
 
-    return false;
+            emit mapScrolledManually();
+        }
+    }
 }
 
 void MapEngine::friendsPositionsUpdated()
@@ -256,8 +257,11 @@ void MapEngine::gpsPositionUpdate(QPointF position, qreal accuracy)
     m_gpsLocationItem->updatePosition(convertLatLonToSceneCoordinate(position), accuracy);
     m_mapScene->spanItems(m_zoomLevel, m_sceneCoordinate, m_viewSize);
 
-    if (m_autoCenteringEnabled)
-        centerToCoordinates(position);
+    if (m_autoCenteringEnabled) {
+        m_lastAutomaticPosition = convertLatLonToSceneCoordinate(position);
+        m_scrollStartedByGps = true;
+        scrollToPosition(m_lastAutomaticPosition);
+    }
 }
 
 qreal MapEngine::greatCircleDistance(QPointF firstLocation, QPointF secondLocation)
@@ -393,24 +397,40 @@ QGraphicsScene* MapEngine::scene()
     return m_mapScene;
 }
 
+void MapEngine::scrollerStateChanged(QAbstractAnimation::State newState)
+{
+    qDebug() << __PRETTY_FUNCTION__;
+
+    if (m_smoothScrollRunning
+        && newState != QAbstractAnimation::Running) {
+            m_smoothScrollRunning = false;
+
+            // don't disable auto centering if current animation was stopped by new update from GPS
+            if (!m_scrollStartedByGps)
+                disableAutoCenteringIfRequired(m_sceneCoordinate);
+    }
+
+    m_scrollStartedByGps = false;
+}
+
 void MapEngine::scrollToPosition(QPoint scenePosition)
 {
+    qDebug() << __PRETTY_FUNCTION__;
+
     m_scroller->stop();
     m_scroller->setEasingCurve(QEasingCurve::InOutQuart);
     m_scroller->setDuration(SMOOTH_CENTERING_TIME_MS);
     m_scroller->setStartValue(m_sceneCoordinate);
     m_scroller->setEndValue(scenePosition);
+    m_smoothScrollRunning = true;
     m_scroller->start();
 }
 
 void MapEngine::setAutoCentering(bool enabled)
 {
-    m_autoCenteringEnabled = enabled;
-}
+    qDebug() << __PRETTY_FUNCTION__;
 
-void MapEngine::setGPSEnabled(bool enabled)
-{
-    m_gpsLocationItem->setEnabled(enabled);
+    m_autoCenteringEnabled = enabled;
 }
 
 void MapEngine::setCenterPosition(QPoint scenePosition)
@@ -423,8 +443,8 @@ void MapEngine::setCenterPosition(QPoint scenePosition)
     // don't allow vertical scene coordinates go out of the map
     scenePosition.setY(qBound(MAP_MIN_PIXEL_Y, scenePosition.y(), MAP_MAX_PIXEL_Y));
 
-    if (disableAutoCentering(scenePosition))
-        emit mapScrolledManually();
+    if (!m_smoothScrollRunning)
+        disableAutoCenteringIfRequired(scenePosition);
 
     m_sceneCoordinate = scenePosition;
     emit locationChanged(m_sceneCoordinate);
@@ -438,6 +458,13 @@ void MapEngine::setCenterPosition(QPoint scenePosition)
     emit newMapResolution(sceneResolution());
 }
 
+void MapEngine::setGPSEnabled(bool enabled)
+{
+    qDebug() << __PRETTY_FUNCTION__;
+
+    m_gpsLocationItem->setEnabled(enabled);
+}
+
 void MapEngine::setZoomLevel(int newZoomLevel)
 {
     qDebug() << __PRETTY_FUNCTION__;

diff --git a/src/map/mapengine.h b/src/map/mapengine.h
index ba505f0..70f4d93 100644 (file)
--- a/src/map/mapengine.h
+++ b/src/map/mapengine.h
@@ -236,12 +236,11 @@ private:
     QRect calculateTileGrid(QPoint sceneCoordinate);
 
     /**
-     * @brief Check if auto centering should be disabled.
+     * @brief Request disabling of auto centering if centered too far from the real location.
      *
      * @param sceneCoordinate scene's center coordinate
-     * @return bool true if auto centering should be disabled
      */
-    bool disableAutoCentering(QPoint sceneCoordinate);
+    void disableAutoCenteringIfRequired(QPoint sceneCoordinate);
 
     /**
      * @brief Get new tiles.
@@ -331,6 +330,16 @@ private slots:
     void mapImageReceived(int zoomLevel, int x, int y, const QPixmap &image);
 
     /**
+    * @brief Called when MapScroller state is changed
+    *
+    * Does check if the smooth scroll effect was interrupted and should the auto centering
+    * feature to be disabled.
+    *
+    * @param newState New state
+    */
+    void scrollerStateChanged(QAbstractAnimation::State newState);
+
+    /**
     * @brief Scroll smoothly to given scene position
     *
     * @param scenePosition Target position in the scene
@@ -434,19 +443,21 @@ signals:
  * DATA MEMBERS
  ******************************************************************************/
 private:
-    bool m_autoCenteringEnabled;   ///< Auto centering enabled
-    bool m_zoomedIn;               ///< Flag for checking if zoomed in when zoom is finished
+    bool m_autoCenteringEnabled;    ///< Auto centering enabled
+    bool m_scrollStartedByGps;      ///< Smooth scroll is started by GPS?
+    bool m_smoothScrollRunning;     ///< Smooth scroll is running?
+    bool m_zoomedIn;                ///< Flag for checking if zoomed in when zoom is finished
 
-    int m_zoomLevel;               ///< Current zoom level
+    int m_zoomLevel;                ///< Current zoom level
 
-    QPoint m_centerTile;           ///< Current center tile
-    QPoint m_lastManualPosition;   ///< Last manually set position in scene coordinate
-    QPoint m_sceneCoordinate;      ///< Current center coordinate
+    QPoint m_centerTile;            ///< Current center tile
+    QPoint m_lastAutomaticPosition; ///< Last automatically set position in scene coordinate
+    QPoint m_sceneCoordinate;       ///< Current center coordinate
 
-    QRect m_viewTilesGrid;         ///< Current grid of tiles in view (includes margin)
+    QRect m_viewTilesGrid;          ///< Current grid of tiles in view (includes margin)
 
-    QSize m_tilesGridSize;         ///< Current size of the tiles grid
-    QSize m_viewSize;              ///< Current view size
+    QSize m_tilesGridSize;          ///< Current size of the tiles grid
+    QSize m_viewSize;               ///< Current view size
 
     FriendItemsHandler *m_friendItemsHandler;   ///< Handler for friend and group items
     GPSLocationItem *m_gpsLocationItem;         ///< Item pointing current location from GPS

diff --git a/src/map/mapscroller.cpp b/src/map/mapscroller.cpp
index d458c91..8b8f554 100644 (file)
--- a/src/map/mapscroller.cpp
+++ b/src/map/mapscroller.cpp
@@ -38,5 +38,6 @@ void MapScroller::updateCurrentValue(const QVariant &value)
 
     Q_ASSERT(value.type() == QVariant::Point);
 
-    emit coordinateUpdated(value.toPoint());
+    if (state() == QAbstractAnimation::Running)
+        emit coordinateUpdated(value.toPoint());
 }

diff --git a/src/map/mapscroller.h b/src/map/mapscroller.h
index 6f44401..1bb1ec6 100644 (file)
--- a/src/map/mapscroller.h
+++ b/src/map/mapscroller.h
@@ -66,7 +66,8 @@ private:
     /**
     * @brief Reimplemented from QVariantAnimation::updateCurrentValue()
     *
-    * Called every time the animation's current value changes
+    * Called every time the animation's current value changes. Does emit coordinateUpdated() signal
+    * if animation is in the running state.
     *
     * @param value New value
     */