Skip to content

Commit

Permalink
Fix freeze on long indexation for snapping on intersections
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Djedouas committed Nov 22, 2024
1 parent c43991a commit 7b3b8d4
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 17 deletions.
23 changes: 7 additions & 16 deletions src/core/qgssnappingutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() )
Expand Down Expand Up @@ -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
Expand All @@ -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() )
Expand Down Expand Up @@ -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 );
}
}

Expand All @@ -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() )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
Expand Down
21 changes: 20 additions & 1 deletion tests/src/app/testqgsadvanceddigitizing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "qgstest.h"

#include "qgsadvanceddigitizingdockwidget.h"
#include "qgsguiutils.h"
#include "qgsapplication.h"
#include "qgsmapcanvas.h"
#include "qgsvectorlayer.h"
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
Expand Down

0 comments on commit 7b3b8d4

Please sign in to comment.