diff --git a/app/layerfeaturesmodel.cpp b/app/layerfeaturesmodel.cpp index 10eab35d8..b7f39cf55 100644 --- a/app/layerfeaturesmodel.cpp +++ b/app/layerfeaturesmodel.cpp @@ -9,13 +9,16 @@ #include "layerfeaturesmodel.h" -#include "inpututils.h" -#include "qgsproject.h" -#include "qgsvectorlayerfeatureiterator.h" +#include "featurelayerpair.h" + +#include "qgsattributetableconfig.h" +#include "qgsfeaturerequest.h" #include "qgsfeedback.h" +#include "qgsvectorlayer.h" +#include "qgsvectorlayerfeatureiterator.h" #include -#include +#include #include @@ -23,14 +26,29 @@ LayerFeaturesModel::LayerFeaturesModel( QObject *parent ) : FeaturesModel( parent ), mLayer( nullptr ) { - mFeedback = std::make_unique(); - connect( &mSearchResultWatcher, &QFutureWatcher::finished, this, &LayerFeaturesModel::onFutureFinished ); } LayerFeaturesModel::~LayerFeaturesModel() { - // cancel any long running request - mFeedback->cancel(); + for ( auto [id, feedbackWatcherPair] : mResultWatchers.asKeyValueRange() ) + { + const int i = id; + QgsFeedback *feedback = feedbackWatcherPair.first; + QFutureWatcher *watcher = feedbackWatcherPair.second; + + // watcher should not call ofFutureFinished after this is deleted + watcher->disconnect( this ); + feedback->cancel(); + + // Self-cleanup: delete both once the background task finishes + connect( watcher, &QFutureWatcher::finished, watcher, [watcher, feedback, i]() + { + watcher->deleteLater(); + feedback->deleteLater(); + qDebug() << QStringLiteral( "Search (%1) cleaned up in the destructor" ).arg( i ); + } ); + } + mResultWatchers.clear(); } QVariant LayerFeaturesModel::data( const QModelIndex &index, int role ) const @@ -71,7 +89,9 @@ QHash LayerFeaturesModel::roleNames() const void LayerFeaturesModel::populate() { - if ( mLayer ) + cancelPendingRequests(); + + if ( mLayer && mLayer->dataProvider() ) { mFetchingResults = true; emit fetchingResultsChanged( mFetchingResults ); @@ -82,41 +102,37 @@ void LayerFeaturesModel::populate() QgsFeatureRequest req; setupFeatureRequest( req ); - req.setFeedback( mFeedback.get() ); + const int searchId = mNextSearchId.fetchAndAddOrdered( 1 ); + QgsFeedback *feedback = new QgsFeedback(); + req.setFeedback( feedback ); - int searchId = mNextSearchId.fetchAndAddOrdered( 1 ); - QgsVectorLayerFeatureSource *source = new QgsVectorLayerFeatureSource( mLayer ); - mSearchResultWatcher.setFuture( QtConcurrent::run( &LayerFeaturesModel::fetchFeatures, this, source, req, searchId ) ); - } -} + QFutureWatcher *watcher = new QFutureWatcher(); -QgsFeatureList LayerFeaturesModel::fetchFeatures( QgsVectorLayerFeatureSource *source, QgsFeatureRequest req, int searchId ) -{ - std::unique_ptr fs( source ); - QgsFeatureList fl; + mResultWatchers[ searchId ] = qMakePair( feedback, watcher ); + connect( watcher, &QFutureWatcher::finished, this, [this, searchId] { this->handleFinishedSearch( searchId ); } ); - // a search might have been queued if no threads were available in the pool, so we also - // check if canceled before we start as the first iteration can potentially be slow - bool canceled = searchId + 1 != mNextSearchId.loadAcquire(); - if ( canceled ) - { - qDebug() << QString( "Search (%1) was cancelled before it started!" ).arg( searchId ); - return fl; + qDebug() << QStringLiteral( "Search (%1) starting on layer %2" ).arg( searchId ).arg( mLayer->id() ); + + watcher->setFuture( QtConcurrent::run( LayerFeaturesModel::fetchFeatures, new QgsVectorLayerFeatureSource( mLayer ), req, searchId ) ); } +} +LayerFeaturesModel::SearchResultData LayerFeaturesModel::fetchFeatures( QgsAbstractFeatureSource *source, const QgsFeatureRequest &req, int searchId ) +{ +#ifdef QT_DEBUG QElapsedTimer t; t.start(); - QgsFeatureIterator it = fs->getFeatures( req ); +#endif + + std::unique_ptr ownedSource( source ); + QgsFeatureList fl; + + QgsFeatureIterator it = ownedSource->getFeatures( req ); + it.setInterruptionChecker( req.feedback() ); QgsFeature f; while ( it.nextFeature( f ) ) { - if ( searchId + 1 != mNextSearchId.loadAcquire() ) - { - canceled = true; - break; - } - if ( FID_IS_NEW( f.id() ) || FID_IS_NULL( f.id() ) ) { continue; // ignore uncommited features @@ -125,27 +141,50 @@ QgsFeatureList LayerFeaturesModel::fetchFeatures( QgsVectorLayerFeatureSource *s fl.append( f ); } - canceled = canceled || ( req.feedback() && req.feedback()->isCanceled() ); - - qDebug() << QString( "Search (%1) %2 after %3ms, results: %4" ).arg( searchId ).arg( canceled ? "was canceled" : "completed" ).arg( t.elapsed() ).arg( fl.count() ); - return fl; +#ifdef QT_DEBUG + const bool canceled = req.feedback()->isCanceled(); + qDebug() << QStringLiteral( "Search (%1) %2 after %3ms, results: %4" ).arg( searchId ).arg( canceled ? QStringLiteral( "was canceled" ) : QStringLiteral( "completed" ) ).arg( t.elapsed() ).arg( fl.count() ); +#endif + return { searchId, fl }; } -void LayerFeaturesModel::onFutureFinished() +void LayerFeaturesModel::handleFinishedSearch( int searchId ) { - QFutureWatcher *watcher = static_cast< QFutureWatcher *>( sender() ); - const QgsFeatureList features = watcher->future().result(); - beginResetModel(); - mFeatures.clear(); - for ( const auto &f : features ) + if ( !mResultWatchers.contains( searchId ) ) { - mFeatures << FeatureLayerPair( f, mLayer ); + // should not happen, this method is called only once for existing searchIds only + Q_ASSERT( false ); + return; + } + + auto [feedback, watcher] = mResultWatchers.take( searchId ); + const SearchResultData data = watcher->future().result(); + + watcher->deleteLater(); + feedback->deleteLater(); + qDebug() << QStringLiteral( "Search (%1) cleaned up" ).arg( searchId ); + + // We ignore the results of cancelled requests + if ( !feedback->isCanceled() ) + { + const QgsFeatureList features = data.features; + beginResetModel(); + mFeatures.clear(); + for ( const QgsFeature &feat : features ) + { + mFeatures.emplaceBack( feat, mLayer ); + } + emit layerFeaturesCountChanged( layerFeaturesCount() ); + emit countChanged( rowCount() ); + endResetModel(); + } + + // Only fire signal if that was the latest request + if ( data.searchId + 1 == mNextSearchId.loadAcquire() ) + { + mFetchingResults = false; + emit fetchingResultsChanged( mFetchingResults ); } - emit layerFeaturesCountChanged( layerFeaturesCount() ); - emit countChanged( rowCount() ); - endResetModel(); - mFetchingResults = false; - emit fetchingResultsChanged( mFetchingResults ); } QString LayerFeaturesModel::searchResultPair( const FeatureLayerPair &pair ) const @@ -179,6 +218,14 @@ QString LayerFeaturesModel::searchResultPair( const FeatureLayerPair &pair ) con return foundPairs.join( ", " ); } +void LayerFeaturesModel::cancelPendingRequests() +{ + for ( auto [id, feedbackWatcherPair] : mResultWatchers.asKeyValueRange() ) + { + feedbackWatcherPair.first->cancel(); + } +} + QString LayerFeaturesModel::buildSearchExpression() { if ( mSearchExpression.isEmpty() || !mLayer ) @@ -256,6 +303,7 @@ int LayerFeaturesModel::layerFeaturesCount() const void LayerFeaturesModel::reset() { + cancelPendingRequests(); mFeatures.clear(); mLayer = nullptr; mSearchExpression.clear(); @@ -285,6 +333,8 @@ void LayerFeaturesModel::setLayer( QgsVectorLayer *newLayer ) { if ( mLayer != newLayer ) { + cancelPendingRequests(); + if ( mLayer ) { disconnect( mLayer ); diff --git a/app/layerfeaturesmodel.h b/app/layerfeaturesmodel.h index 47995c4ed..108365ae9 100644 --- a/app/layerfeaturesmodel.h +++ b/app/layerfeaturesmodel.h @@ -12,13 +12,14 @@ #include #include +#include -#include "qgsvectorlayer.h" -#include "featurelayerpair.h" #include "featuresmodel.h" -class QgsVectorLayerFeatureSource; +class QgsVectorLayer; + +class FeatureLayerPair; /** @@ -88,7 +89,7 @@ class LayerFeaturesModel : public FeaturesModel void layerFeaturesCountChanged( int layerFeaturesCount ); //! \a isFetching is TRUE when still fetching results, FALSE when done fetching - bool fetchingResultsChanged( bool isFetching ); + void fetchingResultsChanged( bool isFetching ); protected: @@ -100,30 +101,36 @@ class LayerFeaturesModel : public FeaturesModel //! These are the attributes that will be fetched from the data source when populating the model QgsAttributeList mAttributeList; - private slots: - void onFutureFinished(); - private: + struct SearchResultData + { + int searchId; + QgsFeatureList features; + }; + QString buildSearchExpression(); - //! Performs getFeatures on layer. Takes ownership of \a layer and tries to move it to current thread. - QgsFeatureList fetchFeatures( QgsVectorLayerFeatureSource *layer, QgsFeatureRequest req, int searchId ); + //! Performs getFeatures on a feature source. Takes ownership of \a source. + static SearchResultData fetchFeatures( QgsAbstractFeatureSource *source, const QgsFeatureRequest &req, int searchId ); + + //! Should be called when a search is done/canceled. Cleans up mResultWatchers entry and populates the model if not canceled. + void handleFinishedSearch( int searchId ); //! Returns found attribute and its value from search expression for feature QString searchResultPair( const FeatureLayerPair &feat ) const; + void cancelPendingRequests(); + const int FEATURES_LIMIT = 10000; //!< Number of maximum features loaded from layer QString mSearchExpression; QgsVectorLayer *mLayer = nullptr; QAtomicInt mNextSearchId = 0; - QFutureWatcher mSearchResultWatcher; + QHash * >> mResultWatchers; //!< owns feedback and future watches objects bool mFetchingResults = false; bool mUseAttributeTableSortOrder = false; - std::unique_ptr mFeedback; - friend class TestModels; };