Skip to content

Commit

Permalink
qgsvectorlayer: Properly invalidate extent cache on feature addition
Browse files Browse the repository at this point in the history
When a new feature is added to a `QgsVectorLayer`
`QgsVectorLayer::addFeature` is called. This function makes mostly 2
things in case of a successful operation:
1. it calls `QgsVectorLayerEditBuffer::addFeature` which itself will
   emit the `QgsVectorLayerEditBuffer::featureAdded` signal. Finally,
   `QgsVectorLayer` listend to this signal to directly emit the
   `QgsVectorLayer::featureAdded` signal
2. Call `QgsVectorLayer::updateExtents()` to invalidate the cache of
   the extent

In practice, this means that the `QgsVectorLayer::featureAdded` signal
may be emitted before the cache of the extent is invalidated. This can
cause some issues.

For example, in the elevation profile tool,
`QgsElevationProfileCanvas` listens to the
`QgsVectorLayer::featureAdded` signal in order to regenerate the
profile of a vector layer when a feature is added. This causes the
creation of a new `QgsVectorLayerProfileGenerator` which needs to copy
the extent of the vector layer. However,
`QgsVectorLayer::updateExtents()` has not been called yet. Thus, it
will use an outdated version of the extent.

This issue is solved by calling `updateExtents()` before emitting the
`featureAdded` signal.
  • Loading branch information
ptitjano authored and nyalldawson committed Feb 6, 2025
1 parent 48b7079 commit 912bf7d
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 6 deletions.
18 changes: 12 additions & 6 deletions src/core/vector/qgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1366,12 +1366,9 @@ bool QgsVectorLayer::addFeature( QgsFeature &feature, Flags )

bool success = mEditBuffer->addFeature( feature );

if ( success )
if ( success && mJoinBuffer->containsJoins() )
{
updateExtents();

if ( mJoinBuffer->containsJoins() )
success = mJoinBuffer->addFeature( feature );
success = mJoinBuffer->addFeature( feature );
}

return success;
Expand Down Expand Up @@ -5080,7 +5077,7 @@ void QgsVectorLayer::createEditBuffer()
connect( mEditBuffer, &QgsVectorLayerEditBuffer::layerModified, this, &QgsVectorLayer::invalidateSymbolCountedFlag );
connect( mEditBuffer, &QgsVectorLayerEditBuffer::layerModified, this, &QgsVectorLayer::layerModified ); // TODO[MD]: necessary?
//connect( mEditBuffer, SIGNAL( layerModified() ), this, SLOT( triggerRepaint() ) ); // TODO[MD]: works well?
connect( mEditBuffer, &QgsVectorLayerEditBuffer::featureAdded, this, &QgsVectorLayer::featureAdded );
connect( mEditBuffer, &QgsVectorLayerEditBuffer::featureAdded, this, &QgsVectorLayer::onFeatureAdded );
connect( mEditBuffer, &QgsVectorLayerEditBuffer::featureDeleted, this, &QgsVectorLayer::onFeatureDeleted );
connect( mEditBuffer, &QgsVectorLayerEditBuffer::geometryChanged, this, &QgsVectorLayer::geometryChanged );
connect( mEditBuffer, &QgsVectorLayerEditBuffer::attributeValueChanged, this, &QgsVectorLayer::attributeValueChanged );
Expand Down Expand Up @@ -5966,6 +5963,15 @@ void QgsVectorLayer::onJoinedFieldsChanged()
updateFields();
}

void QgsVectorLayer::onFeatureAdded( QgsFeatureId fid )
{
QGIS_PROTECT_QOBJECT_THREAD_ACCESS

updateExtents();

emit featureAdded( fid );
}

void QgsVectorLayer::onFeatureDeleted( QgsFeatureId fid )
{
QGIS_PROTECT_QOBJECT_THREAD_ACCESS
Expand Down
1 change: 1 addition & 0 deletions src/core/vector/qgsvectorlayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -2797,6 +2797,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
void onFeatureCounterCompleted();
void onFeatureCounterTerminated();
void onJoinedFieldsChanged();
void onFeatureAdded( QgsFeatureId fid );
void onFeatureDeleted( QgsFeatureId fid );
void onRelationsLoaded();
void onSymbolsCounted();
Expand Down
28 changes: 28 additions & 0 deletions tests/src/core/testqgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class TestQgsVectorLayer : public QgsTest
void testCopyPasteFieldConfiguration_data();
void testFieldExpression();
void testFieldAggregateExpression();
void testAddFeatureExtentUpdated();
};

void TestQgsVectorLayer::initTestCase()
Expand Down Expand Up @@ -495,6 +496,33 @@ void TestQgsVectorLayer::testFieldAggregateExpression()
QVERIFY( qgsDoubleNear( feature2.attribute( vfIndex ).toDouble(), 359065580.0, 1 ) );
}

void TestQgsVectorLayer::testAddFeatureExtentUpdated()
{
QgsVectorLayer *layerLine = new QgsVectorLayer( QStringLiteral( "LineString?crs=EPSG:27700" ), QStringLiteral( "layer line" ), QStringLiteral( "memory" ) );
QVERIFY( layerLine->isValid() );
QCOMPARE( layerLine->featureCount(), static_cast<long>( 0 ) );
QCOMPARE( layerLine->extent(), QgsRectangle() );

const QSignalSpy spyAdded( layerLine, &QgsVectorLayer::featureAdded );

QgsFeature lineF1;
lineF1.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "LineString (2 1, 1 1, 1 5, 7 12)" ) ) );

connect( layerLine, &QgsVectorLayer::featureAdded, this, [&layerLine, &lineF1]( const QgsFeatureId &fid ) {
QCOMPARE( fid, lineF1.id() );
QCOMPARE( layerLine->extent(), QgsRectangle( 1, 1, 7, 12 ) );
QCOMPARE( layerLine->extent3D(), QgsBox3D( 1, 1, std::numeric_limits<double>::quiet_NaN(), 7, 12, std::numeric_limits<double>::quiet_NaN() ) );
} );

layerLine->startEditing();
layerLine->addFeature( lineF1 );
QCOMPARE( spyAdded.count(), 1 );
QCOMPARE( layerLine->featureCount(), static_cast<long>( 1 ) );
QCOMPARE( spyAdded.at( 0 ).at( 0 ), QVariant( lineF1.id() ) );

delete layerLine;
}


QGSTEST_MAIN( TestQgsVectorLayer )
#include "testqgsvectorlayer.moc"

0 comments on commit 912bf7d

Please sign in to comment.