Block signals in the cache only

much better approach ...
qgis · Oct 13, 2018 · afc873d · afc873d
1 parent d79e3ad
commit afc873d
Show file tree

Hide file tree

Showing 2 changed files with 124 additions and 126 deletions.
diff --git a/src/app/qgsattributetabledialog.cpp b/src/app/qgsattributetabledialog.cpp
@@ -715,15 +715,20 @@ void QgsAttributeTableDialog::mActionOpenFieldCalculator_triggered()
   QgsAttributeTableModel *masterModel = mMainView->masterModel();
 
   QgsFieldCalculator calc( mLayer, this );
+  masterModel->layerCache()->blockSignals( true );
   if ( calc.exec() == QDialog::Accepted )
   {
+    masterModel->layerCache()->blockSignals( false );
     int col = masterModel->fieldCol( calc.changedAttributeId() );
-
     if ( col >= 0 )
     {
       masterModel->reload( masterModel->index( 0, col ), masterModel->index( masterModel->rowCount() - 1, col ) );
     }
   }
+  else
+  {
+    masterModel->layerCache()->blockSignals( false );
+  }
 }
 
 void QgsAttributeTableDialog::mActionSaveEdits_triggered()

diff --git a/src/app/qgsfieldcalculator.cpp b/src/app/qgsfieldcalculator.cpp
@@ -167,173 +167,166 @@ void QgsFieldCalculator::accept()
   {
     return;
   }
-  else  // Need a scope for the blocker let's keep the else for clarity
-  {
-
-    QgsSignalBlocker<QgsVectorLayer> vectorBlocker( mVectorLayer );
 
-    // Set up QgsDistanceArea each time we (re-)calculate
-    QgsDistanceArea myDa;
+  // Set up QgsDistanceArea each time we (re-)calculate
+  QgsDistanceArea myDa;
 
-    myDa.setSourceCrs( mVectorLayer->crs(), QgsProject::instance()->transformContext() );
-    myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
+  myDa.setSourceCrs( mVectorLayer->crs(), QgsProject::instance()->transformContext() );
+  myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
 
-    QString calcString = builder->expressionText();
-    QgsExpression exp( calcString );
-    exp.setGeomCalculator( &myDa );
-    exp.setDistanceUnits( QgsProject::instance()->distanceUnits() );
-    exp.setAreaUnits( QgsProject::instance()->areaUnits() );
+  QString calcString = builder->expressionText();
+  QgsExpression exp( calcString );
+  exp.setGeomCalculator( &myDa );
+  exp.setDistanceUnits( QgsProject::instance()->distanceUnits() );
+  exp.setAreaUnits( QgsProject::instance()->areaUnits() );
 
-    QgsExpressionContext expContext( QgsExpressionContextUtils::globalProjectLayerScopes( mVectorLayer ) );
+  QgsExpressionContext expContext( QgsExpressionContextUtils::globalProjectLayerScopes( mVectorLayer ) );
 
-    if ( !exp.prepare( &expContext ) )
-    {
-      QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
-      return;
-    }
+  if ( !exp.prepare( &expContext ) )
+  {
+    QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
+    return;
+  }
 
-    bool updatingGeom = false;
+  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();
+  // 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() )
+    //update existing field
+    if ( mUpdateExistingGroupBox->isChecked() || !mNewFieldGroupBox->isEnabled() )
+    {
+      if ( mExistingFieldComboBox->currentData().toString() == QLatin1String( "geom" ) )
       {
-        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();
-          }
-        }
+        //update geometry
+        mAttributeId = -1;
+        updatingGeom = true;
       }
       else
       {
-        //create new field
-        const QgsField newField = fieldDefinition();
-
-        if ( !mVectorLayer->addAttribute( newField ) )
+        QMap<QString, int>::const_iterator fieldIt = mFieldMap.constFind( mExistingFieldComboBox->currentText() );
+        if ( fieldIt != mFieldMap.constEnd() )
         {
-          QApplication::restoreOverrideCursor();
-          QMessageBox::critical( nullptr, tr( "Create New Field" ), tr( "Could not add the new field to the provider." ) );
-          mVectorLayer->destroyEditCommand();
-          return;
+          mAttributeId = fieldIt.value();
         }
+      }
+    }
+    else
+    {
+      //create new field
+      const QgsField newField = fieldDefinition();
 
-        //get index of the new field
-        const QgsFields &fields = mVectorLayer->fields();
+      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;
+      }
 
-        for ( int idx = 0; idx < fields.count(); ++idx )
-        {
-          if ( fields.at( idx ).name() == mOutputFieldNameLineEdit->text() )
-          {
-            mAttributeId = idx;
-            break;
-          }
-        }
+      //get index of the new field
+      const QgsFields &fields = mVectorLayer->fields();
 
-        //update expression context with new fields
-        expContext.setFields( mVectorLayer->fields() );
-        if ( ! exp.prepare( &expContext ) )
+      for ( int idx = 0; idx < fields.count(); ++idx )
+      {
+        if ( fields.at( idx ).name() == mOutputFieldNameLineEdit->text() )
         {
-          QApplication::restoreOverrideCursor();
-          QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
-          return;
+          mAttributeId = idx;
+          break;
         }
       }
 
-      if ( mAttributeId == -1 && !updatingGeom )
+      //update expression context with new fields
+      expContext.setFields( mVectorLayer->fields() );
+      if ( ! exp.prepare( &expContext ) )
       {
-        mVectorLayer->destroyEditCommand();
         QApplication::restoreOverrideCursor();
+        QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
         return;
       }
+    }
 
-      //go through all the features and change the new attribute
-      QgsFeature feature;
-      bool calculationSuccess = true;
-      QString error;
+    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;
 
-      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 ) );
 
-      QgsFeatureRequest req = QgsFeatureRequest().setFlags( useGeometry ? QgsFeatureRequest::NoFlags : QgsFeatureRequest::NoGeometry );
-      if ( mOnlyUpdateSelectedCheckBox->isChecked() )
+      QVariant value = exp.evaluate( &expContext );
+      if ( exp.hasEvalError() )
       {
-        req.setFilterFids( mVectorLayer->selectedFeatureIds() );
+        calculationSuccess = false;
+        error = exp.evalErrorString();
+        break;
       }
-      QgsFeatureIterator fit = mVectorLayer->getFeatures( req );
-      while ( fit.nextFeature( feature ) )
+      else if ( updatingGeom )
       {
-        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 )
+        if ( value.canConvert< QgsGeometry >() )
         {
-          if ( value.canConvert< QgsGeometry >() )
-          {
-            QgsGeometry geom = value.value< QgsGeometry >();
-            mVectorLayer->changeGeometry( feature.id(), geom );
-          }
+          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 ) );
-        }
-
-        rownum++;
       }
-
-      QApplication::restoreOverrideCursor();
-
-      if ( !calculationSuccess )
+      else
       {
-        QMessageBox::critical( nullptr, tr( "Evaluation Error" ), tr( "An error occurred while evaluating the calculation string:\n%1" ).arg( error ) );
-        mVectorLayer->destroyEditCommand();
-        return;
+        ( void )field.convertCompatible( value );
+        mVectorLayer->changeAttributeValue( feature.id(), mAttributeId, value, newField ? emptyAttribute : feature.attributes().value( mAttributeId ) );
       }
-      mVectorLayer->endEditCommand();
+
+      rownum++;
+    }
+
+    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;
     }
+    mVectorLayer->endEditCommand();
   }
-  // Vector signals unlocked! Tell the world that the layer has changed
-  mVectorLayer->dataChanged();
   QDialog::accept();
 }