Skip to content

Commit

Permalink
fix(reshape-line): keep original direction on reshaping linestrings
Browse files Browse the repository at this point in the history 
When the reshape line goes backwards against the original line, the
result sometimes follows the reshape line direction, sometimes the
original line direction.

We fix this by always choosing the original line direction (except when
the reshape line completely overrides the original line).
  • Loading branch information
@Djedouas @lbartoletti
Djedouas authored and lbartoletti committed Feb 7, 2025
1 parent 1403b21 commit 4677b6d
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 0 deletions.
36 changes: 36 additions & 0 deletions src/core/geometry/qgsgeos.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3392,6 +3392,15 @@ static geos::unique_ptr _mergeLinestrings( const GEOSGeometry *line1, const GEOS
GEOSGeometry *geoms[2] = { g1.release(), g2.release() };
geos::unique_ptr multiGeom( GEOSGeom_createCollection_r( context, GEOS_MULTILINESTRING, geoms, 2 ) );
geos::unique_ptr res( GEOSLineMerge_r( context, multiGeom.get() ) );

//keep the original orientation if the result has a start or end point in common with the original line
//and this point is not the start or the end point for both lines
double x1res, y1res, x2res, y2res;
if ( !_linestringEndpoints( res.get(), x1res, y1res, x2res, y2res ) )
return nullptr;
if ( ( x1res == x2 && y1res == y2 ) || ( x2res == x1 && y2res == y1 ) )
res.reset( GEOSReverse_r( context, res.get() ) );

return res;
}
else
Expand Down Expand Up @@ -3625,6 +3634,33 @@ geos::unique_ptr QgsGeos::reshapeLine( const GEOSGeometry *line, const GEOSGeome
return nullptr;
}

//keep the original orientation
bool reverseLine = false;
if ( isRing )
{
//for closed linestring check clockwise/counter-clockwise
char isResultCCW = 0, isOriginCCW = 0;
if ( GEOSCoordSeq_isCCW_r( context, GEOSGeom_getCoordSeq_r( context, result.get() ), &isResultCCW ) &&
GEOSCoordSeq_isCCW_r( context, GEOSGeom_getCoordSeq_r( context, line ), &isOriginCCW )
)
{
//reverse line if orientations are different
reverseLine = ( isOriginCCW == 1 && isResultCCW == 0 ) || ( isOriginCCW == 0 && isResultCCW == 1 );
}
}
else
{
//for linestring, check if the result has a start or end point in common with the original line
double x1res, y1res, x2res, y2res;
if ( !_linestringEndpoints( result.get(), x1res, y1res, x2res, y2res ) )
return nullptr;
geos::unique_ptr beginResultLineVertex = createGeosPointXY( x1res, y1res, false, 0, false, 0, 2, precision );
geos::unique_ptr endResultLineVertex = createGeosPointXY( x2res, y2res, false, 0, false, 0, 2, precision );
reverseLine = GEOSEquals_r( context, beginLineVertex.get(), endResultLineVertex.get() ) == 1 || GEOSEquals_r( context, endLineVertex.get(), beginResultLineVertex.get() ) == 1;
}
if ( reverseLine )
result.reset( GEOSReverse_r( context, result.get() ) );

return result;
}

Expand Down
71 changes: 71 additions & 0 deletions tests/src/app/testqgsmaptoolreshape.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class TestQgsMapToolReshape : public QObject
void testAvoidIntersectionAndTopoEdit();
void reshapeWithBindingLine();
void testWithTracing();
void testKeepDirection();

private:
QgisApp *mQgisApp = nullptr;
Expand All @@ -57,6 +58,7 @@ class TestQgsMapToolReshape : public QObject
QgsVectorLayer *mLayerPolygonZ = nullptr;
QgsVectorLayer *mLayerTopo = nullptr;
QgsVectorLayer *mLayerTopo2 = nullptr;
QgsVectorLayer *mLayerLine = nullptr;
};

TestQgsMapToolReshape::TestQgsMapToolReshape() = default;
Expand Down Expand Up @@ -106,6 +108,10 @@ void TestQgsMapToolReshape::initTestCase()
mLayerTopo2 = new QgsVectorLayer( QStringLiteral( "Polygon?crs=EPSG:3946" ), QStringLiteral( "topo2" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerTopo2->isValid() );

mLayerLine = new QgsVectorLayer( QStringLiteral( "LineString?crs=EPSG:3946" ), QStringLiteral( "layer line" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerLine->isValid() );
QgsProject::instance()->addMapLayers( QList<QgsMapLayer *>() << mLayerLine );

mLayerLineZ->startEditing();
const QString wkt1 = "LineString Z (0 0 0, 1 1 0, 1 2 0)";
const QString wkt2 = "LineString Z (2 1 5, 3 3 5)";
Expand Down Expand Up @@ -167,6 +173,20 @@ void TestQgsMapToolReshape::initTestCase()
QCOMPARE( mLayerTopo2->featureCount(), 1 );
QCOMPARE( mLayerTopo2->getFeature( 1 ).geometry().asWkt(), QStringLiteral( "Polygon ((0 5, 4 5, 4 7, 0 7))" ) );

mLayerLine->startEditing();
const QString wkt9 = QStringLiteral( "LineString (0 0, 10 10)" );
const QString wkt10 = QStringLiteral( "LineString (2 8, 0 6)" );
const QString wkt11 = QStringLiteral( "LineString (13 0, 13 8, 19 11, 25 8, 25 0, 13 0)" ); // cw oriented linestring ring
QgsFeature f9, f10, f11;
f9.setGeometry( QgsGeometry::fromWkt( wkt9 ) );
f10.setGeometry( QgsGeometry::fromWkt( wkt10 ) );
f11.setGeometry( QgsGeometry::fromWkt( wkt11 ) );
mLayerLine->dataProvider()->addFeatures( QgsFeatureList() << f9 << f10 << f11 );
QCOMPARE( mLayerLine->featureCount(), ( long ) 3 );
QCOMPARE( mLayerLine->getFeature( 1 ).geometry().asWkt(), wkt9 );
QCOMPARE( mLayerLine->getFeature( 2 ).geometry().asWkt(), wkt10 );
QCOMPARE( mLayerLine->getFeature( 3 ).geometry().asWkt(), wkt11 );

QgsSnappingConfig cfg = mCanvas->snappingUtils()->config();
cfg.setMode( Qgis::SnappingMode::AllLayers );
cfg.setTolerance( 100 );
Expand Down Expand Up @@ -408,6 +428,57 @@ void TestQgsMapToolReshape::testWithTracing()
QgsProject::instance()->setTopologicalEditing( topologicalEditing );
}

void TestQgsMapToolReshape::testKeepDirection()
{
TestQgsMapToolAdvancedDigitizingUtils utils( mCaptureTool );
mCanvas->setCurrentLayer( mLayerLine );

// no snapping for this test
QgsSnappingConfig cfg = mCanvas->snappingUtils()->config();
cfg.setEnabled( false );
mCanvas->snappingUtils()->setConfig( cfg );

// intersect linestring 4 times, and go backwards compared to its direction
utils.mouseClick( 9, 8, Qt::LeftButton );
utils.mouseClick( 9, 10, Qt::LeftButton );
utils.mouseClick( 8, 9, Qt::LeftButton );
utils.mouseClick( 8, 7, Qt::LeftButton );
utils.mouseClick( 7, 6, Qt::LeftButton );
utils.mouseClick( 7, 8, Qt::LeftButton );
utils.mouseClick( 6, 7, Qt::LeftButton );
utils.mouseClick( 6, 5, Qt::LeftButton );
utils.mouseClick( 6, 5, Qt::RightButton );

QString wkt1 = QStringLiteral( "LineString (0 0, 6 6, 6 7, 7 8, 7 7, 7 6, 8 7, 8 8, 8 9, 9 10, 9 9, 10 10)" );
QCOMPARE( mLayerLine->getFeature( 1 ).geometry().asWkt(), wkt1 );

// extend linestring from its start point
utils.mouseClick( 2, 8, Qt::LeftButton );
utils.mouseClick( 3, 9, Qt::LeftButton );
utils.mouseClick( 3, 9, Qt::RightButton );

QString wkt2 = QStringLiteral( "LineString (3 9, 2 8, 0 6)" );
QCOMPARE( mLayerLine->getFeature( 2 ).geometry().asWkt(), wkt2 );

// intersect linestring ring 4 times, and go backwards compared to its direction
utils.mouseClick( 14, 7, Qt::LeftButton );
utils.mouseClick( 12, 7, Qt::LeftButton );
utils.mouseClick( 12, 5, Qt::LeftButton );
utils.mouseClick( 14, 5, Qt::LeftButton );
utils.mouseClick( 14, 3, Qt::LeftButton );
utils.mouseClick( 12, 3, Qt::LeftButton );
utils.mouseClick( 12, 1, Qt::LeftButton );
utils.mouseClick( 14, 1, Qt::LeftButton );
utils.mouseClick( 14, 1, Qt::RightButton );

QString wkt3 = QStringLiteral( "LineString (13 1, 12 1, 12 3, 13 3, 14 3, 14 5, 13 5, 12 5, 12 7, 13 7, 13 8, 19 11, 25 8, 25 0, 13 0, 13 1)" );
QCOMPARE( mLayerLine->getFeature( 3 ).geometry().asWkt(), wkt3 );

// activate back snapping
cfg.setEnabled( true );
mCanvas->snappingUtils()->setConfig( cfg );
}


QGSTEST_MAIN( TestQgsMapToolReshape )
#include "testqgsmaptoolreshape.moc"

0 comments on commit 4677b6d

Please sign in to comment.