From 7b3b8d4419e4441b40fa09f82bb54e6fbca7f0ba Mon Sep 17 00:00:00 2001 From: Jacky Volpes Date: Tue, 29 Oct 2024 15:45:36 +0100 Subject: [PATCH] Fix freeze on long indexation for snapping on intersections If snap on intersections is enabled, this specific edge search with the locator was not in relaxed mode, whereas the standard snapping is happening in relaxed mode. As a result, trying to use the snapping during the first indexation was freezing QGIS while the indexation is happening, waiting for it to end. On a layer where the indexation is longer than the timeout (30sec, i.e. a WFS layer as in issue #51179), the locator stops abruptly and resets itself, crashing the indexation and QGIS. --- src/core/qgssnappingutils.cpp | 23 ++++++------------- .../testqgsmaptooladdfeatureline.cpp | 18 +++++++++++++++ tests/src/app/testqgsadvanceddigitizing.cpp | 21 ++++++++++++++++- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/core/qgssnappingutils.cpp b/src/core/qgssnappingutils.cpp index 9c2ac8d6bd7a..609f0169ab5d 100644 --- a/src/core/qgssnappingutils.cpp +++ b/src/core/qgssnappingutils.cpp @@ -290,21 +290,15 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPointXY &pointMap, QgsPointLocator::Match bestMatch; QgsPointLocator::MatchList edges; // for snap on intersection _updateBestMatch( bestMatch, pointMap, loc, type, tolerance, filter, relaxed ); - if ( mSnappingConfig.intersectionSnapping() ) - { - QgsPointLocator *locEdges = locatorForLayerUsingStrategy( mCurrentLayer, pointMap, tolerance ); - if ( !locEdges ) - return QgsPointLocator::Match(); - edges = locEdges->edgesInRect( pointMap, tolerance ); - } + edges = loc->edgesInRect( pointMap, tolerance, filter, relaxed ); for ( QgsVectorLayer *vl : mExtraSnapLayers ) { QgsPointLocator *loc = locatorForLayerUsingStrategy( vl, pointMap, tolerance ); _updateBestMatch( bestMatch, pointMap, loc, type, tolerance, filter, false ); if ( mSnappingConfig.intersectionSnapping() ) - edges << loc->edgesInRect( pointMap, tolerance ); + edges << loc->edgesInRect( pointMap, tolerance, filter, false ); } if ( mSnappingConfig.intersectionSnapping() ) @@ -356,11 +350,9 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPointXY &pointMap, if ( QgsPointLocator *loc = locatorForLayerUsingStrategy( layerConfig.layer, pointMap, tolerance ) ) { _updateBestMatch( bestMatch, pointMap, loc, layerConfig.type, tolerance, filter, relaxed ); - if ( mSnappingConfig.intersectionSnapping() ) - { - edges << loc->edgesInRect( pointMap, tolerance ); - } + edges << loc->edgesInRect( pointMap, tolerance, filter, relaxed ); + // We keep the maximum tolerance for intersection snapping and extra snapping maxTolerance = std::max( maxTolerance, tolerance ); // To avoid yet an additional setting, on extra snappings, we use the combination of all enabled snap types @@ -373,7 +365,7 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPointXY &pointMap, QgsPointLocator *loc = locatorForLayerUsingStrategy( vl, pointMap, maxTolerance ); _updateBestMatch( bestMatch, pointMap, loc, maxTypes, maxTolerance, filter, false ); if ( mSnappingConfig.intersectionSnapping() ) - edges << loc->edgesInRect( pointMap, maxTolerance ); + edges << loc->edgesInRect( pointMap, maxTolerance, filter, false ); } if ( mSnappingConfig.intersectionSnapping() ) @@ -404,9 +396,8 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPointXY &pointMap, if ( QgsPointLocator *loc = locatorForLayerUsingStrategy( vl, pointMap, tolerance ) ) { _updateBestMatch( bestMatch, pointMap, loc, type, tolerance, filter, relaxed ); - if ( mSnappingConfig.intersectionSnapping() ) - edges << loc->edgesInRect( pointMap, tolerance ); + edges << loc->edgesInRect( pointMap, tolerance, filter, relaxed ); } } @@ -415,7 +406,7 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPointXY &pointMap, QgsPointLocator *loc = locatorForLayerUsingStrategy( vl, pointMap, tolerance ); _updateBestMatch( bestMatch, pointMap, loc, type, tolerance, filter, false ); if ( mSnappingConfig.intersectionSnapping() ) - edges << loc->edgesInRect( pointMap, tolerance ); + edges << loc->edgesInRect( pointMap, tolerance, filter, false ); } if ( mSnappingConfig.intersectionSnapping() ) diff --git a/tests/src/app/maptooladdfeatureline/testqgsmaptooladdfeatureline.cpp b/tests/src/app/maptooladdfeatureline/testqgsmaptooladdfeatureline.cpp index 51c22e9e6226..e0ed648bf9f4 100644 --- a/tests/src/app/maptooladdfeatureline/testqgsmaptooladdfeatureline.cpp +++ b/tests/src/app/maptooladdfeatureline/testqgsmaptooladdfeatureline.cpp @@ -964,6 +964,15 @@ void TestQgsMapToolAddFeatureLine::testWithTopologicalEditingDifferentCanvasCrs( snapConfig.project()->setTopologicalEditing( true ); mCanvas->snappingUtils()->setConfig( snapConfig ); + // Wait for indexing to complete + if ( QgsPointLocator *loc = mCanvas->snappingUtils()->locatorForLayer( mLayerCRS3946Line ) ) + { + if ( loc->isIndexing() ) + { + loc->waitForIndexingFinished(); + } + } + // add a line with one vertex near the previous line utils.mouseClick( 10, 0, Qt::LeftButton ); utils.mouseClick( 4.9, 5.1, Qt::LeftButton ); @@ -1044,6 +1053,15 @@ void TestQgsMapToolAddFeatureLine::testWithTopologicalEditingWIthDiffLayerWithDi snapConfig.project()->setTopologicalEditing( true ); mCanvas->snappingUtils()->setConfig( snapConfig ); + // Wait for indexing to complete + if ( QgsPointLocator *loc = mCanvas->snappingUtils()->locatorForLayer( mLayerCRS3945Line ) ) + { + if ( loc->isIndexing() ) + { + loc->waitForIndexingFinished(); + } + } + // test the topological editing utils.mouseClick( 0, 5, Qt::LeftButton ); utils.mouseClick( 10.1, 5, Qt::LeftButton ); diff --git a/tests/src/app/testqgsadvanceddigitizing.cpp b/tests/src/app/testqgsadvanceddigitizing.cpp index 4ab536a7356a..94adff09cb7b 100644 --- a/tests/src/app/testqgsadvanceddigitizing.cpp +++ b/tests/src/app/testqgsadvanceddigitizing.cpp @@ -16,7 +16,6 @@ #include "qgstest.h" #include "qgsadvanceddigitizingdockwidget.h" -#include "qgsguiutils.h" #include "qgsapplication.h" #include "qgsmapcanvas.h" #include "qgsvectorlayer.h" @@ -603,6 +602,16 @@ void TestQgsAdvancedDigitizing::coordinateConstraintWhenSnapping() snapConfig.setEnabled( true ); mCanvas->snappingUtils()->setConfig( snapConfig ); + // move to trigger a re-indexing and wait for it to complete + utils.mouseMove( 0, 0 ); + if ( QgsPointLocator *loc = mCanvas->snappingUtils()->locatorForLayer( mLayer3950 ) ) + { + if ( loc->isIndexing() ) + { + loc->waitForIndexingFinished(); + } + } + // simple snap test utils.mouseClick( 0, 2, Qt::LeftButton ); utils.mouseClick( 2.02, 2, Qt::LeftButton ); @@ -995,6 +1004,16 @@ void TestQgsAdvancedDigitizing::currentPointWhenSanppingWithDiffCanvasCRS() snapConfig.setEnabled( true ); mCanvas->snappingUtils()->setConfig( snapConfig ); + // move to trigger a re-indexing and wait for it to complete + utils.mouseMove( 0, 0 ); + if ( QgsPointLocator *loc = mCanvas->snappingUtils()->locatorForLayer( mLayer4326 ) ) + { + if ( loc->isIndexing() ) + { + loc->waitForIndexingFinished(); + } + } + QCOMPARE( mCanvas->snappingUtils()->currentLayer(), mLayer4326 ); utils.mouseClick( 25, 0, Qt::LeftButton );