From 2bb07804f32e0c9cc7948a5cff0bcef81ae9d8c9 Mon Sep 17 00:00:00 2001 From: Paolo Angelelli Date: Tue, 23 Jul 2019 11:51:41 +0200 Subject: Fix slow Map.removeMapItem This removes connections in map items to react on map changes. A further improvement could be replacing item lists with QSets. This might however have implications with plugins which might expect ordered items. Change-Id: I52dbd64ed22762b1e2d51d1bc38f496346e7a664 Fixes: QTBUG-76950 Reviewed-by: Alex Blasche --- .../declarativemaps/qdeclarativegeomap.cpp | 39 ++++++++++++++++------ .../declarativemaps/qdeclarativegeomapitembase.cpp | 13 ++------ 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/location/declarativemaps/qdeclarativegeomap.cpp b/src/location/declarativemaps/qdeclarativegeomap.cpp index 106c4dae..831c2cf4 100644 --- a/src/location/declarativemaps/qdeclarativegeomap.cpp +++ b/src/location/declarativemaps/qdeclarativegeomap.cpp @@ -623,6 +623,15 @@ void QDeclarativeGeoMap::mappingManagerInitialized() if (!m_map) return; + // Any map items that were added before the plugin was ready + // need to have setMap called again + for (const QPointer &item : qAsConst(m_mapItems)) { + if (item) { + item->setMap(this, m_map); + m_map->addMapItem(item.data()); // m_map filters out what is not supported. + } + } + /* COPY NOTICE SETUP */ m_copyrights = new QDeclarativeGeoMapCopyrightNotice(this); m_copyrights->setCopyrightsZ(m_maxChildZ + 1); @@ -706,15 +715,6 @@ void QDeclarativeGeoMap::mappingManagerInitialized() emit supportedMapTypesChanged(); emit activeMapTypeChanged(); - // Any map items that were added before the plugin was ready - // need to have setMap called again - for (const QPointer &item : qAsConst(m_mapItems)) { - if (item) { - item->setMap(this, m_map); - m_map->addMapItem(item.data()); // m_map filters out what is not supported. - } - } - // Any map item groups that were added before the plugin was ready // DO NOT need to have setMap called again on their children map items // because they have been added to m_mapItems, which is processed right above. @@ -1423,6 +1423,14 @@ void QDeclarativeGeoMap::setVisibleArea(const QRectF &visibleArea) if (m_initialized) { m_map->setVisibleArea(visibleArea); + const QRectF newVisibleArea = QDeclarativeGeoMap::visibleArea(); + if (newVisibleArea != oldVisibleArea) { + // polish map items + for (const QPointer &i: qAsConst(m_mapItems)) { + if (i) + i->visibleAreaChanged(); + } + } } else { m_visibleArea = visibleArea; const QRectF newVisibleArea = QDeclarativeGeoMap::visibleArea(); @@ -1719,6 +1727,11 @@ void QDeclarativeGeoMap::onCameraDataChanged(const QGeoCameraData &cameraData) bool zoomHasChanged = cameraData.zoomLevel() != m_cameraData.zoomLevel(); m_cameraData = cameraData; + // polish map items + for (const QPointer &i: qAsConst(m_mapItems)) { + if (i) + i->baseCameraDataChanged(m_cameraData); // Consider optimizing this further, removing the contained duplicate if conditions. + } if (centerHasChanged) emit centerChanged(m_cameraData.center()); @@ -2200,7 +2213,13 @@ void QDeclarativeGeoMap::geometryChanged(const QRectF &newGeometry, const QRectF QGeoCoordinate coord = cameraData.center(); coord.setLatitude(qBound(m_minimumViewportLatitude, coord.latitude(), m_maximumViewportLatitude)); cameraData.setCenter(coord); - m_map->setCameraData(cameraData); + m_map->setCameraData(cameraData); // this polishes map items + } else if (oldGeometry.size() != newGeometry.size()) { + // polish map items + for (const QPointer &i: qAsConst(m_mapItems)) { + if (i) + i->polishAndUpdate(); + } } } diff --git a/src/location/declarativemaps/qdeclarativegeomapitembase.cpp b/src/location/declarativemaps/qdeclarativegeomapitembase.cpp index 523655c0..7efc773e 100644 --- a/src/location/declarativemaps/qdeclarativegeomapitembase.cpp +++ b/src/location/declarativemaps/qdeclarativegeomapitembase.cpp @@ -126,21 +126,14 @@ void QDeclarativeGeoMapItemBase::setMap(QDeclarativeGeoMap *quickMap, QGeoMap *m return; if (quickMap && quickMap_) return; // don't allow association to more than one map - if (quickMap_) - quickMap_->disconnect(this); - if (map_) - map_->disconnect(this); quickMap_ = quickMap; map_ = map; if (map_ && quickMap_) { - connect(map_, SIGNAL(cameraDataChanged(QGeoCameraData)), - this, SLOT(baseCameraDataChanged(QGeoCameraData))); - connect(map_, SIGNAL(visibleAreaChanged()), - this, SLOT(visibleAreaChanged())); - connect(quickMap, SIGNAL(heightChanged()), this, SLOT(polishAndUpdate())); - connect(quickMap, SIGNAL(widthChanged()), this, SLOT(polishAndUpdate())); + // For performance reasons we're not connecting map_'s and quickMap_'s signals to this. + // Rather, the handling of cameraDataChanged, visibleAreaChanged, heightChanged and widthChanged is done explicitly in QDeclarativeGeoMap by directly calling methods on the items. + // See QTBUG-76950 lastSize_ = QSizeF(quickMap_->width(), quickMap_->height()); lastCameraData_ = map_->cameraData(); } -- cgit v1.2.1