diff --git a/src/core/qgsgeometry.cpp b/src/core/qgsgeometry.cpp index be00e942edb6..f3bf3cf56728 100644 --- a/src/core/qgsgeometry.cpp +++ b/src/core/qgsgeometry.cpp @@ -284,6 +284,8 @@ static GEOSGeometry *createGeosLinearRing( const QgsPolyline& polyline ) } else { + // XXX [MD] this exception should not be silenced! + // this is here just because maptopixel simplification can return invalid linear rings if ( polyline.count() == 3 ) //-> Avoid 'GEOS::IllegalArgumentException: Invalid number of points in LinearRing found 3 - must be 0 or >= 4' return 0; @@ -359,7 +361,16 @@ static GEOSGeometry *createGeosPolygon( const QgsPolygon& polygon ) for ( int i = 0; i < polygon.count(); i++ ) { GEOSGeometry *ring = createGeosLinearRing( polygon[i] ); - if ( ring ) geoms << ring; + if ( !ring ) + { + // something went really wrong - exit + for ( int j = 0; j < geoms.count(); j++ ) + GEOSGeom_destroy_r( geosinit.ctxt, geoms[j] ); + // XXX [MD] we just silently return here - but we shouldn't + // this is just because maptopixel simplification can return invalid linear rings + return 0; + } + geoms << ring; } return createGeosPolygon( geoms ); diff --git a/src/core/qgsmaptopixelgeometrysimplifier.cpp b/src/core/qgsmaptopixelgeometrysimplifier.cpp index d9c749e97dda..c801d7e21378 100644 --- a/src/core/qgsmaptopixelgeometrysimplifier.cpp +++ b/src/core/qgsmaptopixelgeometrysimplifier.cpp @@ -61,7 +61,7 @@ inline static QgsRectangle calculateBoundingBox( QGis::WkbType wkbType, unsigned } //! Generalize the WKB-geometry using the BBOX of the original geometry -inline static bool generalizeWkbGeometry( +static bool generalizeWkbGeometryByBoundingBox( QGis::WkbType wkbType, unsigned char* sourceWkb, size_t sourceWkbSize, unsigned char* targetWkb, size_t& targetWkbSize, @@ -169,7 +169,7 @@ bool QgsMapToPixelSimplifier::simplifyWkbGeometry( if (( simplifyFlags & QgsMapToPixelSimplifier::SimplifyEnvelope ) && isGeneralizableByMapBoundingBox( envelope, map2pixelTol ) ) { - isGeneralizable = generalizeWkbGeometry( wkbType, sourceWkb, sourceWkbSize, targetWkb, targetWkbSize, envelope, writeHeader ); + isGeneralizable = generalizeWkbGeometryByBoundingBox( wkbType, sourceWkb, sourceWkbSize, targetWkb, targetWkbSize, envelope, writeHeader ); if ( isGeneralizable ) return true; } @@ -222,8 +222,8 @@ bool QgsMapToPixelSimplifier::simplifyWkbGeometry( double* ptr = ( double* )targetWkb; map2pixelTol *= map2pixelTol; //-> Use mappixelTol for 'LengthSquare' calculations. - bool isaUngenerizableSegment; - bool hasUngenerizableSegments = false; //-> To avoid replace the simplified geometry by its BBOX when there are 'long' segments. + bool isLongSegment; + bool hasLongSegments = false; //-> To avoid replace the simplified geometry by its BBOX when there are 'long' segments. // Check whether the LinearRing is really closed. if ( isaLinearRing ) @@ -249,44 +249,55 @@ bool QgsMapToPixelSimplifier::simplifyWkbGeometry( memcpy( &x, sourceWkb, sizeof( double ) ); sourceWkb += sizeOfDoubleX; memcpy( &y, sourceWkb, sizeof( double ) ); sourceWkb += sizeOfDoubleY; - isaUngenerizableSegment = false; + isLongSegment = false; if ( i == 0 || !isGeneralizable || - ( isaUngenerizableSegment = ( calculateLengthSquared2D( x, y, lastX, lastY ) > map2pixelTol ) ) || + ( isLongSegment = ( calculateLengthSquared2D( x, y, lastX, lastY ) > map2pixelTol ) ) || ( !isaLinearRing && ( i == 1 || i >= numPoints - 2 ) ) ) { memcpy( ptr, &x, sizeof( double ) ); lastX = x; ptr++; memcpy( ptr, &y, sizeof( double ) ); lastY = y; ptr++; numTargetPoints++; - if ( isaUngenerizableSegment && !hasUngenerizableSegments ) - { - hasUngenerizableSegments = true; - } + hasLongSegments |= isLongSegment; } r.combineExtentWith( x, y ); } targetWkb = wkb2 + 4; - // Fix the topology of the geometry - if ( numTargetPoints <= ( isaLinearRing ? 2 : 1 ) && !hasUngenerizableSegments ) + if ( numTargetPoints < ( isaLinearRing ? 4 : 2 ) ) { - unsigned char* targetTempWkb = targetWkb; - int targetWkbTempSize = targetWkbSize; - - sourceWkb = sourcePrevWkb; - targetWkb = targetPrevWkb; - targetWkbSize = targetWkbPrevSize; - if ( generalizeWkbGeometry( wkbType, sourceWkb, sourceWkbSize, targetWkb, targetWkbSize, r, writeHeader ) ) - return true; - - targetWkb = targetTempWkb; - targetWkbSize = targetWkbTempSize; + // we simplified the geometry too much! + if ( !hasLongSegments ) + { + // approximate the geometry's shape by its bounding box + // (rect for linear ring / one segment for line string) + unsigned char* targetTempWkb = targetWkb; + int targetWkbTempSize = targetWkbSize; + + sourceWkb = sourcePrevWkb; + targetWkb = targetPrevWkb; + targetWkbSize = targetWkbPrevSize; + if ( generalizeWkbGeometryByBoundingBox( wkbType, sourceWkb, sourceWkbSize, targetWkb, targetWkbSize, r, writeHeader ) ) + return true; + + targetWkb = targetTempWkb; + targetWkbSize = targetWkbTempSize; + } + else + { + // Bad luck! The simplified geometry is invalid and approximation by bounding box + // would create artifacts due to long segments. Worst of all, we may have overwritten + // the original coordinates by the simplified ones (source and target WKB ptr can be the same) + // so we cannot even undo the changes here. We will return invalid geometry and hope that + // other pieces of QGIS will survive that :-/ + } } if ( isaLinearRing ) { + // make sure we keep the linear ring closed memcpy( &x, targetWkb + 0, sizeof( double ) ); memcpy( &y, targetWkb + sizeof( double ), sizeof( double ) ); if ( lastX != x || lastY != y )