Don't add track and route layers for LOC files, throw real exceptions instead of strings in GPSData, move all file loading from QgsGPXProvider to GPSData, add a static container with reference counting to let GPX layers from the same file share data, use signaling_NaN instead of -max for 'no data' in waypoint elevation

git-svn-id: http://svn.osgeo.org/qgis/trunk/qgis@1371 c8812cc2-4d05-0410-92ff-de0c093fc19c

plugins/gps_importer/plugin.cpp

@@ -145,11 +145,15 @@ void Plugin::addGPXLayer() {
 				 qgisMainWindowPointer,
 				 "Select a GPX or LOC file",
 				 "Select a GPX or LOC file");
+  
+  // LOC files only has waypoints, don't add track and route layers for them
   if (gpxFileName != 0) {
-    qGisInterface->addVectorLayer(gpxFileName + "?type=track", 
-				  "Tracks", "gpx");
-    qGisInterface->addVectorLayer(gpxFileName + "?type=route",
-				  "Routes", "gpx");
+    if (gpxFileName.right(4) == ".gpx") {
+      qGisInterface->addVectorLayer(gpxFileName + "?type=track", 
+				    "Tracks", "gpx");
+      qGisInterface->addVectorLayer(gpxFileName + "?type=route",
+				    "Routes", "gpx");
+    }
     qGisInterface->addVectorLayer(gpxFileName + "?type=waypoint", 
 				  "Waypoints", "gpx");
   }

providers/gpx/gpsdata.cpp

@@ -16,6 +16,9 @@
  ***************************************************************************/
 
 #include <limits>
+#include <stdexcept>
+
+#include <qfile.h>
 
 #include "gpsdata.h"
 
@@ -55,21 +58,21 @@ int GPSData::getNumberOfTracks() const {
 
 Waypoint& GPSData::getWaypoint(int index) {
   if (index < 0 || index >= waypoints.size())
-    throw "Waypoint index out of range";
+    throw std::out_of_range("Waypoint index is out of range");
   return waypoints[index];
 }
 
 
 Route& GPSData::getRoute(int index) {
   if (index < 0 || index >= routes.size())
-    throw "Route index out of range";
+    throw std::out_of_range("Route index is out of range");
   return routes[index];
 }
 
 
 Track& GPSData::getTrack(int index) {
   if (index < 0 || index >= tracks.size())
-    throw "Track index out of range";
+    throw std::out_of_range("Track index is out of range");
   return tracks[index];
 }
 
@@ -103,9 +106,9 @@ int GPSData::addTrack(std::string name) {
 
 bool GPSData::removeWaypoint(int index, bool checkRoutes) {
   if (checkRoutes)
-    throw "Not implemented";
+    throw std::logic_error("Not implemented");
   if (index < 0 || index >= waypoints.size())
-    throw "Waypoint index is out of range";
+    throw std::out_of_range("Waypoint index is out of range");
   waypoints.erase(waypoints.begin() + index);
   return true;
 }
@@ -113,14 +116,14 @@ bool GPSData::removeWaypoint(int index, bool checkRoutes) {
 
 void GPSData::removeRoute(int index) {
   if (index < 0 || index >= routes.size())
-    throw "Route index is out of range";
+    throw std::out_of_range("Route index is out of range");
   routes.erase(routes.begin() + index);
 }
 
 
 void GPSData::removeTrack(int index) {
   if (index < 0 || index >= tracks.size())
-    throw "Track index is out of range";
+    throw std::out_of_range("Track index is out of range");
   tracks.erase(tracks.begin() + index);
 }
   
@@ -351,8 +354,6 @@ bool GPSData::parseGPX(QDomNode& node) {
     node = node.nextSibling();
   } 
   
-  // update bookkeeping variables
-  
   return true;
 }
 
@@ -393,3 +394,44 @@ bool GPSData::parseLOC(QDomNode& node) {
 }
 
 
+GPSData* GPSData::getData(std::string filename) {
+  
+  // if the data isn't there already, try to load it
+  if (dataObjects.find(filename) == dataObjects.end()) {
+    QDomDocument qdd;
+    QFile file(filename);
+    GPSData data;
+    std::cerr<<"Loading file "<<filename<<std::endl;
+    if (!(qdd.setContent(&file) && data.parseDom(qdd))) {
+      std::cerr<<filename<<"is not valid GPX!"<<std::endl;
+      return 0;
+    }
+    dataObjects[filename] = std::pair<GPSData, unsigned>(data, 0);
+  }
+  else
+    std::cerr<<filename<<" is already loaded"<<std::endl;
+  
+  // return a pointer and increase the reference count for that filename
+  DataMap::iterator iter = dataObjects.find(filename);
+  ++(iter->second.second);
+  return &iter->second.first;
+}
+
+
+void GPSData::releaseData(std::string filename) {
+  
+  /* decrease the reference count for the filename (if it is used), and erase
+     it if the reference count becomes 0 */
+  DataMap::iterator iter = dataObjects.find(filename);
+  if (iter != dataObjects.end()) {
+    std::cerr<<"unrefing "<<filename<<std::endl;
+    if (--(iter->second.second) == 0) {
+      std::cerr<<"No one's using "<<filename<<", I'll erase it"<<std::endl;
+      dataObjects.erase(iter);
+    }
+  }
+}
+
+
+// we have to initialize the static member
+GPSData::DataMap GPSData::dataObjects;

providers/gpx/gpsdata.h

@@ -21,6 +21,7 @@
 #include <iostream>
 #include <limits>
 #include <string>
+#include <map>
 #include <vector>
 
 #include <qdom.h>
@@ -28,7 +29,7 @@
 #include "../../src/qgsrect.h"
 
 
-#define DOUBLE_NA (-std::numeric_limits<double>::max())
+#define DOUBLE_NA (std::numeric_limits<double>::signaling_NaN())
 
 
 /** This is the parent class for all GPS data classes (except tracksegment).
@@ -168,7 +169,25 @@ class GPSData {
       succeeds it will fill this GPSData object with the data from the
       QDomDocument and return true, otherwise it will return false. */
   bool parseDom(QDomDocument& qdd);
-
+  
+  
+  /** This function returns a pointer to the GPSData object associated with
+      the file @c filename. If the file does not exist or can't be parsed,
+      NULL will be returned. If the file is already used by another layer,
+      a pointer to the same GPSData object will be returned. And if the file
+      is not used by any other layer, and can be parsed, a new GPSData object
+      will be created and a pointer to it will be returned. If you use this
+      function you should also call releaseData() with the same @c filename
+      when you're done with the GPSData pointer, otherwise the data will stay
+      in memory forever and you will get an ugly memory leak. */
+  static GPSData* getData(std::string filename);
+  
+  /** Call this function when you're done with a GPSData pointer that you
+      got earlier using getData(). Do NOT call this function if you haven't
+      called getData() earlier with the same @c filename, that can cause data
+      that is still in use to be deleted. */
+  static void releaseData(std::string filename);
+  
   
   /** operator<< is our friend. */
   friend std::ostream& operator<<(std::ostream& os, const GPSData& d);
@@ -199,7 +218,14 @@ class GPSData {
   std::vector<Track> tracks;
   double xMin, xMax, yMin, yMax;
   
-  //static GPSData* dataPtr;
+  /** This is used internally to store GPS data objects (one per file). */
+  typedef std::map<std::string, std::pair<GPSData, unsigned> > DataMap;
+  
+  /** This is the static container that maps filenames to GPSData objects and
+      does reference counting, so several providers can use the same GPSData 
+      object. */
+  static DataMap dataObjects;
+
 };
 
 #endif

providers/gpx/qgsgpxprovider.cpp

@@ -72,13 +72,10 @@ QgsGPXProvider::QgsGPXProvider(QString uri) : mDataSourceUri(uri),
   mSelectionRectangle = 0;
   
   // parse the file
-  mFile = new QFile(mFileName);
-  QDomDocument qdd;
-  if (!(mValid = (qdd.setContent(mFile) && data.parseDom(qdd)))) {
-    std::cerr<<mFileName<<"is not valid GPX!"<<std::endl;
-  }
-  mFile->close();
-  delete mFile;
+  data = GPSData::getData(mFileName);
+  if (data == 0)
+    return;
+  mValid = true;
   
   // resize the cache matrix
   mMinMaxCache=new double*[attributeFields.size()];
@@ -93,6 +90,7 @@ QgsGPXProvider::~QgsGPXProvider() {
     delete mMinMaxCache[i];
   }
   delete[] mMinMaxCache;
+  GPSData::releaseData(mFileName);
 }
 
 
@@ -127,8 +125,8 @@ QgsFeature *QgsGPXProvider::getNextFeature(bool fetchAttributes) {
   if (mFeatureType == "waypoint") {
     // go through the list of waypoints and return the first one that is in
     // the bounds rectangle
-    for (; mFid < data.getNumberOfWaypoints(); ++mFid) {
-      const Waypoint& wpt(data.getWaypoint(mFid));
+    for (; mFid < data->getNumberOfWaypoints(); ++mFid) {
+      const Waypoint& wpt(data->getWaypoint(mFid));
       if (boundsCheck(wpt.lon, wpt.lat)) {
 	result = new QgsFeature(mFid);
 	
@@ -147,7 +145,8 @@ QgsFeature *QgsGPXProvider::getNextFeature(bool fetchAttributes) {
 	  result->addAttribute("name", wpt.name);
 	  result->addAttribute("lat", QString("%1").arg(wpt.lat));
 	  result->addAttribute("lon", QString("%1").arg(wpt.lon));
-	  result->addAttribute("ele", QString("%1").arg(wpt.ele));
+	  if (!isnan(wpt.ele))
+	    result->addAttribute("ele", QString("%1").arg(wpt.ele));
 	}
 	
 	++mFid;
@@ -159,8 +158,8 @@ QgsFeature *QgsGPXProvider::getNextFeature(bool fetchAttributes) {
   else if (mFeatureType == "route") {
     // go through the routes and return the first one that is in the bounds
     // rectangle
-    for (; mFid < data.getNumberOfRoutes(); ++mFid) {
-      const Route& rte(data.getRoute(mFid));
+    for (; mFid < data->getNumberOfRoutes(); ++mFid) {
+      const Route& rte(data->getRoute(mFid));
       if (rte.points.size() == 0)
 	continue;
       const Routepoint& rtept(rte.points[0]);
@@ -197,8 +196,8 @@ QgsFeature *QgsGPXProvider::getNextFeature(bool fetchAttributes) {
   else if (mFeatureType == "track") {
     // go through the tracks and return the first one that is in the bounds
     // rectangle
-    for (; mFid < data.getNumberOfTracks(); ++mFid) {
-      const Track& trk(data.getTrack(mFid));
+    for (; mFid < data->getNumberOfTracks(); ++mFid) {
+      const Track& trk(data->getTrack(mFid));
       if (trk.segments.size() == 0)
 	continue;
       if (trk.segments[0].points.size() == 0)
@@ -301,7 +300,7 @@ int QgsGPXProvider::endian() {
 
 // Return the extent of the layer
 QgsRect *QgsGPXProvider::extent() {
-  return data.getExtent();
+  return data->getExtent();
 }
 
 
@@ -318,11 +317,11 @@ int QgsGPXProvider::geometryType() {
  */
 long QgsGPXProvider::featureCount() {
   if (mFeatureType == "waypoint")
-    return data.getNumberOfWaypoints();
+    return data->getNumberOfWaypoints();
   if (mFeatureType == "route")
-    return data.getNumberOfRoutes();
+    return data->getNumberOfRoutes();
   if (mFeatureType == "track")
-    return data.getNumberOfTracks();
+    return data->getNumberOfTracks();
   return 0;
 }
 

providers/gpx/qgsgpxprovider.h

@@ -138,7 +138,7 @@ public:
   
  private:
   
-  GPSData data;
+  GPSData* data;
   void fillMinMaxCash();
   //! Fields
   std::vector<QgsField> attributeFields;
@@ -149,8 +149,6 @@ public:
   QString mFeatureType;
   //! Current selection rectangle
   QgsRect *mSelectionRectangle;
-  //! Text file
-  QFile *mFile;
   bool mValid;
   int mGeomType;
   long mNumberFeatures;