Fix performance issue with copyright notice

Skip evaluating copyrights when the notice is not visible.
The implication of this patch is that copyright information
won't be up to date or even available unless there is an
"attached" copyright notice that has the visible property
set to true.

Task-number: QTBUG-64880
Change-Id: I3750b61913becb0cbf31273ad9a76ae1a2b6a393
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

9 files changed, 141 insertions, 43 deletions

diff --git a/src/location/declarativemaps/qdeclarativegeomap.cpp b/src/location/declarativemaps/qdeclarativegeomap.cpp
index 52610ab6..2ee6836b 100644
--- a/src/location/declarativemaps/qdeclarativegeomap.cpp
+++ b/src/location/declarativemaps/qdeclarativegeomap.cpp
@@ -239,7 +239,8 @@ QDeclarativeGeoMap::~QDeclarativeGeoMap()
     }
     m_mapItemGroups.clear();
 
-    delete m_copyrights.data();
+    if (m_copyrights.data())
+        delete m_copyrights.data();
     m_copyrights.clear();
 
     delete m_map;
@@ -250,7 +251,7 @@ QDeclarativeGeoMap::~QDeclarativeGeoMap()
 */
 void QDeclarativeGeoMap::onMapChildrenChanged()
 {
-    if (!m_componentCompleted || !m_map)
+    if (!m_componentCompleted || !m_initialized)
         return;
 
     int maxChildZ = 0;
@@ -423,7 +424,7 @@ void QDeclarativeGeoMap::initialize()
 
     emit mapReadyChanged(true);
 
-    if (m_copyrights)
+    if (m_copyrights) // To not update during initialize()
          update();
 }
 
@@ -720,7 +721,6 @@ void QDeclarativeGeoMap::onCameraCapabilitiesChanged(const QGeoCameraCapabilitie
     }
 }
 
-
 /*!
     \internal
     this function will only be ever called once
@@ -732,6 +732,11 @@ void QDeclarativeGeoMap::mappingManagerInitialized()
     if (!m_map)
         return;
 
+    /* COPY NOTICE SETUP */
+    m_copyrights = new QDeclarativeGeoMapCopyrightNotice(this);
+    m_copyrights->setCopyrightsVisible(m_copyrightsVisible);
+    m_copyrights->setMapSource(this);
+
     m_gestureArea->setMap(m_map);
 
     QList<QGeoMapType> types = m_mappingManager->supportedMapTypes();
@@ -775,40 +780,26 @@ void QDeclarativeGeoMap::mappingManagerInitialized()
                         QOverload<const QImage &>::of(&QGeoMap::copyrightsChanged),
                         [&copyrightImage](const QImage &copy){ copyrightImage = copy; });
         m_map->setViewportSize(QSize(width(), height()));
-        initialize();
+        initialize(); // This emits the caught signals above
         QObject::disconnect(copyrightStringCatcherConnection);
         QObject::disconnect(copyrightImageCatcherConnection);
     }
 
-    m_copyrights = new QDeclarativeGeoMapCopyrightNotice(this);
-    m_copyrights->onCopyrightsStyleSheetChanged(m_map->copyrightsStyleSheet());
 
-    connect(m_map, SIGNAL(copyrightsChanged(QImage)),
-            m_copyrights.data(), SLOT(copyrightsChanged(QImage)));
+    /* COPYRIGHT SIGNALS REWIRING */
     connect(m_map, SIGNAL(copyrightsChanged(QImage)),
             this,  SIGNAL(copyrightsChanged(QImage)));
-
-    connect(m_map, SIGNAL(copyrightsChanged(QString)),
-            m_copyrights.data(), SLOT(copyrightsChanged(QString)));
     connect(m_map, SIGNAL(copyrightsChanged(QString)),
             this,  SIGNAL(copyrightsChanged(QString)));
-
     if (!copyrightString.isEmpty())
         emit m_map->copyrightsChanged(copyrightString);
     else if (!copyrightImage.isNull())
         emit m_map->copyrightsChanged(copyrightImage);
 
-    connect(m_map, SIGNAL(copyrightsStyleSheetChanged(QString)),
-            m_copyrights.data(), SLOT(onCopyrightsStyleSheetChanged(QString)));
 
-    connect(m_copyrights.data(), SIGNAL(linkActivated(QString)),
-            this, SIGNAL(copyrightLinkActivated(QString)));
     connect(m_map, &QGeoMap::sgNodeChanged, this, &QQuickItem::update);
     connect(m_map, &QGeoMap::cameraCapabilitiesChanged, this, &QDeclarativeGeoMap::onCameraCapabilitiesChanged);
 
-    // set visibility of copyright notice
-    m_copyrights->setCopyrightsVisible(m_copyrightsVisible);
-
     // This prefetches a buffer around the map
     m_map->prefetchData();
 
@@ -1604,6 +1595,32 @@ bool QDeclarativeGeoMap::isInteractive()
     return (m_gestureArea->enabled() && m_gestureArea->acceptedGestures()) || m_gestureArea->isActive();
 }
 
+void QDeclarativeGeoMap::attachCopyrightNotice(bool initialVisibility)
+{
+    if (initialVisibility) {
+        ++m_copyNoticesVisible;
+        if (m_map)
+            m_map->setCopyrightVisible(m_copyNoticesVisible > 0);
+    }
+}
+
+void QDeclarativeGeoMap::detachCopyrightNotice(bool currentVisibility)
+{
+    if (currentVisibility) {
+        --m_copyNoticesVisible;
+        if (m_map)
+            m_map->setCopyrightVisible(m_copyNoticesVisible > 0);
+    }
+}
+
+void QDeclarativeGeoMap::onAttachedCopyrightNoticeVisibilityChanged()
+{
+    QDeclarativeGeoMapCopyrightNotice *copy = static_cast<QDeclarativeGeoMapCopyrightNotice *>(sender());
+    m_copyNoticesVisible += ( int(copy->copyrightsVisible()) * 2 - 1);
+    if (m_map)
+        m_map->setCopyrightVisible(m_copyNoticesVisible > 0);
+}
+
 /*!
     \internal
 */
diff --git a/src/location/declarativemaps/qdeclarativegeomap_p.h b/src/location/declarativemaps/qdeclarativegeomap_p.h
index e8ff3265..6b765269 100644
--- a/src/location/declarativemaps/qdeclarativegeomap_p.h
+++ b/src/location/declarativemaps/qdeclarativegeomap_p.h
@@ -236,6 +236,7 @@ private Q_SLOTS:
     void onMapChildrenChanged();
     void onSupportedMapTypesChanged();
     void onCameraCapabilitiesChanged(const QGeoCameraCapabilities &oldCameraCapabilities);
+    void onAttachedCopyrightNoticeVisibilityChanged();
 
 private:
     void setupMapView(QDeclarativeGeoMapItemView *view);
@@ -244,6 +245,8 @@ private:
     void fitViewportToMapItemsRefine(bool refine, bool onlyVisible);
     void fitViewportToGeoShape();
     bool isInteractive();
+    void attachCopyrightNotice(bool initialVisibility);
+    void detachCopyrightNotice(bool currentVisibility);
 
 private:
     QDeclarativeGeoServiceProvider *m_plugin;
@@ -281,6 +284,8 @@ private:
     qreal m_userMinimumFieldOfView;
     qreal m_userMaximumFieldOfView;
 
+    int m_copyNoticesVisible = 0;
+
     friend class QDeclarativeGeoMapItem;
     friend class QDeclarativeGeoMapItemView;
     friend class QQuickGeoMapGestureArea;
diff --git a/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice.cpp b/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice.cpp
index 1a8489eb..9edcafed 100644
--- a/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice.cpp
+++ b/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice.cpp
@@ -43,9 +43,17 @@
 #include <QtQuick/private/qquickanchors_p.h>
 #include <QtQuick/private/qquickanchors_p_p.h>
 #include <QtLocation/private/qdeclarativegeomap_p.h>
+#include <QtQuick/private/qquickpainteditem_p.h>
 
 QT_BEGIN_NAMESPACE
 
+class QDeclarativeGeoMapCopyrightNoticePrivate: public QQuickPaintedItemPrivate
+{
+    Q_DECLARE_PUBLIC(QDeclarativeGeoMapCopyrightNotice)
+public:
+    virtual void setVisible(bool visible);
+};
+
 /*!
     \qmltype MapCopyrightNotice
     \instantiates QDeclarativeGeoMapCopyrightNotice
@@ -99,6 +107,7 @@ QDeclarativeGeoMapCopyrightNotice::QDeclarativeGeoMapCopyrightNotice(QQuickItem
 
 QDeclarativeGeoMapCopyrightNotice::~QDeclarativeGeoMapCopyrightNotice()
 {
+    setMapSource(nullptr);
 }
 
 void QDeclarativeGeoMapCopyrightNotice::anchorToBottomLeft()
@@ -112,35 +121,41 @@ void QDeclarativeGeoMapCopyrightNotice::anchorToBottomLeft()
     }
 }
 
-void QDeclarativeGeoMapCopyrightNotice::setMapSource(QDeclarativeGeoMap *mapSource)
+void QDeclarativeGeoMapCopyrightNotice::setMapSource(QDeclarativeGeoMap *map)
 {
-    if (m_mapSource == mapSource)
+    if (m_mapSource == map)
         return;
 
     if (m_mapSource) {
         // disconnect this object from current map source
+        m_mapSource->detachCopyrightNotice(copyrightsVisible());
         m_mapSource->disconnect(this);
         m_mapSource->m_map->disconnect(this);
-        m_copyrightsHtml->clear();
+        if (m_copyrightsHtml)
+            m_copyrightsHtml->clear();
         m_copyrightsImage = QImage();
         m_mapSource = Q_NULLPTR;
     }
 
-    if (mapSource) {
-        m_mapSource = mapSource;
+    if (map) {
+        m_mapSource = map;
+        m_mapSource->attachCopyrightNotice(copyrightsVisible());
+        connect(this, &QDeclarativeGeoMapCopyrightNotice::copyrightsVisibleChanged,
+                mapSource(), &QDeclarativeGeoMap::onAttachedCopyrightNoticeVisibilityChanged);
+
         // First update the copyright. Only Image will do here, no need to store HTML right away.
-        if (mapSource->m_copyrights && !mapSource->m_copyrights->m_copyrightsImage.isNull())
-            m_copyrightsImage = mapSource->m_copyrights->m_copyrightsImage;
+        if (m_mapSource->m_copyrights && !m_mapSource->m_copyrights->m_copyrightsImage.isNull())
+            m_copyrightsImage = m_mapSource->m_copyrights->m_copyrightsImage;
 
-        connect(m_mapSource, SIGNAL(copyrightsChanged(QImage)),
+        connect(mapSource(), SIGNAL(copyrightsChanged(QImage)),
                 this, SLOT(copyrightsChanged(QImage)));
-        connect(m_mapSource, SIGNAL(copyrightsChanged(QString)),
+        connect(mapSource(), SIGNAL(copyrightsChanged(QString)),
                 this, SLOT(copyrightsChanged(QString)));
 
         if (m_mapSource->m_map)
             connectMap();
         else
-            connect(m_mapSource, &QDeclarativeGeoMap::mapReadyChanged, this, &QDeclarativeGeoMapCopyrightNotice::connectMap);
+            connect(mapSource(), &QDeclarativeGeoMap::mapReadyChanged, this, &QDeclarativeGeoMapCopyrightNotice::connectMap);
     }
 }
 
@@ -149,7 +164,7 @@ void QDeclarativeGeoMapCopyrightNotice::connectMap()
     connect(m_mapSource->m_map, SIGNAL(copyrightsStyleSheetChanged(QString)),
             this, SLOT(onCopyrightsStyleSheetChanged(QString)));
     connect(this, SIGNAL(linkActivated(QString)),
-            m_mapSource, SIGNAL(copyrightLinkActivated(QString)));
+            mapSource(), SIGNAL(copyrightLinkActivated(QString)));
 
     onCopyrightsStyleSheetChanged(m_mapSource->m_map->copyrightsStyleSheet());
 
@@ -159,7 +174,7 @@ void QDeclarativeGeoMapCopyrightNotice::connectMap()
 
 QDeclarativeGeoMap *QDeclarativeGeoMapCopyrightNotice::mapSource()
 {
-    return m_mapSource;
+    return m_mapSource.data();
 }
 
 QString QDeclarativeGeoMapCopyrightNotice::styleSheet() const
@@ -253,14 +268,30 @@ void QDeclarativeGeoMapCopyrightNotice::createCopyright()
     m_copyrightsHtml->setDocumentMargin(0);
 }
 
+void QDeclarativeGeoMapCopyrightNoticePrivate::setVisible(bool visible)
+{
+    Q_Q(QDeclarativeGeoMapCopyrightNotice);
+    q->m_copyrightsVisible = visible;
+    QQuickItemPrivate::setVisible(visible);
+}
+
 /*!
     \internal
 */
 void QDeclarativeGeoMapCopyrightNotice::setCopyrightsVisible(bool visible)
 {
+    Q_D(QDeclarativeGeoMapCopyrightNotice);
+    if (visible == m_copyrightsVisible)
+        return;
+
     m_copyrightsVisible = visible;
+    d->QQuickItemPrivate::setVisible(!m_copyrightsImage.isNull() && visible);
+    emit copyrightsVisibleChanged();
+}
 
-    setVisible(!m_copyrightsImage.isNull() && visible);
+bool QDeclarativeGeoMapCopyrightNotice::copyrightsVisible() const
+{
+    return m_copyrightsVisible;
 }
 
 /*!
@@ -277,6 +308,7 @@ void QDeclarativeGeoMapCopyrightNotice::setCopyrightsZ(int copyrightsZ)
 */
 void QDeclarativeGeoMapCopyrightNotice::copyrightsChanged(const QImage &copyrightsImage)
 {
+    Q_D(QDeclarativeGeoMapCopyrightNotice);
     delete m_copyrightsHtml;
     m_copyrightsHtml = 0;
 
@@ -286,20 +318,19 @@ void QDeclarativeGeoMapCopyrightNotice::copyrightsChanged(const QImage &copyrigh
 
     setKeepMouseGrab(false);
     setAcceptedMouseButtons(Qt::NoButton);
-    setVisible(m_copyrightsVisible);
+    d->QQuickItemPrivate::setVisible(m_copyrightsVisible && !m_copyrightsImage.isNull());
 
     update();
 }
 
 void QDeclarativeGeoMapCopyrightNotice::copyrightsChanged(const QString &copyrightsHtml)
 {
+    Q_D(QDeclarativeGeoMapCopyrightNotice);
     if (copyrightsHtml.isEmpty()) {
-        setVisible(false);
+        d->QQuickItemPrivate::setVisible(false);
         return;
-    } else if (!m_copyrightsVisible) {
-        setVisible(false);
-    } else {
-        setVisible(true);
+    } else  {
+        d->QQuickItemPrivate::setVisible(m_copyrightsVisible);
     }
 
     // Divfy, so we can style the background. The extra <span> is a
diff --git a/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice_p.h b/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice_p.h
index 0d7f7a20..9b28299f 100644
--- a/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice_p.h
+++ b/src/location/declarativemaps/qdeclarativegeomapcopyrightsnotice_p.h
@@ -52,13 +52,14 @@
 #include <QtLocation/private/qlocationglobal_p.h>
 
 #include <QtGui/QImage>
+#include <QPointer>
 #include <QtQuick/QQuickPaintedItem>
 
 QT_BEGIN_NAMESPACE
 
 class QTextDocument;
 class QDeclarativeGeoMap;
-
+class QDeclarativeGeoMapCopyrightNoticePrivate;
 class Q_LOCATION_PRIVATE_EXPORT QDeclarativeGeoMapCopyrightNotice : public QQuickPaintedItem
 {
     Q_OBJECT
@@ -72,6 +73,7 @@ public:
     void setCopyrightsZ(int copyrightsZ);
 
     void setCopyrightsVisible(bool visible);
+    bool copyrightsVisible() const;
     void anchorToBottomLeft();
 
     void setMapSource(QDeclarativeGeoMap *mapSource);
@@ -90,6 +92,7 @@ signals:
     void mapSourceChanged();
     void backgroundColorChanged(const QColor &color);
     void styleSheetChanged(const QString &styleSheet);
+    void copyrightsVisibleChanged();
 
 protected:
     void paint(QPainter *painter) Q_DECL_OVERRIDE;
@@ -106,10 +109,13 @@ private:
     QImage m_copyrightsImage;
     QString m_activeAnchor;
     bool m_copyrightsVisible;
-    QDeclarativeGeoMap *m_mapSource;
+    QPointer<QDeclarativeGeoMap> m_mapSource;
     QColor m_backgroundColor;
     QString m_styleSheet;
     bool m_userDefinedStyleSheet;
+
+    Q_DISABLE_COPY(QDeclarativeGeoMapCopyrightNotice)
+    Q_DECLARE_PRIVATE(QDeclarativeGeoMapCopyrightNotice)
 };
 
 QT_END_NAMESPACE
diff --git a/src/location/maps/qgeomap.cpp b/src/location/maps/qgeomap.cpp
index 0c6ce0a7..46a88003 100644
--- a/src/location/maps/qgeomap.cpp
+++ b/src/location/maps/qgeomap.cpp
@@ -230,6 +230,15 @@ QString QGeoMap::copyrightsStyleSheet() const
     return QStringLiteral("#copyright-root { background: rgba(255, 255, 255, 128) }");
 }
 
+void QGeoMap::setCopyrightVisible(bool visible)
+{
+    Q_D(QGeoMap);
+    if (d->m_copyrightVisible == visible)
+        return;
+
+    d->m_copyrightVisible = visible;
+}
+
 QGeoMapPrivate::QGeoMapPrivate(QGeoMappingManagerEngine *engine, QGeoProjection *geoProjection)
     : QObjectPrivate(),
       m_geoProjection(geoProjection),
@@ -287,4 +296,14 @@ void QGeoMapPrivate::removeMapItem(QDeclarativeGeoMapItemBase *item)
     Q_UNUSED(item)
 }
 
+void QGeoMapPrivate::setCopyrightVisible(bool visible)
+{
+    m_copyrightVisible = visible;
+}
+
+bool QGeoMapPrivate::copyrightVisible() const
+{
+    return m_copyrightVisible;
+}
+
 QT_END_NAMESPACE
diff --git a/src/location/maps/qgeomap_p.h b/src/location/maps/qgeomap_p.h
index bb7ade55..f465be1d 100644
--- a/src/location/maps/qgeomap_p.h
+++ b/src/location/maps/qgeomap_p.h
@@ -124,6 +124,8 @@ public:
 
     virtual QString copyrightsStyleSheet() const;
 
+    virtual void setCopyrightVisible(bool visible);
+
 protected:
     QGeoMap(QGeoMapPrivate &dd, QObject *parent = 0);
     void setCameraData(const QGeoCameraData &cameraData);
diff --git a/src/location/maps/qgeomap_p_p.h b/src/location/maps/qgeomap_p_p.h
index ec498484..2c366610 100644
--- a/src/location/maps/qgeomap_p_p.h
+++ b/src/location/maps/qgeomap_p_p.h
@@ -90,6 +90,9 @@ protected:
     virtual void changeCameraData(const QGeoCameraData &oldCameraData) = 0; // called by QGeoMap::setCameraData()
     virtual void changeActiveMapType(const QGeoMapType mapType) = 0; // called by QGeoMap::setActiveMapType()
 
+    virtual void setCopyrightVisible(bool visible);
+    virtual bool copyrightVisible() const;
+
 protected:
     QSize m_viewportSize;
     QGeoProjection *m_geoProjection;
@@ -99,6 +102,7 @@ protected:
     QList<QGeoMapParameter *> m_mapParameters;
     QList<QDeclarativeGeoMapItemBase *> m_mapItems;
     QGeoCameraCapabilities m_cameraCapabilities;
+    bool m_copyrightVisible = true;
 };
 
 QT_END_NAMESPACE
diff --git a/src/location/maps/qgeotiledmap.cpp b/src/location/maps/qgeotiledmap.cpp
index 0eeb189d..815a8b99 100644
--- a/src/location/maps/qgeotiledmap.cpp
+++ b/src/location/maps/qgeotiledmap.cpp
@@ -143,6 +143,17 @@ void QGeoTiledMap::clearData()
     d->m_mapScene->clearTexturedTiles();
 }
 
+void QGeoTiledMap::setCopyrightVisible(bool visible)
+{
+    Q_D(QGeoTiledMap);
+    if (visible == d->m_copyrightVisible)
+        return;
+
+    QGeoMap::setCopyrightVisible(visible);
+    if (visible)
+        evaluateCopyrights(d->m_mapScene->visibleTiles());
+}
+
 void QGeoTiledMap::clearScene(int mapId)
 {
     Q_D(QGeoTiledMap);
@@ -319,7 +330,7 @@ void QGeoTiledMapPrivate::updateScene()
     bool newTilesIntroduced = !m_mapScene->visibleTiles().contains(tiles);
     m_mapScene->setVisibleTiles(tiles);
 
-    if (newTilesIntroduced)
+    if (newTilesIntroduced && m_copyrightVisible)
         q->evaluateCopyrights(tiles);
 
     // don't request tiles that are already built and textured
@@ -381,7 +392,8 @@ void QGeoTiledMapPrivate::changeViewportSize(const QSize& size)
         m_cache->setMinTextureUsage(newSize);
     }
 
-    q->evaluateCopyrights(m_visibleTiles->createTiles());
+    if (m_copyrightVisible)
+        q->evaluateCopyrights(m_mapScene->visibleTiles());
     updateScene();
 }
 
diff --git a/src/location/maps/qgeotiledmap_p.h b/src/location/maps/qgeotiledmap_p.h
index b709cb57..89dd1285 100644
--- a/src/location/maps/qgeotiledmap_p.h
+++ b/src/location/maps/qgeotiledmap_p.h
@@ -87,6 +87,8 @@ public:
     void prefetchData() Q_DECL_OVERRIDE;
     void clearData() Q_DECL_OVERRIDE;
 
+    void setCopyrightVisible(bool visible) override;
+
 public Q_SLOTS:
     virtual void clearScene(int mapId);