Skip to content
Permalink
Browse files

[OGR provider] Update layer extent for GPKG layers

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 c74c722 commit 70186ee2a839d374f9bdb6a84bad2de58e8ac37e
@@ -285,6 +285,7 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri )
: QgsVectorDataProvider( uri )
, ogrDataSource( nullptr )
, mExtent( nullptr )
, mForceRecomputeExtent( false )
, ogrLayer( nullptr )
, ogrOrigLayer( nullptr )
, mLayerIndex( 0 )
@@ -482,7 +483,7 @@ bool QgsOgrProvider::setSubsetString( const QString& theSQL, bool updateFeatureC
loadFields();
QgsDebugMsg( "Done checking validity" );

updateExtents();
invalidateCachedExtent( false );

emit dataChanged();

@@ -882,6 +883,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 )
{
@@ -925,6 +937,12 @@ QgsRectangle QgsOgrProvider::extent()

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

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

invalidateCachedExtent( true );

OGR_F_Destroy( theOGRFeature );
}
QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() );
@@ -1473,7 +1493,7 @@ bool QgsOgrProvider::deleteFeatures( const QgsFeatureIds & id )

clearMinMaxCache();

updateExtents();
invalidateCachedExtent( true );

return returnvalue;
}
@@ -3184,7 +3204,7 @@ void QgsOgrProvider::close()
mValid = false;
setProperty( "_debug_open_mode", "invalid" );

updateExtents();
invalidateCachedExtent( false );
}

void QgsOgrProvider::reloadData()
@@ -296,6 +296,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,
@@ -314,6 +317,7 @@ class QgsOgrProvider : public QgsVectorDataProvider
QgsFields mAttributeFields;
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*/
@@ -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

@@ -160,5 +160,50 @@ def testBug15351_commit_closeIter_closeProvider(self):
return
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 70186ee

Please sign in to comment.
You can’t perform that action at this time.