Skip to content

Commit

Permalink
Remove caching of GEOS representation within QgsGeometry (legacy)
Browse files Browse the repository at this point in the history
  • Loading branch information
wonder-sk committed Nov 14, 2016
1 parent d729951 commit 9872b48
Show file tree
Hide file tree
Showing 15 changed files with 89 additions and 74 deletions.
1 change: 1 addition & 0 deletions doc/api_break.dox
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ value instead of a pointer. The biggest impact with this change is that PyQGIS c
result to None, but instead either use a boolean test (`if g.buffer(10):`) or explicitly use the isEmpty()
method to determine if a geometry is valid.
- wkbSize() and asWkb() has been replaced by exportToWkb(). WKB representation is no longer cached within QgsGeometry
- asGeos() has been replaced by exportToGeos(). GEOS representation is no longer cached within QgsGeometry
- int addPart( const QList<QgsPoint> &points, QgsWkbTypes::GeometryType geomType ) has been renamed to addPoints
- int addPart( const QList<QgsPointV2> &points, QgsWkbTypes::GeometryType geomType ) has been renamed to addPointsV2
- static bool compare( const QgsPolyline& p1, const QgsPolyline& p2, double epsilon ) has been renamed to comparePolylines
Expand Down
6 changes: 3 additions & 3 deletions python/core/geometry/qgsgeometry.sip
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ class QgsGeometry
*/
void fromWkb( const QByteArray& wkb );

/** Returns a geos geometry. QgsGeometry retains ownership of the geometry, so the returned object should not be deleted.
/** Returns a geos geometry - caller takes ownership of the object (should be deleted with GEOSGeom_destroy_r)
* @param precision The precision of the grid to which to snap the geometry vertices. If 0, no snapping is performed.
* @note this method was added in version 1.1
* @note added in version 3.0
* @note not available in python bindings
*/
// const GEOSGeometry* asGeos( double precision = 0 ) const;
// GEOSGeometry* exportToGeos( double precision = 0 ) const;

/** Returns type of the geometry as a WKB type (point / linestring / polygon etc.)
* @see type
Expand Down
4 changes: 3 additions & 1 deletion src/analysis/vector/qgsgeometryanalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,9 @@ QgsGeometry QgsGeometryAnalyzer::createOffsetGeometry( const QgsGeometry& geom,
{
if ( geom.type() == QgsWkbTypes::LineGeometry )
{
GEOSGeometry* offsetGeom = GEOSOffsetCurve_r( geosctxt, ( *inputGeomIt ).asGeos(), -offset, 8 /*quadSegments*/, 0 /*joinStyle*/, 5.0 /*mitreLimit*/ );
GEOSGeometry* inputGeomItGeos = inputGeomIt->exportToGeos();
GEOSGeometry* offsetGeom = GEOSOffsetCurve_r( geosctxt, inputGeomItGeos, -offset, 8 /*quadSegments*/, 0 /*joinStyle*/, 5.0 /*mitreLimit*/ );
GEOSGeom_destroy_r( geosctxt, inputGeomItGeos );
if ( !offsetGeom || !GEOSisValid_r( geosctxt, offsetGeom ) )
{
return QgsGeometry();
Expand Down
6 changes: 4 additions & 2 deletions src/analysis/vector/qgszonalstatistics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -419,16 +419,17 @@ void QgsZonalStatistics::statisticsFromMiddlePointTest( void* band, const QgsGeo
cellCenterY = rasterBBox.yMaximum() - pixelOffsetY * cellSizeY - cellSizeY / 2;
stats.reset();

const GEOSGeometry* polyGeos = poly.asGeos();
GEOSGeometry* polyGeos = poly.exportToGeos();
if ( !polyGeos )
{
return;
}

GEOSContextHandle_t geosctxt = QgsGeometry::getGEOSHandler();
const GEOSPreparedGeometry* polyGeosPrepared = GEOSPrepare_r( geosctxt, poly.asGeos() );
const GEOSPreparedGeometry* polyGeosPrepared = GEOSPrepare_r( geosctxt, polyGeos );
if ( !polyGeosPrepared )
{
GEOSGeom_destroy_r( geosctxt, polyGeos );
return;
}

Expand Down Expand Up @@ -464,6 +465,7 @@ void QgsZonalStatistics::statisticsFromMiddlePointTest( void* band, const QgsGeo
GEOSGeom_destroy_r( geosctxt, currentCellCenter );
CPLFree( scanLine );
GEOSPreparedGeom_destroy_r( geosctxt, polyGeosPrepared );
GEOSGeom_destroy_r( geosctxt, polyGeos );
}

void QgsZonalStatistics::statisticsFromPreciseIntersection( void* band, const QgsGeometry& poly, int pixelOffsetX,
Expand Down
3 changes: 2 additions & 1 deletion src/app/qgsmaptooloffsetcurve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ void QgsMapToolOffsetCurve::setOffsetForRubberBand( double offset )
}

QgsGeometry geomCopy( mOriginalGeometry );
const GEOSGeometry* geosGeom = geomCopy.asGeos();
GEOSGeometry* geosGeom = geomCopy.exportToGeos();
if ( geosGeom )
{
QSettings s;
Expand All @@ -377,6 +377,7 @@ void QgsMapToolOffsetCurve::setOffsetForRubberBand( double offset )
double mitreLimit = s.value( QStringLiteral( "/qgis/digitizing/offset_miter_limit" ), 5.0 ).toDouble();

GEOSGeometry* offsetGeom = GEOSOffsetCurve_r( QgsGeometry::getGEOSHandler(), geosGeom, offset, quadSegments, joinStyle, mitreLimit );
GEOSGeom_destroy_r( QgsGeometry::getGEOSHandler(), geosGeom );
if ( !offsetGeom )
{
deleteRubberBandAndGeometry();
Expand Down
15 changes: 5 additions & 10 deletions src/core/geometry/qgsgeometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,10 @@ email : morb at ozemail dot com dot au

struct QgsGeometryPrivate
{
QgsGeometryPrivate(): ref( 1 ), geometry( nullptr ), mGeos( nullptr ) {}
~QgsGeometryPrivate() { delete geometry; GEOSGeom_destroy_r( QgsGeos::getGEOSHandler(), mGeos ); }
QgsGeometryPrivate(): ref( 1 ), geometry( nullptr ) {}
~QgsGeometryPrivate() { delete geometry; }
QAtomicInt ref;
QgsAbstractGeometry* geometry;
mutable GEOSGeometry* mGeos;
};

QgsGeometry::QgsGeometry(): d( new QgsGeometryPrivate() )
Expand Down Expand Up @@ -269,18 +268,14 @@ void QgsGeometry::fromWkb( const QByteArray &wkb )
d->geometry = QgsGeometryFactory::geomFromWkb( ptr );
}

const GEOSGeometry* QgsGeometry::asGeos( double precision ) const
GEOSGeometry* QgsGeometry::exportToGeos( double precision ) const
{
if ( !d->geometry )
{
return nullptr;
}

if ( !d->mGeos )
{
d->mGeos = QgsGeos::asGeos( d->geometry, precision );
}
return d->mGeos;
return QgsGeos::asGeos( d->geometry, precision );
}


Expand Down Expand Up @@ -320,7 +315,7 @@ void QgsGeometry::fromGeos( GEOSGeometry *geos )
detach( false );
delete d->geometry;
d->geometry = QgsGeos::fromGeos( geos );
d->mGeos = geos;
GEOSGeom_destroy_r( QgsGeos::getGEOSHandler(), geos );
}

QgsPoint QgsGeometry::closestVertex( const QgsPoint& point, int& atVertex, int& beforeVertex, int& afterVertex, double& sqrDist ) const
Expand Down
6 changes: 3 additions & 3 deletions src/core/geometry/qgsgeometry.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,12 @@ class CORE_EXPORT QgsGeometry
*/
void fromWkb( const QByteArray& wkb );

/** Returns a geos geometry. QgsGeometry retains ownership of the geometry, so the returned object should not be deleted.
/** Returns a geos geometry - caller takes ownership of the object (should be deleted with GEOSGeom_destroy_r)
* @param precision The precision of the grid to which to snap the geometry vertices. If 0, no snapping is performed.
* @note this method was added in version 1.1
* @note added in 3.0
* @note not available in python bindings
*/
const GEOSGeometry* asGeos( double precision = 0 ) const;
GEOSGeometry* exportToGeos( double precision = 0 ) const;

/** Returns type of the geometry as a WKB type (point / linestring / polygon etc.)
* @see type
Expand Down
3 changes: 3 additions & 0 deletions src/core/geometry/qgswkbptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class CORE_EXPORT QgsWkbPtr
}

public:
//! Construct WKB pointer from QByteArray
QgsWkbPtr( QByteArray& wkb );
QgsWkbPtr( unsigned char *p, int size );

Expand All @@ -82,6 +83,7 @@ class CORE_EXPORT QgsWkbPtr
inline QgsWkbPtr &operator<<( const unsigned int &v ) { write( v ); return *this; }
inline QgsWkbPtr &operator<<( const char &v ) { write( v ); return *this; }
inline QgsWkbPtr &operator<<( const QgsWkbTypes::Type &v ) { write( v ); return *this; }
//! Append data from a byte array
inline QgsWkbPtr &operator<<( const QByteArray &data ) { write( data ); return *this; }

inline void operator+=( int n ) { verifyBound( n ); mP += n; }
Expand Down Expand Up @@ -119,6 +121,7 @@ class CORE_EXPORT QgsConstWkbPtr
}

public:
//! Construct WKB pointer from QByteArray
explicit QgsConstWkbPtr( const QByteArray& wkb );
QgsConstWkbPtr( const unsigned char *p, int size );
QgsWkbTypes::Type readHeader() const;
Expand Down
6 changes: 4 additions & 2 deletions src/core/qgsgeometryvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ void QgsGeometryValidator::run()
if ( settings.value( QStringLiteral( "/qgis/digitizing/validate_geometries" ), 1 ).toInt() == 2 )
{
char *r = nullptr;
const GEOSGeometry *g0 = mG.asGeos();
GEOSGeometry *g0 = mG.exportToGeos();
GEOSContextHandle_t handle = QgsGeometry::getGEOSHandler();
if ( !g0 )
{
Expand All @@ -229,7 +229,9 @@ void QgsGeometryValidator::run()
else
{
GEOSGeometry *g1 = nullptr;
if ( GEOSisValidDetail_r( handle, g0, GEOSVALID_ALLOW_SELFTOUCHING_RING_FORMING_HOLE, &r, &g1 ) != 1 )
char res = GEOSisValidDetail_r( handle, g0, GEOSVALID_ALLOW_SELFTOUCHING_RING_FORMING_HOLE, &r, &g1 );
GEOSGeom_destroy_r( handle, g0 );
if ( res != 1 )
{
if ( g1 )
{
Expand Down
12 changes: 9 additions & 3 deletions src/core/qgslabelfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ QgsLabelFeature::QgsLabelFeature( QgsFeatureId id, GEOSGeometry* geometry, QSize
, mIsObstacle( false )
, mObstacleFactor( 1 )
, mInfo( nullptr )
, mPermissibleZoneGeos( nullptr )
, mPermissibleZoneGeosPrepared( nullptr )
{
}
Expand All @@ -48,7 +49,10 @@ QgsLabelFeature::~QgsLabelFeature()
GEOSGeom_destroy_r( QgsGeometry::getGEOSHandler(), mObstacleGeometry );

if ( mPermissibleZoneGeosPrepared )
{
GEOSPreparedGeom_destroy_r( QgsGeometry::getGEOSHandler(), mPermissibleZoneGeosPrepared );
GEOSGeom_destroy_r( QgsGeometry::getGEOSHandler(), mPermissibleZoneGeos );
}

delete mInfo;
}
Expand All @@ -68,15 +72,17 @@ void QgsLabelFeature::setPermissibleZone( const QgsGeometry &geometry )
if ( mPermissibleZoneGeosPrepared )
{
GEOSPreparedGeom_destroy_r( QgsGeometry::getGEOSHandler(), mPermissibleZoneGeosPrepared );
GEOSGeom_destroy_r( QgsGeometry::getGEOSHandler(), mPermissibleZoneGeos );
mPermissibleZoneGeosPrepared = nullptr;
mPermissibleZoneGeos = nullptr;
}

if ( mPermissibleZone.isEmpty() )
return;

const GEOSGeometry* zoneGeos = mPermissibleZone.asGeos();
if ( !zoneGeos )
mPermissibleZoneGeos = mPermissibleZone.exportToGeos();
if ( !mPermissibleZoneGeos )
return;

mPermissibleZoneGeosPrepared = GEOSPrepare_r( QgsGeometry::getGEOSHandler(), zoneGeos );
mPermissibleZoneGeosPrepared = GEOSPrepare_r( QgsGeometry::getGEOSHandler(), mPermissibleZoneGeos );
}
3 changes: 3 additions & 0 deletions src/core/qgslabelfeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ class CORE_EXPORT QgsLabelFeature

private:

//! GEOS geometry on which mPermissibleZoneGeosPrepared is based on
GEOSGeometry* mPermissibleZoneGeos;

// TODO - not required when QgsGeometry caches geos preparedness
const GEOSPreparedGeometry* mPermissibleZoneGeosPrepared;

Expand Down
31 changes: 12 additions & 19 deletions src/core/qgspallabeling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1851,17 +1851,16 @@ void QgsPalLayerSettings::registerFeature( QgsFeature& f, QgsRenderContext &cont
geom = QgsGeometry( geom.geometry()->boundary() );
}

const GEOSGeometry* geos_geom = nullptr;
GEOSGeometry* geos_geom_clone = nullptr;
if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, doClip ? &extentGeom : nullptr ) )
{
geom = QgsPalLabeling::prepareGeometry( geom, context, ct, doClip ? &extentGeom : nullptr );

if ( geom.isEmpty() )
return;
}
geos_geom = geom.asGeos();
geos_geom_clone = geom.exportToGeos();

const GEOSGeometry* geosObstacleGeom = nullptr;
QScopedPointer<QgsGeometry> scopedObstacleGeom;
if ( isObstacle )
{
Expand All @@ -1870,16 +1869,12 @@ void QgsPalLayerSettings::registerFeature( QgsFeature& f, QgsRenderContext &cont
scopedObstacleGeom.reset( new QgsGeometry( QgsPalLabeling::prepareGeometry( *obstacleGeometry, context, ct, doClip ? &extentGeom : nullptr ) ) );
obstacleGeometry = scopedObstacleGeom.data();
}
if ( obstacleGeometry )
{
geosObstacleGeom = obstacleGeometry->asGeos();
}
}

if ( minFeatureSize > 0 && !checkMinimumSizeMM( context, geom, minFeatureSize ) )
return;

if ( !geos_geom )
if ( !geos_geom_clone )
return; // invalid geometry

// likelihood exists label will be registered with PAL and may be drawn
Expand Down Expand Up @@ -1907,11 +1902,10 @@ void QgsPalLayerSettings::registerFeature( QgsFeature& f, QgsRenderContext &cont
}
}

GEOSGeometry* geos_geom_clone = GEOSGeom_clone_r( QgsGeometry::getGEOSHandler(), geos_geom );
GEOSGeometry* geosObstacleGeomClone = nullptr;
if ( geosObstacleGeom )
if ( obstacleGeometry )
{
geosObstacleGeomClone = GEOSGeom_clone_r( QgsGeometry::getGEOSHandler(), geosObstacleGeom );
geosObstacleGeomClone = obstacleGeometry->exportToGeos();
}


Expand Down Expand Up @@ -2384,21 +2378,18 @@ void QgsPalLayerSettings::registerObstacleFeature( QgsFeature& f, QgsRenderConte
geom = simplifier.simplify( geom );
}

const GEOSGeometry* geos_geom = nullptr;
GEOSGeometry* geos_geom_clone = nullptr;
QScopedPointer<QgsGeometry> scopedPreparedGeom;

if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, &extentGeom ) )
{
geom = QgsPalLabeling::prepareGeometry( geom, context, ct, &extentGeom );
}
geos_geom = geom.asGeos();
geos_geom_clone = geom.exportToGeos();

if ( !geos_geom )
if ( !geos_geom_clone )
return; // invalid geometry

GEOSGeometry* geos_geom_clone;
geos_geom_clone = GEOSGeom_clone_r( QgsGeometry::getGEOSHandler(), geos_geom );

// feature to the layer
*obstacleFeature = new QgsLabelFeature( f.id(), geos_geom_clone, QSizeF( 0, 0 ) );
( *obstacleFeature )->setIsObstacle( true );
Expand Down Expand Up @@ -3416,8 +3407,10 @@ QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry& geometry, QgsRen
}
}

if ( !geom.asGeos() )
return QgsGeometry(); // there is something really wrong with the geometry
// MD: exporting geometry to GEOS just to see if the geometry is wrong...?
// if still needed, we could have a more specific test here
//if ( !geom.asGeos() )
// return QgsGeometry(); // there is something really wrong with the geometry

// fix invalid polygons
if ( geom.type() == QgsWkbTypes::PolygonGeometry && !geom.isGeosValid() )
Expand Down
4 changes: 3 additions & 1 deletion src/core/qgstracer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,9 @@ bool QgsTracer::initGraph()
{
t2a.start();
// GEOSNode_r may throw an exception
GEOSGeometry* allNoded = GEOSNode_r( QgsGeometry::getGEOSHandler(), allGeom.asGeos() );
GEOSGeometry* allGeomGeos = allGeom.exportToGeos();
GEOSGeometry* allNoded = GEOSNode_r( QgsGeometry::getGEOSHandler(), allGeomGeos );
GEOSGeom_destroy_r( QgsGeometry::getGEOSHandler(), allGeomGeos );
timeNodingCall = t2a.elapsed();

QgsGeometry* noded = new QgsGeometry;
Expand Down
22 changes: 7 additions & 15 deletions src/core/qgsvectorlayerdiagramprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,44 +224,36 @@ QgsLabelFeature* QgsVectorLayerDiagramProvider::registerDiagram( QgsFeature& fea
extentGeom.rotate( -mapSettings.rotation(), mapSettings.visibleExtent().center() );
}

const GEOSGeometry* geos_geom = nullptr;
GEOSGeometry* geomCopy = nullptr;
QScopedPointer<QgsGeometry> scopedPreparedGeom;
if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, mSettings.coordinateTransform(), &extentGeom ) )
{
scopedPreparedGeom.reset( new QgsGeometry( QgsPalLabeling::prepareGeometry( geom, context, mSettings.coordinateTransform(), &extentGeom ) ) );
QgsGeometry* preparedGeom = scopedPreparedGeom.data();
if ( preparedGeom->isEmpty() )
return nullptr;
geos_geom = preparedGeom->asGeos();
geomCopy = preparedGeom->exportToGeos();
}
else
{
geos_geom = geom.asGeos();
geomCopy = geom.exportToGeos();
}

if ( !geos_geom )
if ( !geomCopy )
return nullptr; // invalid geometry

GEOSGeometry* geomCopy = GEOSGeom_clone_r( QgsGeometry::getGEOSHandler(), geos_geom );

const GEOSGeometry* geosObstacleGeom = nullptr;
GEOSGeometry* geosObstacleGeomClone = nullptr;
QScopedPointer<QgsGeometry> scopedObstacleGeom;
if ( mSettings.isObstacle() && obstacleGeometry && QgsPalLabeling::geometryRequiresPreparation( *obstacleGeometry, context, mSettings.coordinateTransform(), &extentGeom ) )
{
QgsGeometry preparedObstacleGeom = QgsPalLabeling::prepareGeometry( *obstacleGeometry, context, mSettings.coordinateTransform(), &extentGeom );
geosObstacleGeom = preparedObstacleGeom.asGeos();
geosObstacleGeomClone = preparedObstacleGeom.exportToGeos();
}
else if ( mSettings.isObstacle() && obstacleGeometry )
{
geosObstacleGeom = obstacleGeometry->asGeos();
}
GEOSGeometry* geosObstacleGeomClone = nullptr;
if ( geosObstacleGeom )
{
geosObstacleGeomClone = GEOSGeom_clone_r( QgsGeometry::getGEOSHandler(), geosObstacleGeom );
geosObstacleGeomClone = obstacleGeometry->exportToGeos();
}


double diagramWidth = 0;
double diagramHeight = 0;
if ( dr )
Expand Down
Loading

0 comments on commit 9872b48

Please sign in to comment.