qgis / QGIS

QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)
https://qgis.org
GNU General Public License v2.0
10.59k stars 3.01k forks source link

QgsFeatureSource not synchronised with QgsVectorLayer for memory provider #58113

Open Djedouas opened 3 months ago

Djedouas commented 3 months ago

What is the bug or the crash?

While (still) working on https://github.com/qgis/QGIS-Enhancement-Proposals/issues/236 (porting the geometry checker to the processing toolbox), I stumbled across an incredibly specific bug, which took me days to understand that it was a bug, and days to try to find it...

I need help understanding the bug, and fixing it.

Trying to explain it simply: there is a difference when working with temporary layers VS working with other providers (GPKG in this example).

Steps to reproduce the issue

The simple test with memory layer:

import tempfile

# Create memory layer and insert a feature
memoryLayer = QgsVectorLayer("Point", "memory", "memory")
f = QgsFeature()
f.setGeometry(QgsGeometry.fromWkt("Point (1 1)"))
assert memoryLayer.startEditing(), "impossible to edit layer"
assert memoryLayer.addFeature(f), "feature not added"
assert memoryLayer.commitChanges(), "changes not committed"

## Test step 1: create a feature source from the layer
featureSource = QgsVectorLayerFeatureSource(memoryLayer)
## Test step 2: change the geometry of the feature
assert memoryLayer.dataProvider().changeGeometryValues({1: QgsGeometry.fromWkt("Point (2 2)")}), "changing geometry on gpkg layer failed"
## Test step 3: verify that the geometry has changed, getting the feature from THE LAYER
assert list(memoryLayer.getFeatures(QgsFeatureRequest(1)))[0].geometry().asWkt() == "Point (2 2)"
## Test step 4: verify that the geometry has changed, getting the feature from THE FEATURE SOURCE
## HERE IS THE ERROR! what do I get here if it is not the changed feature?
assert list(featureSource.getFeatures(QgsFeatureRequest(1)))[0].geometry().asWkt() == "Point (2 2)"

The test with memory and GPKG layer showing that GPKG does not fail whereas memory does:

import tempfile

# Create memory layer and insert a feature
memoryLayer = QgsVectorLayer("Point", "memory", "memory")
f = QgsFeature()
f.setGeometry(QgsGeometry.fromWkt("Point (1 1)"))
assert memoryLayer.startEditing(), "impossible to edit layer"
assert memoryLayer.addFeature(f), "feature not added"
assert memoryLayer.commitChanges(), "changes not committed"

# Create GPKG layer from memory layer to have identic layers
with tempfile.NamedTemporaryFile() as tf:
    QgsVectorFileWriter.writeAsVectorFormatV3(memoryLayer, tf.name + ".gpkg", QgsCoordinateTransformContext(), QgsVectorFileWriter.SaveVectorOptions())
    gpkgLayer = QgsVectorLayer(tf.name + ".gpkg")
    assert gpkgLayer.isValid(), "gpkg layer not valid"

    ## Test step 1: create a feature source from the layer
    featureSource = QgsVectorLayerFeatureSource(gpkgLayer)
    ## Test step 2: change the geometry of the feature
    assert gpkgLayer.dataProvider().changeGeometryValues({1: QgsGeometry.fromWkt("Point (2 2)")}), "changing geometry on gpkg layer failed"
    ## Test step 3: verify that the geometry has changed, getting the feature from THE LAYER
    assert list(gpkgLayer.getFeatures(QgsFeatureRequest(1)))[0].geometry().asWkt() == "Point (2 2)"
    ## Test step 4: verify that the geometry has changed, getting the feature from THE FEATURE SOURCE
    assert list(featureSource.getFeatures(QgsFeatureRequest(1)))[0].geometry().asWkt() == "Point (2 2)"

## Test step 1: create a feature source from the layer
featureSource = QgsVectorLayerFeatureSource(memoryLayer)
## Test step 2: change the geometry of the feature
assert memoryLayer.dataProvider().changeGeometryValues({1: QgsGeometry.fromWkt("Point (2 2)")}), "changing geometry on gpkg layer failed"
## Test step 3: verify that the geometry has changed, getting the feature from THE LAYER
assert list(memoryLayer.getFeatures(QgsFeatureRequest(1)))[0].geometry().asWkt() == "Point (2 2)"
## Test step 4: verify that the geometry has changed, getting the feature from THE FEATURE SOURCE
## HERE IS THE ERROR! what do I get here if it is not the changed feature?
assert list(featureSource.getFeatures(QgsFeatureRequest(1)))[0].geometry().asWkt() == "Point (2 2)"

The CPP test:

void TestQgsProcessingFixGeometry::memoryVsGpkg()
{
  QgsVectorLayer memoryLayer = QgsVectorLayer( "Point?crs=2154", "memory", "memory" );

  {
    memoryLayer.startEditing();
    QgsFeature f;
    f.setGeometry( QgsGeometry::fromWkt( "Point (1 1)" ) );
    QVERIFY( memoryLayer.addFeature( f ) );
    QVERIFY( memoryLayer.commitChanges() );
  }

  {
    QgsVectorFileWriter::writeAsVectorFormatV3( &memoryLayer, "/home/jvolpes/Desktop/test.gpkg", QgsCoordinateTransformContext(), QgsVectorFileWriter::SaveVectorOptions() );
    QgsVectorLayer gpkgLayer = QgsVectorLayer( "/home/jvolpes/Desktop/test.gpkg|layername=test" );
    QgsVectorLayerFeatureSource mFeatureSource( &gpkgLayer );

    QgsGeometryMap geometryMap;
    geometryMap.insert( 1, QgsGeometry::fromWkt( "Point (2 2)" ) );
    QVERIFY( gpkgLayer.dataProvider()->changeGeometryValues( geometryMap ) );

    QgsFeature nf;
    gpkgLayer.getFeatures( QgsFeatureRequest( 1 ) ).nextFeature( nf );
    QCOMPARE( nf.geometry().asWkt(), "Point (2 2)" );

    mFeatureSource.getFeatures( QgsFeatureRequest( 1 ) ).nextFeature( nf );
    QCOMPARE( nf.geometry().asWkt(), "Point (2 2)" );
  }

  {
    QgsVectorLayerFeatureSource mFeatureSource( &memoryLayer );

    QgsGeometryMap geometryMap;
    geometryMap.insert( 1, QgsGeometry::fromWkt( "Point (2 2)" ) );
    QVERIFY( memoryLayer.dataProvider()->changeGeometryValues( geometryMap ) );

    QgsFeature nf;
    memoryLayer.getFeatures( QgsFeatureRequest( 1 ) ).nextFeature( nf );
    QCOMPARE( nf.geometry().asWkt(), "Point (2 2)" );

    mFeatureSource.getFeatures( QgsFeatureRequest( 1 ) ).nextFeature( nf );
    QCOMPARE( nf.geometry().asWkt(), "Point (2 2)" );
  }
}

Versions

master and LTR 3.34.8

Supported QGIS version

New profile

Additional context

Any help is welcome, I need to know if it's supposed to work as it works with memory or ogr provider, and advices on how to fix it, or point code locations...

Thank you.

nyalldawson commented 3 months ago

The short answer is that it's not supposed to work. As soon as you make a feature source it's independent from the layer and won't pick up changes to the layer. The only reason it works for the other formats is because there's the backend storage getting altered and the modification are being done outside of the whole QgsVectorLayer world.

The API is designed to require you to make the feature source after changing the layer.

Djedouas commented 3 months ago

Thanks @nyalldawson for this explanation.

Djedouas commented 3 months ago

Here is a video illustrating the problem, that the geometry checker plugin does not work properly with memory layers (and thus blocking me from continuing https://github.com/qgis/QGIS-Enhancement-Proposals/issues/236):

Delete small angle vertices, on a GPKG layer (OK) VS a memory layer

Memory layer: only one vertex removed, the original geometry is always taken on each vertex deletion iteration

GPKG layer: all the vertices are removed, the iteration on the geometry is correct

https://github.com/user-attachments/assets/374152db-ceeb-421f-8fb3-7cb07d485125