[afs] Don't request intersecting features from server if all

nyalldawson · nyalldawson · commit c0d4ce70f257 · 2018-02-20T13:20:38.000+10:00
features are already locally cached
diff --git a/src/providers/arcgisrest/qgsafsfeatureiterator.cpp b/src/providers/arcgisrest/qgsafsfeatureiterator.cpp
@@ -71,12 +71,15 @@ QgsAfsFeatureIterator::QgsAfsFeatureIterator( QgsAfsFeatureSource *source, bool
     requestIds.insert( mRequest.filterFid() );
   }
 
-  if ( !mFilterRect.isNull() )
+  if ( !mFilterRect.isNull() && !mSource->sharedData()->hasCachedAllFeatures() )
   {
     // defer request to find features in filter rect until first feature is requested
     // this allows time for a interruption checker to be installed on the iterator
     // and avoids performing this expensive check in the main thread when just
     // preparing iterators
+
+    // (but if we've already cached ALL the features, we skip this -- there's no need for
+    // firing off another request to the server)
     mDeferredFeaturesInFilterRectCheck = true;
   }
 
@@ -129,6 +132,10 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
       mFeatureIterator = mRemainingFeatureIds.at( 0 );
 
     mDeferredFeaturesInFilterRectCheck = false;
+
+    // discard the filter rect - we know that the features in mRemainingFeatureIds are gauranteed
+    // to be intersecting the rect, so avoid any extra unnecessary checks
+    mFilterRect = QgsRectangle();
   }
 
   if ( !mFeatureIdList.empty() && mRemainingFeatureIds.empty() )
@@ -171,6 +178,10 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
         {
           ++mFeatureIterator;
         }
+
+        if ( !mFilterRect.isNull() && ( !f.hasGeometry() || !f.geometry().intersects( mFilterRect ) ) )
+          success = false;
+
         if ( !success )
           continue;
         geometryToDestinationCrs( f, mTransform );
diff --git a/src/providers/arcgisrest/qgsafsshareddata.cpp b/src/providers/arcgisrest/qgsafsshareddata.cpp
@@ -161,3 +161,8 @@ QgsFeatureIds QgsAfsSharedData::getFeatureIdsInExtent( const QgsRectangle &exten
   }
   return ids;
 }
+
+bool QgsAfsSharedData::hasCachedAllFeatures() const
+{
+  return mCache.count() == mObjectIds.count();
+}
diff --git a/src/providers/arcgisrest/qgsafsshareddata.h b/src/providers/arcgisrest/qgsafsshareddata.h
@@ -41,6 +41,8 @@ class QgsAfsSharedData : public QObject
     bool getFeature( QgsFeatureId id, QgsFeature &f, const QgsRectangle &filterRect = QgsRectangle(), QgsFeedback *feedback = nullptr );
     QgsFeatureIds getFeatureIdsInExtent( const QgsRectangle &extent, QgsFeedback *feedback );
 
+    bool hasCachedAllFeatures() const;
+
   private:
     friend class QgsAfsProvider;
     QMutex mMutex;

Original file line number	Diff line number	Diff line change
`@@ -71,12 +71,15 @@ QgsAfsFeatureIterator::QgsAfsFeatureIterator( QgsAfsFeatureSource *source, bool`
`71`	`71`	`requestIds.insert( mRequest.filterFid() );`
`72`	`72`	`}`
`73`	`73`
`74`		`- if ( !mFilterRect.isNull() )`
	`74`	`+ if ( !mFilterRect.isNull() && !mSource->sharedData()->hasCachedAllFeatures() )`
`75`	`75`	`{`
`76`	`76`	`// defer request to find features in filter rect until first feature is requested`
`77`	`77`	`// this allows time for a interruption checker to be installed on the iterator`
`78`	`78`	`// and avoids performing this expensive check in the main thread when just`
`79`	`79`	`// preparing iterators`
	`80`	`+`
	`81`	`+ // (but if we've already cached ALL the features, we skip this -- there's no need for`
	`82`	`+ // firing off another request to the server)`
`80`	`83`	`mDeferredFeaturesInFilterRectCheck = true;`
`81`	`84`	`}`
`82`	`85`
`@@ -129,6 +132,10 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )`
`129`	`132`	`mFeatureIterator = mRemainingFeatureIds.at( 0 );`
`130`	`133`
`131`	`134`	`mDeferredFeaturesInFilterRectCheck = false;`
	`135`	`+`
	`136`	`+ // discard the filter rect - we know that the features in mRemainingFeatureIds are gauranteed`
	`137`	`+ // to be intersecting the rect, so avoid any extra unnecessary checks`
	`138`	`+ mFilterRect = QgsRectangle();`
`132`	`139`	`}`
`133`	`140`
`134`	`141`	`if ( !mFeatureIdList.empty() && mRemainingFeatureIds.empty() )`
`@@ -171,6 +178,10 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )`
`171`	`178`	`{`
`172`	`179`	`++mFeatureIterator;`
`173`	`180`	`}`
	`181`	`+`
	`182`	`+ if ( !mFilterRect.isNull() && ( !f.hasGeometry() \|\| !f.geometry().intersects( mFilterRect ) ) )`
	`183`	`+ success = false;`
	`184`	`+`
`174`	`185`	`if ( !success )`
`175`	`186`	`continue;`
`176`	`187`	`geometryToDestinationCrs( f, mTransform );`
Original file line number	Diff line number	Diff line change
`@@ -161,3 +161,8 @@ QgsFeatureIds QgsAfsSharedData::getFeatureIdsInExtent( const QgsRectangle &exten`
`161`	`161`	`}`
`162`	`162`	`return ids;`
`163`	`163`	`}`
	`164`	`+`
	`165`	`+bool QgsAfsSharedData::hasCachedAllFeatures() const`
	`166`	`+{`
	`167`	`+ return mCache.count() == mObjectIds.count();`
	`168`	`+}`