Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix a crash in tessellator with self-intersecting rings
Self-intersecting polygon rings may crash poly2tri so we skip them (for now)
  • Loading branch information
wonder-sk committed May 7, 2018
1 parent ea38c73 commit 28d7c8c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
22 changes: 16 additions & 6 deletions src/3d/qgstessellator.cpp
Expand Up @@ -282,6 +282,21 @@ static QgsPolygon *_transform_polygon_to_new_base( const QgsPolygon &polygon, co

static bool _check_intersecting_rings( const QgsPolygon &polygon )
{
QList<QgsGeometry> geomRings;
geomRings << QgsGeometry( polygon.exteriorRing()->clone() );
for ( int i = 0; i < polygon.numInteriorRings(); ++i )
geomRings << QgsGeometry( polygon.interiorRing( i )->clone() );

// we need to make sure that the polygon has no rings with self-intersection: that may
// crash the tessellator. The original geometry maybe have been valid and the self-intersection
// was introduced when transforming to a new base (in a rare case when all points are not in the same plane)

for ( int i = 0; i < geomRings.count(); ++i )
{
if ( !geomRings[i].isSimple() )
return false;
}

// At this point we assume that input polygons are valid according to the OGC definition.
// This means e.g. no duplicate points, polygons are simple (no butterfly shaped polygon with self-intersection),
// internal rings are inside exterior rings, rings do not cross each other, no dangles.
Expand All @@ -301,11 +316,6 @@ static bool _check_intersecting_rings( const QgsPolygon &polygon )

if ( polygon.numInteriorRings() > 0 )
{
QList<QgsGeometry> geomRings;
geomRings << QgsGeometry( polygon.exteriorRing()->clone() );
for ( int i = 0; i < polygon.numInteriorRings(); ++i )
geomRings << QgsGeometry( polygon.interiorRing( i )->clone() );

for ( int i = 0; i < geomRings.count(); ++i )
for ( int j = i + 1; j < geomRings.count(); ++j )
{
Expand Down Expand Up @@ -417,7 +427,7 @@ void QgsTessellator::addPolygon( const QgsPolygon &polygon, float extrusionHeigh
if ( !_check_intersecting_rings( *polygonNew.get() ) )
{
// skip the polygon - it would cause a crash inside poly2tri library
QgsMessageLog::logMessage( QObject::tr( "polygon rings intersect each other - skipping" ), QObject::tr( "3D" ) );
QgsMessageLog::logMessage( QObject::tr( "polygon rings self-intersect or intersect each other - skipping" ), QObject::tr( "3D" ) );
return;
}

Expand Down
14 changes: 14 additions & 0 deletions tests/src/3d/testqgstessellator.cpp
Expand Up @@ -132,6 +132,7 @@ class TestQgsTessellator : public QObject
void asMultiPolygon();
void testBadCoordinates();
void testIssue17745();
void testCrashSelfIntersection();

private:
};
Expand Down Expand Up @@ -302,6 +303,19 @@ void TestQgsTessellator::testIssue17745()
t.addPolygon( p, 0 ); // must not crash - that's all we test here
}

void TestQgsTessellator::testCrashSelfIntersection()
{
// this is a polygon where we get self-intersecting exterior ring that would crash poly2tri if not skipped

QgsTessellator t( 0, 0, true );
QgsPolygon p;
bool resWktRead = p.fromWkt( "PolygonZ ((-744809.80499999970197678 -1042371.96730000153183937 260.460968017578125, -744809.80299999937415123 -1042371.92199999839067459 260.460968017578125, -744810.21599999815225601 -1042381.09099999815225601 260.460968017578125, -744810.21499999985098839 -1042381.0689999982714653 260.460968017578125, -744812.96469999849796295 -1042375.32499999925494194 263.734283447265625, -744809.80499999970197678 -1042371.96730000153183937 260.460968017578125))" );

QVERIFY( resWktRead );

t.addPolygon( p, 0 ); // must not crash - that's all we test here
}


QGSTEST_MAIN( TestQgsTessellator )
#include "testqgstessellator.moc"

0 comments on commit 28d7c8c

Please sign in to comment.