Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Address PR review comments
  • Loading branch information
elpaso committed Jan 6, 2022
1 parent aca1200 commit fb7b3e0
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 37 deletions.
8 changes: 4 additions & 4 deletions resources/function_help/json/overlay_intersects
Expand Up @@ -40,13 +40,13 @@
"optional": true
},
{
"arg": "return_measure",
"description": "only valid when used with an expression, set this to true to return a list of maps containing the feature id, the expression result and the overlap value, the radius of the maximum inscribed circle is also returned when the target layer is a polygon.",
"arg": "return_details",
"description": "only valid when used with an expression, set this to true to return a list of maps containing (key names in quotes) the feature 'id', the expression 'result' and the 'overlap' value, the 'radius' of the maximum inscribed circle is also returned when the target layer is a polygon.",
"optional": true
},
{
"arg": "sort_by_intersection_size",
"description": "only valid when used with an expression, set this to true to return the results ordered by the overlap value in descending order.",
"description": "only valid when used with an expression, set this to 'des' to return the results ordered by the overlap value in descending order or set this to 'asc' for ascending order.",
"optional": true
}
],
Expand Down Expand Up @@ -84,7 +84,7 @@
"returns": "true if the current feature spatially intersects a region and the intersection area maximum inscribed circle's radius (of at least one of the parts in case of multipart) is greater or equal to the 0.54"
},
{
"expression": "overlay_intersects(layer:='regions', expression:= geom_to_wkt($geometry), return_measure:=true)",
"expression": "overlay_intersects(layer:='regions', expression:= geom_to_wkt($geometry), return_details:=true)",
"returns": "an array of maps with the feature id, the expression value, the overlap value and the radius of the maximum inscribed circle"
},
{
Expand Down
25 changes: 15 additions & 10 deletions src/core/expression/qgsexpressionfunction.cpp
Expand Up @@ -6649,12 +6649,13 @@ static QVariant executeGeomOverlay( const QVariantList &values, const QgsExpress

// Sixth parameter (for intersects only) is the min overlap (area or length)
// Seventh parameter (for intersects only) is the min inscribed circle radius
// Eighth parameter (for intersects only) is the return_measure
// Eighth parameter (for intersects only) is the return_details
// Ninth parameter (for intersects only) is the sort_by_intersection_size flag
double minOverlap { -1 };
double minInscribedCircleRadius { -1 };
bool returnMeasures = false;
bool returnDetails = false;
bool sortByMeasure = false;
bool sortAscending = false;
bool requireMeasures = false;
bool overlapOrRadiusFilter = false;
if ( isIntersectsFunc )
Expand All @@ -6679,11 +6680,13 @@ static QVariant executeGeomOverlay( const QVariantList &values, const QgsExpress
#endif
node = QgsExpressionUtils::getNode( values.at( 7 ), parent );
// Return measures is only effective when an expression is set
returnMeasures = !testOnly && node->eval( parent, context ).toBool();
returnDetails = !testOnly && node->eval( parent, context ).toBool();
node = QgsExpressionUtils::getNode( values.at( 8 ), parent );
// Sort by measures is only effective when an expression is set
sortByMeasure = !testOnly && node->eval( parent, context ).toBool();
requireMeasures = sortByMeasure || returnMeasures;
const QString sorting { node->eval( parent, context ).toString().toLower() };
sortByMeasure = !testOnly && ( sorting.startsWith( "asc" ) || sorting.startsWith( "des" ) );
sortAscending = sorting.startsWith( "asc" );
requireMeasures = sortByMeasure || returnDetails;
overlapOrRadiusFilter = minInscribedCircleRadius != -1 || minOverlap != -1;
}

Expand Down Expand Up @@ -7026,9 +7029,11 @@ static QVariant executeGeomOverlay( const QVariantList &values, const QgsExpress
{
if ( sortByMeasure )
{
std::sort( results.begin(), results.end(), [ ]( const QVariant & recordA, const QVariant & recordB ) -> bool
std::sort( results.begin(), results.end(), [ sortAscending ]( const QVariant & recordA, const QVariant & recordB ) -> bool
{
return recordA.toMap().value( QStringLiteral( "overlap" ) ).toDouble() > recordB.toMap().value( QStringLiteral( "overlap" ) ).toDouble();
return sortAscending ?
recordB.toMap().value( QStringLiteral( "overlap" ) ).toDouble() > recordA.toMap().value( QStringLiteral( "overlap" ) ).toDouble()
: recordA.toMap().value( QStringLiteral( "overlap" ) ).toDouble() > recordB.toMap().value( QStringLiteral( "overlap" ) ).toDouble();
} );
}
// Resize
Expand All @@ -7037,7 +7042,7 @@ static QVariant executeGeomOverlay( const QVariantList &values, const QgsExpress
results.erase( results.begin() + limit );
}

if ( ! returnMeasures )
if ( ! returnDetails )
{
QVariantList expResults;
for ( auto it = results.constBegin(); it != results.constEnd(); ++it )
Expand Down Expand Up @@ -7521,8 +7526,8 @@ const QList<QgsExpressionFunction *> &QgsExpression::Functions()
<< QgsExpressionFunction::Parameter( QStringLiteral( "cache" ), true, QVariant( false ), false )
<< QgsExpressionFunction::Parameter( QStringLiteral( "min_overlap" ), true, QVariant( -1 ), false )
<< QgsExpressionFunction::Parameter( QStringLiteral( "min_inscribed_circle_radius" ), true, QVariant( -1 ), false )
<< QgsExpressionFunction::Parameter( QStringLiteral( "return_measure" ), true, false, false )
<< QgsExpressionFunction::Parameter( QStringLiteral( "sort_by_intersection_size" ), true, false, false ),
<< QgsExpressionFunction::Parameter( QStringLiteral( "return_details" ), true, false, false )
<< QgsExpressionFunction::Parameter( QStringLiteral( "sort_by_intersection_size" ), true, QString(), false ),
i.value(), QStringLiteral( "GeometryGroup" ), QString(), true, QSet<QString>() << QgsFeatureRequest::ALL_ATTRIBUTES, true );

// The current feature is accessed for the geometry, so this should not be cached
Expand Down
50 changes: 27 additions & 23 deletions tests/src/core/testqgsoverlayexpression.cpp
Expand Up @@ -273,23 +273,23 @@ void TestQgsOverlayExpression::testOverlayMeasure_data()
#if GEOS_VERSION_MAJOR>3 || ( GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR>=9 )
expected1.insert( QStringLiteral( "radius" ), 0.46454276882989376 );
#endif
QTest::newRow( "intersects min_overlap multi match return measure" ) << "overlay_intersects('polys', expression:=$id, min_overlap:=1.34, return_measure:=true)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << expected3 ) ;
QTest::newRow( "intersects min_overlap multi match return measure" ) << "overlay_intersects('polys', expression:=$id, min_overlap:=1.34, return_details:=true)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << expected3 ) ;

QTest::newRow( "intersects multi match return measure" ) << "overlay_intersects('polys', expression:=$id, return_measure:=true)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << expected1 << expected3 ) ;
QTest::newRow( "intersects multi match return measure" ) << "overlay_intersects('polys', expression:=$id, return_details:=true)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << expected1 << expected3 ) ;

QTest::newRow( "intersects multi match return sorted measure" ) << "overlay_intersects('polys', expression:=$id, sort_by_intersection_size:=true, return_measure:=true)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << expected3 << expected1 ) ;
QTest::newRow( "intersects multi match return sorted measure" ) << "overlay_intersects('polys', expression:=$id, sort_by_intersection_size:='des', return_details:=true)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << expected3 << expected1 ) ;

QTest::newRow( "intersects multi match return sorted" ) << "overlay_intersects('polys', expression:=$id, sort_by_intersection_size:=true)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << 3LL << 1LL ) ;
QTest::newRow( "intersects multi match return sorted" ) << "overlay_intersects('polys', expression:=$id, sort_by_intersection_size:='des')" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << 3LL << 1LL ) ;

QTest::newRow( "intersects multi match return unsorted" ) << "overlay_intersects('polys', expression:=$id)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << 1LL << 3LL ) ;

QTest::newRow( "intersects multi match return unsorted limit " ) << "overlay_intersects('polys', limit:=1, expression:=$id)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << 1LL ) ;

QTest::newRow( "intersects multi match return sorted limit " ) << "overlay_intersects('polys', sort_by_intersection_size:=true, limit:=1, expression:=$id)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << 3LL ) ;
QTest::newRow( "intersects multi match return sorted limit " ) << "overlay_intersects('polys', sort_by_intersection_size:='des', limit:=1, expression:=$id)" << "POLYGON((-107.37 33.75, -102.76 33.75, -102.76 36.97, -107.37 36.97, -107.37 33.75))" << ( QVariantList() << 3LL ) ;

QTest::newRow( "intersects multi match points" ) << "overlay_intersects('polys', expression:=$id)" << "MULTIPOINT((-107.37 33.75), (-102.8 36.97))" << ( QVariantList() << 1LL << 3LL ) ;

QTest::newRow( "intersects multi match points sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:=true, expression:=$id)" << "MULTIPOINT((-107.37 33.75), (-102.8 36.97))" << ( QVariantList() << 3LL << 1LL );
QTest::newRow( "intersects multi match points sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:='des', expression:=$id)" << "MULTIPOINT((-107.37 33.75), (-102.8 36.97))" << ( QVariantList() << 3LL << 1LL );

{
// Check returned values from point intersection
Expand All @@ -302,7 +302,7 @@ void TestQgsOverlayExpression::testOverlayMeasure_data()
expected1.insert( QStringLiteral( "result" ), 1 );
expected1.insert( QStringLiteral( "overlap" ), 18.569843123334977 );

QTest::newRow( "intersects multi match points return sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:=true, return_measure:=true, expression:=$id)" << "MULTIPOINT((-107.37 33.75), (-102.8 36.97))" << ( QVariantList() << expected3 << expected1 ) ;
QTest::newRow( "intersects multi match points return sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:='des', return_details:=true, expression:=$id)" << "MULTIPOINT((-107.37 33.75), (-102.8 36.97))" << ( QVariantList() << expected3 << expected1 ) ;

}

Expand All @@ -319,20 +319,24 @@ void TestQgsOverlayExpression::testOverlayMeasure_data()

QTest::newRow( "intersects linestring multi match" ) << "overlay_intersects('polys', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 1LL << 3LL );

QTest::newRow( "intersects linestring multi match sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 3LL << 1LL );
QTest::newRow( "intersects linestring multi match sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:='des', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 3LL << 1LL );

QTest::newRow( "intersects linestring multi match sorted limit" ) << "overlay_intersects('polys', limit:=1, sort_by_intersection_size:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 3LL );
QTest::newRow( "intersects linestring multi match sorted limit" ) << "overlay_intersects('polys', limit:=1, sort_by_intersection_size:='des', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 3LL );

QTest::newRow( "intersects linestring multi match measure sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 3LL << 1LL );
QTest::newRow( "intersects linestring multi match measure sorted" ) << "overlay_intersects('polys', sort_by_intersection_size:='des', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 3LL << 1LL );

QTest::newRow( "intersects linestring multi match measure sorted asc" ) << "overlay_intersects('polys', sort_by_intersection_size:='asc', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << 1LL << 3LL );

// Return measure
QTest::newRow( "intersects linestring multi match" ) << "overlay_intersects('polys', return_measure:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine1 << expectedLine3 );
QTest::newRow( "intersects linestring multi match" ) << "overlay_intersects('polys', return_details:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine1 << expectedLine3 );

QTest::newRow( "intersects linestring multi match sorted" ) << "overlay_intersects('polys', return_details:=true, sort_by_intersection_size:='des', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine3 << expectedLine1 );

QTest::newRow( "intersects linestring multi match sorted" ) << "overlay_intersects('polys', return_measure:=true, sort_by_intersection_size:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine3 << expectedLine1 );
QTest::newRow( "intersects linestring multi match sorted asc" ) << "overlay_intersects('polys', return_details:=true, sort_by_intersection_size:='asc', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine1 << expectedLine3 );

QTest::newRow( "intersects linestring multi match sorted limit" ) << "overlay_intersects('polys', return_measure:=true, limit:=1, sort_by_intersection_size:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine3 );
QTest::newRow( "intersects linestring multi match sorted limit" ) << "overlay_intersects('polys', return_details:=true, limit:=1, sort_by_intersection_size:='des', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine3 );

QTest::newRow( "intersects linestring multi match measure sorted" ) << "overlay_intersects('polys', return_measure:=true, sort_by_intersection_size:=true, expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine3 << expectedLine1 );
QTest::newRow( "intersects linestring multi match measure sorted" ) << "overlay_intersects('polys', return_details:=true, sort_by_intersection_size:='des', expression:=$id)" << "LINESTRING(-102.76 33.74, -102.76 36.44)" << ( QVariantList() << expectedLine3 << expectedLine1 );


// Test linestring intersections
Expand All @@ -346,19 +350,19 @@ void TestQgsOverlayExpression::testOverlayMeasure_data()
expectedLine2.insert( QStringLiteral( "result" ), 2 );
expectedLine2.insert( QStringLiteral( "overlap" ), 0 );

QTest::newRow( "intersects linestring single match" ) << "overlay_intersects('linestrings', return_measure:=true, expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1)" << ( QVariantList() << expectedLine1 );
QTest::newRow( "intersects linestring single match fail overlap" ) << "overlay_intersects('linestrings', min_overlap:=0.5, return_measure:=true, expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1)" << ( QVariantList() );
QTest::newRow( "intersects linestring no match" ) << "overlay_intersects('linestrings', return_measure:=true, expression:=$id)" << "LINESTRING(1.5 2, 1.5 1)" << ( QVariantList() );
QTest::newRow( "intersects linestring multi match" ) << "overlay_intersects('linestrings', return_measure:=true, expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1, 4 -1, 4 1)" << ( QVariantList() << expectedLine1 << expectedLine2 );
QTest::newRow( "intersects linestring single match" ) << "overlay_intersects('linestrings', return_details:=true, expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1)" << ( QVariantList() << expectedLine1 );
QTest::newRow( "intersects linestring single match fail overlap" ) << "overlay_intersects('linestrings', min_overlap:=0.5, return_details:=true, expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1)" << ( QVariantList() );
QTest::newRow( "intersects linestring no match" ) << "overlay_intersects('linestrings', return_details:=true, expression:=$id)" << "LINESTRING(1.5 2, 1.5 1)" << ( QVariantList() );
QTest::newRow( "intersects linestring multi match" ) << "overlay_intersects('linestrings', return_details:=true, expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1, 4 -1, 4 1)" << ( QVariantList() << expectedLine1 << expectedLine2 );
expectedLine2[QStringLiteral( "overlap" )] = 0.5;
QTest::newRow( "intersects linestring multi match sorted" ) << "overlay_intersects('linestrings', return_measure:=true, sort_by_intersection_size:=true, expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1, 4 -1, 4 0, 4.5 0)" << ( QVariantList() << expectedLine2 << expectedLine1 );
QTest::newRow( "intersects linestring multi match sorted" ) << "overlay_intersects('linestrings', return_details:=true, sort_by_intersection_size:='des', expression:=$id)" << "LINESTRING(1.5 1, 1.5 -1, 4 -1, 4 0, 4.5 0)" << ( QVariantList() << expectedLine2 << expectedLine1 );

// Full length of targets is expected
expectedLine2[QStringLiteral( "overlap" )] = 2.0;
expectedLine1[QStringLiteral( "overlap" )] = 1.0;

QTest::newRow( "intersects linestring multi match points" ) << "overlay_intersects('linestrings', return_measure:=true, expression:=$id)" << "MULTIPOINT((1.5 0), (3.5 0))" << ( QVariantList() << expectedLine1 << expectedLine2 );
QTest::newRow( "intersects linestring multi match points sorted" ) << "overlay_intersects('linestrings', sort_by_intersection_size:=true, return_measure:=true, expression:=$id)" << "MULTIPOINT((1.5 0), (3.5 0))" << ( QVariantList() << expectedLine2 << expectedLine1 );
QTest::newRow( "intersects linestring multi match points" ) << "overlay_intersects('linestrings', return_details:=true, expression:=$id)" << "MULTIPOINT((1.5 0), (3.5 0))" << ( QVariantList() << expectedLine1 << expectedLine2 );
QTest::newRow( "intersects linestring multi match points sorted" ) << "overlay_intersects('linestrings', sort_by_intersection_size:='des', return_details:=true, expression:=$id)" << "MULTIPOINT((1.5 0), (3.5 0))" << ( QVariantList() << expectedLine2 << expectedLine1 );

}

Expand All @@ -373,8 +377,8 @@ void TestQgsOverlayExpression::testOverlayMeasure_data()
expectedPoint2.insert( QStringLiteral( "result" ), 2 );
expectedPoint2.insert( QStringLiteral( "overlap" ), 0 );

QTest::newRow( "intersects points single match" ) << "overlay_intersects('points', return_measure:=true, expression:=$id)" << "POLYGON((0 -1, 1.5 -1, 1.5 1, 0 1, 0 -1))" << ( QVariantList() << expectedPoint1 );
QTest::newRow( "intersects points multi match" ) << "overlay_intersects('points', return_measure:=true, expression:=$id)" << "POLYGON((0 -1, 3.5 -1, 3.5 1, 0 1, 0 -1))" << ( QVariantList() << expectedPoint1 << expectedPoint2 );
QTest::newRow( "intersects points single match" ) << "overlay_intersects('points', return_details:=true, expression:=$id)" << "POLYGON((0 -1, 1.5 -1, 1.5 1, 0 1, 0 -1))" << ( QVariantList() << expectedPoint1 );
QTest::newRow( "intersects points multi match" ) << "overlay_intersects('points', return_details:=true, expression:=$id)" << "POLYGON((0 -1, 3.5 -1, 3.5 1, 0 1, 0 -1))" << ( QVariantList() << expectedPoint1 << expectedPoint2 );
}

}
Expand Down

0 comments on commit fb7b3e0

Please sign in to comment.