Fix a crash in tessellator with self-intersecting rings

wonder-sk · wonder-sk · commit 28d7c8c46937 · 2018-05-07T22:11:27.000+02:00
Self-intersecting polygon rings may crash poly2tri so we skip them (for now)
diff --git a/src/3d/qgstessellator.cpp b/src/3d/qgstessellator.cpp
@@ -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.
@@ -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 )
       {
@@ -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;
     }
 
diff --git a/tests/src/3d/testqgstessellator.cpp b/tests/src/3d/testqgstessellator.cpp
@@ -132,6 +132,7 @@ class TestQgsTessellator : public QObject
     void asMultiPolygon();
     void testBadCoordinates();
     void testIssue17745();
+    void testCrashSelfIntersection();
 
   private:
 };
@@ -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"
