Copying selected features, and starting editing, no longer force all …

…geometries of the copied-from layer to be cached, which was an (in my opinion) unfortunate side-effect of r5489.

Instead, for copying, the geometry and attributes are now retrieved from the provider in a random-access fashion.

For starting editing, the behaviour is regressed to the pre-r5489 idiom of geometry being cached as part of the screen redraw in editing mode.

This returns to the much better response when copying from large layers, with the additional benefit of copying selected features that had been panned off the canvas (previously a bug).

However, only Postgres and OGR providers have been updated to suit.  Hopefully this is enough for a 0.8 release.

Bonus feature: The layer properties now shows what access method is used to access individual geometries (random or sequential)

Since this is a large change it would be appreciated if others can double checking my testing.  (I have tested against PostGIS and MapInfo tables, and many different geometry adding/vertex editing combinations.)



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

src/core/qgsvectordataprovider.cpp

@@ -172,7 +172,7 @@ QString QgsVectorDataProvider::capabilitiesString() const
   if (abilities & QgsVectorDataProvider::SelectAtId)
   {
     // Not really meaningful to the user.
-    // abilitiesList = "Select at ID";
+    // abilitiesList += "Select at ID";
 #ifdef QGISDEBUG
         std::cerr << "QgsVectorDataProvider::capabilitiesString "
           << "Select at ID" << std::endl;
@@ -188,6 +188,35 @@ QString QgsVectorDataProvider::capabilitiesString() const
 #endif
   }
 
+  if (abilities & QgsVectorDataProvider::SelectGeometryAtId)
+  {
+
+    if (abilities & QgsVectorDataProvider::RandomSelectGeometryAtId)
+    {
+      abilitiesList += "Select Geometries by ID (random access)";
+#ifdef QGISDEBUG
+          std::cerr << "QgsVectorDataProvider::capabilitiesString "
+            << "Select Geometries by ID (random access)" << std::endl;
+#endif
+    }
+    else if (abilities & QgsVectorDataProvider::SequentialSelectGeometryAtId)
+    {
+      abilitiesList += "Select Geometries by ID (sequential access)";
+#ifdef QGISDEBUG
+          std::cerr << "QgsVectorDataProvider::capabilitiesString "
+            << "Select Geometries by ID (sequential access)" << std::endl;
+#endif
+    }
+    else
+    {
+      abilitiesList += "Select Geometries by ID (unknown access method)";
+#ifdef QGISDEBUG
+          std::cerr << "QgsVectorDataProvider::capabilitiesString "
+            << "Select Geometries by ID (unknown access method)" << std::endl;
+#endif
+    }
+  }
+
   return abilitiesList.join(", ");
 
 }

src/core/qgsvectordataprovider.h

@@ -43,16 +43,19 @@ class QgsVectorDataProvider : public QgsDataProvider
       // If you add to this, please also add to capabilitiesString()
       enum Capability
       {
-        NoCapabilities =              0,
-        AddFeatures =                 1,
-        DeleteFeatures =        1 <<  1,
-        ChangeAttributeValues = 1 <<  2,
-        AddAttributes =         1 <<  3,    // TODO: what is this exactly?
-        DeleteAttributes =      1 <<  4,
-        SaveAsShapefile =       1 <<  5,
-        CreateSpatialIndex =    1 <<  6,
-        SelectAtId =            1 <<  7,
-        ChangeGeometries =      1 <<  8
+        NoCapabilities =                     0,
+        AddFeatures =                        1,
+        DeleteFeatures =               1 <<  1,
+        ChangeAttributeValues =        1 <<  2,
+        AddAttributes =                1 <<  3,    // TODO: what is this exactly?
+        DeleteAttributes =             1 <<  4,
+        SaveAsShapefile =              1 <<  5,
+        CreateSpatialIndex =           1 <<  6,
+        SelectAtId =                   1 <<  7,
+        ChangeGeometries =             1 <<  8,
+        SelectGeometryAtId =           1 <<  9,
+        RandomSelectGeometryAtId =     1 << 10,
+        SequentialSelectGeometryAtId = 1 << 11
       };
 
       QgsVectorDataProvider();
@@ -137,12 +140,20 @@ class QgsVectorDataProvider : public QgsDataProvider
       virtual long featureCount() const = 0;
 
       /**
-     * Get the attributes associated with a feature
-     * TODO: Get rid of "row" and set up provider-internal caching instead
-     */
-    virtual void getFeatureAttributes(int oid, int& row, QgsFeature *f) {};
-
-    /**
+       * Get the attributes associated with a feature
+       * TODO: Get rid of "row" and set up provider-internal caching instead
+       */
+      virtual void getFeatureAttributes(int key, int& row, QgsFeature *f) {};
+
+      /**
+       * Fetch geometry for a particular feature with id "key",
+       * modifies "f" in-place.
+       *
+       * This function is enabled if capabilities() returns "SelectGeometryAtId".
+       */
+      virtual void getFeatureGeometry(int key, QgsFeature *f) {};
+
+      /**
        * Number of attribute fields for a feature in the layer
        */
       virtual int fieldCount() const = 0;

src/gui/qgsvectorlayer.cpp

@@ -174,14 +174,8 @@ QgsVectorLayer::~QgsVectorLayer()
   delete myLib;
   delete mLabel;
 
-  // Destroy and cached geometries and clear the references to them
-  for (std::map<int, QgsGeometry*>::iterator it  = mCachedGeometries.begin(); 
-      it != mCachedGeometries.end();
-      ++it )
-  {
-    delete (*it).second;
-  }
-  mCachedGeometries.clear();
+  // Destroy any cached geometries and clear the references to them
+  deleteCachedGeometries();
 }
 
 QString QgsVectorLayer::storageType() const
@@ -830,13 +824,10 @@ void QgsVectorLayer::draw(QPainter * p,
     /*Scale factor of the marker image*/
     double markerScaleFactor=1.;
 
-    // Destroy all cached geometries and clear the references to them
-
     if(mEditable)
     {
       // Destroy all cached geometries and clear the references to them
       deleteCachedGeometries();
-
     }
 
     dataProvider->reset();
@@ -988,7 +979,10 @@ void QgsVectorLayer::cacheGeometries()
 {
   if(dataProvider)
     {
-      QgsFeature* f = 0;
+      QgsFeature* f;
+
+      dataProvider->reset();
+
       while(f = dataProvider->getNextFeature(false))
 	{
 	  mCachedGeometries.insert(std::make_pair(f->featureId(), f->geometryAndOwnership()));
@@ -999,6 +993,8 @@ void QgsVectorLayer::cacheGeometries()
 
 void QgsVectorLayer::deleteCachedGeometries()
 {
+  // Destroy any cached geometries and clear the references to them
+
   for (std::map<int, QgsGeometry*>::iterator it  = mCachedGeometries.begin(); it != mCachedGeometries.end(); ++it )
     {
       delete (*it).second;
@@ -1918,7 +1914,11 @@ void QgsVectorLayer::startEditing()
     }
     else
     {
-      cacheGeometries();
+      // No longer need to cache all geometries
+      // instead, mCachedGeometries is refreshed every time the 
+      // screen is redrawn.
+      //cacheGeometries();
+
       mEditable=true;
       if(isValid())
       {
@@ -2604,6 +2604,13 @@ bool QgsVectorLayer::rollBack()
   return true;
 }
 
+
+int QgsVectorLayer::selectedFeatureCount()
+{
+  return mSelectedFeatureIds.size();
+}
+
+
 std::vector<QgsFeature>* QgsVectorLayer::selectedFeatures()
 {
   if (!dataProvider)
@@ -2612,40 +2619,80 @@ std::vector<QgsFeature>* QgsVectorLayer::selectedFeatures()
   }
 
   std::vector<QgsFeature>* features = new std::vector<QgsFeature>;
-  if(mSelectedFeatureIds.size() == 0)
-    {
-      return features;
-    }
 
-  //we need to cache all the features first (which has already been done if a layer is editable)
-  if(!mEditable)
-    {
-      deleteCachedGeometries();
-      cacheGeometries();
-    }
+  if (mSelectedFeatureIds.size() == 0)
+  {
+    // short cut
+    return features;
+  }
+
+  // we don't need to cache features ... it just adds unnecessary time
+  // as we don't need to pull *everything* from disk
+
+  // Go through each selected feature ID and determine
+  // its current geometry and attributes
 
   for (std::set<int>::iterator it  = mSelectedFeatureIds.begin(); it != mSelectedFeatureIds.end(); ++it)
   {
-    // Check this selected item against the committed or cached features
-    if ( mCachedGeometries.find(*it) != mCachedGeometries.end() )
-    {
-      QgsFeature* f = new QgsFeature();
-      f->setGeometry(*mCachedGeometries[*it]);//makes a deep copy of the geometry
-      features->push_back(*f);
-      continue;
-    }
-
+    QgsFeature* initialFeature;
+
+    // Pull the original version of the feature from disk or memory
+    // as appropriate
+
     // Check this selected item against the uncommitted added features
-    /*for (std::vector<QgsFeature*>::iterator iter  = mAddedFeatures.begin();
+    bool selectionIsAddedFeature = FALSE;
+
+    for (std::vector<QgsFeature*>::iterator iter  = mAddedFeatures.begin();
         iter != mAddedFeatures.end();
         ++iter)
     {
       if ( (*it) == (*iter)->featureId() )
       {
-        features->push_back( **iter ); //shouldn't we make a deep copy here?
-        break;
+#ifdef QGISDEBUG
+        std::cout << "QgsVectorLayer::selectedFeatures: found an added geometry: " 
+          << std::endl;
+#endif
+        initialFeature = new QgsFeature(**iter);
+        selectionIsAddedFeature = TRUE;
       }
-      }*/
+    }
+
+    if (!selectionIsAddedFeature)
+    {
+      // pull committed version from disk
+      initialFeature = new QgsFeature(*it);
+
+      int row = 0;  //TODO: Get rid of this, but getFeatureAttributes()
+                    //      needs it for some reason
+      dataProvider->getFeatureAttributes(*it, row, initialFeature);
+
+      if ( mChangedGeometries.find(*it) == mChangedGeometries.end() )
+      {
+        // also pull committed geometry from disk as we will
+        // not need to overwrite it later
+
+        if (dataProvider->capabilities() & QgsVectorDataProvider::SelectGeometryAtId)
+        {
+          dataProvider->getFeatureGeometry(*it, initialFeature);
+        }
+        else
+        {
+          QMessageBox::information(0, tr("Cannot retrieve features"),
+              tr("The provider for the current layer cannot retrieve geometry for the selected features.  This version of the provider does not have this capability."));
+        }
+      }
+
+    }
+
+    // Transform the feature to the "current" in-memory version
+    QgsFeature finalFeature =
+      QgsFeature(*initialFeature,
+                 mChangedAttributes,
+                 mChangedGeometries);
+
+    delete initialFeature;
+
+    features->push_back(finalFeature);
 
   } // for each selected
 

src/gui/qgsvectorlayer.h

@@ -147,6 +147,9 @@ const QString displayField() const { return fieldIndex; }
 
   QgsAttributeAction* actions() { return &mActions; }
 
+  //! The number of features that are selected in this layer
+  int selectedFeatureCount();
+
   /**
       Get a copy of the user-selected features
    */  
@@ -476,7 +479,7 @@ public slots:
   /**Pointer to the table display object if there is one, else a pointer to 0*/
   QgsAttributeTableDisplay * tabledisplay;
 
-  /** cache of the committed geometries retrieved for the current display */
+  /** cache of the committed geometries retrieved *for the current display* */
   std::map<int, QgsGeometry*> mCachedGeometries;
 
   /** Set holding the feature IDs that are activated.  Note that if a feature
@@ -630,7 +633,7 @@ protected slots:
   };
 private:                       // Private methods
 
-  /**Caches all the (commited) geometries to mCachedFeatures, e.g. when entering editing mode*/
+  /**Caches all the (commited) geometries to mCachedGeometries - somewhat out of date as mCachedGeometries should only contain geometries currently visible on the canvas */
   void cacheGeometries();
   /**Deletes the geometries in mCachedGeometries*/
   void deleteCachedGeometries();

src/providers/ogr/qgsogrprovider.cpp

@@ -719,6 +719,56 @@ void QgsOgrProvider::getFeatureAttributes(OGRFeature *ogrFet, QgsFeature *f){
   }
 }
 
+void QgsOgrProvider::getFeatureAttributes(int key, int &row, QgsFeature *f)
+{
+  if(!valid)
+  {
+      QgsLogger::critical("Read attempt on an invalid shapefile data source");
+      return;
+  }
+
+  OGRFeature* fet;
+  OGRGeometry* geom;
+
+  if ((fet = ogrLayer->GetFeature(key)) != NULL)
+  {
+    getFeatureAttributes(fet, f);
+
+    delete fet;
+  }
+}
+
+void QgsOgrProvider::getFeatureGeometry(int key, QgsFeature *f)
+{
+  if(!valid)
+  {
+      QgsLogger::critical("Read attempt on an invalid shapefile data source");
+      return;
+  }
+
+  OGRFeature* fet;
+  OGRGeometry* geom;
+
+  if ((fet = ogrLayer->GetFeature(key)) != NULL)
+  {
+    if (geom = fet->GetGeometryRef())
+    {
+      geom = fet->GetGeometryRef();
+      // get the wkb representation
+      unsigned char *feature = new unsigned char[geom->WkbSize()];
+      geom->exportToWkb((OGRwkbByteOrder) endian(), feature);
+      OGRFeatureDefn * featureDefinition = fet->GetDefnRef();
+      QString featureTypeName = featureDefinition ? 
+                                QString(featureDefinition->GetName()) : 
+                                QString("");
+
+      f->setGeometryAndOwnership(feature, geom->WkbSize());
+
+    }
+    delete fet;
+  }
+}
+
 std::vector<QgsField> const & QgsOgrProvider::fields() const
 {
   return attributeFields;
@@ -1155,10 +1205,18 @@ int QgsOgrProvider::capabilities() const
     // the #defines we want to test for here.
 
     if (ogrLayer->TestCapability("RandomRead"))
-    // TRUE if the GetFeature() method works for this layer.
+    // TRUE if the GetFeature() method works *efficiently* for this layer.
+    // TODO: Perhaps influence if QGIS caches into memory 
+    //       (vs read from disk every time) based on this setting.
     {
-      // TODO: Perhaps influence if QGIS caches into memory (vs read from disk every time) based on this setting.
+      ability |= QgsVectorDataProvider::RandomSelectGeometryAtId;
     }
+    else
+    {
+      ability |= QgsVectorDataProvider::SequentialSelectGeometryAtId;
+    }
+    ability |= QgsVectorDataProvider::SelectGeometryAtId;
+
 
     if (ogrLayer->TestCapability("SequentialWrite"))
     // TRUE if the CreateFeature() method works for this layer.