From 5680f9a5314537990b8bbb8b8fb1fe95169caf64 Mon Sep 17 00:00:00 2001 From: Jacky Volpes Date: Tue, 16 Jul 2024 12:46:09 +0200 Subject: [PATCH] fix(qgsfeaturepool): cache management - fixes #58113 Geometry checker does not work with memory layers, so we change how the cache in the QgsFeaturePool object is managed, to refresh it more often. --- .../geometry_checker/qgsfeaturepool.cpp | 1 + .../qgsvectorlayerfeaturepool.cpp | 14 +++++++------ .../testqgsvectorlayerfeaturepool.cpp | 20 ++++++------------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp b/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp index 9bb43091a6b01..4ae9be1674e52 100644 --- a/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp +++ b/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp @@ -58,6 +58,7 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature ) { // Feature not in cache, retrieve from layer // TODO: avoid always querying all attributes (attribute values are needed when merging by attribute) + mFeatureSource = std::make_unique( mLayer ); if ( !mFeatureSource->getFeatures( QgsFeatureRequest( id ) ).nextFeature( feature ) ) { return false; diff --git a/src/analysis/vector/geometry_checker/qgsvectorlayerfeaturepool.cpp b/src/analysis/vector/geometry_checker/qgsvectorlayerfeaturepool.cpp index 88f4fde910597..0df21c07fd2b3 100644 --- a/src/analysis/vector/geometry_checker/qgsvectorlayerfeaturepool.cpp +++ b/src/analysis/vector/geometry_checker/qgsvectorlayerfeaturepool.cpp @@ -137,12 +137,14 @@ void QgsVectorLayerFeaturePool::onGeometryChanged( QgsFeatureId fid, const QgsGe { Q_UNUSED( geometry ) - if ( isFeatureCached( fid ) ) - { - QgsFeature feature; - getFeature( fid, feature ); - refreshCache( feature ); - } + // Always update the cache and the spatial index on geometry change. + // A feature that was not in the cache previously might now have to be. + // For instance a geometry that was not in a previously requested area was not cached, + // but if it is now in the area it will be correctly added + + QgsFeature feature; + getFeature( fid, feature ); + refreshCache( feature ); } void QgsVectorLayerFeaturePool::onFeatureDeleted( QgsFeatureId fid ) diff --git a/tests/src/geometry_checker/testqgsvectorlayerfeaturepool.cpp b/tests/src/geometry_checker/testqgsvectorlayerfeaturepool.cpp index 62b8c6e742f88..80924b1d42aea 100644 --- a/tests/src/geometry_checker/testqgsvectorlayerfeaturepool.cpp +++ b/tests/src/geometry_checker/testqgsvectorlayerfeaturepool.cpp @@ -140,29 +140,21 @@ void TestQgsVectorLayerFeaturePool::changeGeometry() // Update a feature to be outside the AOI feat.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "Polygon((100 100, 110 100, 110 110, 100 110, 100 100))" ) ) ); - vl->updateFeature( feat ); + vl->updateFeature( feat ); // updates the geometry and triggers refreshing the cache - // Still working on the cached data + // Now no feature inside the AOI const QgsFeatureIds ids3 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) ); - QCOMPARE( ids3.size(), 1 ); - - // Repopulate the cache + QCOMPARE( ids3.size(), 0 ); const QgsFeatureIds ids4 = pool.getFeatures( QgsFeatureRequest().setFilterRect( QgsRectangle( 0, 0, 10, 10 ) ) ); QCOMPARE( ids4.size(), 0 ); - // Still working on the cached data - const QgsFeatureIds ids5 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) ); - QCOMPARE( ids5.size(), 0 ); - // Update a feature to be inside the AOI feat.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "Polygon((0 0, 10 0, 10 10, 0 10, 0 0))" ) ) ); - vl->updateFeature( feat ); + vl->updateFeature( feat ); // updates the geometry and triggers refreshing the cache - // Still cached + // Now one feature inside the AOI again const QgsFeatureIds ids6 = pool.getIntersects( QgsRectangle( 0, 0, 10, 10 ) ); - QCOMPARE( ids6.size(), 0 ); - - // One in there again + QCOMPARE( ids6.size(), 1 ); const QgsFeatureIds ids7 = pool.getFeatures( QgsFeatureRequest().setFilterRect( QgsRectangle( 0, 0, 10, 10 ) ) ); QCOMPARE( ids7.size(), 1 ); }