From ae4e4cd4bb0e709fcee3f65b7ba531f72c4cdcc2 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 6 Jul 2018 21:42:44 +1000 Subject: [PATCH] Return full point container for matches, remove redundant point method --- .../qgsspatialindexkdbush.sip.in | 16 ++--- src/core/qgsspatialindexkdbush.cpp | 17 ++--- src/core/qgsspatialindexkdbush.h | 16 ++--- src/core/qgsspatialindexkdbush_p.h | 11 ---- tests/src/core/testqgsspatialindexkdbush.cpp | 62 +++++++++---------- 5 files changed, 45 insertions(+), 77 deletions(-) diff --git a/python/core/auto_generated/qgsspatialindexkdbush.sip.in b/python/core/auto_generated/qgsspatialindexkdbush.sip.in index b40b18d63c2..ccf39bf68ca 100644 --- a/python/core/auto_generated/qgsspatialindexkdbush.sip.in +++ b/python/core/auto_generated/qgsspatialindexkdbush.sip.in @@ -65,27 +65,19 @@ Copy constructor ~QgsSpatialIndexKDBush(); - QSet intersect( const QgsRectangle &rectangle ) const; + QList intersect( const QgsRectangle &rectangle ) const; %Docstring -Returns the set of features which fall within the specified ``rectangle``. +Returns the list of features which fall within the specified ``rectangle``. %End - QSet within( const QgsPointXY &point, double radius ) const; + QList within( const QgsPointXY &point, double radius ) const; %Docstring -Returns the set of features which are within the given search ``radius`` +Returns the list of features which are within the given search ``radius`` of ``point``. %End - bool point( QgsFeatureId id, QgsPointXY &point ) const; -%Docstring -Fetches the point from the index with matching ``id`` and stores it in ``point``. - -Returns true if the point was found, or false if no matching feature ID is present -in the index. -%End - qgssize size() const; %Docstring Returns the size of the index, i.e. the number of points contained within the index. diff --git a/src/core/qgsspatialindexkdbush.cpp b/src/core/qgsspatialindexkdbush.cpp index ef3a1963abb..412dad803aa 100644 --- a/src/core/qgsspatialindexkdbush.cpp +++ b/src/core/qgsspatialindexkdbush.cpp @@ -56,10 +56,10 @@ QgsSpatialIndexKDBush::~QgsSpatialIndexKDBush() delete d; } -QSet QgsSpatialIndexKDBush::within( const QgsPointXY &point, double radius ) const +QList QgsSpatialIndexKDBush::within( const QgsPointXY &point, double radius ) const { - QSet result; - d->index->within( point.x(), point.y(), radius, [&result]( const QgsSpatialIndexKDBushData & p ) { result.insert( p.id ); } ); + QList result; + d->index->within( point.x(), point.y(), radius, [&result]( const QgsSpatialIndexKDBushData & p ) { result << p; } ); return result; } @@ -68,23 +68,18 @@ void QgsSpatialIndexKDBush::within( const QgsPointXY &point, double radius, cons d->index->within( point.x(), point.y(), radius, visitor ); } -bool QgsSpatialIndexKDBush::point( QgsFeatureId id, QgsPointXY &point ) const -{ - return d->index->point( id, point ); -} - qgssize QgsSpatialIndexKDBush::size() const { return d->index->size(); } -QSet QgsSpatialIndexKDBush::intersect( const QgsRectangle &rectangle ) const +QList QgsSpatialIndexKDBush::intersect( const QgsRectangle &rectangle ) const { - QSet result; + QList result; d->index->range( rectangle.xMinimum(), rectangle.yMinimum(), rectangle.xMaximum(), - rectangle.yMaximum(), [&result]( const QgsSpatialIndexKDBushData & p ) { result << p.id; } ); + rectangle.yMaximum(), [&result]( const QgsSpatialIndexKDBushData & p ) { result << p; } ); return result; } diff --git a/src/core/qgsspatialindexkdbush.h b/src/core/qgsspatialindexkdbush.h index af55a0a9189..accd5fac3dd 100644 --- a/src/core/qgsspatialindexkdbush.h +++ b/src/core/qgsspatialindexkdbush.h @@ -84,9 +84,9 @@ class CORE_EXPORT QgsSpatialIndexKDBush ~QgsSpatialIndexKDBush(); /** - * Returns the set of features which fall within the specified \a rectangle. + * Returns the list of features which fall within the specified \a rectangle. */ - QSet intersect( const QgsRectangle &rectangle ) const; + QList intersect( const QgsRectangle &rectangle ) const; /** * Calls a \a visitor function for all features which fall within the specified \a rectangle. @@ -96,10 +96,10 @@ class CORE_EXPORT QgsSpatialIndexKDBush void intersect( const QgsRectangle &rectangle, const std::function &visitor ) const SIP_SKIP; /** - * Returns the set of features which are within the given search \a radius + * Returns the list of features which are within the given search \a radius * of \a point. */ - QSet within( const QgsPointXY &point, double radius ) const; + QList within( const QgsPointXY &point, double radius ) const; /** * Calls a \a visitor function for all features which are within the given search \a radius @@ -109,14 +109,6 @@ class CORE_EXPORT QgsSpatialIndexKDBush */ void within( const QgsPointXY &point, double radius, const std::function &visitor ) SIP_SKIP; - /** - * Fetches the point from the index with matching \a id and stores it in \a point. - * - * Returns true if the point was found, or false if no matching feature ID is present - * in the index. - */ - bool point( QgsFeatureId id, QgsPointXY &point ) const; - /** * Returns the size of the index, i.e. the number of points contained within the index. */ diff --git a/src/core/qgsspatialindexkdbush_p.h b/src/core/qgsspatialindexkdbush_p.h index dd8dfada235..c30ff1ebdd4 100644 --- a/src/core/qgsspatialindexkdbush_p.h +++ b/src/core/qgsspatialindexkdbush_p.h @@ -90,17 +90,6 @@ class PointXYKDBush : public kdbush::KDBush< std::pair, QgsSpati sortKD( 0, size - 1, 0 ); } - bool point( QgsFeatureId id, QgsPointXY &point ) const - { - auto it = std::find_if( points.begin(), points.end(), - [id]( const QgsSpatialIndexKDBushData & d ) { return d.id == id; } ); - if ( it == points.end() ) - return false; - - point = QgsPointXY( it->coords.first, it->coords.second ); - return true; - } - std::size_t size() const { return points.size(); diff --git a/tests/src/core/testqgsspatialindexkdbush.cpp b/tests/src/core/testqgsspatialindexkdbush.cpp index 97f8000ec76..2d04fed1317 100644 --- a/tests/src/core/testqgsspatialindexkdbush.cpp +++ b/tests/src/core/testqgsspatialindexkdbush.cpp @@ -51,6 +51,18 @@ static QList _pointFeatures() return feats; } +bool testContains( const QList &data, QgsFeatureId id, const QgsPointXY &point ) +{ + for ( const QgsSpatialIndexKDBushData &d : data ) + { + if ( d.id == id ) + { + return d.point() == point; + } + } + return false; +} + class TestQgsSpatialIndexKdBush : public QObject { Q_OBJECT @@ -75,42 +87,30 @@ class TestQgsSpatialIndexKdBush : public QObject QgsSpatialIndexKDBush index( *vl->dataProvider() ); QCOMPARE( index.size(), 4 ); - QSet fids = index.intersect( QgsRectangle( 0, 0, 10, 10 ) ); + QList fids = index.intersect( QgsRectangle( 0, 0, 10, 10 ) ); QVERIFY( fids.count() == 1 ); - QVERIFY( fids.contains( 1 ) ); + QVERIFY( testContains( fids, 1, QgsPointXY( 1, 1 ) ) ); - QSet fids2 = index.intersect( QgsRectangle( -10, -10, 0, 10 ) ); + QList fids2 = index.intersect( QgsRectangle( -10, -10, 0, 10 ) ); QCOMPARE( fids2.count(), 2 ); - QVERIFY( fids2.contains( 2 ) ); - QVERIFY( fids2.contains( 3 ) ); + QVERIFY( testContains( fids2, 2, QgsPointXY( -1, 1 ) ) ); + QVERIFY( testContains( fids2, 3, QgsPointXY( -1, -1 ) ) ); - QSet fids3 = index.within( QgsPointXY( 0, 0 ), 2 ); + QList fids3 = index.within( QgsPointXY( 0, 0 ), 2 ); QCOMPARE( fids3.count(), 4 ); - QVERIFY( fids3.contains( 1 ) ); - QVERIFY( fids3.contains( 2 ) ); - QVERIFY( fids3.contains( 3 ) ); - QVERIFY( fids3.contains( 4 ) ); + QVERIFY( testContains( fids3, 1, QgsPointXY( 1, 1 ) ) ); + QVERIFY( testContains( fids3, 2, QgsPointXY( -1, 1 ) ) ); + QVERIFY( testContains( fids3, 3, QgsPointXY( -1, -1 ) ) ); + QVERIFY( testContains( fids3, 4, QgsPointXY( 1, -1 ) ) ); - QSet fids4 = index.within( QgsPointXY( 0, 0 ), 1 ); + QList fids4 = index.within( QgsPointXY( 0, 0 ), 1 ); QCOMPARE( fids4.count(), 0 ); - QSet fids5 = index.within( QgsPointXY( -1, -1 ), 2.1 ); + QList fids5 = index.within( QgsPointXY( -1, -1 ), 2.1 ); QCOMPARE( fids5.count(), 3 ); - QVERIFY( fids5.contains( 2 ) ); - QVERIFY( fids5.contains( 3 ) ); - QVERIFY( fids5.contains( 4 ) ); - - QgsPointXY p; - QVERIFY( !index.point( -1, p ) ); - QVERIFY( !index.point( 5, p ) ); - QVERIFY( index.point( 1, p ) ); - QCOMPARE( p, QgsPointXY( 1, 1 ) ); - QVERIFY( index.point( 2, p ) ); - QCOMPARE( p, QgsPointXY( -1, 1 ) ); - QVERIFY( index.point( 3, p ) ); - QCOMPARE( p, QgsPointXY( -1, -1 ) ); - QVERIFY( index.point( 4, p ) ); - QCOMPARE( p, QgsPointXY( 1, -1 ) ); + QVERIFY( testContains( fids5, 2, QgsPointXY( -1, 1 ) ) ); + QVERIFY( testContains( fids5, 3, QgsPointXY( -1, -1 ) ) ); + QVERIFY( testContains( fids5, 4, QgsPointXY( 1, -1 ) ) ); } void testCopy() @@ -128,9 +128,9 @@ class TestQgsSpatialIndexKdBush : public QObject QVERIFY( index->d->ref == 2 ); // test that copied index works - QSet fids = indexCopy->intersect( QgsRectangle( 0, 0, 10, 10 ) ); + QList fids = indexCopy->intersect( QgsRectangle( 0, 0, 10, 10 ) ); QVERIFY( fids.count() == 1 ); - QVERIFY( fids.contains( 1 ) ); + QVERIFY( testContains( fids, 1, QgsPointXY( 1, 1 ) ) ); // check that the index is still shared QVERIFY( index->d == indexCopy->d ); @@ -141,7 +141,7 @@ class TestQgsSpatialIndexKdBush : public QObject // test that copied index still works fids = indexCopy->intersect( QgsRectangle( 0, 0, 10, 10 ) ); QVERIFY( fids.count() == 1 ); - QVERIFY( fids.contains( 1 ) ); + QVERIFY( testContains( fids, 1, QgsPointXY( 1, 1 ) ) ); QVERIFY( indexCopy->d->ref == 1 ); // assignment operator @@ -157,7 +157,7 @@ class TestQgsSpatialIndexKdBush : public QObject QVERIFY( index3.d->ref == 2 ); fids = index3.intersect( QgsRectangle( 0, 0, 10, 10 ) ); QVERIFY( fids.count() == 1 ); - QVERIFY( fids.contains( 1 ) ); + QVERIFY( testContains( fids, 1, QgsPointXY( 1, 1 ) ) ); indexCopy.reset(); QVERIFY( index3.d->ref == 1 );