Skip to content
Permalink
Browse files

Attribute table performance when deleting features

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 committed May 27, 2015
1 parent bb9d413 commit 8eca38ca5f05c8d930c8d82c200452d75fada5c0
@@ -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.
@@ -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
@@ -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() ) );

@@ -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 )
@@ -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 );
}
@@ -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 ) );
}
@@ -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;
@@ -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 );
@@ -145,6 +145,7 @@ QgsVectorLayer::QgsVectorLayer( QString vectorLayerPath,
, mValidExtent( false )
, mLazyExtent( true )
, mSymbolFeatureCounted( false )
, mEditCommandActive( false )
{
mActions = new QgsAttributeAction( this );

@@ -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 ) ) );
@@ -2783,6 +2784,7 @@ void QgsVectorLayer::beginEditCommand( QString text )
if ( !mDataProvider->transaction() )
{
undoStack()->beginMacro( text );
mEditCommandActive = true;
emit editCommandStarted( text );
}
}
@@ -2796,6 +2798,12 @@ void QgsVectorLayer::endEditCommand()
if ( !mDataProvider->transaction() )
{
undoStack()->endMacro();
mEditCommandActive = false;
if ( mDeletedFids.count() )
{
emit featuresDeleted( mDeletedFids );
mDeletedFids.clear();
}
emit editCommandEnded();
}
}
@@ -2810,6 +2818,8 @@ void QgsVectorLayer::destroyEditCommand()
{
undoStack()->endMacro();
undoStack()->undo();
mEditCommandActive = false;
mDeletedFids.clear();
emit editCommandDestroyed();
}
}
@@ -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" )
@@ -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.
@@ -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 */
@@ -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;
};

@@ -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() ) );
@@ -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 );
}
@@ -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;
}

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

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

mAttributeWidgetCaches.clear();
mAttributes.clear();
@@ -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
}
@@ -291,9 +339,7 @@ void QgsAttributeTableModel::loadLayer()

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

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

0 comments on commit 8eca38c

Please sign in to comment.
You can’t perform that action at this time.