[OGR provider] Report curve geometry types. Do geometry type conversions when needed on feature creation/modification

rouault · rouault · commit 1ba2bc0fa9ab · 2016-04-23T19:36:17.000+02:00
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -280,7 +280,7 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri )
     , mOgrGeometryTypeFilter( wkbUnknown )
     , ogrDriver( nullptr )
     , mValid( false )
-    , geomType( wkbUnknown )
+    , mOGRGeomType( wkbUnknown )
     , mFeaturesCounted( -1 )
     , mWriteAccess( false )
     , mShapefileMayBeCorrupted( false )
@@ -497,6 +497,38 @@ QString QgsOgrProvider::ogrWkbGeometryTypeName( OGRwkbGeometryType type ) const
     case wkbGeometryCollection:
       geom = "GeometryCollection";
       break;
+#if defined(GDAL_COMPUTE_VERSION) && GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(2,0,0)
+    case wkbCircularString:
+      geom = "CircularString";
+      break;
+    case wkbCompoundCurve:
+      geom = "CompoundCurve";
+      break;
+    case wkbCurvePolygon:
+      geom = "CurvePolygon";
+      break;
+    case wkbMultiCurve:
+      geom = "MultiCurve";
+      break;
+    case wkbMultiSurface:
+      geom = "MultiSurface";
+      break;
+    case wkbCircularStringZ:
+      geom = "CircularStringZ";
+      break;
+    case wkbCompoundCurveZ:
+      geom = "CompoundCurveZ";
+      break;
+    case wkbCurvePolygonZ:
+      geom = "CurvePolygonZ";
+      break;
+    case wkbMultiCurveZ:
+      geom = "MultiCurveZ";
+      break;
+    case wkbMultiSurfaceZ:
+      geom = "MultiSurfaceZ";
+      break;
+#endif
     case wkbNone:
       geom = "None";
       break;
@@ -525,7 +557,8 @@ QString QgsOgrProvider::ogrWkbGeometryTypeName( OGRwkbGeometryType type ) const
       geom = "GeometryCollection25D";
       break;
     default:
-      geom = QString( "Unknown WKB: %1" ).arg( type );
+      // Do not use ':', as it will mess with the separator used by QgsSublayersDialog::populateLayers()
+      geom = QString( "Unknown WKB (%1)" ).arg( type );
   }
   return geom;
 }
@@ -691,11 +724,11 @@ void QgsOgrProvider::loadFields()
 
   if ( mOgrGeometryTypeFilter != wkbUnknown )
   {
-    geomType = mOgrGeometryTypeFilter;
+    mOGRGeomType = mOgrGeometryTypeFilter;
   }
   else
   {
-    geomType = getOgrGeomType( ogrLayer );
+    mOGRGeomType = getOgrGeomType( ogrLayer );
   }
   OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( ogrLayer );
   if ( fdef )
@@ -907,7 +940,7 @@ size_t QgsOgrProvider::layerCount() const
  */
 QGis::WkbType QgsOgrProvider::geometryType() const
 {
-  return static_cast<QGis::WkbType>( geomType );
+  return static_cast<QGis::WkbType>( mOGRGeomType );
 }
 
 /**
@@ -933,6 +966,36 @@ bool QgsOgrProvider::isValid()
   return mValid;
 }
 
+// Drivers may be more tolerant than we really wish (e.g. GeoPackage driver
+// may accept any geometry type)
+OGRGeometryH QgsOgrProvider::ConvertGeometryIfNecessary( OGRGeometryH hGeom )
+{
+  if ( hGeom == nullptr )
+    return hGeom;
+  OGRwkbGeometryType layerGeomType = OGR_L_GetGeomType( ogrLayer );
+  OGRwkbGeometryType flattenLayerGeomType = wkbFlatten( layerGeomType );
+  OGRwkbGeometryType geomType = OGR_G_GetGeometryType( hGeom );
+  OGRwkbGeometryType flattenGeomType = wkbFlatten( geomType );
+
+  if ( flattenLayerGeomType == wkbUnknown || flattenLayerGeomType == flattenGeomType )
+  {
+    return hGeom;
+  }
+  if ( flattenLayerGeomType == wkbMultiPolygon && flattenGeomType == wkbPolygon )
+  {
+    return OGR_G_ForceToMultiPolygon( hGeom );
+  }
+  if ( flattenLayerGeomType == wkbMultiLineString && flattenGeomType == wkbLineString )
+  {
+    return OGR_G_ForceToMultiLineString( hGeom );
+  }
+#if defined(GDAL_COMPUTE_VERSION) && GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(2,0,0)
+  return OGR_G_ForceTo( hGeom, layerGeomType, nullptr );
+#else
+  return hGeom;
+#endif
+}
+
 bool QgsOgrProvider::addFeature( QgsFeature& f )
 {
   bool returnValue = true;
@@ -951,6 +1014,9 @@ bool QgsOgrProvider::addFeature( QgsFeature& f )
         pushError( tr( "OGR error creating wkb for feature %1: %2" ).arg( f.id() ).arg( CPLGetLastErrorMsg() ) );
         return false;
       }
+
+      geom = ConvertGeometryIfNecessary( geom );
+
       OGR_F_SetGeometryDirectly( feature, geom );
     }
   }
@@ -1320,6 +1386,8 @@ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
       continue;
     }
 
+    theNewGeometry = ConvertGeometryIfNecessary( theNewGeometry );
+
     //set the new geometry
     if ( OGR_F_SetGeometryDirectly( theOGRFeature, theNewGeometry ) != OGRERR_NONE )
     {
@@ -2677,7 +2745,7 @@ void QgsOgrProvider::recalculateFeatureCount()
 bool QgsOgrProvider::doesStrictFeatureTypeCheck() const
 {
   // FIXME probably other drivers too...
-  return ogrDriverName != "ESRI Shapefile" || ( geomType == wkbPoint || geomType == wkbPoint25D );
+  return ogrDriverName != "ESRI Shapefile" || ( mOGRGeomType == wkbPoint || mOGRGeomType == wkbPoint25D );
 }
 
 OGRwkbGeometryType QgsOgrProvider::ogrWkbSingleFlatten( OGRwkbGeometryType type )
diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h
@@ -335,7 +335,7 @@ class QgsOgrProvider : public QgsVectorDataProvider
 
     bool mValid;
 
-    OGRwkbGeometryType geomType;
+    OGRwkbGeometryType mOGRGeomType;
     long mFeaturesCounted;
 
     mutable QStringList mSubLayerList;
@@ -356,6 +356,9 @@ class QgsOgrProvider : public QgsVectorDataProvider
     bool mWriteAccess;
 
     bool mShapefileMayBeCorrupted;
+
+    /** Converts the geometry to the layer type if necessary. Takes ownership of the passed geometry */
+    OGRGeometryH ConvertGeometryIfNecessary( OGRGeometryH );
 };
 
 
diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt
@@ -47,6 +47,7 @@ ADD_PYTHON_TEST(PyQgsMultiEditToolButton test_qgsmultiedittoolbutton.py)
 ADD_PYTHON_TEST(PyQgsNetworkContentFetcher test_qgsnetworkcontentfetcher.py)
 ADD_PYTHON_TEST(PyQgsNullSymbolRenderer test_qgsnullsymbolrenderer.py)
 ADD_PYTHON_TEST(PyQgsNewGeoPackageLayerDialog test_qgsnewgeopackagelayerdialog.py)
+ADD_PYTHON_TEST(PyQgsOGRProviderGpkg test_provider_ogr_gpkg.py)
 ADD_PYTHON_TEST(PyQgsPalLabelingBase test_qgspallabeling_base.py)
 ADD_PYTHON_TEST(PyQgsPalLabelingCanvas test_qgspallabeling_canvas.py)
 ADD_PYTHON_TEST(PyQgsPalLabelingComposer test_qgspallabeling_composer.py)
diff --git a/tests/src/python/test_provider_ogr_gpkg.py b/tests/src/python/test_provider_ogr_gpkg.py
@@ -0,0 +1,86 @@
+# -*- coding: utf-8 -*-
+"""QGIS Unit tests for the OGR/GPKG provider.
+
+.. note:: This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+"""
+__author__ = 'Even Rouault'
+__date__ = '2016-04-21'
+__copyright__ = 'Copyright 2016, Even Rouault'
+# This will get replaced with a git SHA1 when you do a git archive
+__revision__ = '$Format:%H$'
+
+import qgis  # NOQA
+
+import os
+import tempfile
+import shutil
+import glob
+from osgeo import gdal, ogr
+
+from qgis.core import QgsVectorLayer, QgsFeature, QgsGeometry
+from qgis.testing import start_app, unittest
+from utilities import unitTestDataPath
+
+start_app()
+
+
+def GDAL_COMPUTE_VERSION(maj, min, rev):
+    return ((maj) * 1000000 + (min) * 10000 + (rev) * 100)
+
+
+class TestPyQgsOGRProviderGpkg(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        """Run before all tests"""
+        # Create test layer
+        cls.basetestpath = tempfile.mkdtemp()
+
+    @classmethod
+    def tearDownClass(cls):
+        """Run after all tests"""
+        shutil.rmtree(cls.basetestpath, True)
+
+    def testSingleToMultiPolygonPromotion(self):
+
+        version_num = int(gdal.VersionInfo('VERSION_NUM'))
+        if version_num < GDAL_COMPUTE_VERSION(1, 11, 0):
+            return
+
+        tmpfile = os.path.join(self.basetestpath, 'testSingleToMultiPolygonPromotion.gpkg')
+        ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('test', geom_type=ogr.wkbMultiPolygon)
+        ds = None
+
+        vl = QgsVectorLayer(u'{}|layerid=0'.format(tmpfile), u'test', u'ogr')
+        f = QgsFeature()
+        f.setGeometry(QgsGeometry.fromWkt('POLYGON ((0 0,0 1,1 1,0 0))'))
+        vl.dataProvider().addFeatures([f])
+        got = [f.geometry().exportToWkt(0) for f in vl.getFeatures()][0]
+        self.assertEqual(got, 'MultiPolygon (((0 0, 0 1, 1 1, 0 0)))')
+
+    def testCurveGeometryType(self):
+
+        version_num = int(gdal.VersionInfo('VERSION_NUM'))
+        if version_num < GDAL_COMPUTE_VERSION(2, 0, 0):
+            return
+
+        tmpfile = os.path.join(self.basetestpath, 'testCurveGeometryType.gpkg')
+        ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('test', geom_type=ogr.wkbCurvePolygon)
+        ds = None
+
+        vl = QgsVectorLayer(u'{}'.format(tmpfile), u'test', u'ogr')
+        self.assertEqual(vl.dataProvider().subLayers(), [u'0:test:0:CurvePolygon'])
+        f = QgsFeature()
+        f.setGeometry(QgsGeometry.fromWkt('POLYGON ((0 0,0 1,1 1,0 0))'))
+        vl.dataProvider().addFeatures([f])
+        got = [f.geometry().exportToWkt(0) for f in vl.getFeatures()][0]
+        self.assertEqual(got, 'CurvePolygon ((0 0, 0 1, 1 1, 0 0))')
+
+
+if __name__ == '__main__':
+    unittest.main()
