Merge pull request #7465 from elpaso/bugfix-19468-layercache-filter-rect

fix layercache filter rect and a bonus crash
qgis · Jul 26, 2018 · df394c7 · df394c7
2 parents 20e62b4 + d62087e
commit df394c7
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 20 deletions.
diff --git a/src/core/qgsvectorlayercache.cpp b/src/core/qgsvectorlayercache.cpp
@@ -205,11 +205,12 @@ void QgsVectorLayerCache::requestCompleted( const QgsFeatureRequest &featureRequ
   // If a request is too large for the cache don't notify to prevent from indexing incomplete requests
   if ( fids.count() <= mCache.size() )
   {
-    Q_FOREACH ( QgsAbstractCacheIndex *idx, mCacheIndices )
+    for ( const auto &idx : qgis::as_const( mCacheIndices ) )
     {
       idx->requestCompleted( featureRequest, fids );
     }
-    if ( featureRequest.filterType() == QgsFeatureRequest::FilterNone )
+    if ( featureRequest.filterType() == QgsFeatureRequest::FilterNone &&
+         ( featureRequest.filterRect().isNull() || featureRequest.filterRect().contains( mLayer->extent() ) ) )
     {
       mFullCache = true;
     }

diff --git a/src/gui/attributetable/qgsattributetablemodel.cpp b/src/gui/attributetable/qgsattributetablemodel.cpp
@@ -429,32 +429,36 @@ void QgsAttributeTableModel::loadLayer()
     removeRows( 0, rowCount() );
   }
 
-  QgsFeatureIterator features = mLayerCache->getFeatures( mFeatureRequest );
-
-  int i = 0;
+  // Layer might have been deleted and cache set to nullptr!
+  if ( mLayerCache )
+  {
+    QgsFeatureIterator features = mLayerCache->getFeatures( mFeatureRequest );
 
-  QTime t;
-  t.start();
+    int i = 0;
 
-  while ( features.nextFeature( mFeat ) )
-  {
-    ++i;
+    QTime t;
+    t.start();
 
-    if ( t.elapsed() > 1000 )
+    while ( features.nextFeature( mFeat ) )
     {
-      bool cancel = false;
-      emit progress( i, cancel );
-      if ( cancel )
-        break;
+      ++i;
 
-      t.restart();
+      if ( t.elapsed() > 1000 )
+      {
+        bool cancel = false;
+        emit progress( i, cancel );
+        if ( cancel )
+          break;
+
+        t.restart();
+      }
+      featureAdded( mFeat.id(), true );
     }
-    featureAdded( mFeat.id(), true );
-  }
 
-  emit finished();
+    emit finished();
+    connect( mLayerCache, &QgsVectorLayerCache::invalidated, this, &QgsAttributeTableModel::loadLayer, Qt::UniqueConnection );
+  }
 
-  connect( mLayerCache, &QgsVectorLayerCache::invalidated, this, &QgsAttributeTableModel::loadLayer, Qt::UniqueConnection );
   endResetModel();
 }
 

diff --git a/tests/src/core/testqgsvectorlayercache.cpp b/tests/src/core/testqgsvectorlayercache.cpp
@@ -53,6 +53,7 @@ class TestVectorLayerCache : public QObject
     void testFullCacheThroughRequest();
     void testCanUseCacheForRequest();
     void testCacheGeom();
+    void testFullCacheWithRect(); // Test that if rect is set then no full cache can exist, see #19468
 
     void onCommittedFeaturesAdded( const QString &, const QgsFeatureList & );
 
@@ -390,6 +391,33 @@ void TestVectorLayerCache::testCacheGeom()
   QVERIFY( !cache.hasFullCache() );
 }
 
+void TestVectorLayerCache::testFullCacheWithRect()
+{
+  QgsVectorLayerCache cache( mPointsLayer, mPointsLayer->dataProvider()->featureCount() );
+  // cache geometry
+  cache.setCacheGeometry( true );
+  QVERIFY( ! cache.hasFullCache() );
+  QgsFeatureRequest req;
+  req.setFilterRect( mPointsLayer->dataProvider()->extent().buffered( - mPointsLayer->dataProvider()->extent().width() / 2 ) );
+  QgsFeatureIterator it = cache.getFeatures( req );
+  QgsFeature f;
+  while ( it.nextFeature( f ) )
+  {
+    QVERIFY( f.hasGeometry() );
+  }
+  QVERIFY( ! cache.hasFullCache() );
+
+  // Filter rect contains extent
+  req.setFilterRect( mPointsLayer->dataProvider()->extent().buffered( 1 ) );
+  it = cache.getFeatures( req );
+  while ( it.nextFeature( f ) )
+  {
+    QVERIFY( f.hasGeometry() );
+  }
+  QVERIFY( cache.hasFullCache() );
+
+}
+
 void TestVectorLayerCache::onCommittedFeaturesAdded( const QString &layerId, const QgsFeatureList &features )
 {
   Q_UNUSED( layerId )