Skip to content

Commit

Permalink
[OGR provider] Update layer extent for GPKG layers
Browse files Browse the repository at this point in the history
When moving or deleting a geometry that previously touched the layer extent,
the layer extent was never shrinked.

This fix requires GDAL 2.1.2 or above as well.

Fixes #15273
  • Loading branch information
rouault committed Oct 18, 2016
1 parent 1b48a74 commit 70ae301
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
26 changes: 23 additions & 3 deletions src/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri )
, mFirstFieldIsFid( false )
, ogrDataSource( nullptr )
, mExtent( nullptr )
, mForceRecomputeExtent( false )
, ogrLayer( nullptr )
, ogrOrigLayer( nullptr )
, mLayerIndex( 0 )
Expand Down Expand Up @@ -481,7 +482,7 @@ bool QgsOgrProvider::setSubsetString( const QString& theSQL, bool updateFeatureC
loadFields();
QgsDebugMsg( "Done checking validity" );

updateExtents();
invalidateCachedExtent( false );

emit dataChanged();

Expand Down Expand Up @@ -976,6 +977,17 @@ QgsRectangle QgsOgrProvider::extent() const
// get the extent_ (envelope) of the layer
QgsDebugMsg( "Starting get extent" );

#if defined(GDAL_COMPUTE_VERSION) && GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(2,1,2)
if ( mForceRecomputeExtent && mValid && ogrDriverName == "GPKG" && ogrDataSource && ogrOrigLayer )
{
QByteArray layerName = OGR_FD_GetName( OGR_L_GetLayerDefn( ogrOrigLayer ) );
// works with unquoted layerName
QByteArray sql = QByteArray( "RECOMPUTE EXTENT ON " ) + layerName;
QgsDebugMsg( QString( "SQL: %1" ).arg( FROM8( sql ) ) );
OGR_DS_ExecuteSQL( ogrDataSource, sql.constData(), nullptr, nullptr );
}
#endif

// TODO: This can be expensive, do we really need it!
if ( ogrLayer == ogrOrigLayer )
{
Expand Down Expand Up @@ -1019,6 +1031,12 @@ QgsRectangle QgsOgrProvider::extent() const

void QgsOgrProvider::updateExtents()
{
invalidateCachedExtent( true );
}

void QgsOgrProvider::invalidateCachedExtent( bool bForceRecomputeExtent )
{
mForceRecomputeExtent = bForceRecomputeExtent;
delete mExtent;
mExtent = nullptr;
}
Expand Down Expand Up @@ -1663,6 +1681,8 @@ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
}
mShapefileMayBeCorrupted = true;

invalidateCachedExtent( true );

OGR_F_Destroy( theOGRFeature );
}
QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() );
Expand Down Expand Up @@ -1732,7 +1752,7 @@ bool QgsOgrProvider::deleteFeatures( const QgsFeatureIds & id )

clearMinMaxCache();

updateExtents();
invalidateCachedExtent( true );

return returnvalue;
}
Expand Down Expand Up @@ -3440,7 +3460,7 @@ void QgsOgrProvider::close()
mValid = false;
setProperty( "_debug_open_mode", "invalid" );

updateExtents();
invalidateCachedExtent( false );
}

void QgsOgrProvider::reloadData()
Expand Down
4 changes: 4 additions & 0 deletions src/providers/ogr/qgsogrprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ class QgsOgrProvider : public QgsVectorDataProvider
/** Clean shapefile from features which are marked as deleted */
void repack();

/** Invalidate extent and optionnaly force its low level recomputation */
void invalidateCachedExtent( bool bForceRecomputeExtent );

enum OpenMode
{
OpenModeInitial,
Expand All @@ -299,6 +302,7 @@ class QgsOgrProvider : public QgsVectorDataProvider
bool mFirstFieldIsFid;
OGRDataSourceH ogrDataSource;
mutable OGREnvelope* mExtent;
bool mForceRecomputeExtent;

/** This member variable receives the same value as extent_
in the method QgsOgrProvider::extent(). The purpose is to prevent a memory leak*/
Expand Down
41 changes: 40 additions & 1 deletion tests/src/python/test_provider_ogr_gpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import shutil
from osgeo import gdal, ogr

from qgis.core import QgsVectorLayer, QgsFeature, QgsGeometry
from qgis.core import QgsVectorLayer, QgsFeature, QgsGeometry, QgsRectangle
from qgis.testing import start_app, unittest

start_app()
Expand Down Expand Up @@ -154,5 +154,44 @@ def testBug15351_commit_closeProvider_closeIter(self):
def testBug15351_commit_closeIter_closeProvider(self):
self.internalTestBug15351('commit_closeIter_closeProvider')

@unittest.expectedFailure(int(gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(2, 1, 2))
def testGeopackageExtentUpdate(self):
''' test http://hub.qgis.org/issues/15273 '''
tmpfile = os.path.join(self.basetestpath, 'testGeopackageExtentUpdate.gpkg')
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPoint)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(ogr.CreateGeometryFromWkt('POINT(0 0)'))
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(ogr.CreateGeometryFromWkt('POINT(1 1)'))
lyr.CreateFeature(f)
f = None
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(ogr.CreateGeometryFromWkt('POINT(1 0.5)'))
lyr.CreateFeature(f)
f = None
ds = None

vl = QgsVectorLayer(u'{}'.format(tmpfile), u'test', u'ogr')

# Test moving a geometry that touches the bbox
self.assertTrue(vl.startEditing())
self.assertTrue(vl.changeGeometry(1, QgsGeometry.fromWkt('Point (0.5 0)')))
self.assertTrue(vl.commitChanges())
reference = QgsGeometry.fromRect(QgsRectangle(0.5, 0.0, 1.0, 1.0))
provider_extent = QgsGeometry.fromRect(vl.extent())
self.assertTrue(QgsGeometry.compare(provider_extent.asPolygon()[0], reference.asPolygon()[0], 0.00001),
provider_extent.asPolygon()[0])

# Test deleting a geometry that touches the bbox
self.assertTrue(vl.startEditing())
self.assertTrue(vl.deleteFeature(2))
self.assertTrue(vl.commitChanges())
reference = QgsGeometry.fromRect(QgsRectangle(0.5, 0.0, 1.0, 0.5))
provider_extent = QgsGeometry.fromRect(vl.extent())
self.assertTrue(QgsGeometry.compare(provider_extent.asPolygon()[0], reference.asPolygon()[0], 0.00001),
provider_extent.asPolygon()[0])

if __name__ == '__main__':
unittest.main()

0 comments on commit 70ae301

Please sign in to comment.