Skip to content

Commit

Permalink
Merge pull request #8200 from elpaso/bugfix-20094-followup-096b4ce
Browse files Browse the repository at this point in the history
Slow field calculator - Bugfix 20094 followup 096b4ce
  • Loading branch information
elpaso authored Oct 18, 2018
2 parents 009fcd8 + 7e9c874 commit 4010e79
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 95 deletions.
2 changes: 0 additions & 2 deletions python/core/auto_generated/qgsapplication.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,6 @@ 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 @@ -853,7 +852,6 @@ In order to register translatable strings, connect to this signal and register t
.. versionadded:: 3.4
%End


};


Expand Down
13 changes: 3 additions & 10 deletions src/app/qgisapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7118,14 +7118,13 @@ void QgisApp::fieldCalculator()

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

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

QgsAttributeTableDialog *mDialog = new QgsAttributeTableDialog( myLayer, filter );
mDialog->show();
// the dialog will be deleted by itself on close
}
Expand Down Expand Up @@ -9217,7 +9216,6 @@ bool QgisApp::toggleEditing( QgsMapLayer *layer, bool allowCancel )
return res;
}


void QgisApp::saveActiveLayerEdits()
{
saveEdits( activeLayer(), true, true );
Expand Down Expand Up @@ -14175,8 +14173,3 @@ void QgisApp::triggerCrashHandler()
RaiseException( 0x12345678, 0, 0, nullptr );
#endif
}

void QgisApp::blockAttributeTableUpdates( const QgsVectorLayer *layer, const bool blocked )
{
emit attributeTableUpdateBlocked( layer, blocked );
}
14 changes: 0 additions & 14 deletions src/app/qgisapp.h
Original file line number Diff line number Diff line change
Expand Up @@ -1012,13 +1012,6 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
*/
void triggerCrashHandler();

/**
* Emits the signal to set the \a blocked state of attribute tables connected a particular \a layer
*
* \since QGIS 3.4
*/
void blockAttributeTableUpdates( const QgsVectorLayer *layer, const bool blocked );

protected:

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

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

private:
void startProfile( const QString &name );
void endProfile();
Expand Down
20 changes: 1 addition & 19 deletions src/app/qgsattributetabledialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,6 @@ 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, this, [ = ]( const QgsVectorLayer * layer, const bool blocked )
{
if ( layer == mLayer )
this->blockCacheUpdateSignals( blocked );
} );
// Massive rollbacks can also freeze the GUI due to the feature cache signals
connect( mLayer, &QgsVectorLayer::beforeRollBack, this, [ = ] { this->blockCacheUpdateSignals( true ); } );
connect( mLayer, &QgsVectorLayer::afterRollBack, this, [ = ] { this->blockCacheUpdateSignals( false ); } );

const QgsFields fields = mLayer->fields();
for ( const QgsField &field : fields )
{
Expand Down Expand Up @@ -743,6 +733,7 @@ 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 @@ -1154,15 +1145,6 @@ 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: 0 additions & 1 deletion src/app/qgsattributetabledialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ 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: 2 additions & 9 deletions src/app/qgsfieldcalculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,8 @@ 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 @@ -262,10 +260,6 @@ void QgsFieldCalculator::accept()
return;
}

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

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

QgisApp::instance()->blockAttributeTableUpdates( mVectorLayer, false );

if ( !calculationSuccess )
{
cursorOverride.release();
Expand All @@ -336,6 +328,7 @@ void QgsFieldCalculator::accept()
mVectorLayer->destroyEditCommand();
return;
}

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


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


private:

static void copyPath( const QString &src, const QString &dst );
Expand Down
92 changes: 65 additions & 27 deletions src/gui/attributetable/qgsattributetablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,19 @@ QgsAttributeTableModel::QgsAttributeTableModel( QgsVectorLayerCache *layerCache,

loadAttributes();

connect( mLayerCache, &QgsVectorLayerCache::attributeValueChanged, this, &QgsAttributeTableModel::attributeValueChanged );
connect( layer(), &QgsVectorLayer::featuresDeleted, this, &QgsAttributeTableModel::featuresDeleted );
connect( layer(), &QgsVectorLayer::attributeDeleted, this, &QgsAttributeTableModel::attributeDeleted );
connect( layer(), &QgsVectorLayer::updatedFields, this, &QgsAttributeTableModel::updatedFields );

connect( layer(), &QgsVectorLayer::editCommandStarted, this, &QgsAttributeTableModel::bulkEditCommandStarted );
connect( layer(), &QgsVectorLayer::beforeRollBack, this, &QgsAttributeTableModel::bulkEditCommandStarted );
connect( layer(), &QgsVectorLayer::afterRollBack, this, &QgsAttributeTableModel::bulkEditCommandEnded );

connect( layer(), &QgsVectorLayer::editCommandEnded, this, &QgsAttributeTableModel::editCommandEnded );
connect( mLayerCache, &QgsVectorLayerCache::attributeValueChanged, this, &QgsAttributeTableModel::attributeValueChanged );
connect( mLayerCache, &QgsVectorLayerCache::featureAdded, this, [ = ]( QgsFeatureId id ) { featureAdded( id ); } );
connect( mLayerCache, &QgsVectorLayerCache::cachedLayerDeleted, this, &QgsAttributeTableModel::layerDeleted );

}

bool QgsAttributeTableModel::loadFeatureAtId( QgsFeatureId fid ) const
Expand Down Expand Up @@ -255,7 +261,7 @@ void QgsAttributeTableModel::editCommandEnded()
{
// do not do reload(...) due would trigger (dataChanged) row sort
// giving issue: https://issues.qgis.org/issues/15976
mChangedCellBounds = QRect();
bulkEditCommandEnded( );
}

void QgsAttributeTableModel::attributeDeleted( int idx )
Expand Down Expand Up @@ -294,7 +300,13 @@ void QgsAttributeTableModel::fieldFormatterRemoved( QgsFieldFormatter *fieldForm

void QgsAttributeTableModel::attributeValueChanged( QgsFeatureId fid, int idx, const QVariant &value )
{
QgsDebugMsgLevel( QStringLiteral( "(%4) fid: %1, idx: %2, value: %3" ).arg( fid ).arg( idx ).arg( value.toString() ).arg( mFeatureRequest.filterType() ), 3 );
// Defer all updates if a bulk edit/rollback command is running
if ( mBulkEditCommandRunning )
{
mAttributeValueChanges.insert( QPair<QgsFeatureId, int>( fid, idx ), value );
return;
}
QgsDebugMsgLevel( QStringLiteral( "(%4) fid: %1, idx: %2, value: %3" ).arg( fid ).arg( idx ).arg( value.toString() ).arg( mFeatureRequest.filterType() ), 2 );

for ( SortCache &cache : mSortCaches )
{
Expand Down Expand Up @@ -731,30 +743,6 @@ bool QgsAttributeTableModel::setData( const QModelIndex &index, const QVariant &
if ( !layer()->isModified() )
return false;

if ( mChangedCellBounds.isNull() )
{
mChangedCellBounds = QRect( index.column(), index.row(), 1, 1 );
}
else
{
if ( index.column() < mChangedCellBounds.left() )
{
mChangedCellBounds.setLeft( index.column() );
}
if ( index.row() < mChangedCellBounds.top() )
{
mChangedCellBounds.setTop( index.row() );
}
if ( index.column() > mChangedCellBounds.right() )
{
mChangedCellBounds.setRight( index.column() );
}
if ( index.row() > mChangedCellBounds.bottom() )
{
mChangedCellBounds.setBottom( index.row() );
}
}

mRowStylesMap.remove( index.row() );

return true;
Expand Down Expand Up @@ -797,6 +785,56 @@ bool QgsAttributeTableModel::fieldIsEditable( const QgsVectorLayer &layer, int f
( ( layer.dataProvider() && layer.dataProvider()->capabilities() & QgsVectorDataProvider::ChangeAttributeValues ) || FID_IS_NEW( fid ) ) );
}

void QgsAttributeTableModel::bulkEditCommandStarted()
{
mBulkEditCommandRunning = true;
mAttributeValueChanges.clear();
}

void QgsAttributeTableModel::bulkEditCommandEnded()
{
mBulkEditCommandRunning = false;
// Full model update if the changed rows are more than half the total rows
// or if their count is > layer cache size
int changeCount( mAttributeValueChanges.count() );
bool fullModelUpdate = changeCount > mLayerCache->cacheSize() ||
changeCount > rowCount() * 0.5;

QgsDebugMsgLevel( QStringLiteral( "Bulk edit command ended with %1 modified rows over (%4), cache size is %2, starting %3 update." )
.arg( changeCount )
.arg( mLayerCache->cacheSize() )
.arg( fullModelUpdate ? QStringLiteral( "full" ) : QStringLiteral( "incremental" ) )
.arg( rowCount() ),
3 );
// Invalidates the whole model
if ( fullModelUpdate )
{
// Invalidates the cache (there is no API for doing this directly)
mLayerCache->layer()->dataChanged();
emit dataChanged( createIndex( 0, 0 ), createIndex( rowCount() - 1, columnCount() - 1 ) );
}
else
{
int minRow = rowCount();
int minCol = columnCount();
int maxRow = 0;
int maxCol = 0;
const auto keys = mAttributeValueChanges.keys();
for ( const auto &key : keys )
{
attributeValueChanged( key.first, key.second, mAttributeValueChanges.value( key ) );
int row( idToRow( key.first ) );
int col( fieldCol( key.second ) );
minRow = std::min<int>( row, minRow );
minCol = std::min<int>( col, minCol );
maxRow = std::max<int>( row, maxRow );
maxCol = std::max<int>( col, maxCol );
}
emit dataChanged( createIndex( minRow, minCol ), createIndex( maxRow, maxCol ) );
}
mAttributeValueChanges.clear();
}

void QgsAttributeTableModel::reload( const QModelIndex &index1, const QModelIndex &index2 )
{
mFeat.setId( std::numeric_limits<int>::min() );
Expand Down
23 changes: 13 additions & 10 deletions src/gui/attributetable/qgsattributetablemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ class GUI_EXPORT QgsAttributeTableModel: public QAbstractTableModel
virtual void attributeValueChanged( QgsFeatureId fid, int idx, const QVariant &value );

/**
* Launched when eatures have been deleted
* Launched when features have been deleted
* \param fids feature ids
*/
virtual void featuresDeleted( const QgsFeatureIds &fids );
Expand Down Expand Up @@ -378,19 +378,22 @@ class GUI_EXPORT QgsAttributeTableModel: public QAbstractTableModel

std::vector<SortCache> mSortCaches;

/**
* Holds the bounds of changed cells while an update operation is running
* top = min row
* left = min column
* bottom = max row
* right = max column
*/
QRect mChangedCellBounds;

QgsAttributeEditorContext mEditorContext;

int mExtraColumns = 0;

//! Flag for massive changes operations, set by edit command or rollback
bool mBulkEditCommandRunning = false;

//! Sets the flag for massive changes operations
void bulkEditCommandStarted();

//! Clears the flag for massive changes operations, updates/rebuilds the layer cache and tells the view to update
void bulkEditCommandEnded();

//! Changed attribute values within a bulk edit command
QMap<QPair<QgsFeatureId, int>, QVariant> mAttributeValueChanges;

friend class TestQgsAttributeTable;

};
Expand Down

0 comments on commit 4010e79

Please sign in to comment.