Fixes slow update in field calculator

by blocking the vector signals ... ... and emitting dataChanged at the end. I'm a bit worried of side effects, but I can't see any other solution. The root of the issue here is that for each changed field/row an attribute valueChanged signal is emitted, and the QgsVectorLayerCache::featureAtId loads the feature again.
qgis · Oct 12, 2018 · d79e3ad · d79e3ad
1 parent 42ea216
commit d79e3ad
Showing 1 changed file with 128 additions and 120 deletions.
diff --git a/src/app/qgsfieldcalculator.cpp b/src/app/qgsfieldcalculator.cpp
@@ -163,169 +163,177 @@ void QgsFieldCalculator::accept()
 {
   builder->saveToRecent( QStringLiteral( "fieldcalc" ) );
 
-  if ( !mVectorLayer )
+  if ( ! mVectorLayer )
+  {
     return;
+  }
+  else  // Need a scope for the blocker let's keep the else for clarity
+  {
 
-  // Set up QgsDistanceArea each time we (re-)calculate
-  QgsDistanceArea myDa;
+    QgsSignalBlocker<QgsVectorLayer> vectorBlocker( mVectorLayer );
 
-  myDa.setSourceCrs( mVectorLayer->crs(), QgsProject::instance()->transformContext() );
-  myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
+    // Set up QgsDistanceArea each time we (re-)calculate
+    QgsDistanceArea myDa;
 
-  QString calcString = builder->expressionText();
-  QgsExpression exp( calcString );
-  exp.setGeomCalculator( &myDa );
-  exp.setDistanceUnits( QgsProject::instance()->distanceUnits() );
-  exp.setAreaUnits( QgsProject::instance()->areaUnits() );
+    myDa.setSourceCrs( mVectorLayer->crs(), QgsProject::instance()->transformContext() );
+    myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
 
-  QgsExpressionContext expContext( QgsExpressionContextUtils::globalProjectLayerScopes( mVectorLayer ) );
+    QString calcString = builder->expressionText();
+    QgsExpression exp( calcString );
+    exp.setGeomCalculator( &myDa );
+    exp.setDistanceUnits( QgsProject::instance()->distanceUnits() );
+    exp.setAreaUnits( QgsProject::instance()->areaUnits() );
 
-  if ( !exp.prepare( &expContext ) )
-  {
-    QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
-    return;
-  }
+    QgsExpressionContext expContext( QgsExpressionContextUtils::globalProjectLayerScopes( mVectorLayer ) );
 
-  bool updatingGeom = false;
+    if ( !exp.prepare( &expContext ) )
+    {
+      QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
+      return;
+    }
 
-  // Test for creating expression field based on ! mUpdateExistingGroupBox checked rather
-  // than on mNewFieldGroupBox checked, as if the provider does not support adding attributes
-  // then mUpdateExistingGroupBox is set to not checkable, and hence is not checked.  This
-  // is a minimum fix to resolve this - better would be some GUI redesign...
-  if ( ! mUpdateExistingGroupBox->isChecked() && mCreateVirtualFieldCheckbox->isChecked() )
-  {
-    mVectorLayer->addExpressionField( calcString, fieldDefinition() );
-  }
-  else
-  {
-    if ( !mVectorLayer->isEditable() )
-      mVectorLayer->startEditing();
+    bool updatingGeom = false;
+
+    // Test for creating expression field based on ! mUpdateExistingGroupBox checked rather
+    // than on mNewFieldGroupBox checked, as if the provider does not support adding attributes
+    // then mUpdateExistingGroupBox is set to not checkable, and hence is not checked.  This
+    // is a minimum fix to resolve this - better would be some GUI redesign...
+    if ( ! mUpdateExistingGroupBox->isChecked() && mCreateVirtualFieldCheckbox->isChecked() )
+    {
+      mVectorLayer->addExpressionField( calcString, fieldDefinition() );
+    }
+    else
+    {
+      if ( !mVectorLayer->isEditable() )
+        mVectorLayer->startEditing();
 
-    QApplication::setOverrideCursor( Qt::WaitCursor );
+      QApplication::setOverrideCursor( Qt::WaitCursor );
 
-    mVectorLayer->beginEditCommand( QStringLiteral( "Field calculator" ) );
+      mVectorLayer->beginEditCommand( QStringLiteral( "Field calculator" ) );
 
-    //update existing field
-    if ( mUpdateExistingGroupBox->isChecked() || !mNewFieldGroupBox->isEnabled() )
-    {
-      if ( mExistingFieldComboBox->currentData().toString() == QLatin1String( "geom" ) )
+      //update existing field
+      if ( mUpdateExistingGroupBox->isChecked() || !mNewFieldGroupBox->isEnabled() )
       {
-        //update geometry
-        mAttributeId = -1;
-        updatingGeom = true;
+        if ( mExistingFieldComboBox->currentData().toString() == QLatin1String( "geom" ) )
+        {
+          //update geometry
+          mAttributeId = -1;
+          updatingGeom = true;
+        }
+        else
+        {
+          QMap<QString, int>::const_iterator fieldIt = mFieldMap.constFind( mExistingFieldComboBox->currentText() );
+          if ( fieldIt != mFieldMap.constEnd() )
+          {
+            mAttributeId = fieldIt.value();
+          }
+        }
       }
       else
       {
-        QMap<QString, int>::const_iterator fieldIt = mFieldMap.constFind( mExistingFieldComboBox->currentText() );
-        if ( fieldIt != mFieldMap.constEnd() )
+        //create new field
+        const QgsField newField = fieldDefinition();
+
+        if ( !mVectorLayer->addAttribute( newField ) )
         {
-          mAttributeId = fieldIt.value();
+          QApplication::restoreOverrideCursor();
+          QMessageBox::critical( nullptr, tr( "Create New Field" ), tr( "Could not add the new field to the provider." ) );
+          mVectorLayer->destroyEditCommand();
+          return;
         }
-      }
-    }
-    else
-    {
-      //create new field
-      const QgsField newField = fieldDefinition();
 
-      if ( !mVectorLayer->addAttribute( newField ) )
-      {
-        QApplication::restoreOverrideCursor();
-        QMessageBox::critical( nullptr, tr( "Create New Field" ), tr( "Could not add the new field to the provider." ) );
-        mVectorLayer->destroyEditCommand();
-        return;
-      }
+        //get index of the new field
+        const QgsFields &fields = mVectorLayer->fields();
 
-      //get index of the new field
-      const QgsFields &fields = mVectorLayer->fields();
+        for ( int idx = 0; idx < fields.count(); ++idx )
+        {
+          if ( fields.at( idx ).name() == mOutputFieldNameLineEdit->text() )
+          {
+            mAttributeId = idx;
+            break;
+          }
+        }
 
-      for ( int idx = 0; idx < fields.count(); ++idx )
-      {
-        if ( fields.at( idx ).name() == mOutputFieldNameLineEdit->text() )
+        //update expression context with new fields
+        expContext.setFields( mVectorLayer->fields() );
+        if ( ! exp.prepare( &expContext ) )
         {
-          mAttributeId = idx;
-          break;
+          QApplication::restoreOverrideCursor();
+          QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
+          return;
         }
       }
 
-      //update expression context with new fields
-      expContext.setFields( mVectorLayer->fields() );
-      if ( ! exp.prepare( &expContext ) )
+      if ( mAttributeId == -1 && !updatingGeom )
       {
+        mVectorLayer->destroyEditCommand();
         QApplication::restoreOverrideCursor();
-        QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
         return;
       }
-    }
-
-    if ( mAttributeId == -1 && !updatingGeom )
-    {
-      mVectorLayer->destroyEditCommand();
-      QApplication::restoreOverrideCursor();
-      return;
-    }
 
-    //go through all the features and change the new attribute
-    QgsFeature feature;
-    bool calculationSuccess = true;
-    QString error;
+      //go through all the features and change the new attribute
+      QgsFeature feature;
+      bool calculationSuccess = true;
+      QString error;
 
-    bool useGeometry = exp.needsGeometry();
-    int rownum = 1;
+      bool useGeometry = exp.needsGeometry();
+      int rownum = 1;
 
-    QgsField field = !updatingGeom ? mVectorLayer->fields().at( mAttributeId ) : QgsField();
+      QgsField field = !updatingGeom ? mVectorLayer->fields().at( mAttributeId ) : QgsField();
 
-    bool newField = !mUpdateExistingGroupBox->isChecked();
-    QVariant emptyAttribute;
-    if ( newField )
-      emptyAttribute = QVariant( field.type() );
+      bool newField = !mUpdateExistingGroupBox->isChecked();
+      QVariant emptyAttribute;
+      if ( newField )
+        emptyAttribute = QVariant( field.type() );
 
-    QgsFeatureRequest req = QgsFeatureRequest().setFlags( useGeometry ? QgsFeatureRequest::NoFlags : QgsFeatureRequest::NoGeometry );
-    if ( mOnlyUpdateSelectedCheckBox->isChecked() )
-    {
-      req.setFilterFids( mVectorLayer->selectedFeatureIds() );
-    }
-    QgsFeatureIterator fit = mVectorLayer->getFeatures( req );
-    while ( fit.nextFeature( feature ) )
-    {
-      expContext.setFeature( feature );
-      expContext.lastScope()->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "row_number" ), rownum, true ) );
-
-      QVariant value = exp.evaluate( &expContext );
-      if ( exp.hasEvalError() )
+      QgsFeatureRequest req = QgsFeatureRequest().setFlags( useGeometry ? QgsFeatureRequest::NoFlags : QgsFeatureRequest::NoGeometry );
+      if ( mOnlyUpdateSelectedCheckBox->isChecked() )
       {
-        calculationSuccess = false;
-        error = exp.evalErrorString();
-        break;
+        req.setFilterFids( mVectorLayer->selectedFeatureIds() );
       }
-      else if ( updatingGeom )
+      QgsFeatureIterator fit = mVectorLayer->getFeatures( req );
+      while ( fit.nextFeature( feature ) )
       {
-        if ( value.canConvert< QgsGeometry >() )
+        expContext.setFeature( feature );
+        expContext.lastScope()->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "row_number" ), rownum, true ) );
+
+        QVariant value = exp.evaluate( &expContext );
+        if ( exp.hasEvalError() )
+        {
+          calculationSuccess = false;
+          error = exp.evalErrorString();
+          break;
+        }
+        else if ( updatingGeom )
         {
-          QgsGeometry geom = value.value< QgsGeometry >();
-          mVectorLayer->changeGeometry( feature.id(), geom );
+          if ( value.canConvert< QgsGeometry >() )
+          {
+            QgsGeometry geom = value.value< QgsGeometry >();
+            mVectorLayer->changeGeometry( feature.id(), geom );
+          }
+        }
+        else
+        {
+          ( void )field.convertCompatible( value );
+          mVectorLayer->changeAttributeValue( feature.id(), mAttributeId, value, newField ? emptyAttribute : feature.attributes().value( mAttributeId ) );
         }
-      }
-      else
-      {
-        ( void )field.convertCompatible( value );
-        mVectorLayer->changeAttributeValue( feature.id(), mAttributeId, value, newField ? emptyAttribute : feature.attributes().value( mAttributeId ) );
-      }
 
-      rownum++;
-    }
+        rownum++;
+      }
 
-    QApplication::restoreOverrideCursor();
+      QApplication::restoreOverrideCursor();
 
-    if ( !calculationSuccess )
-    {
-      QMessageBox::critical( nullptr, tr( "Evaluation Error" ), tr( "An error occurred while evaluating the calculation string:\n%1" ).arg( error ) );
-      mVectorLayer->destroyEditCommand();
-      return;
+      if ( !calculationSuccess )
+      {
+        QMessageBox::critical( nullptr, tr( "Evaluation Error" ), tr( "An error occurred while evaluating the calculation string:\n%1" ).arg( error ) );
+        mVectorLayer->destroyEditCommand();
+        return;
+      }
+      mVectorLayer->endEditCommand();
     }
-
-    mVectorLayer->endEditCommand();
   }
+  // Vector signals unlocked! Tell the world that the layer has changed
+  mVectorLayer->dataChanged();
   QDialog::accept();
 }