Don't use QgsVectorLayer::selectedFeatures() to check for a selection

in a layer

This is incredibly inefficient, because selectedFeatures() actually
fetches a full copy of all selected features (including all
attributes and geometry). Instead use selectedFeatureIds(), which
is just a list of numbers.

Add warning note to docs cautioning against this practice.

Fixes massive ui lockup when right clicking on a layer with
selected features in the layer tree
This commit is contained in:
Nyall Dawson 2018-05-28 09:43:05 +10:00
parent e552ce62be
commit b6f2f7bd40
6 changed files with 46 additions and 21 deletions

View File

@ -524,9 +524,9 @@ which is used by the layer, so any changes are immediately applied.
int selectedFeatureCount() const;
%Docstring
The number of features that are selected in this layer
Returns the number of features that are selected in this layer.
:return: See description
.. seealso:: :py:func:`selectedFeatureIds`
%End
void selectByRect( QgsRectangle &rect, SelectBehavior behavior = SetSelection );
@ -614,8 +614,15 @@ Invert selection of features found within the search rectangle (in layer's coord
%Docstring
Returns a copy of the user-selected features.
.. warning::
Calling this method triggers a request for all attributes and geometry for the selected features.
Consider using the much more efficient selectedFeatureIds() or selectedFeatureCount() if you do not
require access to the feature attributes or geometry.
:return: A list of :py:class:`QgsFeature`
.. seealso:: :py:func:`selectedFeatureIds`
.. seealso:: :py:func:`getSelectedFeatures`
@ -631,6 +638,13 @@ Returns an iterator of the selected features.
:return: Iterator over the selected features
.. warning::
Calling this method returns an iterator for all attributes and geometry for the selected features.
Consider using the much more efficient selectedFeatureIds() or selectedFeatureCount() if you do not
require access to the feature attributes or geometry.
.. seealso:: :py:func:`selectedFeatureIds`
.. seealso:: :py:func:`selectedFeatures`
@ -638,11 +652,11 @@ Returns an iterator of the selected features.
const QgsFeatureIds &selectedFeatureIds() const;
%Docstring
Returns reference to identifiers of selected features
:return: A list of :py:class:`QgsFeatureId`
Returns a list of the selected features IDs in this layer.
.. seealso:: :py:func:`selectedFeatures`
.. seealso:: :py:func:`selectedFeatureCount`
%End
QgsRectangle boundingBoxOfSelected() const;

View File

@ -82,7 +82,7 @@ QVariantMap QgsSaveSelectedFeatures::processAlgorithm( const QVariantMap &parame
int current = 0;
double step = count > 0 ? 100.0 / count : 1;
QgsFeatureIterator it = selectLayer->getSelectedFeatures();;
QgsFeatureIterator it = selectLayer->getSelectedFeatures();
QgsFeature feat;
while ( it.nextFeature( feat ) )
{

View File

@ -7903,7 +7903,7 @@ void QgisApp::mergeAttributesOfSelectedFeatures()
}
//get initial selection (may be altered by attribute merge dialog later)
QgsFeatureList featureList = vl->selectedFeatures(); //get QList<QgsFeature>
QgsFeatureList featureList = vl->selectedFeatures();
//merge the attributes together
QgsMergeAttributesDialog d( featureList, vl, mapCanvas() );
@ -8043,7 +8043,7 @@ void QgisApp::mergeSelectedFeatures()
//get initial selection (may be altered by attribute merge dialog later)
QgsFeatureIds featureIds = vl->selectedFeatureIds();
QgsFeatureList featureList = vl->selectedFeatures(); //get QList<QgsFeature>
QgsFeatureList featureList = vl->selectedFeatures();
bool canceled;
QgsGeometry unionGeom = unionGeometries( vl, featureList, canceled );
if ( unionGeom.isNull() )
@ -13600,11 +13600,7 @@ QgsFeature QgisApp::duplicateFeatures( QgsMapLayer *mlayer, const QgsFeature &fe
}
else
{
const auto selectedFeatures = layer->selectedFeatures();
for ( const QgsFeature &f : selectedFeatures )
{
featureList.append( f );
}
featureList.append( layer->selectedFeatures() );
}
int featureCount = 0;

View File

@ -139,7 +139,7 @@ QMenu *QgsAppLayerTreeViewMenuProvider::createContextMenu()
if ( vlayer )
{
QAction *actionZoomSelected = actions->actionZoomToSelection( mCanvas, menu );
actionZoomSelected->setEnabled( !vlayer->selectedFeatures().isEmpty() );
actionZoomSelected->setEnabled( !vlayer->selectedFeatureIds().isEmpty() );
menu->addAction( actionZoomSelected );
}
menu->addAction( actions->actionShowInOverview( menu ) );

View File

@ -64,9 +64,16 @@ void QgsMapToolAddPart::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
}
bool isGeometryEmpty = false;
QgsFeatureList selectedFeatures = vlayer->selectedFeatures();
if ( !selectedFeatures.isEmpty() && selectedFeatures.at( 0 ).geometry().isNull() )
isGeometryEmpty = true;
if ( vlayer->selectedFeatureCount() > 0 )
{
// be efficient here - only grab the first selected feature if there's a selection, don't
// fetch all the other features which we don't require.
QgsFeatureIterator selectedFeatures = vlayer->getSelectedFeatures();
QgsFeature firstSelectedFeature;
if ( selectedFeatures.nextFeature( firstSelectedFeature ) )
if ( !firstSelectedFeature.geometry().isNull() )
isGeometryEmpty = true;
}
if ( !checkSelection() )
{

View File

@ -588,9 +588,9 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
const QgsActionManager *actions() const SIP_SKIP { return mActions; }
/**
* The number of features that are selected in this layer
* Returns the number of features that are selected in this layer.
*
* \returns See description
* \see selectedFeatureIds()
*/
int selectedFeatureCount() const;
@ -659,6 +659,10 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
/**
* Returns a copy of the user-selected features.
*
* \warning Calling this method triggers a request for all attributes and geometry for the selected features.
* Consider using the much more efficient selectedFeatureIds() or selectedFeatureCount() if you do not
* require access to the feature attributes or geometry.
*
* \returns A list of QgsFeature
*
* \see selectedFeatureIds()
@ -674,16 +678,20 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
*
* \returns Iterator over the selected features
*
* \warning Calling this method returns an iterator for all attributes and geometry for the selected features.
* Consider using the much more efficient selectedFeatureIds() or selectedFeatureCount() if you do not
* require access to the feature attributes or geometry.
*
* \see selectedFeatureIds()
* \see selectedFeatures()
*/
QgsFeatureIterator getSelectedFeatures( QgsFeatureRequest request = QgsFeatureRequest() ) const;
/**
* Returns reference to identifiers of selected features
* Returns a list of the selected features IDs in this layer.
*
* \returns A list of QgsFeatureId
* \see selectedFeatures()
* \see selectedFeatureCount()
*/
const QgsFeatureIds &selectedFeatureIds() const;