Fix vertextool's geometry cache invalidation

Hugo Mercier · Hugo Mercier · commit a1970104d0c4 · 2019-01-14T15:04:38.000+01:00
This a forward port from d79c212 in PR #8724 Another fix is added here (which will be backported to 3.4): layer' signal connections must be destroyed when the cache for this layer is cleared (otherwise I got an "assertion failed" in onCacheGeometryXXXX)
diff --git a/src/app/vertextool/qgsvertextool.cpp b/src/app/vertextool/qgsvertextool.cpp
@@ -980,14 +980,17 @@ void QgsVertexTool::keyPressEvent( QKeyEvent *e )
 
 QgsGeometry QgsVertexTool::cachedGeometry( const QgsVectorLayer *layer, QgsFeatureId fid )
 {
-  if ( !mCache.contains( layer ) )
+  const bool layerWasNotInCache = !mCache.contains( layer );
+  // insert if it was not in cache
+  QHash<QgsFeatureId, QgsGeometry> &layerCache = mCache[layer];
+  if ( layerWasNotInCache )
   {
     connect( layer, &QgsVectorLayer::geometryChanged, this, &QgsVertexTool::onCachedGeometryChanged );
     connect( layer, &QgsVectorLayer::featureDeleted, this, &QgsVertexTool::onCachedGeometryDeleted );
-    // TODO: also clear cache when layer is deleted
+    connect( layer, &QgsVectorLayer::willBeDeleted, this, &QgsVertexTool::clearGeometryCache );
+    connect( layer, &QgsVectorLayer::dataChanged, this, &QgsVertexTool::clearGeometryCache );
   }
 
-  QHash<QgsFeatureId, QgsGeometry> &layerCache = mCache[layer];
   if ( !layerCache.contains( fid ) )
   {
     QgsFeature f;
@@ -1003,6 +1006,16 @@ QgsGeometry QgsVertexTool::cachedGeometryForVertex( const Vertex &vertex )
   return cachedGeometry( vertex.layer, vertex.fid );
 }
 
+void QgsVertexTool::clearGeometryCache()
+{
+  const QgsVectorLayer *layer = qobject_cast<const QgsVectorLayer *>( sender() );
+  mCache.remove( layer );
+  disconnect( layer, &QgsVectorLayer::geometryChanged, this, &QgsVertexTool::onCachedGeometryChanged );
+  disconnect( layer, &QgsVectorLayer::featureDeleted, this, &QgsVertexTool::onCachedGeometryDeleted );
+  disconnect( layer, &QgsVectorLayer::willBeDeleted, this, &QgsVertexTool::clearGeometryCache );
+  disconnect( layer, &QgsVectorLayer::dataChanged, this, &QgsVertexTool::clearGeometryCache );
+}
+
 void QgsVertexTool::onCachedGeometryChanged( QgsFeatureId fid, const QgsGeometry &geom )
 {
   QgsVectorLayer *layer = qobject_cast<QgsVectorLayer *>( sender() );
diff --git a/src/app/vertextool/qgsvertextool.h b/src/app/vertextool/qgsvertextool.h
@@ -97,6 +97,8 @@ class APP_EXPORT QgsVertexTool : public QgsMapToolAdvancedDigitizing
 
     void onCachedGeometryDeleted( QgsFeatureId fid );
 
+    void clearGeometryCache();
+
     void showVertexEditor();  //#spellok
 
     void deleteVertexEditorSelection();

Original file line number	Diff line number	Diff line change
`@@ -980,14 +980,17 @@ void QgsVertexTool::keyPressEvent( QKeyEvent *e )`
`980`	`980`
`981`	`981`	`QgsGeometry QgsVertexTool::cachedGeometry( const QgsVectorLayer *layer, QgsFeatureId fid )`
`982`	`982`	`{`
`983`		`- if ( !mCache.contains( layer ) )`
	`983`	`+ const bool layerWasNotInCache = !mCache.contains( layer );`
	`984`	`+ // insert if it was not in cache`
	`985`	`+ QHash<QgsFeatureId, QgsGeometry> &layerCache = mCache[layer];`
	`986`	`+ if ( layerWasNotInCache )`
`984`	`987`	`{`
`985`	`988`	`connect( layer, &QgsVectorLayer::geometryChanged, this, &QgsVertexTool::onCachedGeometryChanged );`
`986`	`989`	`connect( layer, &QgsVectorLayer::featureDeleted, this, &QgsVertexTool::onCachedGeometryDeleted );`
`987`		`- // TODO: also clear cache when layer is deleted`
	`990`	`+ connect( layer, &QgsVectorLayer::willBeDeleted, this, &QgsVertexTool::clearGeometryCache );`
	`991`	`+ connect( layer, &QgsVectorLayer::dataChanged, this, &QgsVertexTool::clearGeometryCache );`
`988`	`992`	`}`
`989`	`993`
`990`		`- QHash<QgsFeatureId, QgsGeometry> &layerCache = mCache[layer];`
`991`	`994`	`if ( !layerCache.contains( fid ) )`
`992`	`995`	`{`
`993`	`996`	`QgsFeature f;`
`@@ -1003,6 +1006,16 @@ QgsGeometry QgsVertexTool::cachedGeometryForVertex( const Vertex &vertex )`
`1003`	`1006`	`return cachedGeometry( vertex.layer, vertex.fid );`
`1004`	`1007`	`}`
`1005`	`1008`
	`1009`	`+void QgsVertexTool::clearGeometryCache()`
	`1010`	`+{`
	`1011`	`+ const QgsVectorLayer layer = qobject_cast<const QgsVectorLayer >( sender() );`
	`1012`	`+ mCache.remove( layer );`
	`1013`	`+ disconnect( layer, &QgsVectorLayer::geometryChanged, this, &QgsVertexTool::onCachedGeometryChanged );`
	`1014`	`+ disconnect( layer, &QgsVectorLayer::featureDeleted, this, &QgsVertexTool::onCachedGeometryDeleted );`
	`1015`	`+ disconnect( layer, &QgsVectorLayer::willBeDeleted, this, &QgsVertexTool::clearGeometryCache );`
	`1016`	`+ disconnect( layer, &QgsVectorLayer::dataChanged, this, &QgsVertexTool::clearGeometryCache );`
	`1017`	`+}`
	`1018`	`+`
`1006`	`1019`	`void QgsVertexTool::onCachedGeometryChanged( QgsFeatureId fid, const QgsGeometry &geom )`
`1007`	`1020`	`{`
`1008`	`1021`	`QgsVectorLayer layer = qobject_cast<QgsVectorLayer >( sender() );`