Skip to content

Commit d79e3ad

Browse files
committed
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.
1 parent 42ea216 commit d79e3ad

File tree

1 file changed

+128
-120
lines changed

1 file changed

+128
-120
lines changed

src/app/qgsfieldcalculator.cpp

Lines changed: 128 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -163,169 +163,177 @@ void QgsFieldCalculator::accept()
163163
{
164164
builder->saveToRecent( QStringLiteral( "fieldcalc" ) );
165165

166-
if ( !mVectorLayer )
166+
if ( ! mVectorLayer )
167+
{
167168
return;
169+
}
170+
else // Need a scope for the blocker let's keep the else for clarity
171+
{
168172

169-
// Set up QgsDistanceArea each time we (re-)calculate
170-
QgsDistanceArea myDa;
173+
QgsSignalBlocker<QgsVectorLayer> vectorBlocker( mVectorLayer );
171174

172-
myDa.setSourceCrs( mVectorLayer->crs(), QgsProject::instance()->transformContext() );
173-
myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
175+
// Set up QgsDistanceArea each time we (re-)calculate
176+
QgsDistanceArea myDa;
174177

175-
QString calcString = builder->expressionText();
176-
QgsExpression exp( calcString );
177-
exp.setGeomCalculator( &myDa );
178-
exp.setDistanceUnits( QgsProject::instance()->distanceUnits() );
179-
exp.setAreaUnits( QgsProject::instance()->areaUnits() );
178+
myDa.setSourceCrs( mVectorLayer->crs(), QgsProject::instance()->transformContext() );
179+
myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
180180

181-
QgsExpressionContext expContext( QgsExpressionContextUtils::globalProjectLayerScopes( mVectorLayer ) );
181+
QString calcString = builder->expressionText();
182+
QgsExpression exp( calcString );
183+
exp.setGeomCalculator( &myDa );
184+
exp.setDistanceUnits( QgsProject::instance()->distanceUnits() );
185+
exp.setAreaUnits( QgsProject::instance()->areaUnits() );
182186

183-
if ( !exp.prepare( &expContext ) )
184-
{
185-
QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
186-
return;
187-
}
187+
QgsExpressionContext expContext( QgsExpressionContextUtils::globalProjectLayerScopes( mVectorLayer ) );
188188

189-
bool updatingGeom = false;
189+
if ( !exp.prepare( &expContext ) )
190+
{
191+
QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
192+
return;
193+
}
190194

191-
// Test for creating expression field based on ! mUpdateExistingGroupBox checked rather
192-
// than on mNewFieldGroupBox checked, as if the provider does not support adding attributes
193-
// then mUpdateExistingGroupBox is set to not checkable, and hence is not checked. This
194-
// is a minimum fix to resolve this - better would be some GUI redesign...
195-
if ( ! mUpdateExistingGroupBox->isChecked() && mCreateVirtualFieldCheckbox->isChecked() )
196-
{
197-
mVectorLayer->addExpressionField( calcString, fieldDefinition() );
198-
}
199-
else
200-
{
201-
if ( !mVectorLayer->isEditable() )
202-
mVectorLayer->startEditing();
195+
bool updatingGeom = false;
196+
197+
// Test for creating expression field based on ! mUpdateExistingGroupBox checked rather
198+
// than on mNewFieldGroupBox checked, as if the provider does not support adding attributes
199+
// then mUpdateExistingGroupBox is set to not checkable, and hence is not checked. This
200+
// is a minimum fix to resolve this - better would be some GUI redesign...
201+
if ( ! mUpdateExistingGroupBox->isChecked() && mCreateVirtualFieldCheckbox->isChecked() )
202+
{
203+
mVectorLayer->addExpressionField( calcString, fieldDefinition() );
204+
}
205+
else
206+
{
207+
if ( !mVectorLayer->isEditable() )
208+
mVectorLayer->startEditing();
203209

204-
QApplication::setOverrideCursor( Qt::WaitCursor );
210+
QApplication::setOverrideCursor( Qt::WaitCursor );
205211

206-
mVectorLayer->beginEditCommand( QStringLiteral( "Field calculator" ) );
212+
mVectorLayer->beginEditCommand( QStringLiteral( "Field calculator" ) );
207213

208-
//update existing field
209-
if ( mUpdateExistingGroupBox->isChecked() || !mNewFieldGroupBox->isEnabled() )
210-
{
211-
if ( mExistingFieldComboBox->currentData().toString() == QLatin1String( "geom" ) )
214+
//update existing field
215+
if ( mUpdateExistingGroupBox->isChecked() || !mNewFieldGroupBox->isEnabled() )
212216
{
213-
//update geometry
214-
mAttributeId = -1;
215-
updatingGeom = true;
217+
if ( mExistingFieldComboBox->currentData().toString() == QLatin1String( "geom" ) )
218+
{
219+
//update geometry
220+
mAttributeId = -1;
221+
updatingGeom = true;
222+
}
223+
else
224+
{
225+
QMap<QString, int>::const_iterator fieldIt = mFieldMap.constFind( mExistingFieldComboBox->currentText() );
226+
if ( fieldIt != mFieldMap.constEnd() )
227+
{
228+
mAttributeId = fieldIt.value();
229+
}
230+
}
216231
}
217232
else
218233
{
219-
QMap<QString, int>::const_iterator fieldIt = mFieldMap.constFind( mExistingFieldComboBox->currentText() );
220-
if ( fieldIt != mFieldMap.constEnd() )
234+
//create new field
235+
const QgsField newField = fieldDefinition();
236+
237+
if ( !mVectorLayer->addAttribute( newField ) )
221238
{
222-
mAttributeId = fieldIt.value();
239+
QApplication::restoreOverrideCursor();
240+
QMessageBox::critical( nullptr, tr( "Create New Field" ), tr( "Could not add the new field to the provider." ) );
241+
mVectorLayer->destroyEditCommand();
242+
return;
223243
}
224-
}
225-
}
226-
else
227-
{
228-
//create new field
229-
const QgsField newField = fieldDefinition();
230244

231-
if ( !mVectorLayer->addAttribute( newField ) )
232-
{
233-
QApplication::restoreOverrideCursor();
234-
QMessageBox::critical( nullptr, tr( "Create New Field" ), tr( "Could not add the new field to the provider." ) );
235-
mVectorLayer->destroyEditCommand();
236-
return;
237-
}
245+
//get index of the new field
246+
const QgsFields &fields = mVectorLayer->fields();
238247

239-
//get index of the new field
240-
const QgsFields &fields = mVectorLayer->fields();
248+
for ( int idx = 0; idx < fields.count(); ++idx )
249+
{
250+
if ( fields.at( idx ).name() == mOutputFieldNameLineEdit->text() )
251+
{
252+
mAttributeId = idx;
253+
break;
254+
}
255+
}
241256

242-
for ( int idx = 0; idx < fields.count(); ++idx )
243-
{
244-
if ( fields.at( idx ).name() == mOutputFieldNameLineEdit->text() )
257+
//update expression context with new fields
258+
expContext.setFields( mVectorLayer->fields() );
259+
if ( ! exp.prepare( &expContext ) )
245260
{
246-
mAttributeId = idx;
247-
break;
261+
QApplication::restoreOverrideCursor();
262+
QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
263+
return;
248264
}
249265
}
250266

251-
//update expression context with new fields
252-
expContext.setFields( mVectorLayer->fields() );
253-
if ( ! exp.prepare( &expContext ) )
267+
if ( mAttributeId == -1 && !updatingGeom )
254268
{
269+
mVectorLayer->destroyEditCommand();
255270
QApplication::restoreOverrideCursor();
256-
QMessageBox::critical( nullptr, tr( "Evaluation Error" ), exp.evalErrorString() );
257271
return;
258272
}
259-
}
260-
261-
if ( mAttributeId == -1 && !updatingGeom )
262-
{
263-
mVectorLayer->destroyEditCommand();
264-
QApplication::restoreOverrideCursor();
265-
return;
266-
}
267273

268-
//go through all the features and change the new attribute
269-
QgsFeature feature;
270-
bool calculationSuccess = true;
271-
QString error;
274+
//go through all the features and change the new attribute
275+
QgsFeature feature;
276+
bool calculationSuccess = true;
277+
QString error;
272278

273-
bool useGeometry = exp.needsGeometry();
274-
int rownum = 1;
279+
bool useGeometry = exp.needsGeometry();
280+
int rownum = 1;
275281

276-
QgsField field = !updatingGeom ? mVectorLayer->fields().at( mAttributeId ) : QgsField();
282+
QgsField field = !updatingGeom ? mVectorLayer->fields().at( mAttributeId ) : QgsField();
277283

278-
bool newField = !mUpdateExistingGroupBox->isChecked();
279-
QVariant emptyAttribute;
280-
if ( newField )
281-
emptyAttribute = QVariant( field.type() );
284+
bool newField = !mUpdateExistingGroupBox->isChecked();
285+
QVariant emptyAttribute;
286+
if ( newField )
287+
emptyAttribute = QVariant( field.type() );
282288

283-
QgsFeatureRequest req = QgsFeatureRequest().setFlags( useGeometry ? QgsFeatureRequest::NoFlags : QgsFeatureRequest::NoGeometry );
284-
if ( mOnlyUpdateSelectedCheckBox->isChecked() )
285-
{
286-
req.setFilterFids( mVectorLayer->selectedFeatureIds() );
287-
}
288-
QgsFeatureIterator fit = mVectorLayer->getFeatures( req );
289-
while ( fit.nextFeature( feature ) )
290-
{
291-
expContext.setFeature( feature );
292-
expContext.lastScope()->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "row_number" ), rownum, true ) );
293-
294-
QVariant value = exp.evaluate( &expContext );
295-
if ( exp.hasEvalError() )
289+
QgsFeatureRequest req = QgsFeatureRequest().setFlags( useGeometry ? QgsFeatureRequest::NoFlags : QgsFeatureRequest::NoGeometry );
290+
if ( mOnlyUpdateSelectedCheckBox->isChecked() )
296291
{
297-
calculationSuccess = false;
298-
error = exp.evalErrorString();
299-
break;
292+
req.setFilterFids( mVectorLayer->selectedFeatureIds() );
300293
}
301-
else if ( updatingGeom )
294+
QgsFeatureIterator fit = mVectorLayer->getFeatures( req );
295+
while ( fit.nextFeature( feature ) )
302296
{
303-
if ( value.canConvert< QgsGeometry >() )
297+
expContext.setFeature( feature );
298+
expContext.lastScope()->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "row_number" ), rownum, true ) );
299+
300+
QVariant value = exp.evaluate( &expContext );
301+
if ( exp.hasEvalError() )
302+
{
303+
calculationSuccess = false;
304+
error = exp.evalErrorString();
305+
break;
306+
}
307+
else if ( updatingGeom )
304308
{
305-
QgsGeometry geom = value.value< QgsGeometry >();
306-
mVectorLayer->changeGeometry( feature.id(), geom );
309+
if ( value.canConvert< QgsGeometry >() )
310+
{
311+
QgsGeometry geom = value.value< QgsGeometry >();
312+
mVectorLayer->changeGeometry( feature.id(), geom );
313+
}
314+
}
315+
else
316+
{
317+
( void )field.convertCompatible( value );
318+
mVectorLayer->changeAttributeValue( feature.id(), mAttributeId, value, newField ? emptyAttribute : feature.attributes().value( mAttributeId ) );
307319
}
308-
}
309-
else
310-
{
311-
( void )field.convertCompatible( value );
312-
mVectorLayer->changeAttributeValue( feature.id(), mAttributeId, value, newField ? emptyAttribute : feature.attributes().value( mAttributeId ) );
313-
}
314320

315-
rownum++;
316-
}
321+
rownum++;
322+
}
317323

318-
QApplication::restoreOverrideCursor();
324+
QApplication::restoreOverrideCursor();
319325

320-
if ( !calculationSuccess )
321-
{
322-
QMessageBox::critical( nullptr, tr( "Evaluation Error" ), tr( "An error occurred while evaluating the calculation string:\n%1" ).arg( error ) );
323-
mVectorLayer->destroyEditCommand();
324-
return;
326+
if ( !calculationSuccess )
327+
{
328+
QMessageBox::critical( nullptr, tr( "Evaluation Error" ), tr( "An error occurred while evaluating the calculation string:\n%1" ).arg( error ) );
329+
mVectorLayer->destroyEditCommand();
330+
return;
331+
}
332+
mVectorLayer->endEditCommand();
325333
}
326-
327-
mVectorLayer->endEditCommand();
328334
}
335+
// Vector signals unlocked! Tell the world that the layer has changed
336+
mVectorLayer->dataChanged();
329337
QDialog::accept();
330338
}
331339

0 commit comments

Comments
 (0)