Skip to content

Commit

Permalink
Respect selection in layers in vertex tool (fixes #17782) (#6421)
Browse files Browse the repository at this point in the history
This fixes issues in situations when there are multiple vertices in one location:

1. when clicking a location, if there are selected features,
   the closest vertex from a selected feature will be used with priority.

2. when dragging a rectangle, if there is a selected feature,
   only vertices from selected features will be used.

If there is selection in any editable layers, but away from the location where
user clicked to pick vertex (or dragged rectangle to pick multiple vertices),
the existing vertex tool behavior is not affected (so it cannot happen that
vertex tool suddenly appears to have stopped working just because there is
selection somewhere possibly outside of the current map view).
  • Loading branch information
wonder-sk authored and NathanW2 committed Feb 23, 2018
1 parent 5114d60 commit 64aa400
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 2 deletions.
61 changes: 59 additions & 2 deletions src/app/vertextool/qgsvertextool.cpp
Expand Up @@ -192,6 +192,36 @@ class MatchCollectingFilter : public QgsPointLocator::MatchFilter
}
};


/**
* Keeps the best match from a selected feature so that we can possibly use it with higher priority.
* If we do not encounter any selected feature within tolerance, we use the best match as usual.
*/
class SelectedMatchFilter : public QgsPointLocator::MatchFilter
{
public:
explicit SelectedMatchFilter( double tol )
: mTolerance( tol ) {}

virtual bool acceptMatch( const QgsPointLocator::Match &match )
{
if ( match.distance() <= mTolerance && match.layer() && match.layer()->selectedFeatureIds().contains( match.featureId() ) )
{
if ( !mBestSelectedMatch.isValid() || match.distance() < mBestSelectedMatch.distance() )
mBestSelectedMatch = match;
}
return true;
}

bool hasSelectedMatch() const { return mBestSelectedMatch.isValid(); }
QgsPointLocator::Match bestSelectedMatch() const { return mBestSelectedMatch; }

private:
double mTolerance;
QgsPointLocator::Match mBestSelectedMatch;
};


//
//
//
Expand Down Expand Up @@ -440,6 +470,7 @@ void QgsVertexTool::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
QgsPointXY pt1 = toMapCoordinates( e->pos() );
QgsRectangle map_rect( pt0, pt1 );
QList<Vertex> vertices;
QList<Vertex> selectedVertices;

// for each editable layer, select vertices
const auto layers = canvas()->layers();
Expand All @@ -454,16 +485,26 @@ void QgsVertexTool::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
QgsFeatureIterator fi = vlayer->getFeatures( QgsFeatureRequest( layerRect ).setSubsetOfAttributes( QgsAttributeList() ) );
while ( fi.nextFeature( f ) )
{
bool isFeatureSelected = vlayer->selectedFeatureIds().contains( f.id() );
QgsGeometry g = f.geometry();
for ( int i = 0; i < g.constGet()->nCoordinates(); ++i )
{
QgsPointXY pt = g.vertexAt( i );
if ( layerRect.contains( pt ) )
{
vertices << Vertex( vlayer, f.id(), i );
if ( isFeatureSelected )
selectedVertices << Vertex( vlayer, f.id(), i );
}
}
}
}

// If there were any vertices that come from selected features, use just vertices from selected features.
// This allows user to select a bunch of features in complex situations to constrain the selection.
if ( !selectedVertices.isEmpty() )
vertices = selectedVertices;

HighlightMode mode = ModeReset;
if ( e->modifiers() & Qt::ShiftModifier )
mode = ModeAdd;
Expand Down Expand Up @@ -639,7 +680,15 @@ QgsPointLocator::Match QgsVertexTool::snapToEditableLayer( QgsMapMouseEvent *e )
}

snapUtils->setConfig( config );
m = snapUtils->snapToMap( mapPoint );
SelectedMatchFilter filter( tol );
m = snapUtils->snapToMap( mapPoint, &filter );

// we give priority to snap matches that are from selected features
if ( filter.hasSelectedMatch() )
{
m = filter.bestSelectedMatch();
mLastSnap.reset();
}
}
}

Expand All @@ -658,7 +707,15 @@ QgsPointLocator::Match QgsVertexTool::snapToEditableLayer( QgsMapMouseEvent *e )
}

snapUtils->setConfig( config );
m = snapUtils->snapToMap( mapPoint );
SelectedMatchFilter filter( tol );
m = snapUtils->snapToMap( mapPoint, &filter );

// we give priority to snap matches that are from selected features
if ( filter.hasSelectedMatch() )
{
m = filter.bestSelectedMatch();
mLastSnap.reset();
}
}

// try to stay snapped to previously used feature
Expand Down
83 changes: 83 additions & 0 deletions tests/src/app/testqgsvertextool.cpp
Expand Up @@ -65,6 +65,7 @@ class TestQgsVertexTool : public QObject
void testMoveVertexTopo();
void testDeleteVertexTopo();
void testActiveLayerPriority();
void testSelectedFeaturesPriority();

private:
QPoint mapToScreen( double mapX, double mapY )
Expand Down Expand Up @@ -571,10 +572,92 @@ void TestQgsVertexTool::testActiveLayerPriority()
QCOMPARE( layerLine2->getFeature( fidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(0 1, 0 0, 1 0)" ) );
layerLine2->undoStack()->undo();

mCanvas->setCurrentLayer( nullptr );

// get rid of the temporary layer
mCanvas->setLayers( QList<QgsMapLayer *>() << mLayerLine << mLayerPolygon << mLayerPoint );
QgsProject::instance()->removeMapLayer( layerLine2 );
}

void TestQgsVertexTool::testSelectedFeaturesPriority()
{
// preparation: make the polygon feature touch line feature
mouseClick( 4, 1, Qt::LeftButton );
mouseClick( 2, 1, Qt::LeftButton );

//
// test that clicking a location with selected and non-selected feature will always pick the selected feature
//

mLayerLine->selectByIds( QgsFeatureIds() << mFidLineF1 );
mLayerPolygon->selectByIds( QgsFeatureIds() );

mouseClick( 2, 1, Qt::LeftButton );
mouseClick( 3, 1, Qt::LeftButton );

// check that move of (2,1) to (3,1) affects only line layer
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(3 1, 1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((2 1, 7 1, 7 4, 4 4, 2 1))" ) );
mLayerLine->undoStack()->undo();

mLayerLine->selectByIds( QgsFeatureIds() );
mLayerPolygon->selectByIds( QgsFeatureIds() << mFidPolygonF1 );

mouseClick( 2, 1, Qt::LeftButton );
mouseClick( 3, 1, Qt::LeftButton );

// check that move of (2,1) to (3,1) affects only polygon layer
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((3 1, 7 1, 7 4, 4 4, 3 1))" ) );
mLayerPolygon->undoStack()->undo();

//
// test that dragging rectangle to pick vertices in location with selected and non-selected feature
// will always pick vertices only from the selected feature
//

mLayerLine->selectByIds( QgsFeatureIds() );
mLayerPolygon->selectByIds( QgsFeatureIds() );

mousePress( 1.5, 0.5, Qt::LeftButton );
mouseMove( 2.5, 1.5 );
mouseRelease( 2.5, 1.5, Qt::LeftButton );
keyClick( Qt::Key_Delete );

// check we have deleted vertex at (2,1) from both line and polygon features
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((7 1, 7 4, 4 4, 7 1))" ) );
mLayerLine->undoStack()->undo();
mLayerPolygon->undoStack()->undo();

mLayerLine->selectByIds( QgsFeatureIds() << mFidLineF1 );
mLayerPolygon->selectByIds( QgsFeatureIds() );

mousePress( 1.5, 0.5, Qt::LeftButton );
mouseMove( 2.5, 1.5 );
mouseRelease( 2.5, 1.5, Qt::LeftButton );
keyClick( Qt::Key_Delete );

// check we have deleted vertex at (2,1) just from line feature
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((2 1, 7 1, 7 4, 4 4, 2 1))" ) );
mLayerLine->undoStack()->undo();

mLayerLine->selectByIds( QgsFeatureIds() );
mLayerPolygon->selectByIds( QgsFeatureIds() << mFidPolygonF1 );

mousePress( 1.5, 0.5, Qt::LeftButton );
mouseMove( 2.5, 1.5 );
mouseRelease( 2.5, 1.5, Qt::LeftButton );
keyClick( Qt::Key_Delete );

// check we have deleted vertex at (2,1) just from polygon feature
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((7 1, 7 4, 4 4, 7 1))" ) );
mLayerPolygon->undoStack()->undo();

mLayerPolygon->undoStack()->undo(); // undo the initial change
}

QGSTEST_MAIN( TestQgsVertexTool )
#include "testqgsvertextool.moc"

0 comments on commit 64aa400

Please sign in to comment.