Optimise conversion of geometry from OGR -> QGIS

Avoid conversion to/from WKB at OGR/QGIS side, and just directly
utilise OGR geometry API to construct QGIS geometries.

Shaves ~10% off rendering time for a large point layer (GPKG)

Author: nyalldawson

python/core/auto_generated/geometry/qgslinestring.sip.in

@@ -38,14 +38,20 @@ in the vector.
 
     QgsLineString( const QVector<double> &x, const QVector<double> &y,
                    const QVector<double> &z = QVector<double>(),
-                   const QVector<double> &m = QVector<double>() );
+                   const QVector<double> &m = QVector<double>(), bool is25DType = false );
 %Docstring
 Construct a linestring from arrays of coordinates. If the z or m
 arrays are non-empty then the resultant linestring will have
 z and m types accordingly.
 This constructor is more efficient then calling setPoints()
 or repeatedly calling addVertex()
 
+If the \z vector is filled, then the geometry type will either
+be a LineStringZ(M) or LineString25D depending on the ``is25DType``
+argument. If ``is25DType`` is true (and the ``m`` vector is unfilled) then
+the created Linestring will be a LineString25D type. Otherwise, the
+LineString will be LineStringZ (or LineStringZM) type.
+
 .. versionadded:: 3.0
 %End
 

src/core/geometry/qgslinestring.cpp

@@ -79,7 +79,7 @@ QgsLineString::QgsLineString( const QVector<QgsPoint> &points )
   }
 }
 
-QgsLineString::QgsLineString( const QVector<double> &x, const QVector<double> &y, const QVector<double> &z, const QVector<double> &m )
+QgsLineString::QgsLineString( const QVector<double> &x, const QVector<double> &y, const QVector<double> &z, const QVector<double> &m, bool is25DType )
 {
   mWkbType = QgsWkbTypes::LineString;
   int pointCount = std::min( x.size(), y.size() );
@@ -101,7 +101,7 @@ QgsLineString::QgsLineString( const QVector<double> &x, const QVector<double> &y
   }
   if ( !z.isEmpty() && z.count() >= pointCount )
   {
-    mWkbType = QgsWkbTypes::addZ( mWkbType );
+    mWkbType = is25DType ? QgsWkbTypes::LineString25D : QgsWkbTypes::LineStringZ;
     if ( z.size() == pointCount )
     {
       mZ = z;

src/core/geometry/qgslinestring.h

@@ -59,11 +59,18 @@ class CORE_EXPORT QgsLineString: public QgsCurve
      * z and m types accordingly.
      * This constructor is more efficient then calling setPoints()
      * or repeatedly calling addVertex()
+     *
+     * If the \z vector is filled, then the geometry type will either
+     * be a LineStringZ(M) or LineString25D depending on the \a is25DType
+     * argument. If \a is25DType is true (and the \a m vector is unfilled) then
+     * the created Linestring will be a LineString25D type. Otherwise, the
+     * LineString will be LineStringZ (or LineStringZM) type.
+     *
      * \since QGIS 3.0
      */
     QgsLineString( const QVector<double> &x, const QVector<double> &y,
                    const QVector<double> &z = QVector<double>(),
-                   const QVector<double> &m = QVector<double>() );
+                   const QVector<double> &m = QVector<double>(), bool is25DType = false );
 
     /**
      * Constructs a linestring with a single segment from \a p1 to \a p2.

src/core/qgsogrutils.cpp

@@ -18,6 +18,7 @@
 #include "qgslogger.h"
 #include "qgsgeometry.h"
 #include "qgsfields.h"
+#include "qgslinestring.h"
 #include <QTextCodec>
 #include <QUuid>
 #include <cpl_error.h>
@@ -269,11 +270,68 @@ bool QgsOgrUtils::readOgrFeatureGeometry( OGRFeatureH ogrFet, QgsFeature &featur
   return true;
 }
 
+std::unique_ptr< QgsPoint > ogrGeometryToQgsPoint( OGRGeometryH geom )
+{
+  QgsWkbTypes::Type wkbType = static_cast<QgsWkbTypes::Type>( OGR_G_GetGeometryType( geom ) );
+
+  double x, y, z, m;
+  OGR_G_GetPointZM( geom, 0, &x, &y, &z, &m );
+  return qgis::make_unique< QgsPoint >( wkbType, x, y, z, m );
+}
+
+std::unique_ptr< QgsLineString > ogrGeometryToQgsLineString( OGRGeometryH geom )
+{
+  QgsWkbTypes::Type wkbType = static_cast<QgsWkbTypes::Type>( OGR_G_GetGeometryType( geom ) );
+
+  int count = OGR_G_GetPointCount( geom );
+  QVector< double > x( count );
+  QVector< double > y( count );
+  QVector< double > z;
+  double *pz = nullptr;
+  if ( QgsWkbTypes::hasZ( wkbType ) )
+  {
+    z.resize( count );
+    pz = z.data();
+  }
+  double *pm = nullptr;
+  QVector< double > m;
+  if ( QgsWkbTypes::hasM( wkbType ) )
+  {
+    m.resize( count );
+    pm = m.data();
+  }
+  OGR_G_GetPointsZM( geom, x.data(), sizeof( double ), y.data(), sizeof( double ), pz, sizeof( double ), pm, sizeof( double ) );
+
+  return qgis::make_unique< QgsLineString>( x, y, z, m, wkbType == QgsWkbTypes::LineString25D );
+}
+
 QgsGeometry QgsOgrUtils::ogrGeometryToQgsGeometry( OGRGeometryH geom )
 {
   if ( !geom )
     return QgsGeometry();
 
+  QgsWkbTypes::Type wkbType = static_cast<QgsWkbTypes::Type>( OGR_G_GetGeometryType( geom ) );
+  // optimised case for some geometry classes, avoiding wkb conversion on OGR/QGIS sides
+  // TODO - extend to other classes!
+  switch ( QgsWkbTypes::flatType( wkbType ) )
+  {
+    case QgsWkbTypes::Point:
+    {
+      return QgsGeometry( ogrGeometryToQgsPoint( geom ) );
+    }
+
+    case QgsWkbTypes::LineString:
+    {
+      // optimised case for line -- avoid wkb conversion
+      return QgsGeometry( ogrGeometryToQgsLineString( geom ) );
+    }
+
+    default:
+      break;
+  };
+
+  // Fallback to inefficient WKB conversions
+
   // get the wkb representation
   int memorySize = OGR_G_WkbSize( geom );
   unsigned char *wkb = new unsigned char[memorySize];

tests/src/core/testqgsgeometry.cpp

@@ -2853,6 +2853,19 @@ void TestQgsGeometry::lineString()
   QCOMPARE( fromArray4.xAt( 2 ), 3.0 );
   QCOMPARE( fromArray4.yAt( 2 ), 13.0 );
   QCOMPARE( fromArray4.zAt( 2 ), 23.0 );
+  fromArray4 = QgsLineString( xx, yy, zz, QVector< double >(), true );  // LineString25D
+  QCOMPARE( fromArray4.wkbType(), QgsWkbTypes::LineString25D );
+  QCOMPARE( fromArray4.numPoints(), 3 );
+  QCOMPARE( fromArray4.xAt( 0 ), 1.0 );
+  QCOMPARE( fromArray4.yAt( 0 ), 11.0 );
+  QCOMPARE( fromArray4.zAt( 0 ), 21.0 );
+  QCOMPARE( fromArray4.xAt( 1 ), 2.0 );
+  QCOMPARE( fromArray4.yAt( 1 ), 12.0 );
+  QCOMPARE( fromArray4.zAt( 1 ), 22.0 );
+  QCOMPARE( fromArray4.xAt( 2 ), 3.0 );
+  QCOMPARE( fromArray4.yAt( 2 ), 13.0 );
+  QCOMPARE( fromArray4.zAt( 2 ), 23.0 );
+
   // unbalanced -> z ignored
   zz = QVector< double >() << 21 << 22;
   QgsLineString fromArray5( xx, yy, zz );

tests/src/core/testqgsogrutils.cpp

@@ -38,6 +38,8 @@ class TestQgsOgrUtils: public QObject
     void init();// will be called before each testfunction is executed.
     void cleanup();// will be called after every testfunction.
     void ogrGeometryToQgsGeometry();
+    void ogrGeometryToQgsGeometry2_data();
+    void ogrGeometryToQgsGeometry2();
     void readOgrFeatureGeometry();
     void getOgrFeatureAttribute();
     void readOgrFeatureAttributes();
@@ -102,6 +104,55 @@ void TestQgsOgrUtils::ogrGeometryToQgsGeometry()
 
   OGR_F_Destroy( oFeat );
   OGR_DS_Destroy( hDS );
+
+  ogrGeom = nullptr;
+  QByteArray wkt( "point( 1.1 2.2)" );
+  char *wktChar = wkt.data();
+  OGR_G_CreateFromWkt( &wktChar, nullptr, &ogrGeom );
+  geom = QgsOgrUtils::ogrGeometryToQgsGeometry( ogrGeom );
+  QCOMPARE( geom.asWkt( 3 ), QStringLiteral( "Point (1.1 2.2)" ) );
+
+}
+
+void TestQgsOgrUtils::ogrGeometryToQgsGeometry2_data()
+{
+  QTest::addColumn<QString>( "wkt" );
+  QTest::addColumn<int>( "type" );
+
+  QTest::newRow( "point" ) << QStringLiteral( "Point (1.1 2.2)" ) << static_cast< int >( QgsWkbTypes::Point );
+  QTest::newRow( "pointz" ) << QStringLiteral( "PointZ (1.1 2.2 3.3)" ) <<  static_cast< int >( QgsWkbTypes::Point25D ); // ogr uses 25d for z
+  QTest::newRow( "pointm" ) << QStringLiteral( "PointM (1.1 2.2 3.3)" ) <<  static_cast< int >( QgsWkbTypes::PointM );
+  QTest::newRow( "pointzm" ) << QStringLiteral( "PointZM (1.1 2.2 3.3 4.4)" ) <<  static_cast< int >( QgsWkbTypes::PointZM );
+  QTest::newRow( "point25d" ) << QStringLiteral( "Point25D (1.1 2.2 3.3)" ) <<  static_cast< int >( QgsWkbTypes::Point25D );
+
+  QTest::newRow( "linestring" ) << QStringLiteral( "LineString (1.1 2.2, 3.3 4.4)" ) << static_cast< int >( QgsWkbTypes::LineString );
+  QTest::newRow( "linestringz" ) << QStringLiteral( "LineStringZ (1.1 2.2 3.3, 4.4 5.5 6.6)" ) <<  static_cast< int >( QgsWkbTypes::LineString25D ); // ogr uses 25d for z
+  QTest::newRow( "linestringm" ) << QStringLiteral( "LineStringM (1.1 2.2 3.3, 4.4 5.5 6.6)" ) <<  static_cast< int >( QgsWkbTypes::LineStringM );
+  QTest::newRow( "linestringzm" ) << QStringLiteral( "LineStringZM (1.1 2.2 3.3 4.4, 5.5 6.6 7.7 8.8)" ) <<  static_cast< int >( QgsWkbTypes::LineStringZM );
+  QTest::newRow( "linestring25d" ) << QStringLiteral( "LineString25D (1.1 2.2 3.3, 4.4 5.5 6.6)" ) <<  static_cast< int >( QgsWkbTypes::LineString25D );
+}
+
+void TestQgsOgrUtils::ogrGeometryToQgsGeometry2()
+{
+  QFETCH( QString, wkt );
+  QFETCH( int, type );
+
+  QgsGeometry input = QgsGeometry::fromWkt( wkt );
+  QVERIFY( !input.isNull() );
+
+  // to OGR Geometry
+  QByteArray wkb( input.asWkb() );
+  OGRGeometryH ogrGeom = nullptr;
+
+  QCOMPARE( OGR_G_CreateFromWkb( reinterpret_cast<unsigned char *>( const_cast<char *>( wkb.constData() ) ), nullptr, &ogrGeom, wkb.length() ), OGRERR_NONE );
+
+  // back again!
+  QgsGeometry geom = QgsOgrUtils::ogrGeometryToQgsGeometry( ogrGeom );
+  QCOMPARE( static_cast< int >( geom.wkbType() ), type );
+
+  // bit of trickiness here - QGIS wkt conversion changes 25D -> Z, so account for that
+  wkt.replace( QStringLiteral( "25D" ), QStringLiteral( "Z" ) );
+  QCOMPARE( geom.asWkt( 3 ), wkt );
 }
 
 void TestQgsOgrUtils::readOgrFeatureGeometry()