From c90eb46a44c7bf2c987ec115af84376eb41e06a0 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 6 Feb 2025 14:39:49 +0100 Subject: [PATCH] Address PR comments --- .../providers/ogr/qgsogrproviderutils.cpp | 63 +++++++- tests/src/python/test_provider_ogr.py | 144 +++++++++++++++++- 2 files changed, 197 insertions(+), 10 deletions(-) diff --git a/src/core/providers/ogr/qgsogrproviderutils.cpp b/src/core/providers/ogr/qgsogrproviderutils.cpp index f16972f13b0..f3bde630893 100644 --- a/src/core/providers/ogr/qgsogrproviderutils.cpp +++ b/src/core/providers/ogr/qgsogrproviderutils.cpp @@ -2764,12 +2764,11 @@ QList< QgsProviderSublayerDetails > QgsOgrProviderUtils::querySubLayerList( int OGRwkbGeometryType pointBaseType { wkbPoint }; // Last type in the list is the winner - const static QList pointHyerarchy { wkbPoint, wkbMultiPoint }; - // Note: compound curve takes precedence over multilinestring - const static QList lineHyerarchy { wkbLineString, wkbCircularString, wkbMultiCurve, wkbMultiLineString, wkbCompoundCurve, wkbMultiCurve }; - const static QList polyHyerarchy { wkbPolyhedralSurface, wkbTIN, wkbPolygon, wkbCurvePolygon, wkbMultiSurface, wkbMultiPolygon }; + const static QList pointHierarchy { wkbPoint, wkbMultiPoint }; + const static QList lineHierarchy { wkbLineString, wkbCircularString, wkbMultiLineString, wkbCompoundCurve, wkbMultiCurve }; + const static QList polyHierarchy { wkbPolyhedralSurface, wkbTIN, wkbPolygon, wkbCurvePolygon, wkbMultiPolygon, wkbMultiSurface }; - for ( const auto t : std::as_const( pointHyerarchy ) ) + for ( const auto t : std::as_const( pointHierarchy ) ) { if ( fCount.contains( t ) ) { @@ -2779,7 +2778,39 @@ QList< QgsProviderSublayerDetails > QgsOgrProviderUtils::querySubLayerList( int } } - for ( const auto t : std::as_const( lineHyerarchy ) ) + // For lines use a three-step approach + // 1. First collapse linestring and circularstring into compoundcurve + if ( fCount.contains( wkbLineString ) && fCount.contains( wkbCircularString ) ) + { + baseTypeCount[Qgis::GeometryType::Line] += fCount.value( wkbLineString ); + baseTypeCount[Qgis::GeometryType::Line] += fCount.value( wkbCircularString ); + lineBaseType = wkbCompoundCurve; + if ( ! fCount.contains( wkbCompoundCurve ) ) + { + fCount[wkbCompoundCurve] = baseTypeCount[Qgis::GeometryType::Line]; + baseTypeCount[Qgis::GeometryType::Line] = 0; + } + fCount.remove( wkbLineString ); + fCount.remove( wkbCircularString ); + } + + // 2. Then collapse multilinestring and compoundcurve into multicurve + if ( fCount.contains( wkbMultiLineString ) && fCount.contains( wkbCompoundCurve ) ) + { + baseTypeCount[Qgis::GeometryType::Line] += fCount.value( wkbMultiLineString ); + baseTypeCount[Qgis::GeometryType::Line] += fCount.value( wkbCompoundCurve ); + lineBaseType = wkbMultiCurve; + if ( ! fCount.contains( wkbMultiCurve ) ) + { + fCount[wkbMultiCurve] = baseTypeCount[Qgis::GeometryType::Line]; + baseTypeCount[Qgis::GeometryType::Line] = 0; + } + fCount.remove( wkbMultiLineString ); + fCount.remove( wkbCompoundCurve ); + } + + // 3. Then follow the hierarchy + for ( const auto t : std::as_const( lineHierarchy ) ) { if ( fCount.contains( t ) ) { @@ -2789,7 +2820,25 @@ QList< QgsProviderSublayerDetails > QgsOgrProviderUtils::querySubLayerList( int } } - for ( const auto t : std::as_const( polyHyerarchy ) ) + + // For polygons use a two-step approach: + // 1. First collapse multipolygon and curvepolygon into multisurface + if ( fCount.contains( wkbMultiPolygon ) && fCount.contains( wkbCurvePolygon ) ) + { + baseTypeCount[Qgis::GeometryType::Polygon] += fCount.value( wkbMultiPolygon ); + baseTypeCount[Qgis::GeometryType::Polygon] += fCount.value( wkbMultiSurface ); + polyBaseType = wkbMultiSurface; + if ( ! fCount.contains( wkbMultiSurface ) ) + { + fCount[wkbMultiSurface] = baseTypeCount[Qgis::GeometryType::Polygon]; + baseTypeCount[Qgis::GeometryType::Polygon] = 0; + } + fCount.remove( wkbMultiPolygon ); + fCount.remove( wkbCurvePolygon ); + } + + // 2. Then collapse following the hierarchy + for ( const auto t : std::as_const( polyHierarchy ) ) { if ( fCount.contains( t ) ) { diff --git a/tests/src/python/test_provider_ogr.py b/tests/src/python/test_provider_ogr.py index f405bdf807b..cd6c58f9bb8 100644 --- a/tests/src/python/test_provider_ogr.py +++ b/tests/src/python/test_provider_ogr.py @@ -174,7 +174,7 @@ class PyQgsOGRProvider(QgisTestCase): self.assertEqual( vl.dataProvider().subLayers()[0], QgsDataProvider.SUBLAYER_SEPARATOR.join( - ["0", "testMixOfPolygonCurvePolygon", "4", "MultiPolygon", "", ""] + ["0", "testMixOfPolygonCurvePolygon", "4", "MultiSurface", "", ""] ), ) @@ -188,7 +188,6 @@ class PyQgsOGRProvider(QgisTestCase): f.write('1,"LINESTRING(0 0,0 1)"\n') f.write('2,"COMPOUNDCURVE((0 0,0 1))"\n') f.write('3,"MULTILINESTRING((0 0,0 1))"\n') - f.write('4,"MULTICURVE((0 0,0 1))"\n') f.write('5,"CIRCULARSTRING(0 0,1 1,2 0)"\n') vl = QgsVectorLayer(f"{datasource}|layerid=0", "test", "ogr") @@ -197,7 +196,146 @@ class PyQgsOGRProvider(QgisTestCase): self.assertEqual( vl.dataProvider().subLayers()[0], QgsDataProvider.SUBLAYER_SEPARATOR.join( - ["0", "testMixOfLineStringCompoundCurve", "5", "CompoundCurve", "", ""] + ["0", "testMixOfLineStringCompoundCurve", "4", "MultiCurve", "", ""] + ), + ) + + def testMixOfCurvePolygonAndMultiPolygon(self): + + datasource = os.path.join( + self.basetestpath, "testMixOfCurvePolygonAndMultiPolygon.csv" + ) + with open(datasource, "w") as f: + f.write("id,WKT\n") + f.write('1,"CURVEPOLYGON((0 0,0 1,1 1,0 0))"\n') + f.write('2,"MULTIPOLYGON(((0 0,0 1,1 1,0 0)))"\n') + + vl = QgsVectorLayer(f"{datasource}|layerid=0", "test", "ogr") + self.assertTrue(vl.isValid()) + self.assertEqual(len(vl.dataProvider().subLayers()), 1) + self.assertEqual( + vl.dataProvider().subLayers()[0], + QgsDataProvider.SUBLAYER_SEPARATOR.join( + [ + "0", + "testMixOfCurvePolygonAndMultiPolygon", + "2", + "MultiSurface", + "", + "", + ] + ), + ) + + def testMixOfLineStringAndCircularString(self): + + datasource = os.path.join( + self.basetestpath, "testMixOfLineStringAndCircularString.csv" + ) + with open(datasource, "w") as f: + f.write("id,WKT\n") + f.write('1,"LINESTRING(0 0,0 1)"\n') + f.write('2,"CIRCULARSTRING(0 0,1 1,2 0)"\n') + + vl = QgsVectorLayer(f"{datasource}|layerid=0", "test", "ogr") + self.assertTrue(vl.isValid()) + self.assertEqual(len(vl.dataProvider().subLayers()), 1) + self.assertEqual( + vl.dataProvider().subLayers()[0], + QgsDataProvider.SUBLAYER_SEPARATOR.join( + [ + "0", + "testMixOfLineStringAndCircularString", + "2", + "CompoundCurve", + "", + "", + ] + ), + ) + + def testMixOfLineStringAndCircularStringAndCompoundCurve(self): + + datasource = os.path.join( + self.basetestpath, + "testMixOfLineStringAndCircularStringAndCompoundCurve.csv", + ) + with open(datasource, "w") as f: + f.write("id,WKT\n") + f.write('1,"LINESTRING(0 0,0 1)"\n') + f.write('2,"CIRCULARSTRING(0 0,1 1,2 0)"\n') + f.write('3,"COMPOUNDCURVE((0 0,0 1))"\n') + + vl = QgsVectorLayer(f"{datasource}|layerid=0", "test", "ogr") + self.assertTrue(vl.isValid()) + self.assertEqual(len(vl.dataProvider().subLayers()), 1) + self.assertEqual( + vl.dataProvider().subLayers()[0], + QgsDataProvider.SUBLAYER_SEPARATOR.join( + [ + "0", + "testMixOfLineStringAndCircularStringAndCompoundCurve", + "3", + "CompoundCurve", + "", + "", + ] + ), + ) + + def testMixOfMultiLineStringAndCompoundCurve(self): + + datasource = os.path.join( + self.basetestpath, "testMixOfMultiLineStringAndCompoundCurve.csv" + ) + with open(datasource, "w") as f: + f.write("id,WKT\n") + f.write('1,"MULTILINESTRING((0 0,0 1))"\n') + f.write('2,"COMPOUNDCURVE((0 0,0 1))"\n') + + vl = QgsVectorLayer(f"{datasource}|layerid=0", "test", "ogr") + self.assertTrue(vl.isValid()) + self.assertEqual(len(vl.dataProvider().subLayers()), 1) + self.assertEqual( + vl.dataProvider().subLayers()[0], + QgsDataProvider.SUBLAYER_SEPARATOR.join( + [ + "0", + "testMixOfMultiLineStringAndCompoundCurve", + "2", + "MultiCurve", + "", + "", + ] + ), + ) + + def testMixOfMultiLineStringAndCompoundCurveAndMultiCurve(self): + + datasource = os.path.join( + self.basetestpath, + "testMixOfMultiLineStringAndCompoundCurveAndMultiCurve.csv", + ) + with open(datasource, "w") as f: + f.write("id,WKT\n") + f.write('1,"MULTILINESTRING((0 0,0 1))"\n') + f.write('2,"COMPOUNDCURVE((0 0,0 1))"\n') + f.write('3,"MULTICURVE((0 0,0 1))"\n') + + vl = QgsVectorLayer(f"{datasource}|layerid=0", "test", "ogr") + self.assertTrue(vl.isValid()) + self.assertEqual(len(vl.dataProvider().subLayers()), 1) + self.assertEqual( + vl.dataProvider().subLayers()[0], + QgsDataProvider.SUBLAYER_SEPARATOR.join( + [ + "0", + "testMixOfMultiLineStringAndCompoundCurveAndMultiCurve", + "3", + "MultiCurve", + "", + "", + ] ), )