Skip to content

Commit

Permalink
Use GEOS reentrant API (_r functions) and update requirement to GEOS …
Browse files Browse the repository at this point in the history
…>= 3.1.0

Currently QGIS uses the 'classic' GEOS API that uses a global context.
This can conflict with libraries that would also use the global context
and potentially finalize it whereas QGIS will still use it later.

See https://groups.google.com/forum/#!topic/spatialite-users/9YSU6c5AVQ4 for
such an example of such a recent issue with Spatialite.

The _r API is available since GEOS 3.1.0, which is already an ancient GEOS
version. For example, old-old-stable Ubuntu (Lucid 10.04) and Debian (squeeze)
ship with GEOS 3.1.0 or later.

Such move has also been done in GDAL 1.11
(http://lists.osgeo.org/pipermail/gdal-dev/2013-August/036877.html)
and MapServer 7.0 (MapServer/MapServer#4733)

There's no easy way unfortunately to check at compile time that you don't
use the non _r API. I have patched my geos_c.h header to #ifdef that API (quite
painfull to do..). A postprocessing check can be done however with :

objdump -T output/lib/*.so | grep -v Base | grep GEOS | grep -v _r | grep -v "_ZN" | grep -v GEOSversion

It should return nothing.
  • Loading branch information
rouault committed Jun 17, 2014
1 parent 45ebe53 commit 6354dd3
Show file tree
Hide file tree
Showing 14 changed files with 464 additions and 501 deletions.
6 changes: 3 additions & 3 deletions cmake/FindGEOS.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ ELSE(WIN32)
STRING(REGEX REPLACE "([0-9]+)\\.([0-9]+)\\.([0-9]+)" "\\1" GEOS_VERSION_MAJOR "${GEOS_VERSION}")
STRING(REGEX REPLACE "([0-9]+)\\.([0-9]+)\\.([0-9]+)" "\\2" GEOS_VERSION_MINOR "${GEOS_VERSION}")

IF (GEOS_VERSION_MAJOR LESS 3)
MESSAGE (FATAL_ERROR "GEOS version is too old (${GEOS_VERSION}). Use 3.0.0 or higher.")
ENDIF (GEOS_VERSION_MAJOR LESS 3)
IF (GEOS_VERSION_MAJOR LESS 3 OR (GEOS_VERSION_MAJOR EQUAL 3 AND GEOS_VERSION_MINOR LESS 1) )
MESSAGE (FATAL_ERROR "GEOS version is too old (${GEOS_VERSION}). Use 3.1.0 or higher.")
ENDIF (GEOS_VERSION_MAJOR LESS 3 OR (GEOS_VERSION_MAJOR EQUAL 3 AND GEOS_VERSION_MINOR LESS 1) )

# set INCLUDE_DIR to prefix+include
EXEC_PROGRAM(${GEOS_CONFIG}
Expand Down
23 changes: 12 additions & 11 deletions src/analysis/vector/qgsgeometryanalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1100,36 +1100,37 @@ bool QgsGeometryAnalyzer::createOffsetGeometry( QgsGeometry* geom, QgsGeometry*

QList<GEOSGeometry*> outputGeomList;
QList<QgsGeometry*>::const_iterator inputGeomIt = inputGeomList.constBegin();
GEOSContextHandle_t geosctxt = QgsGeometry::getGEOSHandler();
for ( ; inputGeomIt != inputGeomList.constEnd(); ++inputGeomIt )
{
if ( geom->type() == QGis::Line )
{
//geos 3.3 needed for line offsets
#if defined(GEOS_VERSION_MAJOR) && defined(GEOS_VERSION_MINOR) && \
((GEOS_VERSION_MAJOR>3) || ((GEOS_VERSION_MAJOR==3) && (GEOS_VERSION_MINOR>=3)))
GEOSGeometry* offsetGeom = GEOSOffsetCurve(( *inputGeomIt )->asGeos(), -offset, 8 /*quadSegments*/, 0 /*joinStyle*/, 5.0 /*mitreLimit*/ );
if ( !offsetGeom || !GEOSisValid( offsetGeom ) )
GEOSGeometry* offsetGeom = GEOSOffsetCurve_r(geosctxt, ( *inputGeomIt )->asGeos(), -offset, 8 /*quadSegments*/, 0 /*joinStyle*/, 5.0 /*mitreLimit*/ );
if ( !offsetGeom || !GEOSisValid_r(geosctxt, offsetGeom ) )
{
return false;
}
if ( !GEOSisValid( offsetGeom ) || GEOSGeomTypeId( offsetGeom ) != GEOS_LINESTRING || GEOSGeomGetNumPoints( offsetGeom ) < 1 )
if ( !GEOSisValid_r(geosctxt, offsetGeom ) || GEOSGeomTypeId_r(geosctxt, offsetGeom ) != GEOS_LINESTRING || GEOSGeomGetNumPoints_r(geosctxt, offsetGeom ) < 1 )
{
GEOSGeom_destroy( offsetGeom );
GEOSGeom_destroy_r(geosctxt, offsetGeom );
return false;
}
outputGeomList.push_back( offsetGeom );
#else
outputGeomList.push_back( GEOSGeom_clone(( *inputGeomIt )->asGeos() ) );
outputGeomList.push_back( GEOSGeom_clone_r(geosctxt, ( *inputGeomIt )->asGeos() ) );
#endif
}
else if ( geom->type() == QGis::Point )
{
QgsPoint p = ( *inputGeomIt )->asPoint();
p = createPointOffset( p.x(), p.y(), offset, lineGeom );
GEOSCoordSequence* ptSeq = GEOSCoordSeq_create( 1, 2 );
GEOSCoordSeq_setX( ptSeq, 0, p.x() );
GEOSCoordSeq_setY( ptSeq, 0, p.y() );
GEOSGeometry* geosPt = GEOSGeom_createPoint( ptSeq );
GEOSCoordSequence* ptSeq = GEOSCoordSeq_create_r(geosctxt, 1, 2 );
GEOSCoordSeq_setX_r(geosctxt, ptSeq, 0, p.x() );
GEOSCoordSeq_setY_r(geosctxt, ptSeq, 0, p.y() );
GEOSGeometry* geosPt = GEOSGeom_createPoint_r(geosctxt, ptSeq );
outputGeomList.push_back( geosPt );
}
}
Expand All @@ -1152,11 +1153,11 @@ bool QgsGeometryAnalyzer::createOffsetGeometry( QgsGeometry* geom, QgsGeometry*
GEOSGeometry* collection = 0;
if ( geom->type() == QGis::Point )
{
collection = GEOSGeom_createCollection( GEOS_MULTIPOINT, geomArray, outputGeomList.size() );
collection = GEOSGeom_createCollection_r(geosctxt, GEOS_MULTIPOINT, geomArray, outputGeomList.size() );
}
else if ( geom->type() == QGis::Line )
{
collection = GEOSGeom_createCollection( GEOS_MULTILINESTRING, geomArray, outputGeomList.size() );
collection = GEOSGeom_createCollection_r(geosctxt, GEOS_MULTILINESTRING, geomArray, outputGeomList.size() );
}
geom->fromGeos( collection );
delete[] geomArray;
Expand Down
17 changes: 9 additions & 8 deletions src/analysis/vector/qgszonalstatistics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,8 @@ void QgsZonalStatistics::statisticsFromMiddlePointTest( void* band, QgsGeometry*
return;
}

const GEOSPreparedGeometry* polyGeosPrepared = GEOSPrepare( poly->asGeos() );
GEOSContextHandle_t geosctxt = QgsGeometry::getGEOSHandler();
const GEOSPreparedGeometry* polyGeosPrepared = GEOSPrepare_r( geosctxt, poly->asGeos() );
if ( !polyGeosPrepared )
{
return;
Expand All @@ -300,15 +301,15 @@ void QgsZonalStatistics::statisticsFromMiddlePointTest( void* band, QgsGeometry*
cellCenterX = rasterBBox.xMinimum() + pixelOffsetX * cellSizeX + cellSizeX / 2;
for ( int j = 0; j < nCellsX; ++j )
{
GEOSGeom_destroy( currentCellCenter );
cellCenterCoords = GEOSCoordSeq_create( 1, 2 );
GEOSCoordSeq_setX( cellCenterCoords, 0, cellCenterX );
GEOSCoordSeq_setY( cellCenterCoords, 0, cellCenterY );
currentCellCenter = GEOSGeom_createPoint( cellCenterCoords );
GEOSGeom_destroy_r( geosctxt, currentCellCenter );
cellCenterCoords = GEOSCoordSeq_create_r( geosctxt, 1, 2 );
GEOSCoordSeq_setX_r( geosctxt, cellCenterCoords, 0, cellCenterX );
GEOSCoordSeq_setY_r( geosctxt, cellCenterCoords, 0, cellCenterY );
currentCellCenter = GEOSGeom_createPoint_r( geosctxt, cellCenterCoords );

if ( scanLine[j] != mInputNodataValue ) //don't consider nodata values
{
if ( GEOSPreparedContains( polyGeosPrepared, currentCellCenter ) )
if ( GEOSPreparedContains_r( geosctxt, polyGeosPrepared, currentCellCenter ) )
{
if ( !qIsNaN( scanLine[j] ) )
{
Expand All @@ -322,7 +323,7 @@ void QgsZonalStatistics::statisticsFromMiddlePointTest( void* band, QgsGeometry*
cellCenterY -= cellSizeY;
}
CPLFree( scanLine );
GEOSPreparedGeom_destroy( polyGeosPrepared );
GEOSPreparedGeom_destroy_r( geosctxt, polyGeosPrepared );
}

void QgsZonalStatistics::statisticsFromPreciseIntersection( void* band, QgsGeometry* poly, int pixelOffsetX,
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsmaptooloffsetcurve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ void QgsMapToolOffsetCurve::setOffsetForRubberBand( double offset, bool leftSide
int quadSegments = s.value( "/qgis/digitizing/offset_quad_seg", 8 ).toInt();
double mitreLimit = s.value( "/qgis/digitizing/offset_miter_limit", 5.0 ).toDouble();

GEOSGeometry* offsetGeom = GEOSOffsetCurve( geosGeom, leftSide ? offset : -offset, quadSegments, joinStyle, mitreLimit );
GEOSGeometry* offsetGeom = GEOSOffsetCurve_r( QgsGeometry::getGEOSHandler(), geosGeom, leftSide ? offset : -offset, quadSegments, joinStyle, mitreLimit );
if ( !offsetGeom )
{
deleteRubberBandAndGeometry();
Expand Down
56 changes: 30 additions & 26 deletions src/core/pal/feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#endif

#include <qglobal.h>
#include <qgsgeometry.h>

#include <cmath>
#include <cstring>
Expand Down Expand Up @@ -109,7 +110,7 @@ namespace pal

if ( ownsGeom )
{
GEOSGeom_destroy( the_geom );
GEOSGeom_destroy_r( QgsGeometry::getGEOSHandler(), the_geom );
the_geom = NULL;
}
}
Expand All @@ -123,36 +124,37 @@ namespace pal
int i, j;

const GEOSCoordSequence *coordSeq;
GEOSContextHandle_t geosctxt = QgsGeometry::getGEOSHandler();

type = GEOSGeomTypeId( geom );
type = GEOSGeomTypeId_r( geosctxt, geom );

if ( type == GEOS_POLYGON )
{
if ( GEOSGetNumInteriorRings( geom ) > 0 )
if ( GEOSGetNumInteriorRings_r( geosctxt, geom ) > 0 )
{
// set nbHoles, holes member variables
nbHoles = GEOSGetNumInteriorRings( geom );
nbHoles = GEOSGetNumInteriorRings_r( geosctxt, geom );
holes = new PointSet*[nbHoles];

for ( i = 0; i < nbHoles; i++ )
{
holes[i] = new PointSet();
holes[i]->holeOf = NULL;

const GEOSGeometry* interior = GEOSGetInteriorRingN( geom, i );
holes[i]->nbPoints = GEOSGetNumCoordinates( interior );
const GEOSGeometry* interior = GEOSGetInteriorRingN_r( geosctxt, geom, i );
holes[i]->nbPoints = GEOSGetNumCoordinates_r( geosctxt, interior );
holes[i]->x = new double[holes[i]->nbPoints];
holes[i]->y = new double[holes[i]->nbPoints];

holes[i]->xmin = holes[i]->ymin = DBL_MAX;
holes[i]->xmax = holes[i]->ymax = -DBL_MAX;

coordSeq = GEOSGeom_getCoordSeq( interior );
coordSeq = GEOSGeom_getCoordSeq_r( geosctxt, interior );

for ( j = 0; j < holes[i]->nbPoints; j++ )
{
GEOSCoordSeq_getX( coordSeq, j, &holes[i]->x[j] );
GEOSCoordSeq_getY( coordSeq, j, &holes[i]->y[j] );
GEOSCoordSeq_getX_r( geosctxt, coordSeq, j, &holes[i]->x[j] );
GEOSCoordSeq_getY_r( geosctxt, coordSeq, j, &holes[i]->y[j] );

holes[i]->xmax = holes[i]->x[j] > holes[i]->xmax ? holes[i]->x[j] : holes[i]->xmax;
holes[i]->xmin = holes[i]->x[j] < holes[i]->xmin ? holes[i]->x[j] : holes[i]->xmin;
Expand All @@ -166,7 +168,7 @@ namespace pal
}

// use exterior ring for the extraction of coordinates that follows
geom = GEOSGetExteriorRing( geom );
geom = GEOSGetExteriorRing_r( geosctxt, geom );
}
else
{
Expand All @@ -175,8 +177,8 @@ namespace pal
}

// find out number of points
nbPoints = GEOSGetNumCoordinates( geom );
coordSeq = GEOSGeom_getCoordSeq( geom );
nbPoints = GEOSGetNumCoordinates_r( geosctxt, geom );
coordSeq = GEOSGeom_getCoordSeq_r( geosctxt, geom );

// initialize bounding box
xmin = ymin = DBL_MAX;
Expand All @@ -188,8 +190,8 @@ namespace pal

for ( i = 0; i < nbPoints; i++ )
{
GEOSCoordSeq_getX( coordSeq, i, &x[i] );
GEOSCoordSeq_getY( coordSeq, i, &y[i] );
GEOSCoordSeq_getX_r( geosctxt, coordSeq, i, &x[i] );
GEOSCoordSeq_getY_r( geosctxt, coordSeq, i, &y[i] );

xmax = x[i] > xmax ? x[i] : xmax;
xmin = x[i] < xmin ? x[i] : xmin;
Expand Down Expand Up @@ -1393,13 +1395,14 @@ namespace pal

void FeaturePart::addSizePenalty( int nbp, LabelPosition** lPos, double bbx[4], double bby[4] )
{
int geomType = GEOSGeomTypeId( the_geom );
GEOSContextHandle_t ctxt = QgsGeometry::getGEOSHandler();
int geomType = GEOSGeomTypeId_r( ctxt, the_geom );

double sizeCost = 0;
if ( geomType == GEOS_LINESTRING )
{
double length;
if ( GEOSLength( the_geom, &length ) != 1 )
if ( GEOSLength_r( ctxt, the_geom, &length ) != 1 )
return; // failed to calculate length
double bbox_length = max( bbx[2] - bbx[0], bby[2] - bby[0] );
if ( length >= bbox_length / 4 )
Expand All @@ -1410,7 +1413,7 @@ namespace pal
else if ( geomType == GEOS_POLYGON )
{
double area;
if ( GEOSArea( the_geom, &area ) != 1 )
if ( GEOSArea_r( ctxt, the_geom, &area ) != 1 )
return;
double bbox_area = ( bbx[2] - bbx[0] ) * ( bby[2] - bby[0] );
if ( area >= bbox_area / 16 )
Expand All @@ -1432,27 +1435,28 @@ namespace pal

bool FeaturePart::isConnected( FeaturePart* p2 )
{
return ( GEOSTouches( the_geom, p2->the_geom ) == 1 );
return ( GEOSTouches_r( QgsGeometry::getGEOSHandler(), the_geom, p2->the_geom ) == 1 );
}

bool FeaturePart::mergeWithFeaturePart( FeaturePart* other )
{
GEOSGeometry* g1 = GEOSGeom_clone( the_geom );
GEOSGeometry* g2 = GEOSGeom_clone( other->the_geom );
GEOSContextHandle_t ctxt = QgsGeometry::getGEOSHandler();
GEOSGeometry* g1 = GEOSGeom_clone_r( ctxt, the_geom );
GEOSGeometry* g2 = GEOSGeom_clone_r( ctxt, other->the_geom );
GEOSGeometry* geoms[2] = { g1, g2 };
GEOSGeometry* g = GEOSGeom_createCollection( GEOS_MULTILINESTRING, geoms, 2 );
GEOSGeometry* gTmp = GEOSLineMerge( g );
GEOSGeom_destroy( g );
GEOSGeometry* g = GEOSGeom_createCollection_r( ctxt, GEOS_MULTILINESTRING, geoms, 2 );
GEOSGeometry* gTmp = GEOSLineMerge_r( ctxt, g );
GEOSGeom_destroy_r( ctxt, g );

if ( GEOSGeomTypeId( gTmp ) != GEOS_LINESTRING )
if ( GEOSGeomTypeId_r( ctxt, gTmp ) != GEOS_LINESTRING )
{
// sometimes it's not possible to merge lines (e.g. they don't touch at endpoints)
GEOSGeom_destroy( gTmp );
GEOSGeom_destroy_r( ctxt, gTmp );
return false;
}

if ( ownsGeom ) // delete old geometry if we own it
GEOSGeom_destroy( the_geom );
GEOSGeom_destroy_r( ctxt, the_geom );
// set up new geometry
the_geom = gTmp;
ownsGeom = true;
Expand Down
36 changes: 19 additions & 17 deletions src/core/pal/layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <cmath>
#include <vector>

#include <qgsgeometry.h>

#include <pal/pal.h>
#include <pal/layer.h>
Expand Down Expand Up @@ -285,27 +286,28 @@ namespace pal
throw InternalException::UnknownGeometry();
}

GEOSContextHandle_t geosctxt = QgsGeometry::getGEOSHandler();
// if multiple labels are requested for lines, split the line in pieces of desired distance
if ( repeatDistance > 0 )
{
int nSimpleGeometries = simpleGeometries->size();
for ( int i = 0; i < nSimpleGeometries; ++i )
{
const GEOSGeometry* geom = simpleGeometries->pop_front();
if ( GEOSGeomTypeId( geom ) == GEOS_LINESTRING )
if ( GEOSGeomTypeId_r( geosctxt, geom ) == GEOS_LINESTRING )
{
const GEOSCoordSequence *cs = GEOSGeom_getCoordSeq( geom );
const GEOSCoordSequence *cs = GEOSGeom_getCoordSeq_r( geosctxt, geom );

// get number of points
unsigned int n;
GEOSCoordSeq_getSize( cs, &n );
GEOSCoordSeq_getSize_r( geosctxt, cs, &n );

// Read points
std::vector<Point> points( n );
for ( unsigned int i = 0; i < n; ++i )
{
GEOSCoordSeq_getX( cs, i, &points[i].x );
GEOSCoordSeq_getY( cs, i, &points[i].y );
GEOSCoordSeq_getX_r( geosctxt, cs, i, &points[i].x );
GEOSCoordSeq_getY_r( geosctxt, cs, i, &points[i].y );
}

// Cumulative length vector
Expand Down Expand Up @@ -337,26 +339,26 @@ namespace pal
p.x = points[cur - 1].x + c * ( points[cur].x - points[cur - 1].x );
p.y = points[cur - 1].y + c * ( points[cur].y - points[cur - 1].y );
part.push_back( p );
GEOSCoordSequence* cooSeq = GEOSCoordSeq_create( part.size(), 2 );
GEOSCoordSequence* cooSeq = GEOSCoordSeq_create_r( geosctxt, part.size(), 2 );
for ( std::size_t i = 0; i < part.size(); ++i )
{
GEOSCoordSeq_setX( cooSeq, i, part[i].x );
GEOSCoordSeq_setY( cooSeq, i, part[i].y );
GEOSCoordSeq_setX_r( geosctxt, cooSeq, i, part[i].x );
GEOSCoordSeq_setY_r( geosctxt, cooSeq, i, part[i].y );
}

simpleGeometries->push_back( GEOSGeom_createLineString( cooSeq ) );
simpleGeometries->push_back( GEOSGeom_createLineString_r( geosctxt, cooSeq ) );
part.clear();
part.push_back( p );
}
// Create final part
part.push_back( points[n - 1] );
GEOSCoordSequence* cooSeq = GEOSCoordSeq_create( part.size(), 2 );
GEOSCoordSequence* cooSeq = GEOSCoordSeq_create_r( geosctxt, part.size(), 2 );
for ( std::size_t i = 0; i < part.size(); ++i )
{
GEOSCoordSeq_setX( cooSeq, i, part[i].x );
GEOSCoordSeq_setY( cooSeq, i, part[i].y );
GEOSCoordSeq_setX_r( geosctxt, cooSeq, i, part[i].x );
GEOSCoordSeq_setY_r( geosctxt, cooSeq, i, part[i].y );
}
simpleGeometries->push_back( GEOSGeom_createLineString( cooSeq ) );
simpleGeometries->push_back( GEOSGeom_createLineString_r( geosctxt, cooSeq ) );
}
else
{
Expand All @@ -370,13 +372,13 @@ namespace pal
const GEOSGeometry* geom = simpleGeometries->pop_front();

// ignore invalid geometries (e.g. polygons with self-intersecting rings)
if ( GEOSisValid( geom ) != 1 ) // 0=invalid, 1=valid, 2=exception
if ( GEOSisValid_r( geosctxt, geom ) != 1 ) // 0=invalid, 1=valid, 2=exception
{
std::cerr << "ignoring invalid feature " << geom_id << std::endl;
continue;
}

int type = GEOSGeomTypeId( geom );
int type = GEOSGeomTypeId_r( geosctxt, geom );

if ( type != GEOS_POINT && type != GEOS_LINESTRING && type != GEOS_POLYGON )
{
Expand Down Expand Up @@ -404,9 +406,9 @@ namespace pal
if ( mode == LabelPerFeature && repeatDistance == 0.0 && ( type == GEOS_POLYGON || type == GEOS_LINESTRING ) )
{
if ( type == GEOS_LINESTRING )
GEOSLength( geom, &geom_size );
GEOSLength_r( geosctxt, geom, &geom_size );
else if ( type == GEOS_POLYGON )
GEOSArea( geom, &geom_size );
GEOSArea_r( geosctxt, geom, &geom_size );

if ( geom_size > biggest_size )
{
Expand Down

0 comments on commit 6354dd3

Please sign in to comment.