Skip to content
Permalink
Browse files

[Geometry] Fix deleteVertex() for CircularString, CompoundCurve and C…

…urvePolygon

In some situations, deleteVertex() could generate an invalid geometry, causing
later crashes due to unexpected structure.

Fix #15087
  • Loading branch information
rouault committed Jun 20, 2016
1 parent b015fb4 commit dabc3b16d72184da2867e0b3ae37b1d45928cddd
@@ -779,9 +779,10 @@ bool QgsCircularStringV2::deleteVertex( QgsVertexId position )
int nVertices = this->numPoints();
if ( nVertices < 4 ) //circular string must have at least 3 vertices
{
return false;
clear();
return true;
}
if ( position.vertex < 1 || position.vertex > ( nVertices - 2 ) )
if ( position.vertex < 0 || position.vertex > ( nVertices - 1 ) )
{
return false;
}
@@ -539,10 +539,87 @@ bool QgsCompoundCurveV2::moveVertex( QgsVertexId position, const QgsPointV2& new
bool QgsCompoundCurveV2::deleteVertex( QgsVertexId position )
{
QList< QPair<int, QgsVertexId> > curveIds = curveVertexId( position );
QList< QPair<int, QgsVertexId> >::const_iterator idIt = curveIds.constBegin();
for ( ; idIt != curveIds.constEnd(); ++idIt )
if ( curveIds.size() == 1 )
{
if ( !mCurves.at( curveIds.at( 0 ).first )->deleteVertex( curveIds.at( 0 ).second ) )
{
clearCache(); //bbox may have changed
return false;
}
if ( mCurves.at( curveIds.at( 0 ).first )->numPoints() == 0 )
{
removeCurve( curveIds.at( 0 ).first );
}
}
else if ( curveIds.size() == 2 )
{
mCurves.at( idIt->first )->deleteVertex( idIt->second );
Q_ASSERT( curveIds.at( 1 ).first == curveIds.at( 0 ).first + 1 );
Q_ASSERT( curveIds.at( 0 ).second.vertex == mCurves.at( curveIds.at( 0 ).first )->numPoints() - 1 );
Q_ASSERT( curveIds.at( 1 ).second.vertex == 0 );
QgsPointV2 startPoint = mCurves.at( curveIds.at( 0 ).first ) ->startPoint();
QgsPointV2 endPoint = mCurves.at( curveIds.at( 1 ).first ) ->endPoint();
if ( QgsWKBTypes::flatType( mCurves.at( curveIds.at( 0 ).first )->wkbType() ) == QgsWKBTypes::LineString &&
QgsWKBTypes::flatType( mCurves.at( curveIds.at( 1 ).first )->wkbType() ) == QgsWKBTypes::CircularString &&
mCurves.at( curveIds.at( 1 ).first )->numPoints() > 3 )
{
QgsPointV2 intermediatePoint;
QgsVertexId::VertexType type;
mCurves.at( curveIds.at( 1 ).first ) ->pointAt( 2, intermediatePoint, type );
mCurves.at( curveIds.at( 0 ).first )->moveVertex(
QgsVertexId( 0, 0, mCurves.at( curveIds.at( 0 ).first )->numPoints() - 1 ), intermediatePoint );
}
else if ( !mCurves.at( curveIds.at( 0 ).first )->deleteVertex( curveIds.at( 0 ).second ) )
{
clearCache(); //bbox may have changed
return false;
}
if ( QgsWKBTypes::flatType( mCurves.at( curveIds.at( 0 ).first )->wkbType() ) == QgsWKBTypes::CircularString &&
mCurves.at( curveIds.at( 0 ).first )->numPoints() > 0 &&
QgsWKBTypes::flatType( mCurves.at( curveIds.at( 1 ).first )->wkbType() ) == QgsWKBTypes::LineString )
{
QgsPointV2 intermediatePoint = mCurves.at( curveIds.at( 0 ).first ) ->endPoint();
mCurves.at( curveIds.at( 1 ).first )->moveVertex( QgsVertexId( 0, 0, 0 ), intermediatePoint );
}
else if ( !mCurves.at( curveIds.at( 1 ).first )->deleteVertex( curveIds.at( 1 ).second ) )
{
clearCache(); //bbox may have changed
return false;
}
if ( mCurves.at( curveIds.at( 0 ).first )->numPoints() == 0 &&
mCurves.at( curveIds.at( 1 ).first )->numPoints() != 0 )
{
removeCurve( curveIds.at( 0 ).first );
mCurves.at( curveIds.at( 1 ).first )->moveVertex( QgsVertexId( 0, 0, 0 ), startPoint );
}
else if ( mCurves.at( curveIds.at( 0 ).first )->numPoints() != 0 &&
mCurves.at( curveIds.at( 1 ).first )->numPoints() == 0 )
{
removeCurve( curveIds.at( 1 ).first );
mCurves.at( curveIds.at( 0 ).first )->moveVertex(
QgsVertexId( 0, 0, mCurves.at( curveIds.at( 0 ).first )->numPoints() - 1 ), endPoint );
}
else if ( mCurves.at( curveIds.at( 0 ).first )->numPoints() == 0 &&
mCurves.at( curveIds.at( 1 ).first )->numPoints() == 0 )
{
removeCurve( curveIds.at( 1 ).first );
removeCurve( curveIds.at( 0 ).first );
QgsLineStringV2* line = new QgsLineStringV2();
line->insertVertex( QgsVertexId( 0, 0, 0 ), startPoint );
line->insertVertex( QgsVertexId( 0, 0, 1 ), endPoint );
mCurves.insert( curveIds.at( 0 ).first, line );
}
else
{
QgsPointV2 endPointOfFirst = mCurves.at( curveIds.at( 0 ).first ) ->endPoint();
QgsPointV2 startPointOfSecond = mCurves.at( curveIds.at( 1 ).first ) ->startPoint();
if ( endPointOfFirst != startPointOfSecond )
{
QgsLineStringV2* line = new QgsLineStringV2();
line->insertVertex( QgsVertexId( 0, 0, 0 ), endPointOfFirst );
line->insertVertex( QgsVertexId( 0, 0, 1 ), startPointOfSecond );
mCurves.insert( curveIds.at( 1 ).first, line );
}
}
}

bool success = !curveIds.isEmpty();
@@ -574,6 +651,7 @@ QList< QPair<int, QgsVertexId> > QgsCompoundCurveV2::curveVertexId( QgsVertexId
vid.vertex = 0;
curveIds.append( qMakePair( i + 1, vid ) );
}
break;
}
currentVertexIndex += increment;
}
@@ -161,10 +161,13 @@ bool QgsCurvePolygonV2::fromWkt( const QString& wkt )
{
QPair<QgsWKBTypes::Type, QString> childParts = QgsGeometryUtils::wktReadBlock( childWkt );

if ( QgsWKBTypes::flatType( childParts.first ) == QgsWKBTypes::LineString )
QgsWKBTypes::Type flatCurveType = QgsWKBTypes::flatType( childParts.first );
if ( flatCurveType == QgsWKBTypes::LineString )
mInteriorRings.append( new QgsLineStringV2() );
else if ( QgsWKBTypes::flatType( childParts.first ) == QgsWKBTypes::CircularString )
else if ( flatCurveType == QgsWKBTypes::CircularString )
mInteriorRings.append( new QgsCircularStringV2() );
else if ( flatCurveType == QgsWKBTypes::CompoundCurve )
mInteriorRings.append( new QgsCompoundCurveV2() );
else
{
clear();
@@ -739,10 +742,12 @@ bool QgsCurvePolygonV2::deleteVertex( QgsVertexId vId )
if ( success )
{
// If first or last vertex is removed, re-sync the last/first vertex
// Do not use "n - 2", but "ring->numPoints() - 1" as more than one vertex
// may have been deleted (e.g. with CircularString)
if ( vId.vertex == 0 )
ring->moveVertex( QgsVertexId( 0, 0, n - 2 ), ring->vertexAt( QgsVertexId( 0, 0, 0 ) ) );
ring->moveVertex( QgsVertexId( 0, 0, ring->numPoints() - 1 ), ring->vertexAt( QgsVertexId( 0, 0, 0 ) ) );
else if ( vId.vertex == n - 1 )
ring->moveVertex( QgsVertexId( 0, 0, 0 ), ring->vertexAt( QgsVertexId( 0, 0, n - 2 ) ) );
ring->moveVertex( QgsVertexId( 0, 0, 0 ), ring->vertexAt( QgsVertexId( 0, 0, ring->numPoints() - 1 ) ) );
clearCache();
}
return success;
@@ -3146,6 +3146,164 @@ def testWkbTypes(self):
self.assertEqual(QgsWKBTypes.zmType(QgsWKBTypes.MultiSurfaceZM, False, True), QgsWKBTypes.MultiSurfaceM)
self.assertEqual(QgsWKBTypes.zmType(QgsWKBTypes.MultiSurfaceZM, True, True), QgsWKBTypes.MultiSurfaceZM)

def testDeleteVertexCircularString(self):

wkt = "CircularString ((0 0,1 1,2 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(0)
self.assertEqual(geom.exportToWkt(), QgsCircularStringV2().asWkt())

wkt = "CircularString ((0 0,1 1,2 0,3 -1,4 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(0)
expected_wkt = "CircularString (2 0, 3 -1, 4 0)"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CircularString ((0 0,1 1,2 0,3 -1,4 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(1)
expected_wkt = "CircularString (0 0, 3 -1, 4 0)"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CircularString ((0 0,1 1,2 0,3 -1,4 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(2)
expected_wkt = "CircularString (0 0, 1 1, 4 0)"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CircularString ((0 0,1 1,2 0,3 -1,4 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(3)
expected_wkt = "CircularString (0 0, 1 1, 4 0)"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CircularString ((0 0,1 1,2 0,3 -1,4 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(4)
expected_wkt = "CircularString (0 0,1 1,2 0)"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CircularString ((0 0,1 1,2 0,3 -1,4 0))"
geom = QgsGeometry.fromWkt(wkt)
assert not geom.deleteVertex(-1)
assert not geom.deleteVertex(5)

def testDeleteVertexCompoundCurve(self):

wkt = "CompoundCurve ((0 0,1 1))"
geom = QgsGeometry.fromWkt(wkt)
assert not geom.deleteVertex(-1)
assert not geom.deleteVertex(2)
assert geom.deleteVertex(0)
self.assertEqual(geom.exportToWkt(), QgsCompoundCurveV2().asWkt())

wkt = "CompoundCurve ((0 0,1 1))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(1)
self.assertEqual(geom.exportToWkt(), QgsCompoundCurveV2().asWkt())

wkt = "CompoundCurve ((0 0,1 1),(1 1,2 2))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(0)
expected_wkt = "CompoundCurve ((1 1,2 2))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CompoundCurve ((0 0,1 1),(1 1,2 2))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(1)
expected_wkt = "CompoundCurve ((0 0,2 2))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CompoundCurve ((0 0,1 1),(1 1,2 2))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(2)
expected_wkt = "CompoundCurve ((0 0,1 1))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CompoundCurve ((0 0,1 1),CircularString(1 1,2 0,1 -1))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(1)
expected_wkt = "CompoundCurve ((0 0,1 -1))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CompoundCurve (CircularString(0 0,1 1,2 0),CircularString(2 0,3 -1,4 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(2)
expected_wkt = "CompoundCurve ((0 0, 4 0))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CompoundCurve (CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1),(1 -1,0 0))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(4)
expected_wkt = "CompoundCurve (CircularString (0 0, 1 1, 2 0),(2 0, 0 0))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CompoundCurve ((-1 0,0 0),CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(1)
expected_wkt = "CompoundCurve ((-1 0, 2 0),CircularString (2 0, 1.5 -0.5, 1 -1))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CompoundCurve (CircularString(-1 -1,-1.5 -0.5,-2 0,-1 1,0 0),CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(4)
expected_wkt = "CompoundCurve (CircularString (-1 -1, -1.5 -0.5, -2 0),(-2 0, 2 0),CircularString (2 0, 1.5 -0.5, 1 -1))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

def testDeleteVertexCurvePolygon(self):

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0),(2 0,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert not geom.deleteVertex(-1)
assert not geom.deleteVertex(4)
assert geom.deleteVertex(0)
self.assertEqual(geom.exportToWkt(), QgsCurvePolygonV2().asWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0),(2 0,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(1)
self.assertEqual(geom.exportToWkt(), QgsCurvePolygonV2().asWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0),(2 0,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(2)
self.assertEqual(geom.exportToWkt(), QgsCurvePolygonV2().asWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0),(2 0,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(3)
self.assertEqual(geom.exportToWkt(), QgsCurvePolygonV2().asWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1),(1 -1,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(0)
expected_wkt = "CurvePolygon (CompoundCurve (CircularString (2 0, 1.5 -0.5, 1 -1),(1 -1, 2 0)))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1),(1 -1,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(1)
expected_wkt = "CurvePolygon (CompoundCurve (CircularString (0 0, 1.5 -0.5, 1 -1),(1 -1, 0 0)))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1),(1 -1,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(2)
expected_wkt = "CurvePolygon (CompoundCurve (CircularString (0 0, 1 1, 1 -1),(1 -1, 0 0)))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1),(1 -1,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(3)
expected_wkt = "CurvePolygon (CompoundCurve (CircularString (0 0, 1 1, 1 -1),(1 -1, 0 0)))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())

wkt = "CurvePolygon (CompoundCurve (CircularString(0 0,1 1,2 0,1.5 -0.5,1 -1),(1 -1,0 0)))"
geom = QgsGeometry.fromWkt(wkt)
assert geom.deleteVertex(4)
expected_wkt = "CurvePolygon (CompoundCurve (CircularString (0 0, 1 1, 2 0),(2 0, 0 0)))"
self.assertEqual(geom.exportToWkt(), QgsGeometry.fromWkt(expected_wkt).exportToWkt())


if __name__ == '__main__':
unittest.main()

0 comments on commit dabc3b1

Please sign in to comment.
You can’t perform that action at this time.