Skip to content
Permalink
Browse files

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
  • Loading branch information
nyalldawson committed May 27, 2018
1 parent e552ce6 commit b6f2f7bd404f98325dd941717c9eb4663305c4d3
@@ -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,18 +638,25 @@ 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`
%End

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

0 comments on commit b6f2f7b

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