Skip to content

Commit

Permalink
fix(vertextool): topo edition when map CRS != layer CRS
Browse files Browse the repository at this point in the history
  • Loading branch information
@Djedouas @lbartoletti
Djedouas authored and lbartoletti committed Nov 25, 2024
1 parent 989853a commit a17650d
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 46 deletions.
68 changes: 23 additions & 45 deletions src/app/vertextool/qgsvertextool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,55 +164,33 @@ class OneFeatureFilter : public QgsPointLocator::MatchFilter
};


//! a filter just to gather all matches at the same place
/**
* For polygons we need to filter out the last vertex, because it will have
* the same coordinates than the first and we would have a duplicate match which
* would make topology editing mode behave incorrectly
*/
class MatchCollectingFilter : public QgsPointLocator::MatchFilter
{
public:
QList<QgsPointLocator::Match> matches;
QgsVertexTool *vertextool = nullptr;

MatchCollectingFilter( QgsVertexTool *vertextool )
: vertextool( vertextool ) {}

bool acceptMatch( const QgsPointLocator::Match &match ) override
{
if ( match.distance() > 0 )
return false;
if ( match.layer()->geometryType() != Qgis::GeometryType::Polygon )
return true;

// 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() == Qgis::GeometryType::Polygon;
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() )
{
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;
}
}
QgsGeometry matchGeom = vertextool->cachedGeometry( match.layer(), match.featureId() );
if ( !matchGeom.vertexIdFromVertexNr( match.vertexIndex(), vid ) )
// should not happen because vertex index in match object was created with vertexNrFromVertexId
// so the methods are reversible and we will have a vid
return false;

QgsPointLocator::Match extra_match( match.type(), match.layer(), match.featureId(),
0, match.point(), vindex );
matches.append( extra_match );
}
}
return true;
// filter out the vertex if it is the last one (of its ring, in its part)
return vid.vertex != matchGeom.constGet()->vertexCount( vid.part, vid.ring ) - 1;
}
};

Expand Down Expand Up @@ -1013,7 +991,7 @@ QgsPointLocator::Match QgsVertexTool::snapToPolygonInterior( QgsMapMouseEvent *e
}


QList<QgsPointLocator::Match> QgsVertexTool::findEditableLayerMatches( const QgsPointXY &mapPoint, QgsVectorLayer *layer )
QgsPointLocator::MatchList QgsVertexTool::findEditableLayerMatches( const QgsPointXY &mapPoint, QgsVectorLayer *layer )
{
QgsPointLocator::MatchList matchList;

Expand Down Expand Up @@ -1936,29 +1914,29 @@ void QgsVertexTool::buildDragBandsForVertices( const QSet<Vertex> &movingVertice
}
}

QList<QgsPointLocator::Match> QgsVertexTool::layerVerticesSnappedToPoint( QgsVectorLayer *layer, const QgsPointXY &mapPoint )
QgsPointLocator::MatchList QgsVertexTool::layerVerticesSnappedToPoint( QgsVectorLayer *layer, const QgsPointXY &mapPoint )
{
MatchCollectingFilter myfilter( this );
QgsPointLocator *loc = canvas()->snappingUtils()->locatorForLayer( layer );
loc->nearestVertex( mapPoint, 0, &myfilter, true );
return myfilter.matches;
double tol = QgsTolerance::vertexSearchRadius( canvas()->mapSettings() );
return loc->verticesInRect( mapPoint, tol, &myfilter, true );
}

QList<QgsPointLocator::Match> QgsVertexTool::layerSegmentsSnappedToSegment( QgsVectorLayer *layer, const QgsPointXY &mapPoint1, const QgsPointXY &mapPoint2 )
QgsPointLocator::MatchList QgsVertexTool::layerSegmentsSnappedToSegment( QgsVectorLayer *layer, const QgsPointXY &mapPoint1, const QgsPointXY &mapPoint2 )
{
QList<QgsPointLocator::Match> finalMatches;
QgsPointLocator::MatchList finalMatches;
// we want segment matches that have exactly the same vertices as the given segment (mapPoint1, mapPoint2)
// so rather than doing nearest edge search which could return any segment within a tolerance,
// we first find matches for one endpoint and then see if there is a matching other endpoint.
const QList<QgsPointLocator::Match> matches1 = layerVerticesSnappedToPoint( layer, mapPoint1 );
const QgsPointLocator::MatchList matches1 = layerVerticesSnappedToPoint( layer, mapPoint1 );
for ( const QgsPointLocator::Match &m : matches1 )
{
QgsGeometry g = cachedGeometry( layer, m.featureId() );
int v0, v1;
g.adjacentVertices( m.vertexIndex(), v0, v1 );
if ( v0 != -1 && QgsPointXY( g.vertexAt( v0 ) ) == mapPoint2 )
if ( v0 != -1 && toMapCoordinates( layer, QgsPointXY( g.vertexAt( v0 ) ) ) == mapPoint2 )
finalMatches << QgsPointLocator::Match( QgsPointLocator::Edge, layer, m.featureId(), 0, m.point(), v0 );
else if ( v1 != -1 && QgsPointXY( g.vertexAt( v1 ) ) == mapPoint2 )
else if ( v1 != -1 && toMapCoordinates( layer, QgsPointXY( g.vertexAt( v1 ) ) ) == mapPoint2 )
finalMatches << QgsPointLocator::Match( QgsPointLocator::Edge, layer, m.featureId(), 0, m.point(), m.vertexIndex() );
}
return finalMatches;
Expand Down
44 changes: 43 additions & 1 deletion tests/src/app/testqgsvertextool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class TestQgsVertexTool : public QObject
void testActiveLayerPriority();
void testSelectedFeaturesPriority();
void testVertexToolCompoundCurve();
void testMoveVertexTopoOtherMapCrs();

private:
QPoint mapToScreen( double mapX, double mapY )
Expand Down Expand Up @@ -1614,7 +1615,6 @@ void TestQgsVertexTool::testVertexToolCompoundCurve()
mouseMove( 17, 10 );
mouseClick( 17 + offsetInMapUnits, 10, Qt::LeftButton );
mouseClick( 7, 2, Qt::LeftButton );
mouseClick( 7, 1, Qt::RightButton );

// verifying that it's not possible to add a extra vertex to a CircularString
QCOMPARE( mLayerCompoundCurve->undoStack()->index(), 2 );
Expand Down Expand Up @@ -1647,5 +1647,47 @@ void TestQgsVertexTool::testSelectVerticesByPolygon()
QCOMPARE( mLayerMultiPolygon->getFeature( mFidMultiPolygonF1 ).geometry(), QgsGeometry::fromWkt( "MultiPolygon (((1 5, 2 5, 2 6.5, 2 8, 1 8, 1 6.5, 1 5),(1.25 5.5, 1.25 6, 1.75 6, 1.75 5.5, 1.25 5.5),(1.25 7, 1.75 7, 1.75 7.5, 1.25 7.5, 1.25 7)),((3 5, 3 6.5, 3 8, 4 8, 4 6.5, 4 5, 3 5),(3.25 5.5, 3.75 5.5, 3.75 6, 3.25 6, 3.25 5.5),(3.25 7, 3.75 7, 3.75 7.5, 3.25 7.5, 3.25 7)))" ) );
}

void TestQgsVertexTool::testMoveVertexTopoOtherMapCrs()
{
// test moving of vertices of two features at once

QgsProject::instance()->setTopologicalEditing( true );
QgsCoordinateReferenceSystem prevCrs = QgsProject::instance()->crs();
QgsCoordinateReferenceSystem tmpCrs = QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3857" ) );

// move linestring vertex to connect with polygon at point (7, 1)
mouseClick( 2, 1, Qt::LeftButton );
mouseClick( 7, 1, Qt::LeftButton );

// change CRS so that the map canvas and the layers CRSs are different
mCanvas->setDestinationCrs( tmpCrs );
mCanvas->snappingUtils()->locatorForLayer( mLayerLine )->init();
mCanvas->snappingUtils()->locatorForLayer( mLayerPolygon )->init();

// Start point is 7, 1 in layer coordinates, to snap to the linestring and polygon vertices.
// End point is 3, 3 in layer coordinates.
// Get the map coordinates for these points to click on it.
QgsPointXY mapPointStart = mCanvas->mapSettings().layerToMapCoordinates( mLayerPolygon, QgsPointXY( 7, 1 ) );
QgsPointXY mapPointEnd = mCanvas->mapSettings().layerToMapCoordinates( mLayerPolygon, QgsPointXY( 3, 3 ) );
mouseClick( mapPointStart.x(), mapPointStart.y(), Qt::LeftButton );
mouseClick( mapPointEnd.x(), mapPointEnd.y(), Qt::LeftButton );

// polygon and line features have changed, within the CRS conversion precision
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry().asWkt( 2 ), "LineString (3 3, 1 1, 1 3)" );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry().asWkt( 2 ), "Polygon ((4 1, 3 3, 7 4, 4 4, 4 1))" );

QCOMPARE( mLayerLine->undoStack()->index(), 3 ); // one more move of vertex from earlier
QCOMPARE( mLayerPolygon->undoStack()->index(), 2 );
mLayerLine->undoStack()->undo();
mLayerLine->undoStack()->undo();
mLayerPolygon->undoStack()->undo();

// back to the original state
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 4 4, 4 1))" ) );
mCanvas->setDestinationCrs( prevCrs );
QgsProject::instance()->setTopologicalEditing( false );
}

QGSTEST_MAIN( TestQgsVertexTool )
#include "testqgsvertextool.moc"

0 comments on commit a17650d

Please sign in to comment.