Skip to content

/ QGIS

Permalink
Browse files
Fix is_closed (and other expression functions) fail with multi* input… 
… geometries

Fixes #19704

(cherry picked from commit e5e14dd)
  • Loading branch information
@nyalldawson
nyalldawson committed Aug 31, 2018
1 parent 8aaeb7b commit c9442a9f0a0f30796cddabe7d694180889585837
Unified Split
Showing with 213 additions and 8 deletions.
  1. +210 −8 src/core/expression/qgsexpressionfunction.cpp
  2. +3 −0 tests/src/core/testqgsexpression.cpp
218 src/core/expression/qgsexpressionfunction.cpp
View file
@@ -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 )
3 tests/src/core/testqgsexpression.cpp
View file
@@ -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)" );

0 comments on commit c9442a9

Please sign in to comment.