Skip to content

Commit

Permalink
[vertex tool] Fix vertex addition to polygon's first segment (fixes #…
Browse files Browse the repository at this point in the history
…20774)

With topo editing mode enabled, addition of extra points to keep the topology
correct wasn't working correctly because for the first segment we were getting
two matches due to duplicated first and last vertex in the ring. The fix
ensures that only one match will be returned for the first duplicated vertex.

(cherry picked from commit 3769faa)
  • Loading branch information
wonder-sk authored and nyalldawson committed Feb 1, 2019
1 parent ef1634c commit 4423caf
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
21 changes: 19 additions & 2 deletions src/app/vertextool/qgsvertextool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,35 @@ class MatchCollectingFilter : public QgsPointLocator::MatchFilter
{
if ( match.distance() > 0 )
return false;
matches.append( match );

// there may be multiple points at the same location, but we get only one
// result... the locator API needs a new method verticesInRect()
QgsGeometry matchGeom = vertextool->cachedGeometry( match.layer(), match.featureId() );
bool isPolygon = matchGeom.type() == QgsWkbTypes::PolygonGeometry;
QgsVertexId polygonRingVid;
QgsVertexId vid;
QgsPoint pt;
while ( matchGeom.constGet()->nextVertex( vid, pt ) )
{
int vindex = matchGeom.vertexNrFromVertexId( vid );
if ( pt.x() == match.point().x() && pt.y() == match.point().y() && vindex != match.vertexIndex() )
if ( pt.x() == match.point().x() && pt.y() == match.point().y() )
{
if ( isPolygon )
{
// for polygons we need to handle the case where the first vertex is matching because the
// last point will have the same coordinates and we would have a duplicate match which
// would make subsequent code behave incorrectly (topology editing mode would add a single
// vertex twice)
if ( vid.vertex == 0 )
{
polygonRingVid = vid;
}
else if ( vid.ringEqual( polygonRingVid ) && vid.vertex == matchGeom.constGet()->vertexCount( vid.part, vid.ring ) - 1 )
{
continue;
}
}

QgsPointLocator::Match extra_match( match.type(), match.layer(), match.featureId(),
0, match.point(), vindex );
matches.append( extra_match );
Expand Down
22 changes: 22 additions & 0 deletions tests/src/app/testqgsvertextool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class TestQgsVertexTool : public QObject
void testDeleteVertexTopo();
void testAddVertexTopo();
void testMoveEdgeTopo();
void testAddVertexTopoFirstSegment();
void testActiveLayerPriority();
void testSelectedFeaturesPriority();

Expand Down Expand Up @@ -642,6 +643,27 @@ void TestQgsVertexTool::testMoveEdgeTopo()
QgsProject::instance()->setTopologicalEditing( false );
}

void TestQgsVertexTool::testAddVertexTopoFirstSegment()
{
// check that when adding a vertex to the first segment of a polygon's ring with topo editing
// enabled, the geometry does not get corrupted (#20774)

QgsProject::instance()->setTopologicalEditing( true );

mouseClick( 5.5, 1, Qt::LeftButton );
mouseClick( 5, 2, Qt::LeftButton );

QCOMPARE( mLayerPolygon->undoStack()->index(), 2 );

QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 5 2, 7 1, 7 4, 4 4, 4 1))" ) );

mLayerPolygon->undoStack()->undo();

QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 4 4, 4 1))" ) );

QgsProject::instance()->setTopologicalEditing( false );
}

void TestQgsVertexTool::testActiveLayerPriority()
{
// check that features from current layer get priority when picking points
Expand Down

0 comments on commit 4423caf

Please sign in to comment.