Skip to content

Commit

Permalink
Better geometry memory management for afs provider
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Feb 20, 2018
1 parent 8b0b826 commit 1e95433
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 57 deletions.
5 changes: 2 additions & 3 deletions src/providers/arcgisrest/qgsafsfeatureiterator.cpp
View file
Expand Up @@ -70,7 +70,6 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
if ( mFeatureIterator >= mSource->sharedData()->featureCount() )
return false;

bool fetchGeometries = ( mRequest.flags() & QgsFeatureRequest::NoGeometry ) == 0;
QgsAttributeList fetchAttribures;
if ( ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) != 0 )
fetchAttribures = mRequest.subsetOfAttributes();
Expand All @@ -82,7 +81,7 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )

if ( mRequest.filterType() == QgsFeatureRequest::FilterFid )
{
bool result = mSource->sharedData()->getFeature( mRequest.filterFid(), f, fetchGeometries, fetchAttribures );
bool result = mSource->sharedData()->getFeature( mRequest.filterFid(), f, fetchAttribures );
geometryToDestinationCrs( f, mTransform );
f.setValid( result );
return result;
Expand All @@ -94,7 +93,7 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
filterRect = filterRect.intersect( &mFilterRect );
while ( mFeatureIterator < mSource->sharedData()->featureCount() )
{
bool success = mSource->sharedData()->getFeature( mFeatureIterator, f, fetchGeometries, fetchAttribures, filterRect );
bool success = mSource->sharedData()->getFeature( mFeatureIterator, f, fetchAttribures, filterRect );
++mFeatureIterator;
if ( !success )
continue;
Expand Down
19 changes: 8 additions & 11 deletions src/providers/arcgisrest/qgsafsshareddata.cpp
View file
Expand Up @@ -23,7 +23,7 @@ void QgsAfsSharedData::clearCache()
mCache.clear();
}

bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, bool fetchGeometry, const QList<int> & /*fetchAttributes*/, const QgsRectangle &filterRect )
bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QList<int> & /*fetchAttributes*/, const QgsRectangle &filterRect )
{
QMutexLocker locker( &mMutex );

Expand All @@ -44,7 +44,6 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, bool fetchGeo
fetchAttribNames.append( mFields.at( idx ).name() );
fetchAttribIdx.append( idx );
}
fetchGeometry = true;

// Fetch 100 features at the time
int startId = ( id / 100 ) * 100;
Expand All @@ -60,7 +59,7 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, bool fetchGeo
// Query
QString errorTitle, errorMessage;
const QVariantMap queryData = QgsArcGisRestUtils::getObjects(
mDataSource.param( QStringLiteral( "url" ) ), objectIds, mDataSource.param( QStringLiteral( "crs" ) ), fetchGeometry,
mDataSource.param( QStringLiteral( "url" ) ), objectIds, mDataSource.param( QStringLiteral( "crs" ) ), true,
fetchAttribNames, QgsWkbTypes::hasM( mGeometryType ), QgsWkbTypes::hasZ( mGeometryType ),
filterRect, errorTitle, errorMessage );
if ( queryData.isEmpty() )
Expand Down Expand Up @@ -105,14 +104,12 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, bool fetchGeo
feature.setId( featureId );

// Set geometry
if ( fetchGeometry )
{
const QVariantMap geometryData = featureData[QStringLiteral( "geometry" )].toMap();
QgsAbstractGeometry *geometry = QgsArcGisRestUtils::parseEsriGeoJSON( geometryData, queryData[QStringLiteral( "geometryType" )].toString(),
QgsWkbTypes::hasM( mGeometryType ), QgsWkbTypes::hasZ( mGeometryType ) );
// Above might return 0, which is OK since in theory empty geometries are allowed
feature.setGeometry( QgsGeometry( geometry ) );
}
const QVariantMap geometryData = featureData[QStringLiteral( "geometry" )].toMap();
std::unique_ptr< QgsAbstractGeometry > geometry = QgsArcGisRestUtils::parseEsriGeoJSON( geometryData, queryData[QStringLiteral( "geometryType" )].toString(),
QgsWkbTypes::hasM( mGeometryType ), QgsWkbTypes::hasZ( mGeometryType ) );
// Above might return 0, which is OK since in theory empty geometries are allowed
if ( geometry )
feature.setGeometry( QgsGeometry( std::move( geometry ) ) );
feature.setValid( true );
mCache.insert( feature.id(), feature );
}
Expand Down
2 changes: 1 addition & 1 deletion src/providers/arcgisrest/qgsafsshareddata.h
View file
Expand Up @@ -36,7 +36,7 @@ class QgsAfsSharedData : public QObject
QgsCoordinateReferenceSystem crs() const { return mSourceCRS; }
void clearCache();

bool getFeature( QgsFeatureId id, QgsFeature &f, bool fetchGeometry, const QList<int> &fetchAttributes, const QgsRectangle &filterRect = QgsRectangle() );
bool getFeature( QgsFeatureId id, QgsFeature &f, const QList<int> &fetchAttributes, const QgsRectangle &filterRect = QgsRectangle() );

private:
friend class QgsAfsProvider;
Expand Down
4 changes: 2 additions & 2 deletions src/providers/arcgisrest/qgsamsprovider.cpp
View file
Expand Up @@ -427,9 +427,9 @@ QgsRasterIdentifyResult QgsAmsProvider::identify( const QgsPointXY &point, QgsRa
featureAttributes.append( it.value().toString() );
}
QgsCoordinateReferenceSystem crs;
QgsAbstractGeometry *geometry = QgsArcGisRestUtils::parseEsriGeoJSON( resultMap[QStringLiteral( "geometry" )].toMap(), resultMap[QStringLiteral( "geometryType" )].toString(), false, false, &crs );
std::unique_ptr< QgsAbstractGeometry > geometry = QgsArcGisRestUtils::parseEsriGeoJSON( resultMap[QStringLiteral( "geometry" )].toMap(), resultMap[QStringLiteral( "geometryType" )].toString(), false, false, &crs );
QgsFeature feature( fields );
feature.setGeometry( QgsGeometry( geometry ) );
feature.setGeometry( QgsGeometry( std::move( geometry ) ) );
feature.setAttributes( featureAttributes );
feature.setValid( true );
QgsFeatureStore store( fields, crs );
Expand Down
70 changes: 31 additions & 39 deletions src/providers/arcgisrest/qgsarcgisrestutils.cpp
View file
Expand Up @@ -100,7 +100,7 @@ QgsWkbTypes::Type QgsArcGisRestUtils::mapEsriGeometryType( const QString &esriGe
return QgsWkbTypes::Unknown;
}

static QgsPoint *parsePoint( const QVariantList &coordList, QgsWkbTypes::Type pointType )
static std::unique_ptr< QgsPoint > parsePoint( const QVariantList &coordList, QgsWkbTypes::Type pointType )
{
int nCoords = coordList.size();
if ( nCoords < 2 )
Expand All @@ -112,10 +112,10 @@ static QgsPoint *parsePoint( const QVariantList &coordList, QgsWkbTypes::Type po
return nullptr;
double z = nCoords >= 3 ? coordList[2].toDouble() : 0;
double m = nCoords >= 4 ? coordList[3].toDouble() : 0;
return new QgsPoint( pointType, x, y, z, m );
return qgis::make_unique< QgsPoint >( pointType, x, y, z, m );
}

static QgsCircularString *parseCircularString( const QVariantMap &curveData, QgsWkbTypes::Type pointType, const QgsPoint &startPoint )
static std::unique_ptr< QgsCircularString > parseCircularString( const QVariantMap &curveData, QgsWkbTypes::Type pointType, const QgsPoint &startPoint )
{
QVariantList coordsList = curveData[QStringLiteral( "c" )].toList();
if ( coordsList.isEmpty() )
Expand All @@ -124,53 +124,49 @@ static QgsCircularString *parseCircularString( const QVariantMap &curveData, Qgs
points.append( startPoint );
foreach ( const QVariant &coordData, coordsList )
{
QgsPoint *point = parsePoint( coordData.toList(), pointType );
std::unique_ptr< QgsPoint > point = parsePoint( coordData.toList(), pointType );
if ( !point )
{
return nullptr;
}
points.append( *point );
delete point;
}
QgsCircularString *curve = new QgsCircularString();
std::unique_ptr< QgsCircularString > curve = qgis::make_unique< QgsCircularString> ();
curve->setPoints( points );
return curve;
}

static QgsCompoundCurve *parseCompoundCurve( const QVariantList &curvesList, QgsWkbTypes::Type pointType )
static std::unique_ptr< QgsCompoundCurve > parseCompoundCurve( const QVariantList &curvesList, QgsWkbTypes::Type pointType )
{
// [[6,3],[5,3],{"b":[[3,2],[6,1],[2,4]]},[1,2],{"c": [[3,3],[1,4]]}]
QgsCompoundCurve *compoundCurve = new QgsCompoundCurve();
std::unique_ptr< QgsCompoundCurve > compoundCurve = qgis::make_unique< QgsCompoundCurve >();
QgsLineString *lineString = new QgsLineString();
compoundCurve->addCurve( lineString );
foreach ( const QVariant &curveData, curvesList )
{
if ( curveData.type() == QVariant::List )
{
QgsPoint *point = parsePoint( curveData.toList(), pointType );
std::unique_ptr< QgsPoint > point = parsePoint( curveData.toList(), pointType );
if ( !point )
{
delete compoundCurve;
return nullptr;
}
lineString->addVertex( *point );
delete point;
}
else if ( curveData.type() == QVariant::Map )
{
// The last point of the linestring is the start point of this circular string
QgsCircularString *circularString = parseCircularString( curveData.toMap(), pointType, lineString->endPoint() );
std::unique_ptr< QgsCircularString > circularString = parseCircularString( curveData.toMap(), pointType, lineString->endPoint() );
if ( !circularString )
{
delete compoundCurve;
return nullptr;
}

// If the previous curve had less than two points, remove it
if ( compoundCurve->curveAt( compoundCurve->nCurves() - 1 )->nCoordinates() < 2 )
compoundCurve->removeCurve( compoundCurve->nCurves() - 1 );

compoundCurve->addCurve( circularString );
compoundCurve->addCurve( circularString.release() );

// Prepare a new line string
lineString = new QgsLineString;
Expand All @@ -181,7 +177,7 @@ static QgsCompoundCurve *parseCompoundCurve( const QVariantList &curvesList, Qgs
return compoundCurve;
}

static QgsAbstractGeometry *parseEsriGeometryPoint( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
static std::unique_ptr< QgsPoint > parseEsriGeometryPoint( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
{
// {"x" : <x>, "y" : <y>, "z" : <z>, "m" : <m>}
bool xok = false, yok = false;
Expand All @@ -191,32 +187,31 @@ static QgsAbstractGeometry *parseEsriGeometryPoint( const QVariantMap &geometryD
return nullptr;
double z = geometryData[QStringLiteral( "z" )].toDouble();
double m = geometryData[QStringLiteral( "m" )].toDouble();
return new QgsPoint( pointType, x, y, z, m );
return qgis::make_unique< QgsPoint >( pointType, x, y, z, m );
}

static QgsAbstractGeometry *parseEsriGeometryMultiPoint( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
static std::unique_ptr< QgsMultiPoint > parseEsriGeometryMultiPoint( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
{
// {"points" : [[ <x1>, <y1>, <z1>, <m1> ] , [ <x2>, <y2>, <z2>, <m2> ], ... ]}
QVariantList coordsList = geometryData[QStringLiteral( "points" )].toList();
if ( coordsList.isEmpty() )
return nullptr;

QgsMultiPoint *multiPoint = new QgsMultiPoint();
std::unique_ptr< QgsMultiPoint > multiPoint = qgis::make_unique< QgsMultiPoint >();
Q_FOREACH ( const QVariant &coordData, coordsList )
{
QVariantList coordList = coordData.toList();
QgsPoint *p = parsePoint( coordList, pointType );
std::unique_ptr< QgsPoint > p = parsePoint( coordList, pointType );
if ( !p )
{
delete multiPoint;
return nullptr;
}
multiPoint->addGeometry( p );
multiPoint->addGeometry( p.release() );
}
return multiPoint;
}

static QgsAbstractGeometry *parseEsriGeometryPolyline( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
static std::unique_ptr< QgsMultiCurve > parseEsriGeometryPolyline( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
{
// {"curvePaths": [[[0,0], {"c": [[3,3],[1,4]]} ]]}
QVariantList pathsList;
Expand All @@ -226,21 +221,20 @@ static QgsAbstractGeometry *parseEsriGeometryPolyline( const QVariantMap &geomet
pathsList = geometryData[QStringLiteral( "curvePaths" )].toList();
if ( pathsList.isEmpty() )
return nullptr;
QgsMultiCurve *multiCurve = new QgsMultiCurve();
std::unique_ptr< QgsMultiCurve > multiCurve = qgis::make_unique< QgsMultiCurve >();
foreach ( const QVariant &pathData, pathsList )
{
QgsCompoundCurve *curve = parseCompoundCurve( pathData.toList(), pointType );
std::unique_ptr< QgsCompoundCurve > curve = parseCompoundCurve( pathData.toList(), pointType );
if ( !curve )
{
delete multiCurve;
return nullptr;
}
multiCurve->addGeometry( curve );
multiCurve->addGeometry( curve.release() );
}
return multiCurve;
}

static QgsAbstractGeometry *parseEsriGeometryPolygon( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
static std::unique_ptr< QgsCurvePolygon > parseEsriGeometryPolygon( const QVariantMap &geometryData, QgsWkbTypes::Type pointType )
{
// {"curveRings": [[[0,0], {"c": [[3,3],[1,4]]} ]]}
QVariantList ringsList;
Expand All @@ -250,28 +244,26 @@ static QgsAbstractGeometry *parseEsriGeometryPolygon( const QVariantMap &geometr
ringsList = geometryData[QStringLiteral( "ringPaths" )].toList();
if ( ringsList.isEmpty() )
return nullptr;
QgsCurvePolygon *polygon = new QgsCurvePolygon();
QgsCompoundCurve *ext = parseCompoundCurve( ringsList.front().toList(), pointType );
std::unique_ptr< QgsCurvePolygon > polygon = qgis::make_unique< QgsCurvePolygon >();
std::unique_ptr< QgsCompoundCurve > ext = parseCompoundCurve( ringsList.front().toList(), pointType );
if ( !ext )
{
delete polygon;
return nullptr;
}
polygon->setExteriorRing( ext );
polygon->setExteriorRing( ext.release() );
for ( int i = 1, n = ringsList.size(); i < n; ++i )
{
QgsCompoundCurve *curve = parseCompoundCurve( ringsList[i].toList(), pointType );
std::unique_ptr< QgsCompoundCurve > curve = parseCompoundCurve( ringsList[i].toList(), pointType );
if ( !curve )
{
delete polygon;
return nullptr;
}
polygon->addInteriorRing( curve );
polygon->addInteriorRing( curve.release() );
}
return polygon;
}

static QgsAbstractGeometry *parseEsriEnvelope( const QVariantMap &geometryData )
static std::unique_ptr< QgsPolygon > parseEsriEnvelope( const QVariantMap &geometryData )
{
// {"xmin" : -109.55, "ymin" : 25.76, "xmax" : -86.39, "ymax" : 49.94}
bool xminOk = false, yminOk = false, xmaxOk = false, ymaxOk = false;
Expand All @@ -281,18 +273,18 @@ static QgsAbstractGeometry *parseEsriEnvelope( const QVariantMap &geometryData )
double ymax = geometryData[QStringLiteral( "ymax" )].toDouble( &ymaxOk );
if ( !xminOk || !yminOk || !xmaxOk || !ymaxOk )
return nullptr;
QgsLineString *ext = new QgsLineString();
std::unique_ptr< QgsLineString > ext = qgis::make_unique< QgsLineString> ();
ext->addVertex( QgsPoint( xmin, ymin ) );
ext->addVertex( QgsPoint( xmax, ymin ) );
ext->addVertex( QgsPoint( xmax, ymax ) );
ext->addVertex( QgsPoint( xmin, ymax ) );
ext->addVertex( QgsPoint( xmin, ymin ) );
QgsPolygon *poly = new QgsPolygon();
poly->setExteriorRing( ext );
std::unique_ptr< QgsPolygon > poly = qgis::make_unique< QgsPolygon >();
poly->setExteriorRing( ext.release() );
return poly;
}

QgsAbstractGeometry *QgsArcGisRestUtils::parseEsriGeoJSON( const QVariantMap &geometryData, const QString &esriGeometryType, bool readM, bool readZ, QgsCoordinateReferenceSystem *crs )
std::unique_ptr<QgsAbstractGeometry> QgsArcGisRestUtils::parseEsriGeoJSON( const QVariantMap &geometryData, const QString &esriGeometryType, bool readM, bool readZ, QgsCoordinateReferenceSystem *crs )
{
QgsWkbTypes::Type pointType = QgsWkbTypes::zmType( QgsWkbTypes::Point, readZ, readM );
if ( crs )
Expand Down
2 changes: 1 addition & 1 deletion src/providers/arcgisrest/qgsarcgisrestutils.h
View file
Expand Up @@ -31,7 +31,7 @@ class QgsArcGisRestUtils
public:
static QVariant::Type mapEsriFieldType( const QString &esriFieldType );
static QgsWkbTypes::Type mapEsriGeometryType( const QString &esriGeometryType );
static QgsAbstractGeometry *parseEsriGeoJSON( const QVariantMap &geometryData, const QString &esriGeometryType, bool readM, bool readZ, QgsCoordinateReferenceSystem *crs = nullptr );
static std::unique_ptr< QgsAbstractGeometry > parseEsriGeoJSON( const QVariantMap &geometryData, const QString &esriGeometryType, bool readM, bool readZ, QgsCoordinateReferenceSystem *crs = nullptr );
static QgsCoordinateReferenceSystem parseSpatialReference( const QVariantMap &spatialReferenceMap );

static QVariantMap getServiceInfo( const QString &baseurl, QString &errorTitle, QString &errorText );
Expand Down

0 comments on commit 1e95433

Please sign in to comment.