Fix is_closed (and other expression functions) fail with multi* input…

… geometries Fixes #19704 (cherry picked from commit e5e14dd)
qgis · Aug 31, 2018 · c9442a9 · c9442a9
1 parent 8aaeb7b
commit c9442a9
Show file tree

Hide file tree

Showing 2 changed files with 213 additions and 8 deletions.
diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -1655,6 +1655,17 @@ static QVariant fcnGeomZ( const QVariantList &values, const QgsExpressionContext
     if ( point )
       return point->z();
   }
+  else if ( geom.type() == QgsWkbTypes::PointGeometry && geom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( geom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        if ( const QgsPoint *point = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) ) )
+          return point->z();
+      }
+    }
+  }
 
   return QVariant();
 }
@@ -1672,6 +1683,17 @@ static QVariant fcnGeomM( const QVariantList &values, const QgsExpressionContext
     if ( point )
       return point->m();
   }
+  else if ( geom.type() == QgsWkbTypes::PointGeometry && geom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( geom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        if ( const QgsPoint *point = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) ) )
+          return point->m();
+      }
+    }
+  }
 
   return QVariant();
 }
@@ -1802,6 +1824,17 @@ static QVariant fcnInteriorRingN( const QVariantList &values, const QgsExpressio
     return QVariant();
 
   const QgsCurvePolygon *curvePolygon = qgsgeometry_cast< const QgsCurvePolygon * >( geom.constGet() );
+  if ( !curvePolygon && geom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( geom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        curvePolygon = qgsgeometry_cast< const QgsCurvePolygon * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+
   if ( !curvePolygon )
     return QVariant();
 
@@ -1811,7 +1844,7 @@ static QVariant fcnInteriorRingN( const QVariantList &values, const QgsExpressio
   if ( idx >= curvePolygon->numInteriorRings() || idx < 0 )
     return QVariant();
 
-  QgsCurve *curve = static_cast< QgsCurve * >( curvePolygon->interiorRing( idx )->clone() );
+  QgsCurve *curve = static_cast< QgsCurve * >( curvePolygon->interiorRing( static_cast< int >( idx ) )->clone() );
   QVariant result = curve ? QVariant::fromValue( QgsGeometry( curve ) ) : QVariant();
   return result;
 }
@@ -1833,7 +1866,7 @@ static QVariant fcnGeometryN( const QVariantList &values, const QgsExpressionCon
   if ( idx < 0 || idx >= collection->numGeometries() )
     return QVariant();
 
-  QgsAbstractGeometry *part = collection->geometryN( idx )->clone();
+  QgsAbstractGeometry *part = collection->geometryN( static_cast< int >( idx ) )->clone();
   QVariant result = part ? QVariant::fromValue( QgsGeometry( part ) ) : QVariant();
   return result;
 }
@@ -1989,25 +2022,57 @@ static QVariant fcnMakePolygon( const QVariantList &values, const QgsExpressionC
   }
 
   QgsGeometry outerRing = QgsExpressionUtils::getGeometry( values.at( 0 ), parent );
-  if ( outerRing.type() != QgsWkbTypes::LineGeometry || outerRing.isMultipart() || outerRing.isNull() )
+  if ( outerRing.type() != QgsWkbTypes::LineGeometry || outerRing.isNull() )
     return QVariant();
 
-  QgsPolygon *polygon = new QgsPolygon();
-  polygon->setExteriorRing( qgsgeometry_cast< QgsCurve * >( outerRing.constGet()->clone() ) );
+  std::unique_ptr< QgsPolygon > polygon = qgis::make_unique< QgsPolygon >();
+
+  const QgsCurve *exteriorRing = qgsgeometry_cast< QgsCurve * >( outerRing.constGet() );
+  if ( !exteriorRing && outerRing.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( outerRing.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        exteriorRing = qgsgeometry_cast< QgsCurve * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+
+  if ( !exteriorRing )
+    return QVariant();
+
+  polygon->setExteriorRing( exteriorRing->segmentize() );
+
 
   for ( int i = 1; i < values.count(); ++i )
   {
     QgsGeometry ringGeom = QgsExpressionUtils::getGeometry( values.at( i ), parent );
     if ( ringGeom.isNull() )
       continue;
 
-    if ( ringGeom.type() != QgsWkbTypes::LineGeometry || ringGeom.isMultipart() || ringGeom.isNull() )
+    if ( ringGeom.type() != QgsWkbTypes::LineGeometry || ringGeom.isNull() )
+      continue;
+
+    const QgsCurve *ring = qgsgeometry_cast< QgsCurve * >( ringGeom.constGet() );
+    if ( !ring && ringGeom.isMultipart() )
+    {
+      if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( ringGeom.constGet() ) )
+      {
+        if ( collection->numGeometries() == 1 )
+        {
+          ring = qgsgeometry_cast< QgsCurve * >( collection->geometryN( 0 ) );
+        }
+      }
+    }
+
+    if ( !ring )
       continue;
 
-    polygon->addInteriorRing( qgsgeometry_cast< QgsCurve * >( ringGeom.constGet()->clone() ) );
+    polygon->addInteriorRing( ring->segmentize() );
   }
 
-  return QVariant::fromValue( QgsGeometry( polygon ) );
+  return QVariant::fromValue( QgsGeometry( std::move( polygon ) ) );
 }
 
 static QVariant fcnMakeTriangle( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
@@ -2026,6 +2091,17 @@ static QVariant fcnMakeTriangle( const QVariantList &values, const QgsExpression
       return QVariant();
 
     const QgsPoint *point = qgsgeometry_cast< const QgsPoint * >( geom.constGet() );
+    if ( !point && geom.isMultipart() )
+    {
+      if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( geom.constGet() ) )
+      {
+        if ( collection->numGeometries() == 1 )
+        {
+          point = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+        }
+      }
+    }
+
     if ( !point )
       return QVariant();
 
@@ -2055,6 +2131,19 @@ static QVariant fcnMakeCircle( const QVariantList &values, const QgsExpressionCo
     return QVariant();
   }
   const QgsPoint *point = qgsgeometry_cast< const QgsPoint * >( geom.constGet() );
+  if ( !point && geom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( geom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        point = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+  if ( !point )
+    return QVariant();
+
   QgsCircle circ( *point, radius );
   return QVariant::fromValue( QgsGeometry( circ.toPolygon( segment ) ) );
 }
@@ -2078,6 +2167,19 @@ static QVariant fcnMakeEllipse( const QVariantList &values, const QgsExpressionC
     return QVariant();
   }
   const QgsPoint *point = qgsgeometry_cast< const QgsPoint * >( geom.constGet() );
+  if ( !point && geom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( geom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        point = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+  if ( !point )
+    return QVariant();
+
   QgsEllipse elp( *point, majorAxis,  minorAxis, azimuth );
   return QVariant::fromValue( QgsGeometry( elp.toPolygon( segment ) ) );
 }
@@ -2112,8 +2214,34 @@ static QVariant fcnMakeRegularPolygon( const QVariantList &values, const QgsExpr
     parent->setEvalErrorString( QObject::tr( "Option can be 0 (inscribed) or 1 (circumscribed)" ) );
     return QVariant();
   }
+
   const QgsPoint *center = qgsgeometry_cast< const QgsPoint * >( pt1.constGet() );
+  if ( !center && pt1.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( pt1.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        center = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+  if ( !center )
+    return QVariant();
+
   const QgsPoint *corner = qgsgeometry_cast< const QgsPoint * >( pt2.constGet() );
+  if ( !corner && pt2.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( pt2.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        corner = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+  if ( !corner )
+    return QVariant();
 
   QgsRegularPolygon rp = QgsRegularPolygon( *center, *corner, nbEdges, option );
 
@@ -2394,6 +2522,17 @@ static QVariant fcnIsClosed( const QVariantList &values, const QgsExpressionCont
     return QVariant();
 
   const QgsCurve *curve = qgsgeometry_cast< const QgsCurve * >( fGeom.constGet() );
+  if ( !curve && fGeom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        curve = qgsgeometry_cast< const QgsCurve * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+
   if ( !curve )
     return QVariant();
 
@@ -2496,6 +2635,17 @@ static QVariant fcnWedgeBuffer( const QVariantList &values, const QgsExpressionC
 {
   QgsGeometry fGeom = QgsExpressionUtils::getGeometry( values.at( 0 ), parent );
   const QgsPoint *pt = qgsgeometry_cast<const QgsPoint *>( fGeom.constGet() );
+  if ( !pt && fGeom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        pt = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+
   if ( !pt )
   {
     parent->setEvalErrorString( QObject::tr( "Function `wedge_buffer` requires a point value for the center." ) );
@@ -2699,6 +2849,17 @@ static QVariant fcnExteriorRing( const QVariantList &values, const QgsExpression
     return QVariant();
 
   const QgsCurvePolygon *curvePolygon = qgsgeometry_cast< const QgsCurvePolygon * >( fGeom.constGet() );
+  if ( !curvePolygon && fGeom.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        curvePolygon = qgsgeometry_cast< const QgsCurvePolygon * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+
   if ( !curvePolygon || !curvePolygon->exteriorRing() )
     return QVariant();
 
@@ -2783,7 +2944,28 @@ static QVariant fcnAzimuth( const QVariantList &values, const QgsExpressionConte
   QgsGeometry fGeom2 = QgsExpressionUtils::getGeometry( values.at( 1 ), parent );
 
   const QgsPoint *pt1 = qgsgeometry_cast<const QgsPoint *>( fGeom1.constGet() );
+  if ( !pt1 && fGeom1.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom1.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        pt1 = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
+
   const QgsPoint *pt2 = qgsgeometry_cast<const QgsPoint *>( fGeom2.constGet() );
+  if ( !pt2 && fGeom2.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom2.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        pt2 = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
 
   if ( !pt1 || !pt2 )
   {
@@ -2866,7 +3048,27 @@ static QVariant fcnInclination( const QVariantList &values, const QgsExpressionC
   QgsGeometry fGeom2 = QgsExpressionUtils::getGeometry( values.at( 1 ), parent );
 
   const QgsPoint *pt1 = qgsgeometry_cast<const QgsPoint *>( fGeom1.constGet() );
+  if ( !pt1 && fGeom1.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom1.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        pt1 = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
   const QgsPoint *pt2 = qgsgeometry_cast<const QgsPoint *>( fGeom2.constGet() );
+  if ( !pt2 && fGeom2.isMultipart() )
+  {
+    if ( const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom2.constGet() ) )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        pt2 = qgsgeometry_cast< const QgsPoint * >( collection->geometryN( 0 ) );
+      }
+    }
+  }
 
   if ( ( fGeom1.type() != QgsWkbTypes::PointGeometry ) || ( fGeom2.type() != QgsWkbTypes::PointGeometry ) ||
        !pt1 || !pt2 )

diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -888,6 +888,9 @@ class TestQgsExpression: public QObject
       QTest::newRow( "is_closed polygon" ) << "is_closed(geom_from_wkt('POLYGON((-1 -1, 4 0, 4 2, 0 2, -1 -1))'))" << false << QVariant();
       QTest::newRow( "is_closed not closed" ) << "is_closed(geom_from_wkt('LINESTRING(0 0, 1 1, 2 2)'))" << false << QVariant( false );
       QTest::newRow( "is_closed closed" ) << "is_closed(geom_from_wkt('LINESTRING(0 0, 1 1, 2 2, 0 0)'))" << false << QVariant( true );
+      QTest::newRow( "is_closed multiline" ) << "is_closed(geom_from_wkt('MultiLineString ((6501338.13976828 4850981.51459331, 6501343.09036573 4850984.01453377, 6501338.13976828 4850988.96491092, 6501335.63971657 4850984.01453377, 6501338.13976828 4850981.51459331))'))" << false << QVariant( true );
+      QTest::newRow( "is_closed multiline" ) << "is_closed(geom_from_wkt('MultiLineString ((6501338.13976828 4850981.51459331, 6501343.09036573 4850984.01453377, 6501338.13976828 4850988.96491092, 6501335.63971657 4850984.01453377, 6501438.13976828 4850981.51459331))'))" << false << QVariant( false );
+      QTest::newRow( "is_closed multiline" ) << "is_closed(geom_from_wkt('MultiLineString ()'))" << false << QVariant();
       QTest::newRow( "make_point" ) << "geom_to_wkt(make_point(2.2,4.4))" << false << QVariant( "Point (2.2 4.4)" );
       QTest::newRow( "make_point z" ) << "geom_to_wkt(make_point(2.2,4.4,5.5))" << false << QVariant( "PointZ (2.2 4.4 5.5)" );
       QTest::newRow( "make_point zm" ) << "geom_to_wkt(make_point(2.2,4.4,5.5,6.6))" << false << QVariant( "PointZM (2.2 4.4 5.5 6.6)" );