Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes slow update in field calculator #8177

Merged
merged 6 commits into from
Oct 15, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions python/core/auto_generated/qgsapplication.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,7 @@ Emits the signal to collect all the strings of .qgs to be included in ts file
.. versionadded:: 3.4
%End


%If (ANDROID)
//dummy method to workaround sip generation issue
bool x11EventFilter( XEvent *event );
Expand Down Expand Up @@ -852,6 +853,7 @@ In order to register translatable strings, connect to this signal and register t
.. versionadded:: 3.4
%End


};


Expand Down
7 changes: 7 additions & 0 deletions python/core/auto_generated/qgsvectorlayer.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -2371,6 +2371,13 @@ Is emitted, before changes are committed to the data provider
void beforeRollBack();
%Docstring
Is emitted, before changes are rolled back
%End

void afterRollBack();
%Docstring
Is emitted, after changes are rolled back

.. versionadded:: 3.4
%End

void attributeAdded( int idx );
Expand Down
18 changes: 15 additions & 3 deletions src/app/qgisapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7090,13 +7090,14 @@ void QgisApp::fieldCalculator()

void QgisApp::attributeTable( QgsAttributeTableFilterModel::FilterMode filter )
{
QgsVectorLayer *myLayer = qobject_cast<QgsVectorLayer *>( activeLayer() );
if ( !myLayer )
QgsVectorLayer *vectorLayer = qobject_cast<QgsVectorLayer *>( activeLayer() );
if ( !vectorLayer )
{
return;
}

QgsAttributeTableDialog *mDialog = new QgsAttributeTableDialog( myLayer, filter );
QgsAttributeTableDialog *mDialog = new QgsAttributeTableDialog( vectorLayer, filter );

mDialog->show();
// the dialog will be deleted by itself on close
}
Expand Down Expand Up @@ -9188,6 +9189,17 @@ bool QgisApp::toggleEditing( QgsMapLayer *layer, bool allowCancel )
return res;
}


void QgisApp::unblockAttributeTableUpdates( const QgsVectorLayer *layer )
{
emit attributeTableUpdateUnblocked( layer );
}

void QgisApp::blockAttributeTableUpdates( const QgsVectorLayer *layer )
{
emit attributeTableUpdateBlocked( layer );
}

void QgisApp::saveActiveLayerEdits()
{
saveEdits( activeLayer(), true, true );
Expand Down
28 changes: 28 additions & 0 deletions src/app/qgisapp.h
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,20 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
*/
void triggerCrashHandler();

/**
* Emits the signal to block updates for attribute tables connected a particular \a layer
*
* \since QGIS 3.4
*/
void blockAttributeTableUpdates( const QgsVectorLayer *layer );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could use one signal and one slot here with a bool blocked argument

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I first implemented it like that. But I was in doubt, because in general we don't like bool switches, do we?
But you solved my doubt, so I'm fine with the bool now. (I've always thought that blockSignals(bool) was not very consistent with the rest of Qt API).


/**
* Emits the signal to unblock updates for attribute tables connected a particular \a layer
*
* \since QGIS 3.4
*/
void unblockAttributeTableUpdates( const QgsVectorLayer *layer );

protected:

//! Handle state changes (WindowTitleChange)
Expand Down Expand Up @@ -1741,6 +1755,20 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
*/
void activeLayerChanged( QgsMapLayer *layer );

/**
* Emitted when attribute table updates for a particular \a layer must be blocked
*
* \since QGIS 3.4
*/
void attributeTableUpdateBlocked( const QgsVectorLayer *layer );

/**
* Emitted when all attribute table updates for a particular \a layer must be unblocked
*
* \since QGIS 3.4
*/
void attributeTableUpdateUnblocked( const QgsVectorLayer *layer );

private:
void startProfile( const QString &name );
void endProfile();
Expand Down
25 changes: 24 additions & 1 deletion src/app/qgsattributetabledialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,21 @@ QgsAttributeTableDialog::QgsAttributeTableDialog( QgsVectorLayer *layer, QgsAttr
connect( mActionExpressionSelect, &QAction::triggered, this, &QgsAttributeTableDialog::mActionExpressionSelect_triggered );
connect( mMainView, &QgsDualView::showContextMenuExternally, this, &QgsAttributeTableDialog::showContextMenu );

// Block/unblock table updates (feature cache signals)
connect( QgisApp::instance(), &QgisApp::attributeTableUpdateBlocked, [ = ]( const QgsVectorLayer * layer )
{
if ( layer == mLayer )
this->blockCacheUpdateSignals( true );
} );
connect( QgisApp::instance(), &QgisApp::attributeTableUpdateUnblocked, [ = ]( const QgsVectorLayer * layer )
{
if ( layer == mLayer )
this->blockCacheUpdateSignals( false );
} );
// Massive rollbacks can also freeze the GUI due to the feature cache signals
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the same issue with attribute addition/deletion too, don't we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I you mean removing the columns as a whole, apparently not, at least I didn't observe the issue when removing the z field, I'll double check it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed: with my PR removing a colum is FAST.

connect( mLayer, &QgsVectorLayer::beforeRollBack, [ = ] { this->blockCacheUpdateSignals( true ); } );
connect( mLayer, &QgsVectorLayer::afterRollBack, [ = ] { this->blockCacheUpdateSignals( false ); } );

const QgsFields fields = mLayer->fields();
for ( const QgsField &field : fields )
{
Expand Down Expand Up @@ -718,7 +733,6 @@ void QgsAttributeTableDialog::mActionOpenFieldCalculator_triggered()
if ( calc.exec() == QDialog::Accepted )
{
int col = masterModel->fieldCol( calc.changedAttributeId() );

if ( col >= 0 )
{
masterModel->reload( masterModel->index( 0, col ), masterModel->index( masterModel->rowCount() - 1, col ) );
Expand Down Expand Up @@ -1130,6 +1144,15 @@ void QgsAttributeTableDialog::setFilterExpression( const QString &filterString,
mMainView->setFilterMode( QgsAttributeTableFilterModel::ShowFilteredList );
}

void QgsAttributeTableDialog::blockCacheUpdateSignals( const bool block )
{
QgsAttributeTableModel *masterModel = mMainView->masterModel();

if ( ! masterModel )
return;

masterModel->layerCache()->blockSignals( block );
}

void QgsAttributeTableDialog::deleteFeature( const QgsFeatureId fid )
{
Expand Down
1 change: 1 addition & 0 deletions src/app/qgsattributetabledialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ class APP_EXPORT QgsAttributeTableDialog : public QDialog, private Ui::QgsAttrib

void updateMultiEditButtonState();
void deleteFeature( QgsFeatureId fid );
void blockCacheUpdateSignals( const bool block );

friend class TestQgsAttributeTable;
};
Expand Down
11 changes: 9 additions & 2 deletions src/app/qgsfieldcalculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,10 @@ void QgsFieldCalculator::accept()
{
builder->saveToRecent( QStringLiteral( "fieldcalc" ) );

if ( !mVectorLayer )
if ( ! mVectorLayer )
{
return;
}

// Set up QgsDistanceArea each time we (re-)calculate
QgsDistanceArea myDa;
Expand Down Expand Up @@ -265,6 +267,10 @@ void QgsFieldCalculator::accept()
return;
}

// Begin feature modifications, block updates for attr tables
// connected to this layer
QgisApp::instance()->blockAttributeTableUpdates( mVectorLayer );

//go through all the features and change the new attribute
QgsFeature feature;
bool calculationSuccess = true;
Expand Down Expand Up @@ -315,6 +321,8 @@ void QgsFieldCalculator::accept()
rownum++;
}

QgisApp::instance()->unblockAttributeTableUpdates( mVectorLayer );

QApplication::restoreOverrideCursor();

if ( !calculationSuccess )
Expand All @@ -323,7 +331,6 @@ void QgsFieldCalculator::accept()
mVectorLayer->destroyEditCommand();
return;
}

mVectorLayer->endEditCommand();
}
QDialog::accept();
Expand Down
1 change: 1 addition & 0 deletions src/core/qgsapplication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,7 @@ void QgsApplication::collectTranslatableObjects( QgsTranslationContext *translat
emit requestForTranslatableObjects( translationContext );
}


QString QgsApplication::nullRepresentation()
{
ApplicationMembers *appMembers = members();
Expand Down
2 changes: 2 additions & 0 deletions src/core/qgsapplication.h
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,7 @@ class CORE_EXPORT QgsApplication : public QApplication
*/
void collectTranslatableObjects( QgsTranslationContext *translationContext );


#ifdef SIP_RUN
SIP_IF_FEATURE( ANDROID )
//dummy method to workaround sip generation issue
Expand Down Expand Up @@ -788,6 +789,7 @@ class CORE_EXPORT QgsApplication : public QApplication
*/
void requestForTranslatableObjects( QgsTranslationContext *translationContext );


private:

static void copyPath( const QString &src, const QString &dst );
Expand Down
2 changes: 2 additions & 0 deletions src/core/qgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2973,6 +2973,8 @@ bool QgsVectorLayer::rollBack( bool deleteBuffer )

mEditBuffer->rollBack();

emit afterRollBack();

if ( isModified() )
{
// new undo stack roll back method
Expand Down
6 changes: 6 additions & 0 deletions src/core/qgsvectorlayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -2167,6 +2167,12 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
//! Is emitted, before changes are rolled back
void beforeRollBack();

/**
* Is emitted, after changes are rolled back
* \since QGIS 3.4
*/
void afterRollBack();

/**
* Will be emitted, when a new attribute has been added to this vector layer.
* Applies only to types QgsFields::OriginEdit, QgsFields::OriginProvider and QgsFields::OriginExpression
Expand Down