From 2b6020a59ef503eeadabc532782f5e9e795fcf3f Mon Sep 17 00:00:00 2001 From: Paolo Angelelli Date: Mon, 11 Dec 2017 14:57:11 +0100 Subject: 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 --- .../declarativemaps/qdeclarativegeomap.cpp | 57 +++++++++++------- .../declarativemaps/qdeclarativegeomap_p.h | 5 ++ .../qdeclarativegeomapcopyrightsnotice.cpp | 69 ++++++++++++++++------ .../qdeclarativegeomapcopyrightsnotice_p.h | 10 +++- src/location/maps/qgeomap.cpp | 19 ++++++ src/location/maps/qgeomap_p.h | 2 + src/location/maps/qgeomap_p_p.h | 4 ++ src/location/maps/qgeotiledmap.cpp | 16 ++++- src/location/maps/qgeotiledmap_p.h | 2 + 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 types = m_mappingManager->supportedMapTypes(); @@ -775,40 +780,26 @@ void QDeclarativeGeoMap::mappingManagerInitialized() QOverload::of(&QGeoMap::copyrightsChanged), [©rightImage](const QImage ©){ 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(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 #include #include +#include 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 ©rightsImage) { + Q_D(QDeclarativeGeoMapCopyrightNotice); delete m_copyrightsHtml; m_copyrightsHtml = 0; @@ -286,20 +318,19 @@ void QDeclarativeGeoMapCopyrightNotice::copyrightsChanged(const QImage ©righ setKeepMouseGrab(false); setAcceptedMouseButtons(Qt::NoButton); - setVisible(m_copyrightsVisible); + d->QQuickItemPrivate::setVisible(m_copyrightsVisible && !m_copyrightsImage.isNull()); update(); } void QDeclarativeGeoMapCopyrightNotice::copyrightsChanged(const QString ©rightsHtml) { + 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 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 #include +#include #include 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 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 m_mapParameters; QList 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); -- cgit v1.2.1