Skip to content

Commit

Permalink
Attribute table performance when deleting features
Browse files Browse the repository at this point in the history 
This fixes performance issues with the attribute table visible when deleting a
large number of features.

The attribute table tries to behave smart in the following way:

 * It tries to remove only the deleted rows as long as they are in one or a few
   single blocks
 * If there are more than 100 rows to delete and it starts to delete blocks
   of a size smaller than 10 it assumes that the selection to delete is widely
   distributed and that a reload of the whole model is less expensive than a
   differential update.

Fix #10167
  • Loading branch information
@m-kuhn
m-kuhn committed May 27, 2015
1 parent bb9d413 commit 8eca38c
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 47 deletions.
10 changes: 10 additions & 0 deletions python/core/qgsvectorlayer.sip
View file
Expand Up @@ -1292,6 +1292,16 @@ class QgsVectorLayer : QgsMapLayer
void attributeDeleted( int idx );
void featureAdded( QgsFeatureId fid );
void featureDeleted( QgsFeatureId fid );
/**
* Emitted when features have been deleted.
*
* If features are deleted within an edit command, this will only be emitted once at the end
* to allow connected slots to minimize the overhead.
* If features are delted outside of an edit command, this signal will be emitted once per feature.
*
* @param fids The feature ids that have been deleted.
*/
void featuresDeleted( QgsFeatureIds fids );
/**
* Is emitted, whenever the fields available from this layer have been changed.
* This can be due to manually adding attributes or due to a join.
Expand Down
6 changes: 3 additions & 3 deletions python/gui/attributetable/qgsattributetablemodel.sip
View file
Expand Up @@ -198,10 +198,10 @@ class QgsAttributeTableModel : QAbstractTableModel
*/
virtual void attributeValueChanged( QgsFeatureId fid, int idx, const QVariant &value );
/**
* Launched when a feature has been deleted
* @param fid feature id
* Launched when eatures have been deleted
* @param fids feature ids
*/
virtual void featureDeleted( QgsFeatureId fid );
virtual void featuresDeleted( QgsFeatureIds fid );
/**
* Launched when a feature has been added
* @param fid feature id
Expand Down
34 changes: 18 additions & 16 deletions src/app/qgsattributetabledialog.cpp
View file
Expand Up @@ -139,6 +139,8 @@ QgsAttributeTableDialog::QgsAttributeTableDialog( QgsVectorLayer *theLayer, QWid
connect( mLayer, SIGNAL( editingStopped() ), this, SLOT( editingToggled() ) );
connect( mLayer, SIGNAL( layerDeleted() ), this, SLOT( close() ) );
connect( mLayer, SIGNAL( selectionChanged() ), this, SLOT( updateTitle() ) );
connect( mLayer, SIGNAL( featureAdded( QgsFeatureId ) ), this, SLOT( updateTitle() ) );
connect( mLayer, SIGNAL( featuresDeleted( QgsFeatureIds ) ), this, SLOT( updateTitle() ) );
connect( mLayer, SIGNAL( attributeAdded( int ) ), this, SLOT( columnBoxInit() ) );
connect( mLayer, SIGNAL( attributeDeleted( int ) ), this, SLOT( columnBoxInit() ) );

Expand Down Expand Up @@ -405,12 +407,12 @@ void QgsAttributeTableDialog::runFieldCalculation( QgsVectorLayer* layer, QStrin
mLayer->endEditCommand();
}

void QgsAttributeTableDialog::replaceSearchWidget(QWidget* oldw, QWidget* neww)
void QgsAttributeTableDialog::replaceSearchWidget( QWidget* oldw, QWidget* neww )
{
mFilterLayout->removeWidget(oldw);
oldw->setVisible(false);
mFilterLayout->addWidget(neww,0,0,0);
neww->setVisible(true);
mFilterLayout->removeWidget( oldw );
oldw->setVisible( false );
mFilterLayout->addWidget( neww, 0, 0, 0 );
neww->setVisible( true );
}

void QgsAttributeTableDialog::filterColumnChanged( QObject* filterAction )
Expand All @@ -427,14 +429,14 @@ void QgsAttributeTableDialog::filterColumnChanged( QObject* filterAction )
QString fieldName = mFilterButton->defaultAction()->text();
// get the search widget
int fldIdx = mLayer->fieldNameIndex( fieldName );
if ( fldIdx < 0 )
return;
if ( fldIdx < 0 )
return;
const QString widgetType = mLayer->editorWidgetV2( fldIdx );
const QgsEditorWidgetConfig widgetConfig = mLayer->editorWidgetV2Config( fldIdx );
mCurrentSearchWidgetWrapper= QgsEditorWidgetRegistry::instance()->
createSearchWidget(widgetType, mLayer, fldIdx, widgetConfig, mFilterContainer);
mCurrentSearchWidgetWrapper = QgsEditorWidgetRegistry::instance()->
createSearchWidget( widgetType, mLayer, fldIdx, widgetConfig, mFilterContainer );

replaceSearchWidget(mFilterQuery, mCurrentSearchWidgetWrapper->widget());
replaceSearchWidget( mFilterQuery, mCurrentSearchWidgetWrapper->widget() );

mApplyFilterButton->setVisible( true );
}
Expand Down Expand Up @@ -723,7 +725,7 @@ void QgsAttributeTableDialog::filterQueryChanged( const QString& query )
QString nullValue = settings.value( "qgis/nullValue", "NULL" ).toString();
QString value = mCurrentSearchWidgetWrapper->value().toString();

if ( value == nullValue )
if ( value == nullValue )
{
str = QString( "%1 IS NULL" ).arg( QgsExpression::quotedColumnRef( fieldName ) );
}
Expand All @@ -745,9 +747,9 @@ void QgsAttributeTableDialog::filterQueryChanged( const QString& query )

void QgsAttributeTableDialog::filterQueryAccepted()
{
if ( (mFilterQuery->isVisible() && mFilterQuery->text().isEmpty()) ||
(mCurrentSearchWidgetWrapper!=0 && mCurrentSearchWidgetWrapper->widget()->isVisible()
&& mCurrentSearchWidgetWrapper->value().toString().isEmpty() ))
if (( mFilterQuery->isVisible() && mFilterQuery->text().isEmpty() ) ||
( mCurrentSearchWidgetWrapper != 0 && mCurrentSearchWidgetWrapper->widget()->isVisible()
&& mCurrentSearchWidgetWrapper->value().toString().isEmpty() ) )
{
filterShowAll();
return;
Expand All @@ -763,9 +765,9 @@ void QgsAttributeTableDialog::setFilterExpression( QString filterString )
mCbxCaseSensitive->setVisible( false );

mFilterQuery->setVisible( true );
if ( mCurrentSearchWidgetWrapper != 0 )
if ( mCurrentSearchWidgetWrapper != 0 )
{
replaceSearchWidget(mCurrentSearchWidgetWrapper->widget(),mFilterQuery);
replaceSearchWidget( mCurrentSearchWidgetWrapper->widget(), mFilterQuery );
}
mApplyFilterButton->setVisible( true );
mMainView->setFilterMode( QgsAttributeTableFilterModel::ShowFilteredList );
Expand Down
22 changes: 21 additions & 1 deletion src/core/qgsvectorlayer.cpp
View file
Expand Up @@ -145,6 +145,7 @@ QgsVectorLayer::QgsVectorLayer( QString vectorLayerPath,
, mValidExtent( false )
, mLazyExtent( true )
, mSymbolFeatureCounted( false )
, mEditCommandActive( false )
{
mActions = new QgsAttributeAction( this );

Expand Down Expand Up @@ -1210,7 +1211,7 @@ bool QgsVectorLayer::startEditing()
connect( mEditBuffer, SIGNAL( layerModified() ), this, SIGNAL( layerModified() ) ); // TODO[MD]: necessary?
//connect( mEditBuffer, SIGNAL( layerModified() ), this, SLOT( triggerRepaint() ) ); // TODO[MD]: works well?
connect( mEditBuffer, SIGNAL( featureAdded( QgsFeatureId ) ), this, SIGNAL( featureAdded( QgsFeatureId ) ) );
connect( mEditBuffer, SIGNAL( featureDeleted( QgsFeatureId ) ), this, SIGNAL( featureDeleted( QgsFeatureId ) ) );
connect( mEditBuffer, SIGNAL( featureDeleted( QgsFeatureId ) ), this, SLOT( onFeatureDeleted( QgsFeatureId ) ) );
connect( mEditBuffer, SIGNAL( geometryChanged( QgsFeatureId, QgsGeometry& ) ), this, SIGNAL( geometryChanged( QgsFeatureId, QgsGeometry& ) ) );
connect( mEditBuffer, SIGNAL( attributeValueChanged( QgsFeatureId, int, QVariant ) ), this, SIGNAL( attributeValueChanged( QgsFeatureId, int, QVariant ) ) );
connect( mEditBuffer, SIGNAL( attributeAdded( int ) ), this, SIGNAL( attributeAdded( int ) ) );
Expand Down Expand Up @@ -2783,6 +2784,7 @@ void QgsVectorLayer::beginEditCommand( QString text )
if ( !mDataProvider->transaction() )
{
undoStack()->beginMacro( text );
mEditCommandActive = true;
emit editCommandStarted( text );
}
}
Expand All @@ -2796,6 +2798,12 @@ void QgsVectorLayer::endEditCommand()
if ( !mDataProvider->transaction() )
{
undoStack()->endMacro();
mEditCommandActive = false;
if ( mDeletedFids.count() )
{
emit featuresDeleted( mDeletedFids );
mDeletedFids.clear();
}
emit editCommandEnded();
}
}
Expand All @@ -2810,6 +2818,8 @@ void QgsVectorLayer::destroyEditCommand()
{
undoStack()->endMacro();
undoStack()->undo();
mEditCommandActive = false;
mDeletedFids.clear();
emit editCommandDestroyed();
}
}
Expand Down Expand Up @@ -3762,6 +3772,16 @@ void QgsVectorLayer::onJoinedFieldsChanged()
updateFields();
}

void QgsVectorLayer::onFeatureDeleted( const QgsFeatureId& fid )
{
if ( mEditCommandActive )
mDeletedFids << fid;
else
emit featuresDeleted( QgsFeatureIds() << fid );

emit featureDeleted( fid );
}

QgsVectorLayer::ValueRelationData QgsVectorLayer::valueRelation( int idx )
{
if ( editorWidgetV2( idx ) == "ValueRelation" )
Expand Down
29 changes: 29 additions & 0 deletions src/core/qgsvectorlayer.h
View file
Expand Up @@ -1656,8 +1656,31 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
* @see updatedFields()
*/
void attributeDeleted( int idx );
/**
* Emitted when a new feature has been added to the layer
*
* @param fid The id of the new feature
*/
void featureAdded( QgsFeatureId fid );
/**
* Emitted when a feature has been deleted.
*
* If you do expensive operations in a slot connected to this, you should prever to use
* {@link featuresDeleted(QgsFeatureIds)}.
*
* @param fid The id of the feature which has been deleted
*/
void featureDeleted( QgsFeatureId fid );
/**
* Emitted when features have been deleted.
*
* If features are deleted within an edit command, this will only be emitted once at the end
* to allow connected slots to minimize the overhead.
* If features are delted outside of an edit command, this signal will be emitted once per feature.
*
* @param fids The feature ids that have been deleted.
*/
void featuresDeleted( QgsFeatureIds fids );
/**
* Is emitted, whenever the fields available from this layer have been changed.
* This can be due to manually adding attributes or due to a join.
Expand Down Expand Up @@ -1734,6 +1757,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
private slots:
void onRelationsLoaded();
void onJoinedFieldsChanged();
void onFeatureDeleted( const QgsFeatureId& fid );

protected:
/** Set the extent */
Expand Down Expand Up @@ -1896,6 +1920,11 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
// Feature counts for each renderer symbol
QMap<QgsSymbolV2*, long> mSymbolFeatureCountMap;

//! True while an undo command is active
bool mEditCommandActive;

QgsFeatureIds mDeletedFids;

friend class QgsVectorLayerFeatureSource;
};

Expand Down
95 changes: 71 additions & 24 deletions src/gui/attributetable/qgsattributetablemodel.cpp
View file
Expand Up @@ -53,7 +53,7 @@ QgsAttributeTableModel::QgsAttributeTableModel( QgsVectorLayerCache *layerCache,
loadAttributes();

connect( mLayerCache, SIGNAL( attributeValueChanged( QgsFeatureId, int, const QVariant& ) ), this, SLOT( attributeValueChanged( QgsFeatureId, int, const QVariant& ) ) );
connect( layer(), SIGNAL( featureDeleted( QgsFeatureId ) ), this, SLOT( featureDeleted( QgsFeatureId ) ) );
connect( layer(), SIGNAL( featuresDeleted( QgsFeatureIds ) ), this, SLOT( featuresDeleted( QgsFeatureIds ) ) );
connect( layer(), SIGNAL( attributeDeleted( int ) ), this, SLOT( attributeDeleted( int ) ) );
connect( layer(), SIGNAL( updatedFields() ), this, SLOT( updatedFields() ) );
connect( layer(), SIGNAL( editCommandEnded() ), this, SLOT( editCommandEnded() ) );
Expand All @@ -73,29 +73,74 @@ bool QgsAttributeTableModel::loadFeatureAtId( QgsFeatureId fid ) const
return mLayerCache->featureAtId( fid, mFeat );
}

void QgsAttributeTableModel::featureDeleted( QgsFeatureId fid )
void QgsAttributeTableModel::featuresDeleted( QgsFeatureIds fids )
{
QgsDebugMsgLevel( QString( "(%2) fid: %1" ).arg( fid ).arg( mFeatureRequest.filterType() ), 4 );
mFieldCache.remove( fid );
QList<int> rows;

Q_FOREACH ( const QgsFeatureId& fid, fids )
{
QgsDebugMsgLevel( QString( "(%2) fid: %1" ).arg( fid ).arg( mFeatureRequest.filterType() ), 4 );

int row = idToRow( fid );
int row = idToRow( fid );
if ( row != -1 )
rows << row;
}

if ( row != -1 )
qSort( rows );

int lastRow = -1;
int beginRow = -1;
int currentRowCount = 0;
int removedRows = 0;
bool reset = false;

Q_FOREACH ( int row, rows )
{
beginRemoveRows( QModelIndex(), row, row );
removeRow( row );
endRemoveRows();
#if 0
qDebug() << "Row: " << row << ", begin " << beginRow << ", last " << lastRow << ", current " << currentRowCount << ", removed " << removedRows;
#endif
if ( lastRow == -1 )
{
beginRow = row;
}

if ( row != lastRow + 1 && lastRow != -1 )
{
if ( rows.count() > 100 && currentRowCount < 10 )
{
reset = true;
break;
}
removeRows( beginRow - removedRows, currentRowCount );

beginRow = row;
removedRows += currentRowCount;
currentRowCount = 0;
}

currentRowCount++;

lastRow = row;
}

if ( !reset )
removeRows( beginRow - removedRows, currentRowCount );
else
resetModel();
}

bool QgsAttributeTableModel::removeRows( int row, int count, const QModelIndex &parent )
{
Q_UNUSED( parent );
QgsDebugMsgLevel( QString( "remove %2 rows at %1 (rows %3, ids %4)" ).arg( row ).arg( count ).arg( mRowIdMap.size() ).arg( mIdRowMap.size() ), 3 );
beginRemoveRows( parent, row, row + count );
#ifdef QGISDEBUG
if ( 3 > QgsLogger::debugLevel() )
QgsDebugMsgLevel( QString( "remove %2 rows at %1 (rows %3, ids %4)" ).arg( row ).arg( count ).arg( mRowIdMap.size() ).arg( mIdRowMap.size() ), 3 );
#endif

// clean old references
for ( int i = row; i < row + count; i++ )
{
mFieldCache.remove( mRowIdMap[i] );
mIdRowMap.remove( mRowIdMap[ i ] );
mRowIdMap.remove( i );
}
Expand All @@ -111,20 +156,25 @@ bool QgsAttributeTableModel::removeRows( int row, int count, const QModelIndex &
}

#ifdef QGISDEBUG
QgsDebugMsgLevel( QString( "after removal rows %1, ids %2" ).arg( mRowIdMap.size() ).arg( mIdRowMap.size() ), 4 );
QgsDebugMsgLevel( "id->row", 4 );
for ( QHash<QgsFeatureId, int>::iterator it = mIdRowMap.begin(); it != mIdRowMap.end(); ++it )
QgsDebugMsgLevel( QString( "%1->%2" ).arg( FID_TO_STRING( it.key() ) ).arg( *it ), 4 );
if ( 4 > QgsLogger::debugLevel() )
{
QgsDebugMsgLevel( QString( "after removal rows %1, ids %2" ).arg( mRowIdMap.size() ).arg( mIdRowMap.size() ), 4 );
QgsDebugMsgLevel( "id->row", 4 );
for ( QHash<QgsFeatureId, int>::iterator it = mIdRowMap.begin(); it != mIdRowMap.end(); ++it )
QgsDebugMsgLevel( QString( "%1->%2" ).arg( FID_TO_STRING( it.key() ) ).arg( *it ), 4 );

QHash<QgsFeatureId, int>::iterator idit;
QHash<QgsFeatureId, int>::iterator idit;

QgsDebugMsgLevel( "row->id", 4 );
for ( QHash<int, QgsFeatureId>::iterator it = mRowIdMap.begin(); it != mRowIdMap.end(); ++it )
QgsDebugMsgLevel( QString( "%1->%2" ).arg( it.key() ).arg( FID_TO_STRING( *it ) ), 4 );
QgsDebugMsgLevel( "row->id", 4 );
for ( QHash<int, QgsFeatureId>::iterator it = mRowIdMap.begin(); it != mRowIdMap.end(); ++it )
QgsDebugMsgLevel( QString( "%1->%2" ).arg( it.key() ).arg( FID_TO_STRING( *it ) ), 4 );
}
#endif

Q_ASSERT( mRowIdMap.size() == mIdRowMap.size() );

endRemoveRows();

return true;
}

Expand Down Expand Up @@ -179,9 +229,7 @@ void QgsAttributeTableModel::layerDeleted()
{
QgsDebugMsg( "entered." );

beginRemoveRows( QModelIndex(), 0, rowCount() - 1 );
removeRows( 0, rowCount() );
endRemoveRows();

mAttributeWidgetCaches.clear();
mAttributes.clear();
Expand Down Expand Up @@ -223,7 +271,7 @@ void QgsAttributeTableModel::attributeValueChanged( QgsFeatureId fid, int idx, c
if ( mIdRowMap.contains( fid ) )
{
// Feature changed such, that it is no longer shown
featureDeleted( fid );
featuresDeleted( QgsFeatureIds() << fid );
}
// else: we don't care
}
Expand Down Expand Up @@ -291,9 +339,7 @@ void QgsAttributeTableModel::loadLayer()

if ( rowCount() != 0 )
{
beginRemoveRows( QModelIndex(), 0, rowCount() - 1 );
removeRows( 0, rowCount() );
endRemoveRows();
}

QgsFeatureIterator features = mLayerCache->getFeatures( mFeatureRequest );
Expand Down Expand Up @@ -575,6 +621,7 @@ void QgsAttributeTableModel::reload( const QModelIndex &index1, const QModelInde
void QgsAttributeTableModel::resetModel()
{
beginResetModel();
loadLayer();
endResetModel();
}

Expand Down

0 comments on commit 8eca38c

Please sign in to comment.