Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QgsFeatureSource not synchronised with QgsVectorLayer for memory provider #58113

Open
2 tasks done
Djedouas opened this issue Jul 15, 2024 · 3 comments · May be fixed by #59330
Open
2 tasks done

QgsFeatureSource not synchronised with QgsVectorLayer for memory provider #58113

Djedouas opened this issue Jul 15, 2024 · 3 comments · May be fixed by #59330
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers

Comments

@Djedouas
Copy link
Member

Djedouas commented Jul 15, 2024

What is the bug or the crash?

While (still) working on qgis/QGIS-Enhancement-Proposals#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

  • I'm running a supported QGIS version according to the roadmap.

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.

@Djedouas Djedouas added Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers labels Jul 15, 2024
@nyalldawson
Copy link
Collaborator

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 added a commit to Djedouas/QGIS that referenced this issue Jul 16, 2024
Geometry checker does not work with memory layers, so we change
how the cache in the QgsFeaturePool object is managed, to refresh it
more often.
@Djedouas
Copy link
Member Author

Thanks @nyalldawson for this explanation.

@Djedouas
Copy link
Member Author

Djedouas commented Jul 16, 2024

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 qgis/QGIS-Enhancement-Proposals#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

geometry_checker_memory_layer-2024-07-16_10.07.09.mp4

Djedouas added a commit to Djedouas/QGIS that referenced this issue Jul 16, 2024
Geometry checker does not work with memory layers, so we change
how the cache in the QgsFeaturePool object is managed, to refresh it
more often.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Jul 17, 2024
Geometry checker does not work with memory layers. We change
how QgsFeaturePool class works with memory layers to avoid
creating a "double-cached" system, as memory layers are already
a cache layer by construction.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Jul 17, 2024
Geometry checker does not work with memory layers. We change
how QgsFeaturePool class works with memory layers to avoid
creating a "double-cached" system, as memory layers are already
a cache layer by construction.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Jul 18, 2024
Geometry checker does not work with memory layers. We change
how QgsFeaturePool class works with memory layers to avoid
creating a "double-cached" system, as memory layers are already
a cache layer by construction.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Jul 18, 2024
Geometry checker does not work properly with memory layers. We change
how QgsFeaturePool class works with memory layers to avoid
creating a "double-cached" system, as memory layers are already
a cache layer by construction.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Jul 18, 2024
Geometry checker does not work properly with memory layers. We change
how QgsFeaturePool class works with memory layers to avoid
creating a "double-cached" system, as memory layers are already
a cache layer by construction.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Aug 1, 2024
Geometry checker cache does not work properly with memory layers.
refreshCache now directly adds the geometry in the feature pool cache.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Aug 1, 2024
Geometry checker cache does not work properly with memory layers.
refreshCache now directly adds the geometry in the feature pool cache.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Aug 1, 2024
Geometry checker cache does not work properly with memory layers.
refreshCache now directly adds the geometry in the feature pool cache.
Djedouas added a commit to Djedouas/QGIS that referenced this issue Nov 4, 2024
Geometry checker cache does not work properly with memory layers.
refreshCache now handles a list of updated features to be thread-safe.

Also, fixes a locker mode, and correctly remove features from spatial
index.
@Djedouas Djedouas linked a pull request Nov 4, 2024 that will close this issue
Djedouas added a commit to Djedouas/QGIS that referenced this issue Nov 5, 2024
Geometry checker cache does not work properly with memory layers.
refreshCache now handles a list of updated features to be thread-safe.

Also, fixes a locker mode, and correctly remove features from spatial
index.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers
Projects
None yet
2 participants