Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Backport queued_ltr_backports] Fix #51245 vertex tool topo crs mismatch #59345

Merged
merged 1 commit into from
Nov 25, 2024
Merged

[Backport queued_ltr_backports] Fix #51245 vertex tool topo crs mismatch #59345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 23 additions & 45 deletions src/app/vertextool/qgsvertextool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,55 +162,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 @@ -1011,7 +989,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 @@ -1934,29 +1912,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class TestQgsVertexTool : public QObject
void testActiveLayerPriority();
void testSelectedFeaturesPriority();
void testVertexToolCompoundCurve();
void testMoveVertexTopoOtherMapCrs();

private:
QPoint mapToScreen( double mapX, double mapY )
Expand Down Expand Up @@ -1557,7 +1558,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 @@ -1590,5 +1590,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"
Loading