Fixed crashing async map loading

fixes #331

gpxsee.pro

@@ -106,6 +106,7 @@ HEADERS += src/common/config.h \
     src/map/IMG/textpathitem.h \
     src/map/IMG/textpointitem.h \
     src/map/bsbmap.h \
+    src/map/invalidmap.h \
     src/map/polyconic.h \
     src/map/projection.h \
     src/map/ellipsoid.h \

src/GUI/gui.cpp

@@ -131,11 +131,17 @@ void GUI::createMapActions()
 	if (mapDir.isNull())
 		return;
 
-	QString unused;
+	QList<Map*> maps(MapList::loadMaps(mapDir));
-	QList<Map*> maps(MapList::loadMaps(mapDir, unused));
 	for (int i = 0; i < maps.count(); i++) {
-		MapAction *a = createMapAction(maps.at(i));
+		Map *map = maps.at(i);
+		if (map->isValid()) {
+			MapAction *a = createMapAction(map);
 			connect(a, SIGNAL(loaded()), this, SLOT(mapInitialized()));
+		} else {
+			qWarning("%s: %s", qPrintable(map->path()),
+			  qPrintable(map->errorString()));
+			delete map;
+		}
 	}
 }
 
@@ -160,7 +166,7 @@ void GUI::mapInitialized()
 		_showMapAction->setEnabled(true);
 		_clearMapCacheAction->setEnabled(true);
 	} else {
-		qWarning("%s: %s", qPrintable(map->name()), qPrintable(map->errorString()));
+		qWarning("%s: %s", qPrintable(map->path()), qPrintable(map->errorString()));
 		action->deleteLater();
 	}
 }
@@ -1463,32 +1469,36 @@ static MapAction *findMapAction(const QList<QAction*> &mapActions,
 
 bool GUI::loadMap(const QString &fileName, MapAction *&action, bool silent)
 {
-	QString error;
+	QList<Map*> maps(MapList::loadMaps(fileName));
-	QList<Map*> maps(MapList::loadMaps(fileName, error));
+	QList<QAction*> existingActions(_mapsActionGroup->actions());
-	if (maps.isEmpty()) {
-		error = tr("Error loading map:") + "\n\n" + fileName + "\n\n" + error;
-		if (!silent)
-			QMessageBox::critical(this, APP_NAME, error);
-		return false;
-	}
-
 	MapAction *lastReady = 0;
-	QList<QAction*> mapActions(_mapsActionGroup->actions());
+	bool valid = false;
 
 	for (int i = 0; i < maps.size(); i++) {
 		Map *map = maps.at(i);
 		MapAction *a;
 
-		if (!(a = findMapAction(mapActions, map))) {
+		if (!(a = findMapAction(existingActions, map))) {
+			if (!map->isValid()) {
+				if (!silent)
+					QMessageBox::critical(this, APP_NAME,
+					  tr("Error loading map:") + "\n\n" + map->path() + "\n\n"
+					  + map->errorString());
+				delete map;
+			} else {
+				valid = true;
 				a = createMapAction(map);
 				_mapMenu->insertAction(_mapsEnd, a);
+
 				if (map->isReady()) {
 					lastReady = a;
 					_showMapAction->setEnabled(true);
 					_clearMapCacheAction->setEnabled(true);
 				} else
 					connect(a, SIGNAL(loaded()), this, SLOT(mapLoaded()));
+			}
 		} else {
+			valid = true;
 			map = a->data().value<Map*>();
 			if (map->isReady())
 				lastReady = a;
@@ -1497,7 +1507,7 @@ bool GUI::loadMap(const QString &fileName, MapAction *&action, bool silent)
 
 	action = lastReady;
 
-	return true;
+	return valid;
 }
 
 void GUI::mapLoaded()
@@ -1510,7 +1520,23 @@ void GUI::mapLoaded()
 		_showMapAction->setEnabled(true);
 		_clearMapCacheAction->setEnabled(true);
 	} else {
-		QString error = tr("Error loading map:") + "\n\n" + map->name() + "\n\n"
+		QString error = tr("Error loading map:") + "\n\n" + map->path() + "\n\n"
+		  + map->errorString();
+		QMessageBox::critical(this, APP_NAME, error);
+		action->deleteLater();
+	}
+}
+
+void GUI::mapLoadedNoTrigger()
+{
+	MapAction *action = static_cast<MapAction*>(QObject::sender());
+	Map *map = action->data().value<Map*>();
+
+	if (map->isValid()) {
+		_showMapAction->setEnabled(true);
+		_clearMapCacheAction->setEnabled(true);
+	} else {
+		QString error = tr("Error loading map:") + "\n\n" + map->path() + "\n\n"
 		  + map->errorString();
 		QMessageBox::critical(this, APP_NAME, error);
 		action->deleteLater();
@@ -1524,34 +1550,38 @@ void GUI::loadMapDir()
 	if (dir.isEmpty())
 		return;
 
-	QString error;
+	QList<Map*> maps(MapList::loadMaps(dir));
-	QList<Map*> maps(MapList::loadMaps(dir, error));
+	QList<MapAction*> actions;
-	if (maps.isEmpty()) {
+	QList<QAction*> existingActions(_mapsActionGroup->actions());
-		QMessageBox::critical(this, APP_NAME, tr("No usable map found"));
-		return;
-	}
-
-	QList<MapItem*> items(_mapView->loadMaps(maps));
-	QList<QAction*> mapActions(_mapsActionGroup->actions());
-
 	QFileInfo fi(dir);
 	QMenu *menu = new QMenu(fi.fileName());
 
 	for (int i = 0; i < maps.size(); i++) {
 		Map *map = maps.at(i);
+		MapAction *a;
 
-		if (!findMapAction(mapActions, map)) {
+		if (!(a = findMapAction(existingActions, map))) {
-			MapAction *a = createMapAction(map);
+			if (!map->isValid()) {
+				QMessageBox::critical(this, APP_NAME, tr("Error loading map:")
+				  + "\n\n" + map->path() + "\n\n" + map->errorString());
+				delete map;
+			} else {
+				a = createMapAction(map);
 				menu->addAction(a);
+				actions.append(a);
+
 				if (map->isReady()) {
 					_showMapAction->setEnabled(true);
 					_clearMapCacheAction->setEnabled(true);
 				} else
-				connect(a, SIGNAL(loaded()), this, SLOT(mapLoaded()));
+					connect(a, SIGNAL(loaded()), this,
+					  SLOT(mapLoadedNoTrigger()));
+			}
+		} else
+			actions.append(a);
+	}
 
-			connect(items.at(i), SIGNAL(triggered()), a, SLOT(trigger()));
+	_mapView->loadMaps(actions);
-		}
-	}
 
 	if (menu->isEmpty())
 		delete menu;

src/GUI/gui.h

@@ -96,6 +96,7 @@ private slots:
 	void logicalDotsPerInchChanged(qreal dpi);
 
 	void mapLoaded();
+	void mapLoadedNoTrigger();
 	void mapInitialized();
 
 private:

src/GUI/mapview.cpp

@@ -18,6 +18,7 @@
 #include "mapitem.h"
 #include "keys.h"
 #include "graphicsscene.h"
+#include "mapaction.h"
 #include "mapview.h"
 
 
@@ -271,13 +272,19 @@ QList<PathItem *> MapView::loadData(const Data &data)
 	return paths;
 }
 
-QList<MapItem *> MapView::loadMaps(const QList<Map*> &maps)
+void MapView::loadMaps(const QList<MapAction *> &maps)
 {
-	QList<MapItem *> items;
 	int zoom = _map->zoom();
 
-	for (int i = 0; i < maps.size(); i++)
+	for (int i = 0; i < maps.size(); i++) {
-		items.append(addMap(maps.at(i)));
+		MapAction *a = maps.at(i);
+		Map *map = a->data().value<Map*>();
+		if (map->isReady()) {
+			MapItem *mi = addMap(map);
+			connect(mi, SIGNAL(triggered()), a, SLOT(trigger()));
+		} else
+			connect(a, SIGNAL(loaded()), this, SLOT(mapLoaded()));
+	}
 
 	if (fitMapZoom() != zoom)
 		rescale();
@@ -287,8 +294,29 @@ QList<MapItem *> MapView::loadMaps(const QList<Map*> &maps)
 	updateZValues(_areas);
 
 	centerOn(contentCenter());
+}
 
-	return items;
+void MapView::mapLoaded()
+{
+	MapAction *action = static_cast<MapAction*>(QObject::sender());
+	Map *map = action->data().value<Map*>();
+
+	if (!map->isValid())
+		return;
+
+	int zoom = _map->zoom();
+
+	MapItem *mi = addMap(map);
+	connect(mi, SIGNAL(triggered()), action, SLOT(trigger()));
+
+	if (fitMapZoom() != zoom)
+		rescale();
+	else
+		updatePOIVisibility();
+
+	updateZValues(_areas);
+
+	centerOn(contentCenter());
 }
 
 int MapView::fitMapZoom() const

src/GUI/mapview.h

@@ -34,6 +34,7 @@ class MapItem;
 class Area;
 class GraphicsScene;
 class QTimeZone;
+class MapAction;
 
 class MapView : public QGraphicsView
 {
@@ -50,7 +51,7 @@ public:
 	MapView(Map *map, POI *poi, QWidget *parent = 0);
 
 	QList<PathItem *> loadData(const Data &data);
-	QList<MapItem *> loadMaps(const QList<Map*> &maps);
+	void loadMaps(const QList<MapAction*> &maps);
 
 	void setPalette(const Palette &palette);
 	void setPOI(POI *poi);
@@ -104,6 +105,7 @@ public slots:
 private slots:
 	void updatePOI();
 	void reloadMap();
+	void mapLoaded();
 
 private:
 	typedef QHash<SearchPointer<Waypoint>, WaypointItem*> POIHash;

src/map/invalidmap.h

@@ -0,0 +1,30 @@
+#ifndef INVALIDMAP_H
+#define INVALIDMAP_H
+
+#include "map.h"
+
+class InvalidMap : public Map
+{
+	Q_OBJECT
+
+public:
+	InvalidMap(const QString &fileName, const QString &error, QObject *parent = 0)
+	  : Map(fileName, parent), _errorString(error) {}
+
+	QString name() const {return QString();}
+
+	QRectF bounds() {return QRectF();}
+
+	QPointF ll2xy(const Coordinates &) {return QPointF();}
+	Coordinates xy2ll(const QPointF &) {return Coordinates();}
+
+	void draw(QPainter *, const QRectF &, Flags) {}
+
+	bool isValid() const {return false;}
+	QString errorString() const {return _errorString;}
+
+private:
+	QString _errorString;
+};
+
+#endif // INVALIDMAP_H

src/map/maplist.cpp

@@ -11,11 +11,11 @@
 #include "imgmap.h"
 #include "IMG/gmap.h"
 #include "bsbmap.h"
+#include "invalidmap.h"
 #include "maplist.h"
 
 
-Map *MapList::loadFile(const QString &path, QString &errorString,
+Map *MapList::loadFile(const QString &path, bool *terminate)
-  bool *terminate)
 {
 	QFileInfo fi(path);
 	QString suffix = fi.suffix().toLower();
@@ -27,12 +27,11 @@ Map *MapList::loadFile(const QString &path, QString &errorString,
 		map = new Atlas(path);
 	} else if (suffix == "xml") {
 		if (MapSource::isMap(path)) {
-			if (!(map = MapSource::loadMap(path, errorString)))
+			map = MapSource::loadMap(path);
-				return 0;
 		} else if (GMAP::isGMAP(path)) {
+			map = new IMGMap(path);
 			if (terminate)
 				*terminate = true;
-			map = new IMGMap(path);
 		}
 	} else if (suffix == "jnx")
 		map = new JNXMap(path);
@@ -49,16 +48,10 @@ Map *MapList::loadFile(const QString &path, QString &errorString,
 	else if (suffix == "kap")
 		map = new BSBMap(path);
 
-	if (map && map->isValid())
+	return map ? map : new InvalidMap(path, "Unknown file format");
-		return map;
-	else {
-		errorString = (map) ? map->errorString() : "Unknown file format";
-		delete map;
-		return 0;
-	}
 }
 
-QList<Map*> MapList::loadDir(const QString &path, QString &errorString)
+QList<Map*> MapList::loadDir(const QString &path)
 {
 	QDir md(path);
 	md.setFilter(QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot);
@@ -72,14 +65,9 @@ QList<Map*> MapList::loadDir(const QString &path, QString &errorString)
 		bool terminate = false;
 
 		if (fi.isDir() && fi.fileName() != "set")
-			list.append(loadDir(fi.absoluteFilePath(), errorString));
+			list.append(loadDir(fi.absoluteFilePath()));
 		else if (filter().contains("*." + suffix)) {
-			Map *map = loadFile(fi.absoluteFilePath(), errorString, &terminate);
+			list.append(loadFile(fi.absoluteFilePath(), &terminate));
-			if (map)
-				list.append(map);
-			else
-				qWarning("%s: %s", qPrintable(fi.absoluteFilePath()),
-				  qPrintable(errorString));
 			if (terminate)
 				break;
 		}
@@ -88,15 +76,13 @@ QList<Map*> MapList::loadDir(const QString &path, QString &errorString)
 	return list;
 }
 
-QList<Map*> MapList::loadMaps(const QString &path, QString &errorString)
+QList<Map*> MapList::loadMaps(const QString &path)
 {
 	if (QFileInfo(path).isDir())
-		return loadDir(path, errorString);
+		return loadDir(path);
 	else {
 		QList<Map*> list;
-		Map *map = loadFile(path, errorString, 0);
+		list.append(loadFile(path, 0));
-		if (map)
-			list.append(map);
 		return list;
 	}
 }

src/map/maplist.h

@@ -8,14 +8,13 @@ class Map;
 class MapList
 {
 public:
-	static QList<Map*> loadMaps(const QString &path, QString &errorString);
+	static QList<Map*> loadMaps(const QString &path);
 	static QString formats();
 	static QStringList filter();
 
 private:
-	static Map *loadFile(const QString &path, QString &errorString,
+	static Map *loadFile(const QString &path, bool *terminate);
-	  bool *terminate);
+	static QList<Map*> loadDir(const QString &path);
-	static QList<Map*> loadDir(const QString &path, QString &errorString);
 };
 
 #endif // MAPLIST_H

src/map/mapsource.cpp

@@ -5,6 +5,7 @@
 #include "wmtsmap.h"
 #include "wmsmap.h"
 #include "osm.h"
+#include "invalidmap.h"
 #include "mapsource.h"
 
 
@@ -236,16 +237,14 @@ bool MapSource::isMap(const QString &path)
 	return false;
 }
 
-Map *MapSource::loadMap(const QString &path, QString &errorString)
+Map *MapSource::loadMap(const QString &path)
 {
 	Config config;
 	QFile file(path);
 
 
-	if (!file.open(QFile::ReadOnly | QFile::Text)) {
+	if (!file.open(QFile::ReadOnly | QFile::Text))
-		errorString = file.errorString();
+		return new InvalidMap(path, file.errorString());
-		return 0;
-	}
 
 	QXmlStreamReader reader(&file);
 	if (reader.readNextStartElement()) {
@@ -254,41 +253,27 @@ Map *MapSource::loadMap(const QString &path, QString &errorString)
 		else
 			reader.raiseError("Not an online map source file");
 	}
-	if (reader.error()) {
+	if (reader.error())
-		errorString = QString("%1: %2").arg(reader.lineNumber())
+		return new InvalidMap(path, QString("%1: %2").arg(reader.lineNumber())
-		  .arg(reader.errorString());
+		  .arg(reader.errorString()));
-		return 0;
-	}
 
-	if (config.name.isEmpty()) {
+	if (config.name.isEmpty())
-		errorString = "Missing name definition";
+		return new InvalidMap(path, "Missing name definition");
-		return 0;
+	if (config.url.isEmpty())
-	}
+		return new InvalidMap(path, "Missing URL definition");
-	if (config.url.isEmpty()) {
-		errorString = "Missing URL definition";
-		return 0;
-	}
 	if (config.type == WMTS || config.type == WMS) {
-		if (config.layer.isEmpty()) {
+		if (config.layer.isEmpty())
-			errorString = "Missing layer definition";
+			return new InvalidMap(path, "Missing layer definition");
-			return 0;
+		if (config.format.isEmpty())
-		}
+			return new InvalidMap(path, "Missing format definition");
-		if (config.format.isEmpty()) {
-			errorString = "Missing format definition";
-			return 0;
-		}
 	}
 	if (config.type == WMTS) {
-		if (config.set.isEmpty()) {
+		if (config.set.isEmpty())
-			errorString = "Missing set definiton";
+			return new InvalidMap(path, "Missing set definiton");
-			return 0;
-		}
 	}
 	if (config.type == WMS) {
-		if (config.crs.isEmpty()) {
+		if (config.crs.isEmpty())
-			errorString = "Missing CRS definiton";
+			return new InvalidMap(path, "Missing CRS definiton");
-			return 0;
-		}
 	}
 
 	switch (config.type) {
@@ -315,6 +300,6 @@ Map *MapSource::loadMap(const QString &path, QString &errorString)
 			 config.bounds, config.tileRatio, config.authorization,
 			 config.tileSize, config.scalable, false, true);
 		default:
-			return 0;
+			return new InvalidMap(path, "Invalid map type");
 	}
 }

src/map/mapsource.h

@@ -14,7 +14,7 @@ class QXmlStreamReader;
 class MapSource
 {
 public:
-	static Map *loadMap(const QString &path, QString &errorString);
+	static Map *loadMap(const QString &path);
 	static bool isMap(const QString &path);
 
 private: