Processing port geometry tools #53787
Conversation
|
I'm +1 to port python processing to C++. But, please, can you make one PR by algorithm as possible? |
|
@alexbruy A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged. Please update the description (not the comments) with helpful description and screenshot to help the work from documentors. Thank you! |
| engine->prepareGeometry(); | ||
| for ( const QgsFeatureId id : intersected ) | ||
| { | ||
| if ( engine->intersects( index.geometry( id ).constGet() ) ) |
There was a problem hiding this comment.
This should be implied when querying the index built using single points -- we could save the cost here and also avoid storing the geometries in the index.
Potentially a big speed boost by using QgsSpatialIndexKDBush here too.
There was a problem hiding this comment.
Not sure I understand. We can't check for exact intersection with the geometry, index supports only intersection with the rectangle, so we need test whether point actually intersects geometry.
|
Regarding concave hull -- can we ifdef in a geos version check and delegate the whole algorithm to geos on newer versions? |
I might be wrong, but from my tests geos implementation creates concave hull using different approach. At least for me, not matter which threshold I use it always produces the same polygon covering all points. |
|
I may be wrong, but seems old Python implementation produces incorrect results by not including all input points into concave hull polygon. |
| { | ||
| addParameter( new QgsProcessingParameterFeatureSource( QStringLiteral( "INPUT" ), QObject::tr( "Input layer" ), QList< int >() << QgsProcessing::TypeVectorPoint ) ); | ||
| std::unique_ptr<QgsProcessingParameterNumber> toleranceParam = std::make_unique<QgsProcessingParameterNumber>( QStringLiteral( "TOLERANCE" ), QObject::tr( "Tolerance" ), QgsProcessingParameterNumber::Double, 0, true, 0 ); | ||
| toleranceParam->setHelp( QObject::tr( "Specifies an optional snapping tolerance which can be used to improve the robustness of the triangulation" ) ); |
There was a problem hiding this comment.
Wow! I didn't know you could do this! Why is it only used on 6 algorithms? This should be everywhere!
There was a problem hiding this comment.
It's relatively new, introduced after the bulk of the algorithms were already made
There was a problem hiding this comment.
But yes, should be everywhere 😁
move step calculation to prepareAlgorithm to avoid crash disable tests for native implementation
|
@alexbruy |
|
Nice one @alexbruy |
| QgsFields fields; | ||
| if ( addAttributes ) | ||
| { | ||
| fields.append( QgsField( QStringLiteral( "POINTA" ), QVariant::LongLong ) ); | ||
| fields.append( QgsField( QStringLiteral( "POINTB" ), QVariant::LongLong ) ); | ||
| fields.append( QgsField( QStringLiteral( "POINTC" ), QVariant::LongLong ) ); | ||
| } | ||
| else | ||
| { | ||
| fields.append( QgsField( QStringLiteral( "id" ), QVariant::LongLong ) ); | ||
| } |
There was a problem hiding this comment.
@alexbruy I'm on my way to document this PR and wonder if it is deliberate that the output layer contains either an id field or the IDs from the input point. Why not always have the generated id field in the output layer (and add or not the original points)? I guess, something like:
QgsFields fields;
fields.append( QgsField( QStringLiteral( "id" ), QVariant::LongLong ) );
if ( addAttributes )
{
fields.append( QgsField( QStringLiteral( "POINTA" ), QVariant::LongLong ) );
fields.append( QgsField( QStringLiteral( "POINTB" ), QVariant::LongLong ) );
fields.append( QgsField( QStringLiteral( "POINTC" ), QVariant::LongLong ) );
}
Same applies to Voronoi afaict. Thanks.
There was a problem hiding this comment.
It is intentional, this is behaviour of the original Python implementation.
Description
Port some Python algorithms to C++ as a part of QEP #271.
Voronoi polygons and Delaunay triangulation algorithms ported to C++ and use GEOS to compute operations. Old Python code removed.
Concave hull algorithm ported to C++, old Python code removed. K-nearest concave hull marked as deprecated, but still available to keep existing scripts/models working.