Differential D6884 Diff 17133 src/lib/marble/layers/GeometryLayer.cpp

Changeset View

Standalone View

View Options

src/lib/marble/layers/GeometryLayer.cpp

Context not available.
110 GeoDataLatLonBox m_cachedLatLonBox; 110​ GeoDataLatLonBox m_cachedLatLonBox;
111 QSet<qint64> m_highlightedRouteRelations; 111​ QSet<qint64> m_highlightedRouteRelations;
112 GeoDataRelation::RelationTypes m_visibleRelationTypes; 112​ GeoDataRelation::RelationTypes m_visibleRelationTypes;
113​ bool m_levelTagsDebugModeEnabled;
114​ int m_levelToDebug;
113}; 115​};
114 116​
115GeometryLayerPrivate::GeometryLayerPrivate(const QAbstractItemModel *model, const StyleBuilder *styleBuilder) : 117​GeometryLayerPrivate::GeometryLayerPrivate(const QAbstractItemModel *model, const StyleBuilder *styleBuilder) :
Context not available.
119 m_lastFeatureAt(nullptr), 121​ m_lastFeatureAt(nullptr),
120 m_dirty(true), 122​ m_dirty(true),
121 m_cachedItemCount(0), 123​ m_cachedItemCount(0),
122 m_visibleRelationTypes(GeoDataRelation::RouteFerry) 124​ m_visibleRelationTypes(GeoDataRelation::RouteFerry),
125​ m_levelTagsDebugModeEnabled(false),
126​ m_levelToDebug(0)
123{ 127​{
124} 128​}
125 129​
Context not available.
248 AbstractGeoPolygonGraphicsItem::s_previousStyle = 0; 252​ AbstractGeoPolygonGraphicsItem::s_previousStyle = 0;
249 GeoLineStringGraphicsItem::s_previousStyle = 0; 253​ GeoLineStringGraphicsItem::s_previousStyle = 0;
250 for (auto item: layerItems) { 254​ for (auto item: layerItems) {
255​ if (const auto placemark = geodata_cast<GeoDataPlacemark>(item->feature())) {
nienhueserUnsubmitted
Done

layerItems grows large and a cast takes some time, so I'd optimize this a bit for performance: Keep the original loop as-is and add a second one in case d->m_levelTagsDebugModeEnabled is true, sth like

if (d->m_levelTagsDebugModeEnabled) {
  for (auto item: layerItems) {
    ...
  }
}
nienhueser: layerItems grows large and a cast takes some time, so I'd optimize this a bit for performance…
256​ if (d->m_levelTagsDebugModeEnabled && placemark->hasOsmData()) {
257​ if (placemark->osmData().containsTagKey(QStringLiteral("level"))) {
nienhueserUnsubmitted
Done

Can't we get rid of this check and instead check that the iterator is valid below?

nienhueser: Can't we get rid of this check and instead check that the iterator is valid below?
258​ QHash<QString, QString>::const_iterator tagIter = placemark->osmData().findTag(QStringLiteral("level"));
259​ const int val = tagIter.value().toInt();
260​ if (val != d->m_levelToDebug) {
261​ continue;
262​ }
263​ }
264​ }
265​ }
251 item->paint(painter, viewport, layer, d->m_tileLevel); 266​ item->paint(painter, viewport, layer, d->m_tileLevel);
252 } 267​ }
253 } 268​ }
Context not available.
667 emit highlightedPlacemarksChanged(selectedPlacemarks); 682​ emit highlightedPlacemarksChanged(selectedPlacemarks);
668} 683​}
669 684​
685​void GeometryLayer::setLevelTagsDebugModeEnabled(bool enabled)
686​{
687​ if (d->m_levelTagsDebugModeEnabled != enabled) {
688​ d->m_levelTagsDebugModeEnabled = enabled;
689​ d->m_levelToDebug = 0;
nienhueserUnsubmitted
Not Done

Why not keep the last one?
Also, is a repaint needed?

nienhueser: Why not keep the last one? Also, is a repaint needed?
690​ }
691​}
692​
693​bool GeometryLayer::isLevelTagsDebugModeEnabled() const
694​{
695​ return d->m_levelTagsDebugModeEnabled;
696​}
697​
698​void GeometryLayer::setLevelToDebug(int level)
699​{
700​ if (d->m_levelToDebug != level) {
701​ d->m_levelToDebug = level;
702​ emit repaintNeeded();
703​ }
704​}
705​
706​int GeometryLayer::levelToDebug() const
707​{
708​ return d->m_levelToDebug;
709​}
710​
670} 711​}
671 712​
672#include "moc_GeometryLayer.cpp" 713​#include "moc_GeometryLayer.cpp"
Context not available.