Skip to content

Commit

Permalink
fix(QgsVectorLayer-dependencies): don't emit too many dataChanged sig…
Browse files Browse the repository at this point in the history
…nals
  • Loading branch information
Djedouas committed Nov 6, 2024
1 parent cc8cd21 commit 6061bcc
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 14 deletions.
12 changes: 10 additions & 2 deletions src/core/vector/qgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6109,6 +6109,14 @@ void QgsVectorLayer::emitDataChanged()
if ( mDataChangedFired )
return;

// If we are asked to fire dataChanged from a layer we depend on,
// be sure that this layer is not in the process of committing its changes, because
// we will be asked to fire dataChanged at the end of his commit, and we don't
// want to fire this signal more than necessary.
if ( QgsVectorLayer *layerWeDependUpon = dynamic_cast<QgsVectorLayer *>( sender() ) )
if ( layerWeDependUpon->mCommitChangesActive )
return;

updateExtents(); // reset cached extent to reflect data changes

mDataChangedFired = true;
Expand Down Expand Up @@ -6143,7 +6151,7 @@ bool QgsVectorLayer::setDependencies( const QSet<QgsMapLayerDependency> &oDeps )
disconnect( lyr, &QgsVectorLayer::geometryChanged, this, &QgsVectorLayer::emitDataChanged );
disconnect( lyr, &QgsVectorLayer::dataChanged, this, &QgsVectorLayer::emitDataChanged );
disconnect( lyr, &QgsVectorLayer::repaintRequested, this, &QgsVectorLayer::triggerRepaint );
disconnect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::reload );
disconnect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::emitDataChanged );
}
}

Expand All @@ -6167,7 +6175,7 @@ bool QgsVectorLayer::setDependencies( const QSet<QgsMapLayerDependency> &oDeps )
connect( lyr, &QgsVectorLayer::geometryChanged, this, &QgsVectorLayer::emitDataChanged );
connect( lyr, &QgsVectorLayer::dataChanged, this, &QgsVectorLayer::emitDataChanged );
connect( lyr, &QgsVectorLayer::repaintRequested, this, &QgsVectorLayer::triggerRepaint );
connect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::reload );
connect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::emitDataChanged );
}
}

Expand Down
30 changes: 18 additions & 12 deletions tests/src/python/test_layer_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,19 @@ def test_circular_dependencies_with_2_layers(self):
self.assertEqual(len(spy_points_data_changed), 3)
self.assertEqual(len(spy_lines_data_changed), 2)

# added feature is deleted and added with its new defined id
# (it was -1 before) so it fires 3 more signal dataChanged on
# depending line (on featureAdded and on featureDeleted and on afterCommitChanges)
# and so 2 more signal on points because it depends on line
self.pointsLayer.commitChanges()
# commit changes fires dataChanged because external changes could happen (provider side)
self.pointsLayer.commitChanges(False)
self.assertEqual(len(spy_points_data_changed), 4)
self.assertEqual(len(spy_lines_data_changed), 3)

# points fire dataChanged on geometryChanged
# line depends on point, so fire dataChanged
self.pointsLayer.changeGeometry(f.id(), QgsGeometry.fromWkt("POINT(0 2)"))
self.assertEqual(len(spy_points_data_changed), 5)
self.assertEqual(len(spy_lines_data_changed), 4)

# commit changes fires dataChanged because external changes could happen (provider side)
self.assertTrue(self.pointsLayer.commitChanges())
self.assertEqual(len(spy_points_data_changed), 6)
self.assertEqual(len(spy_lines_data_changed), 5)

Expand Down Expand Up @@ -228,23 +236,21 @@ def test_circular_dependencies_with_1_layer(self):
self.linesLayer.addFeatures([f])
self.assertEqual(len(spy_lines_data_changed), 2)

# added feature is deleted and added with its new defined id
# (it was -1 before) so it fires 3 more signal dataChanged on
# depending line (on featureAdded and on featureDeleted and on afterCommitChanges)
# line fire dataChanged on commitChanges
self.linesLayer.commitChanges(False)
self.assertEqual(len(spy_lines_data_changed), 5)
self.assertEqual(len(spy_lines_data_changed), 3)

# repaintRequested is called only once on commit changes on line
# (ideally only one repaintRequested signal is fired, but it's harmless to fire multiple ones)
self.assertGreaterEqual(len(spy_lines_repaint_requested), 2)

# line fire dataChanged on geometryChanged
self.linesLayer.changeGeometry(f.id(), QgsGeometry.fromWkt("LINESTRING(0 0, 2 2)"))
self.assertEqual(len(spy_lines_data_changed), 6)
self.assertEqual(len(spy_lines_data_changed), 4)

# line fire dataChanged on commitChanges
# commit changes fires dataChanged because external changes could happen (provider side)
self.linesLayer.commitChanges()
self.assertEqual(len(spy_lines_data_changed), 7)
self.assertEqual(len(spy_lines_data_changed), 5)

def test_layerDefinitionRewriteId(self):
tmpfile = os.path.join(tempfile.tempdir, "test.qlr")
Expand Down

0 comments on commit 6061bcc

Please sign in to comment.