Skip to content

Commit

Permalink
[OGR provider] Map GeometryCollection of TIN coming from multipart sh…
Browse files Browse the repository at this point in the history 
…apefiles to MultiPolygonZ (fixes #29376)
  • Loading branch information
@rouault @nyalldawson
rouault authored and nyalldawson committed May 30, 2019
1 parent 6abc15d commit 50dab59
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 13 deletions.
103 changes: 102 additions & 1 deletion src/core/qgsogrutils.cpp
View file
Expand Up @@ -368,12 +368,100 @@ std::unique_ptr< QgsLineString > ogrGeometryToQgsLineString( OGRGeometryH geom )
return qgis::make_unique< QgsLineString>( x, y, z, m, wkbType == QgsWkbTypes::LineString25D );
}

QgsWkbTypes::Type QgsOgrUtils::ogrGeometryTypeToQgsWkbType( OGRwkbGeometryType ogrGeomType )
{
switch ( ogrGeomType )
{
case wkbUnknown: return QgsWkbTypes::Type::Unknown;
case wkbPoint: return QgsWkbTypes::Type::Point;
case wkbLineString: return QgsWkbTypes::Type::LineString;
case wkbPolygon: return QgsWkbTypes::Type::Polygon;
case wkbMultiPoint: return QgsWkbTypes::Type::MultiPoint;
case wkbMultiLineString: return QgsWkbTypes::Type::MultiLineString;
case wkbMultiPolygon: return QgsWkbTypes::Type::MultiPolygon;
case wkbGeometryCollection: return QgsWkbTypes::Type::GeometryCollection;
case wkbCircularString: return QgsWkbTypes::Type::CircularString;
case wkbCompoundCurve: return QgsWkbTypes::Type::CompoundCurve;
case wkbCurvePolygon: return QgsWkbTypes::Type::CurvePolygon;
case wkbMultiCurve: return QgsWkbTypes::Type::MultiCurve;
case wkbMultiSurface: return QgsWkbTypes::Type::MultiSurface;
case wkbCurve: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbSurface: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbPolyhedralSurface: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTIN: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTriangle: return QgsWkbTypes::Type::Triangle;

case wkbNone: return QgsWkbTypes::Type::NoGeometry;
case wkbLinearRing: return QgsWkbTypes::Type::LineString; // approximate match

case wkbCircularStringZ: return QgsWkbTypes::Type::CircularStringZ;
case wkbCompoundCurveZ: return QgsWkbTypes::Type::CompoundCurveZ;
case wkbCurvePolygonZ: return QgsWkbTypes::Type::CurvePolygonZ;
case wkbMultiCurveZ: return QgsWkbTypes::Type::MultiCurveZ;
case wkbMultiSurfaceZ: return QgsWkbTypes::Type::MultiSurfaceZ;
case wkbCurveZ: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbSurfaceZ: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbPolyhedralSurfaceZ: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTINZ: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTriangleZ: return QgsWkbTypes::Type::TriangleZ;

case wkbPointM: return QgsWkbTypes::Type::PointM;
case wkbLineStringM: return QgsWkbTypes::Type::LineStringM;
case wkbPolygonM: return QgsWkbTypes::Type::PolygonM;
case wkbMultiPointM: return QgsWkbTypes::Type::MultiPointM;
case wkbMultiLineStringM: return QgsWkbTypes::Type::MultiLineStringM;
case wkbMultiPolygonM: return QgsWkbTypes::Type::MultiPolygonM;
case wkbGeometryCollectionM: return QgsWkbTypes::Type::GeometryCollectionM;
case wkbCircularStringM: return QgsWkbTypes::Type::CircularStringM;
case wkbCompoundCurveM: return QgsWkbTypes::Type::CompoundCurveM;
case wkbCurvePolygonM: return QgsWkbTypes::Type::CurvePolygonM;
case wkbMultiCurveM: return QgsWkbTypes::Type::MultiCurveM;
case wkbMultiSurfaceM: return QgsWkbTypes::Type::MultiSurfaceM;
case wkbCurveM: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbSurfaceM: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbPolyhedralSurfaceM: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTINM: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTriangleM: return QgsWkbTypes::Type::TriangleM;

case wkbPointZM: return QgsWkbTypes::Type::PointZM;
case wkbLineStringZM: return QgsWkbTypes::Type::LineStringZM;
case wkbPolygonZM: return QgsWkbTypes::Type::PolygonZM;
case wkbMultiPointZM: return QgsWkbTypes::Type::MultiPointZM;
case wkbMultiLineStringZM: return QgsWkbTypes::Type::MultiLineStringZM;
case wkbMultiPolygonZM: return QgsWkbTypes::Type::MultiPolygonZM;
case wkbGeometryCollectionZM: return QgsWkbTypes::Type::GeometryCollectionZM;
case wkbCircularStringZM: return QgsWkbTypes::Type::CircularStringZM;
case wkbCompoundCurveZM: return QgsWkbTypes::Type::CompoundCurveZM;
case wkbCurvePolygonZM: return QgsWkbTypes::Type::CurvePolygonZM;
case wkbMultiCurveZM: return QgsWkbTypes::Type::MultiCurveZM;
case wkbMultiSurfaceZM: return QgsWkbTypes::Type::MultiSurfaceZM;
case wkbCurveZM: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbSurfaceZM: return QgsWkbTypes::Type::Unknown; // not an actual concrete type
case wkbPolyhedralSurfaceZM: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTINZM: return QgsWkbTypes::Type::Unknown; // no actual matching
case wkbTriangleZM: return QgsWkbTypes::Type::TriangleZM;

case wkbPoint25D: return QgsWkbTypes::Type::PointZ;
case wkbLineString25D: return QgsWkbTypes::Type::LineStringZ;
case wkbPolygon25D: return QgsWkbTypes::Type::PolygonZ;
case wkbMultiPoint25D: return QgsWkbTypes::Type::MultiPointZ;
case wkbMultiLineString25D: return QgsWkbTypes::Type::MultiLineStringZ;
case wkbMultiPolygon25D: return QgsWkbTypes::Type::MultiPolygonZ;
case wkbGeometryCollection25D: return QgsWkbTypes::Type::GeometryCollectionZ;
}

// should not reach that point normally
return QgsWkbTypes::Type::Unknown;
}

QgsGeometry QgsOgrUtils::ogrGeometryToQgsGeometry( OGRGeometryH geom )
{
if ( !geom )
return QgsGeometry();

QgsWkbTypes::Type wkbType = static_cast<QgsWkbTypes::Type>( OGR_G_GetGeometryType( geom ) );
const auto ogrGeomType = OGR_G_GetGeometryType( geom );
QgsWkbTypes::Type wkbType = ogrGeometryTypeToQgsWkbType( ogrGeomType );

// optimised case for some geometry classes, avoiding wkb conversion on OGR/QGIS sides
// TODO - extend to other classes!
switch ( QgsWkbTypes::flatType( wkbType ) )
Expand All @@ -395,6 +483,19 @@ QgsGeometry QgsOgrUtils::ogrGeometryToQgsGeometry( OGRGeometryH geom )

// Fallback to inefficient WKB conversions

if ( wkbFlatten( wkbType ) == wkbGeometryCollection )
{
// Shapefile MultiPatch can be reported as GeometryCollectionZ of TINZ
if ( OGR_G_GetGeometryCount( geom ) >= 1 &&
wkbFlatten( OGR_G_GetGeometryType( OGR_G_GetGeometryRef( geom, 0 ) ) ) == wkbTIN )
{
auto newGeom = OGR_G_ForceToMultiPolygon( OGR_G_Clone( geom ) );
auto ret = ogrGeometryToQgsGeometry( newGeom );
OGR_G_DestroyGeometry( newGeom );
return ret;
}
}

// get the wkb representation
int memorySize = OGR_G_WkbSize( geom );
unsigned char *wkb = new unsigned char[memorySize];
Expand Down
7 changes: 7 additions & 0 deletions src/core/qgsogrutils.h
View file
Expand Up @@ -247,6 +247,13 @@ class CORE_EXPORT QgsOgrUtils
* \since QGIS 3.2
*/
static QStringList cStringListToQStringList( char **stringList );

/**
* Converts a OGRwkbGeometryType to QgsWkbTypes::Type
*
* \since QGIS 3.4.9
*/
static QgsWkbTypes::Type ogrGeometryTypeToQgsWkbType( OGRwkbGeometryType ogrGeomType );
};

#endif // QGSOGRUTILS_H
4 changes: 3 additions & 1 deletion src/providers/ogr/qgsogrdataitems.cpp
View file
Expand Up @@ -336,7 +336,9 @@ static QgsOgrLayerItem *dataItemForLayer( QgsDataItem *parentItem, QString name,
OGRFeatureDefnH hDef = OGR_L_GetLayerDefn( hLayer );

QgsLayerItem::LayerType layerType = QgsLayerItem::Vector;
OGRwkbGeometryType ogrType = QgsOgrProvider::getOgrGeomType( hLayer );
GDALDriverH hDriver = GDALGetDatasetDriver( hDataSource );
QString driverName = QString::fromUtf8( GDALGetDriverShortName( hDriver ) );
OGRwkbGeometryType ogrType = QgsOgrProvider::getOgrGeomType( driverName, hLayer );
QgsWkbTypes::Type wkbType = QgsOgrProviderUtils::qgisTypeFromOgrType( ogrType );
switch ( QgsWkbTypes::geometryType( wkbType ) )
{
Expand Down
23 changes: 16 additions & 7 deletions src/providers/ogr/qgsogrprovider.cpp
View file
Expand Up @@ -967,7 +967,7 @@ void QgsOgrProvider::setEncoding( const QString &e )
}

// This is reused by dataItem
OGRwkbGeometryType QgsOgrProvider::getOgrGeomType( OGRLayerH ogrLayer )
OGRwkbGeometryType QgsOgrProvider::getOgrGeomType( const QString &driverName, OGRLayerH ogrLayer )
{
OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( ogrLayer );
OGRwkbGeometryType geomType = wkbUnknown;
Expand Down Expand Up @@ -998,6 +998,15 @@ OGRwkbGeometryType QgsOgrProvider::getOgrGeomType( OGRLayerH ogrLayer )
if ( geometry )
{
geomType = OGR_G_GetGeometryType( geometry );

// Shapefile MultiPatch can be reported as GeometryCollectionZ of TINZ
if ( wkbFlatten( geomType ) == wkbGeometryCollection &&
driverName == QLatin1String( "ESRI Shapefile" ) &&
OGR_G_GetGeometryCount( geometry ) >= 1 &&
wkbFlatten( OGR_G_GetGeometryType( OGR_G_GetGeometryRef( geometry, 0 ) ) ) == wkbTIN )
{
geomType = wkbMultiPolygon25D;
}
}
if ( geomType != wkbNone )
break;
Expand Down Expand Up @@ -1026,7 +1035,7 @@ void QgsOgrProvider::loadFields()
QMutex *mutex = nullptr;
OGRLayerH ogrLayer = mOgrLayer->getHandleAndMutex( mutex );
QMutexLocker locker( mutex );
mOGRGeomType = getOgrGeomType( ogrLayer );
mOGRGeomType = getOgrGeomType( mGDALDriverName, ogrLayer );
}
QgsOgrFeatureDefn &fdef = mOgrLayer->GetLayerDefn();

Expand Down Expand Up @@ -1395,18 +1404,18 @@ size_t QgsOgrProvider::layerCount() const
*/
QgsWkbTypes::Type QgsOgrProvider::wkbType() const
{
QgsWkbTypes::Type wkb = static_cast<QgsWkbTypes::Type>( mOGRGeomType );
QgsWkbTypes::Type wkb = QgsOgrUtils::ogrGeometryTypeToQgsWkbType( mOGRGeomType );
if ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) && ( wkb == QgsWkbTypes::LineString || wkb == QgsWkbTypes::Polygon ) )
{
wkb = QgsWkbTypes::multiType( wkb );
}
if ( wkb % 1000 == 15 ) // is PolyhedralSurface, PolyhedralSurfaceZ, PolyhedralSurfaceM or PolyhedralSurfaceZM => map to MultiPolygon
if ( mOGRGeomType % 1000 == wkbPolyhedralSurface ) // is PolyhedralSurface, PolyhedralSurfaceZ, PolyhedralSurfaceM or PolyhedralSurfaceZM => map to MultiPolygon
{
wkb = static_cast<QgsWkbTypes::Type>( wkb - 9 );
wkb = static_cast<QgsWkbTypes::Type>( mOGRGeomType - ( wkbPolyhedralSurface - wkbMultiPolygon ) );
}
else if ( wkb % 1000 == 16 ) // is TIN, TINZ, TINM or TINZM => map to MultiPolygon
else if ( mOGRGeomType % 1000 == wkbTIN ) // is TIN, TINZ, TINM or TINZM => map to MultiPolygon
{
wkb = static_cast<QgsWkbTypes::Type>( wkb - 10 );
wkb = static_cast<QgsWkbTypes::Type>( mOGRGeomType - ( wkbTIN - wkbMultiPolygon ) );
}
return wkb;
}
Expand Down
2 changes: 1 addition & 1 deletion src/providers/ogr/qgsogrprovider.h
View file
Expand Up @@ -152,7 +152,7 @@ class QgsOgrProvider : public QgsVectorDataProvider
bool doesStrictFeatureTypeCheck() const override;

//! Returns OGR geometry type
static OGRwkbGeometryType getOgrGeomType( OGRLayerH ogrLayer );
static OGRwkbGeometryType getOgrGeomType( const QString &driverName, OGRLayerH ogrLayer );

//! Gets single flatten geometry type
static OGRwkbGeometryType ogrWkbSingleFlatten( OGRwkbGeometryType type );
Expand Down
4 changes: 2 additions & 2 deletions tests/src/analysis/testqgsprocessing.cpp
View file
Expand Up @@ -1686,14 +1686,14 @@ void TestQgsProcessing::createFeatureSink()
// no extension, should default to shp
destination = QDir::tempPath() + "/create_feature_sink2";
prevDest = QDir::tempPath() + "/create_feature_sink2.gpkg";
sink.reset( QgsProcessingUtils::createFeatureSink( destination, context, fields, QgsWkbTypes::Point25D, QgsCoordinateReferenceSystem::fromEpsgId( 3111 ) ) );
sink.reset( QgsProcessingUtils::createFeatureSink( destination, context, fields, QgsWkbTypes::PointZ, QgsCoordinateReferenceSystem::fromEpsgId( 3111 ) ) );
QVERIFY( sink.get() );
f.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "PointZ(1 2 3)" ) ) );
QVERIFY( sink->addFeature( f ) );
QCOMPARE( destination, prevDest );
sink.reset( nullptr );
layer = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( destination, context, true ) );
QCOMPARE( layer->wkbType(), QgsWkbTypes::Point25D );
QCOMPARE( layer->wkbType(), QgsWkbTypes::PointZ );
QCOMPARE( layer->crs().authid(), QStringLiteral( "EPSG:3111" ) );
QCOMPARE( layer->fields().size(), 2 );
QCOMPARE( layer->fields().at( 0 ).name(), QStringLiteral( "fid" ) );
Expand Down
14 changes: 13 additions & 1 deletion tests/src/python/test_provider_shapefile.py
View file
Expand Up @@ -19,7 +19,7 @@
import osgeo.ogr
import sys

from qgis.core import QgsSettings, QgsFeature, QgsField, QgsGeometry, QgsVectorLayer, QgsFeatureRequest, QgsVectorDataProvider
from qgis.core import QgsSettings, QgsFeature, QgsField, QgsGeometry, QgsVectorLayer, QgsFeatureRequest, QgsVectorDataProvider, QgsWkbTypes
from qgis.PyQt.QtCore import QVariant
from qgis.testing import start_app, unittest
from utilities import unitTestDataPath
Expand Down Expand Up @@ -701,6 +701,18 @@ def _lessdigits(s):
self.assertEqual(_lessdigits(subSet_vl.extent().toString()), filtered_extent)
self.assertNotEqual(_lessdigits(subSet_vl.extent().toString()), unfiltered_extent)

def testMultipatch(self):
"""Check that we can deal with multipatch shapefiles, returned natively by OGR as GeometryCollection of TIN"""

testPath = TEST_DATA_DIR + '/' + 'multipatch.shp'
vl = QgsVectorLayer(testPath, 'test', 'ogr')
self.assertTrue(vl.isValid())
self.assertEqual(vl.wkbType(), QgsWkbTypes.MultiPolygonZ)
f = next(vl.getFeatures())
self.assertEqual(f.geometry().wkbType(), QgsWkbTypes.MultiPolygonZ)
self.assertEqual(f.geometry().constGet().asWkt(),
'MultiPolygonZ (((0 0 0, 0 1 0, 1 1 0, 0 0 0)),((0 0 0, 1 1 0, 1 0 0, 0 0 0)),((0 0 0, 0 -1 0, 1 -1 0, 0 0 0)),((0 0 0, 1 -1 0, 1 0 0, 0 0 0)))')


if __name__ == '__main__':
unittest.main()
Binary file added tests/testdata/multipatch.shp
View file
Binary file not shown.
Binary file added tests/testdata/multipatch.shx
View file
Binary file not shown.
Binary file modified tests/testdata/points_gpkg.gpkg
View file
Binary file not shown.

0 comments on commit 50dab59

Please sign in to comment.