Fix crash in tessellation (polygon 3D symbol) for touching rings

wonder-sk · wonder-sk · commit 0951ff7ee36b · 2017-10-01T21:33:04.000+02:00
diff --git a/src/3d/qgstessellator.cpp b/src/3d/qgstessellator.cpp
@@ -16,6 +16,7 @@
 #include "qgstessellator.h"
 
 #include "qgscurve.h"
+#include "qgsgeometry.h"
 #include "qgspoint.h"
 #include "qgspolygon.h"
 
@@ -111,6 +112,42 @@ static void _makeWalls( const QgsCurve &ring, bool ccw, float extrusionHeight, Q
 
 void QgsTessellator::addPolygon( const QgsPolygonV2 &polygon, float extrusionHeight )
 {
+  // 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.
+
+  // There is however an issue with polygons where rings touch:
+  //  +---+
+  //  |   |
+  //  | +-+-+
+  //  | | | |
+  //  | +-+ |
+  //  |     |
+  //  +-----+
+  // This is a valid polygon with one exterior and one interior ring that touch at one point,
+  // but poly2tri library does not allow interior rings touch each other or exterior ring.
+  // TODO: Handle the situation better - rather than just detecting the problem, try to fix
+  // it by converting touching rings into one ring.
+
+  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 )
+      {
+        if ( geomRings[i].intersects( geomRings[j] ) )
+        {
+          // skip the polygon - it would cause a crash inside poly2tri library
+          qDebug() << "polygon rings intersect each other - skipping";
+          return;
+        }
+      }
+  }
+
   const QgsCurve *exterior = polygon.exteriorRing();
 
   QList< std::vector<p2t::Point *> > polylinesToDelete;
diff --git a/src/3d/terrain/qgsflatterraingenerator.cpp b/src/3d/terrain/qgsflatterraingenerator.cpp
@@ -93,6 +93,10 @@ Qt3DCore::QEntity *FlatTerrainChunkLoader::createEntity( Qt3DCore::QEntity *pare
 // ---------------
 
 
+QgsFlatTerrainGenerator::QgsFlatTerrainGenerator()
+{
+}
+
 QgsChunkLoader *QgsFlatTerrainGenerator::createChunkLoader( QgsChunkNode *node ) const
 {
   return new FlatTerrainChunkLoader( mTerrain, node );
diff --git a/src/3d/terrain/qgsflatterraingenerator.h b/src/3d/terrain/qgsflatterraingenerator.h
@@ -31,7 +31,8 @@
 class _3D_EXPORT QgsFlatTerrainGenerator : public QgsTerrainGenerator
 {
   public:
-    QgsFlatTerrainGenerator() = default;
+    //! Creates flat terrain generator object
+    QgsFlatTerrainGenerator();
 
     virtual QgsChunkLoader *createChunkLoader( QgsChunkNode *node ) const override SIP_FACTORY;
 

Original file line number	Diff line number	Diff line change
`@@ -93,6 +93,10 @@ Qt3DCore::QEntity FlatTerrainChunkLoader::createEntity( Qt3DCore::QEntity pare`
`93`	`93`	`// ---------------`
`94`	`94`
`95`	`95`
	`96`	`+QgsFlatTerrainGenerator::QgsFlatTerrainGenerator()`
	`97`	`+{`
	`98`	`+}`
	`99`	`+`
`96`	`100`	`QgsChunkLoader QgsFlatTerrainGenerator::createChunkLoader( QgsChunkNode node ) const`
`97`	`101`	`{`
`98`	`102`	`return new FlatTerrainChunkLoader( mTerrain, node );`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,8 @@`
`31`	`31`	`class _3D_EXPORT QgsFlatTerrainGenerator : public QgsTerrainGenerator`
`32`	`32`	`{`
`33`	`33`	`public:`
`34`		`- QgsFlatTerrainGenerator() = default;`
	`34`	`+ //! Creates flat terrain generator object`
	`35`	`+ QgsFlatTerrainGenerator();`
`35`	`36`
`36`	`37`	`virtual QgsChunkLoader createChunkLoader( QgsChunkNode node ) const override SIP_FACTORY;`
`37`	`38`