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 566e93e commit d2299c1
Show file tree
Hide file tree
Showing 3 changed files with 73 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 @@ -287,6 +287,7 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri )
, mFirstFieldIsFid( false )
, ogrDataSource( nullptr )
, mExtent( nullptr )
, mForceRecomputeExtent( false )
, ogrLayer( nullptr )
, ogrOrigLayer( nullptr )
, mLayerIndex( 0 )
Expand Down Expand Up @@ -483,7 +484,7 @@ bool QgsOgrProvider::setSubsetString( const QString& theSQL, bool updateFeatureC
loadFields();
QgsDebugMsg( "Done checking validity" );

updateExtents();
invalidateCachedExtent( false );

emit dataChanged();

Expand Down Expand Up @@ -978,6 +979,17 @@ QgsRectangle QgsOgrProvider::extent()
// 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 @@ -1021,6 +1033,12 @@ QgsRectangle QgsOgrProvider::extent()

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

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

invalidateCachedExtent( true );

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

clearMinMaxCache();

updateExtents();
invalidateCachedExtent( true );

return returnvalue;
}
Expand Down Expand Up @@ -3442,7 +3462,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 @@ -286,6 +286,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 @@ -305,6 +308,7 @@ class QgsOgrProvider : public QgsVectorDataProvider
bool mFirstFieldIsFid;
OGRDataSourceH ogrDataSource;
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
47 changes: 46 additions & 1 deletion tests/src/python/test_provider_ogr_gpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import glob
from osgeo import gdal, ogr

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

Expand Down Expand Up @@ -156,5 +156,50 @@ def testBug15351_closeIter_commit_closeProvider(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
gdal.ErrorReset()
ds.ExecuteSQL('RECOMPUTE EXTENT ON test')
has_error = gdal.GetLastErrorMsg() != ''
ds = None
if has_error:
print('Too old GDAL trunk version. Please update')
return

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 d2299c1

Please sign in to comment.