vector layer: save old attribute values where available to speedup un…

…do (and rollback) (fixes #9509) field calculator & app: use wait cursor while calculating and committing or rolling back
qgis · Feb 8, 2014 · 52616b6 · 52616b6
1 parent 48427e1
commit 52616b6
Show file tree

Hide file tree

Showing 15 changed files with 60 additions and 43 deletions.
diff --git a/python/core/qgsvectorlayer.sip b/python/core/qgsvectorlayer.sip
@@ -732,11 +732,12 @@ class QgsVectorLayer : QgsMapLayer
      *
      * @param fid   The feature id of the feature to be changed
      * @param field The index of the field to be updated
-     * @param value The value which will be assigned to the field
+     * @param newValue The value which will be assigned to the field
+     * @param oldValue The previous value to restore on undo (will otherwise be retrieved)
      *
      * @return true in case of success
      */
-    bool changeAttributeValue( QgsFeatureId fid, int field, QVariant value );
+    bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );
 
     /** add an attribute field (but does not commit it)
         returns true if the field was added

diff --git a/python/core/qgsvectorlayereditbuffer.sip b/python/core/qgsvectorlayereditbuffer.sip
@@ -32,7 +32,7 @@ class QgsVectorLayerEditBuffer : QObject
     bool changeGeometry( QgsFeatureId fid, QgsGeometry* geom );
 
     /** changed an attribute value (but does not commit it) */
-    bool changeAttributeValue( QgsFeatureId fid, int field, QVariant value );
+    bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );
 
     /** add an attribute field (but does not commit it)
         returns true if the field was added

diff --git a/python/core/qgsvectorlayerundocommand.sip b/python/core/qgsvectorlayerundocommand.sip
@@ -61,7 +61,7 @@ class QgsVectorLayerUndoCommandChangeAttribute : QgsVectorLayerUndoCommand
 #include "qgsvectorlayerundocommand.h"
 %End
   public:
-    QgsVectorLayerUndoCommandChangeAttribute( QgsVectorLayerEditBuffer* buffer /Transfer/, QgsFeatureId fid, int fieldIndex, const QVariant& newValue );
+    QgsVectorLayerUndoCommandChangeAttribute( QgsVectorLayerEditBuffer* buffer /Transfer/, QgsFeatureId fid, int fieldIndex, const QVariant &newValue, const QVariant &oldValue );
     virtual void undo();
     virtual void redo();
 };

diff --git a/scripts/sipdiff b/scripts/sipdiff
@@ -1,10 +1,12 @@
 #!/bin/bash
 
-file=$1
+for file in $*; do
+	file=$1
 
-d=${file#*/}
-d=${d%/*}
-f=${file##*/}
-f=${f%.*}
+	d=${file#*/}
+	d=${d%/*}
+	f=${file##*/}
+	f=${f%.*}
 
-vimdiff src/$d/$f.h python/$d/$f.sip
+	vimdiff src/$d/$f.h python/$d/$f.sip
+done
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -5943,7 +5943,7 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()
       feature.geometry()->convertToMultiType();
     }
   }
-  if ( ! layer->addFeatures( features ) || ! layer->commitChanges() )
+  if ( ! layer->addFeatures( features ) || !layer->commitChanges() )
   {
     QgsDebugMsg( "Cannot add features or commit changes" );
     delete layer;
@@ -6127,6 +6127,8 @@ bool QgisApp::toggleEditing( QgsMapLayer *layer, bool allowCancel )
         break;
 
       case QMessageBox::Save:
+        QApplication::setOverrideCursor( Qt::WaitCursor );
+
         if ( !vlayer->commitChanges() )
         {
           commitError( vlayer );
@@ -6137,9 +6139,13 @@ bool QgisApp::toggleEditing( QgsMapLayer *layer, bool allowCancel )
         }
 
         vlayer->triggerRepaint();
+
+        QApplication::restoreOverrideCursor();
         break;
 
       case QMessageBox::Discard:
+        QApplication::setOverrideCursor( Qt::WaitCursor );
+
         mMapCanvas->freeze( true );
         if ( !vlayer->rollBack() )
         {
@@ -6151,6 +6157,8 @@ bool QgisApp::toggleEditing( QgsMapLayer *layer, bool allowCancel )
         mMapCanvas->freeze( false );
 
         vlayer->triggerRepaint();
+
+        QApplication::restoreOverrideCursor();
         break;
 
       default:
@@ -8115,6 +8123,7 @@ void QgisApp::hasCrsTransformEnabled( bool theFlag )
   QgsProject::instance()->writeEntry( "SpatialRefSys", "/ProjectionsEnabled", ( theFlag ? 1 : 0 ) );
   updateCRSStatusBar();
 }
+
 // slot to update the progress bar in the status bar
 void QgisApp::showProgress( int theProgress, int theTotalSteps )
 {

diff --git a/src/app/qgsfeatureaction.cpp b/src/app/qgsfeatureaction.cpp
@@ -127,7 +127,7 @@ bool QgsFeatureAction::editFeature()
       {
         if ( dst[i] != src[i] )
         {
-          mLayer->changeAttributeValue( mFeature.id(), i, dst[i] );
+          mLayer->changeAttributeValue( mFeature.id(), i, dst[i], src[i] );
         }
       }
 

diff --git a/src/app/qgsfieldcalculator.cpp b/src/app/qgsfieldcalculator.cpp
@@ -110,6 +110,8 @@ void QgsFieldCalculator::accept()
     return;
   }
 
+  QApplication::setOverrideCursor( Qt::WaitCursor );
+
   mVectorLayer->beginEditCommand( "Field calculator" );
 
   //update existing field
@@ -153,6 +155,7 @@ void QgsFieldCalculator::accept()
   if ( mAttributeId == -1 )
   {
     mVectorLayer->destroyEditCommand();
+    QApplication::restoreOverrideCursor();
     return;
   }
 
@@ -188,12 +191,14 @@ void QgsFieldCalculator::accept()
     }
     else
     {
-      mVectorLayer->changeAttributeValue( feature.id(), mAttributeId, value );
+      mVectorLayer->changeAttributeValue( feature.id(), mAttributeId, value, feature.attributes().value( mAttributeId ) );
     }
 
     rownum++;
   }
 
+  QApplication::restoreOverrideCursor();
+
   if ( !calculationSuccess )
   {
     QMessageBox::critical( 0, tr( "Error" ), tr( "An error occured while evaluating the calculation string:\n%1" ).arg( error ) );

diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -1328,7 +1328,7 @@ bool QgsVectorLayer::updateFeature( QgsFeature &f )
   {
     if ( fa[attr] != ca[attr] )
     {
-      if ( !changeAttributeValue( f.id(), attr, fa[attr] ) )
+      if ( !changeAttributeValue( f.id(), attr, fa[attr], ca[attr] ) )
       {
         QgsDebugMsg( QString( "attribute %1 of feature %2 could not be changed." ).arg( attr ).arg( f.id() ) );
         return false;
@@ -2559,12 +2559,12 @@ bool QgsVectorLayer::changeAttributeValue( QgsFeatureId fid, int field, QVariant
   return changeAttributeValue( fid, field, value );
 }
 
-bool QgsVectorLayer::changeAttributeValue( QgsFeatureId fid, int field, QVariant value )
+bool QgsVectorLayer::changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue )
 {
   if ( !mEditBuffer || !mDataProvider )
     return false;
 
-  return mEditBuffer->changeAttributeValue( fid, field, value );
+  return mEditBuffer->changeAttributeValue( fid, field, newValue, oldValue );
 }
 
 bool QgsVectorLayer::addAttribute( const QgsField &field )

diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h
@@ -1083,11 +1083,12 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
      *
      * @param fid   The feature id of the feature to be changed
      * @param field The index of the field to be updated
-     * @param value The value which will be assigned to the field
+     * @param newValue The value which will be assigned to the field
+     * @param oldValue The previous value to restore on undo (will otherwise be retrieved)
      *
      * @return true in case of success
      */
-    bool changeAttributeValue( QgsFeatureId fid, int field, QVariant value );
+    bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );
 
     /** add an attribute field (but does not commit it)
         returns true if the field was added

diff --git a/src/core/qgsvectorlayereditbuffer.cpp b/src/core/qgsvectorlayereditbuffer.cpp
@@ -169,7 +169,7 @@ bool QgsVectorLayerEditBuffer::changeGeometry( QgsFeatureId fid, QgsGeometry* ge
 }
 
 
-bool QgsVectorLayerEditBuffer::changeAttributeValue( QgsFeatureId fid, int field, QVariant value )
+bool QgsVectorLayerEditBuffer::changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue )
 {
   if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::ChangeAttributeValues ) )
     return false;
@@ -184,7 +184,7 @@ bool QgsVectorLayerEditBuffer::changeAttributeValue( QgsFeatureId fid, int field
        L->pendingFields().fieldOrigin( field ) == QgsFields::OriginJoin )
     return false;
 
-  L->undoStack()->push( new QgsVectorLayerUndoCommandChangeAttribute( this, fid, field, value ) );
+  L->undoStack()->push( new QgsVectorLayerUndoCommandChangeAttribute( this, fid, field, newValue, oldValue ) );
   return true;
 }
 

diff --git a/src/core/qgsvectorlayereditbuffer.h b/src/core/qgsvectorlayereditbuffer.h
@@ -58,7 +58,7 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
     bool changeGeometry( QgsFeatureId fid, QgsGeometry* geom );
 
     /** changed an attribute value (but does not commit it) */
-    bool changeAttributeValue( QgsFeatureId fid, int field, QVariant value );
+    bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );
 
     /** add an attribute field (but does not commit it)
         returns true if the field was added

diff --git a/src/core/qgsvectorlayerundocommand.cpp b/src/core/qgsvectorlayerundocommand.cpp
@@ -217,10 +217,11 @@ void QgsVectorLayerUndoCommandChangeGeometry::redo()
 }
 
 
-QgsVectorLayerUndoCommandChangeAttribute::QgsVectorLayerUndoCommandChangeAttribute( QgsVectorLayerEditBuffer* buffer, QgsFeatureId fid, int fieldIndex, const QVariant& newValue )
+QgsVectorLayerUndoCommandChangeAttribute::QgsVectorLayerUndoCommandChangeAttribute( QgsVectorLayerEditBuffer* buffer, QgsFeatureId fid, int fieldIndex, const QVariant &newValue, const QVariant &oldValue )
     : QgsVectorLayerUndoCommand( buffer )
     , mFid( fid )
     , mFieldIndex( fieldIndex )
+    , mOldValue( oldValue )
     , mNewValue( newValue )
     , mFirstChange( true )
 {
@@ -235,13 +236,10 @@ QgsVectorLayerUndoCommandChangeAttribute::QgsVectorLayerUndoCommandChangeAttribu
       mFirstChange = false;
     }
   }
-  else
+  else if ( mBuffer->mChangedAttributeValues.contains( mFid ) && mBuffer->mChangedAttributeValues[mFid].contains( mFieldIndex ) )
   {
-    if ( mBuffer->mChangedAttributeValues.contains( mFid ) && mBuffer->mChangedAttributeValues[mFid].contains( mFieldIndex ) )
-    {
-      mOldValue = mBuffer->mChangedAttributeValues[mFid][mFieldIndex];
-      mFirstChange = false;
-    }
+    mOldValue = mBuffer->mChangedAttributeValues[mFid][mFieldIndex];
+    mFirstChange = false;
   }
 
 }
@@ -257,15 +255,15 @@ void QgsVectorLayerUndoCommandChangeAttribute::undo()
     Q_ASSERT( it != mBuffer->mAddedFeatures.end() );
     it.value().setAttribute( mFieldIndex, mOldValue );
   }
-  else
+  else if ( mFirstChange )
   {
     // existing feature
-    if ( mFirstChange )
-    {
-      mBuffer->mChangedAttributeValues[mFid].remove( mFieldIndex );
-      if ( mBuffer->mChangedAttributeValues[mFid].isEmpty() )
-        mBuffer->mChangedAttributeValues.remove( mFid );
+    mBuffer->mChangedAttributeValues[mFid].remove( mFieldIndex );
+    if ( mBuffer->mChangedAttributeValues[mFid].isEmpty() )
+      mBuffer->mChangedAttributeValues.remove( mFid );
 
+    if ( !mOldValue.isValid() )
+    {
       // get old value from provider
       QgsFeature tmp;
       QgsFeatureRequest request;
@@ -276,10 +274,10 @@ void QgsVectorLayerUndoCommandChangeAttribute::undo()
       if ( fi.nextFeature( tmp ) )
         original = tmp.attribute( mFieldIndex );
     }
-    else
-    {
-      mBuffer->mChangedAttributeValues[mFid][mFieldIndex] = mOldValue;
-    }
+  }
+  else
+  {
+    mBuffer->mChangedAttributeValues[mFid][mFieldIndex] = mOldValue;
   }
 
   emit mBuffer->attributeValueChanged( mFid, mFieldIndex, original );

diff --git a/src/core/qgsvectorlayerundocommand.h b/src/core/qgsvectorlayerundocommand.h
@@ -98,7 +98,7 @@ class CORE_EXPORT QgsVectorLayerUndoCommandChangeGeometry : public QgsVectorLaye
 class CORE_EXPORT QgsVectorLayerUndoCommandChangeAttribute : public QgsVectorLayerUndoCommand
 {
   public:
-    QgsVectorLayerUndoCommandChangeAttribute( QgsVectorLayerEditBuffer* buffer, QgsFeatureId fid, int fieldIndex, const QVariant& newValue );
+    QgsVectorLayerUndoCommandChangeAttribute( QgsVectorLayerEditBuffer* buffer, QgsFeatureId fid, int fieldIndex, const QVariant &newValue, const QVariant &oldValue );
     virtual void undo();
     virtual void redo();
 

diff --git a/src/gui/attributetable/qgsattributetabledelegate.cpp b/src/gui/attributetable/qgsattributetabledelegate.cpp
@@ -87,13 +87,14 @@ void QgsAttributeTableDelegate::setModelData( QWidget *editor, QAbstractItemMode
 
   int fieldIdx = model->data( index, QgsAttributeTableModel::FieldIndexRole ).toInt();
   QgsFeatureId fid = model->data( index, QgsAttributeTableModel::FeatureIdRole ).toLongLong();
+  QVariant oldValue = model->data( index, Qt::EditRole );
 
-  QVariant value;
-  if ( !QgsAttributeEditor::retrieveValue( editor, vl, fieldIdx, value ) )
+  QVariant newValue;
+  if ( !QgsAttributeEditor::retrieveValue( editor, vl, fieldIdx, newValue ) )
     return;
 
   vl->beginEditCommand( tr( "Attribute changed" ) );
-  vl->changeAttributeValue( fid, fieldIdx, value );
+  vl->changeAttributeValue( fid, fieldIdx, newValue, oldValue );
   vl->endEditCommand();
 }
 

diff --git a/src/gui/attributetable/qgsdualview.cpp b/src/gui/attributetable/qgsdualview.cpp
@@ -350,7 +350,7 @@ bool QgsDualView::saveEditChanges()
           if ( dst[i] == src[i] )
             continue;
 
-          mLayerCache->layer()->changeAttributeValue( fid, i, dst[i] );
+          mLayerCache->layer()->changeAttributeValue( fid, i, dst[i], src[i] );
         }
 
         mLayerCache->layer()->endEditCommand();