Skip to content
Permalink
Browse files

Merge pull request #1099 from ahuarte47/Issue_8725R-ST_simplify

Fix bug #8725R: fix collapsed geometries by ST_simplify function of postgis
  • Loading branch information
timlinux committed Jan 23, 2014
2 parents 49a807a + 2baf403 commit 8b530012d179750ca18d384aef8246a4e330d49a
@@ -29,9 +29,6 @@ class QgsSimplifyMethod
/** Gets the tolerance of simplification */
double tolerance() const;

/** Returns the optimal tolerance for Douglas-Peucker simplification algorithms */
double toleranceForDouglasPeuckerAlgorithms() const;

/** Sets whether the simplification executes after fetch the geometries from provider, otherwise it executes, when supported, in provider before fetch the geometries */
void setForceLocalOptimization( bool localOptimization );
/** Gets whether the simplification executes after fetch the geometries from provider, otherwise it executes, when supported, in provider before fetch the geometries */
@@ -4697,7 +4697,10 @@ bool QgsGeometry::exportWkbToGeos() const
}
sequence << QgsPoint( *x, *y );
}
lines << createGeosLineString( sequence );

// ignore invalid parts, it can come from ST_Simplify operations
if ( sequence.count() > 1 )
lines << createGeosLineString( sequence );
}
mGeos = createGeosCollection( GEOS_MULTILINESTRING, lines );
mDirtyGeos = false;
@@ -18,9 +18,9 @@
#include "qgsmaptopixelgeometrysimplifier.h"
#include "qgsapplication.h"

QgsMapToPixelSimplifier::QgsMapToPixelSimplifier( int simplifyFlags, double map2pixelTol )
QgsMapToPixelSimplifier::QgsMapToPixelSimplifier( int simplifyFlags, double tolerance )
: mSimplifyFlags( simplifyFlags )
, mMapToPixelTol( map2pixelTol )
, mTolerance( tolerance )
{
}
QgsMapToPixelSimplifier::~QgsMapToPixelSimplifier()
@@ -330,13 +330,13 @@ QgsGeometry* QgsMapToPixelSimplifier::simplify( QgsGeometry* geometry ) const
unsigned char* wkb = ( unsigned char* )malloc( wkbSize );
memcpy( wkb, geometry->asWkb(), wkbSize );
g->fromWkb( wkb, wkbSize );
simplifyGeometry( g, mSimplifyFlags, mMapToPixelTol );
simplifyGeometry( g, mSimplifyFlags, mTolerance );

return g;
}

//! Simplifies the geometry (Removing duplicated points) when is applied the specified map2pixel context
bool QgsMapToPixelSimplifier::simplifyGeometry( QgsGeometry* geometry, int simplifyFlags, double map2pixelTol )
bool QgsMapToPixelSimplifier::simplifyGeometry( QgsGeometry* geometry, int simplifyFlags, double tolerance )
{
size_t targetWkbSize = 0;

@@ -351,7 +351,7 @@ bool QgsMapToPixelSimplifier::simplifyGeometry( QgsGeometry* geometry, int simpl
size_t wkbSize = geometry->wkbSize( );

// Simplify the geometry rewriting temporally its WKB-stream for saving calloc's.
if ( simplifyWkbGeometry( simplifyFlags, wkbType, wkb, wkbSize, wkb, targetWkbSize, envelope, map2pixelTol ) )
if ( simplifyWkbGeometry( simplifyFlags, wkbType, wkb, wkbSize, wkb, targetWkbSize, envelope, tolerance ) )
{
unsigned char* targetWkb = ( unsigned char* )malloc( targetWkbSize );
memcpy( targetWkb, wkb, targetWkbSize );
@@ -364,5 +364,5 @@ bool QgsMapToPixelSimplifier::simplifyGeometry( QgsGeometry* geometry, int simpl
//! Simplifies the geometry (Removing duplicated points) when is applied the specified map2pixel context
bool QgsMapToPixelSimplifier::simplifyGeometry( QgsGeometry* geometry ) const
{
return simplifyGeometry( geometry, mSimplifyFlags, mMapToPixelTol );
return simplifyGeometry( geometry, mSimplifyFlags, mTolerance );
}
@@ -32,7 +32,7 @@
class CORE_EXPORT QgsMapToPixelSimplifier : public QgsAbstractGeometrySimplifier
{
public:
QgsMapToPixelSimplifier( int simplifyFlags, double map2pixelTol );
QgsMapToPixelSimplifier( int simplifyFlags, double tolerance );
virtual ~QgsMapToPixelSimplifier();

//! Applicable simplification flags
@@ -51,8 +51,8 @@ class CORE_EXPORT QgsMapToPixelSimplifier : public QgsAbstractGeometrySimplifier
//! Current simplification flags
int mSimplifyFlags;

//! Map2Pixel tolerance for the simplification
double mMapToPixelTol;
//! Distance tolerance for the simplification
double mTolerance;

//! Returns the squared 2D-distance of the vector defined by the two points specified
static float calculateLengthSquared2D( double x1, double y1, double x2, double y2 );
@@ -73,10 +73,10 @@ class CORE_EXPORT QgsMapToPixelSimplifier : public QgsAbstractGeometrySimplifier
static bool canbeGeneralizedByMapBoundingBox( const QgsRectangle& envelope, double map2pixelTol );

//! Returns whether the envelope can be replaced by its BBOX when is applied the specified map2pixel context
inline bool canbeGeneralizedByMapBoundingBox( const QgsRectangle& envelope ) const { return canbeGeneralizedByMapBoundingBox( envelope, mMapToPixelTol ); }
inline bool canbeGeneralizedByMapBoundingBox( const QgsRectangle& envelope ) const { return canbeGeneralizedByMapBoundingBox( envelope, mTolerance ); }

//! Simplifies the geometry when is applied the specified map2pixel context
static bool simplifyGeometry( QgsGeometry* geometry, int simplifyFlags, double map2pixelTol );
static bool simplifyGeometry( QgsGeometry* geometry, int simplifyFlags, double tolerance );

};

@@ -54,12 +54,6 @@ void QgsSimplifyMethod::setForceLocalOptimization( bool localOptimization )
mForceLocalOptimization = localOptimization;
}

double QgsSimplifyMethod::toleranceForDouglasPeuckerAlgorithms() const
{
//TODO: define more precise value, now, it is experimental but conservative
return mTolerance / 5.0;
}

QgsAbstractGeometrySimplifier* QgsSimplifyMethod::createGeometrySimplifier( const QgsSimplifyMethod& simplifyMethod )
{
QgsSimplifyMethod::MethodType methodType = simplifyMethod.methodType();
@@ -48,9 +48,6 @@ class CORE_EXPORT QgsSimplifyMethod
//! Gets the tolerance of simplification
inline double tolerance() const { return mTolerance; }

//! Returns the optimal tolerance for Douglas-Peucker simplification algorithms
double toleranceForDouglasPeuckerAlgorithms() const;

//! Sets whether the simplification executes after fetch the geometries from provider, otherwise it executes, when supported, in provider before fetch the geometries
void setForceLocalOptimization( bool localOptimization );
//! Gets whether the simplification executes after fetch the geometries from provider, otherwise it executes, when supported, in provider before fetch the geometries
@@ -63,8 +63,8 @@ bool QgsOgrTopologyPreservingSimplifier::simplifyGeometry( OGRGeometryH geometry
#if defined(GDAL_VERSION_NUM) && defined(GDAL_COMPUTE_VERSION)
#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(1,11,0)

QgsOgrMapToPixelSimplifier::QgsOgrMapToPixelSimplifier( int simplifyFlags, double map2pixelTol )
: QgsMapToPixelSimplifier( simplifyFlags, map2pixelTol )
QgsOgrMapToPixelSimplifier::QgsOgrMapToPixelSimplifier( int simplifyFlags, double tolerance )
: QgsMapToPixelSimplifier( simplifyFlags, tolerance )
, mPointBufferPtr( NULL )
, mPointBufferCount( 0 )
{
@@ -137,7 +137,7 @@ bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( QGis::GeometryType geometr
if ( geometryType == QGis::Point || geometryType == QGis::UnknownGeometry ) return false;
pointSimplifiedCount = 0;

double map2pixelTol = mMapToPixelTol * mMapToPixelTol; //-> Use mappixelTol for 'LengthSquare' calculations.
double map2pixelTol = mTolerance * mTolerance; //-> Use mappixelTol for 'LengthSquare' calculations.
double x, y, lastX = 0, lastY = 0;

char* xsourcePtr = ( char* )xptr;
@@ -63,7 +63,7 @@ class QgsOgrTopologyPreservingSimplifier : public QgsOgrAbstractGeometrySimplifi
class QgsOgrMapToPixelSimplifier : public QgsOgrAbstractGeometrySimplifier, QgsMapToPixelSimplifier
{
public:
QgsOgrMapToPixelSimplifier( int simplifyFlags, double map2pixelTol );
QgsOgrMapToPixelSimplifier( int simplifyFlags, double tolerance );
virtual ~QgsOgrMapToPixelSimplifier();

private:
@@ -14,6 +14,7 @@
***************************************************************************/
#include "qgspostgresfeatureiterator.h"
#include "qgspostgresprovider.h"
#include "qgsgeometry.h"

#include "qgslogger.h"
#include "qgsmessagelog.h"
@@ -292,6 +293,7 @@ QString QgsPostgresFeatureIterator::whereClauseRect()
bool QgsPostgresFeatureIterator::declareCursor( const QString& whereClause )
{
mFetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) && !P->mGeometryColumn.isNull();
bool simplifyGeometry = false;

try
{
@@ -303,11 +305,10 @@ bool QgsPostgresFeatureIterator::declareCursor( const QString& whereClause )
{
QString simplifyFunctionName = simplifyMethod.methodType() == QgsSimplifyMethod::OptimizeForRendering
? ( P->mConnectionRO->majorVersion() < 2 ? "simplify" : "st_simplify" )
: ( P->mConnectionRO->majorVersion() < 2 ? "simplifypreservetopology" : "st_simplifypreservetopology" );
: ( P->mConnectionRO->majorVersion() < 2 ? "simplifypreservetopology" : "st_simplifypreservetopology" );

double tolerance = simplifyMethod.methodType() == QgsSimplifyMethod::OptimizeForRendering
? simplifyMethod.toleranceForDouglasPeuckerAlgorithms()
: simplifyMethod.tolerance();
double tolerance = simplifyMethod.tolerance() * 0.8; //-> Default factor for the maximum displacement distance for simplification, similar as GeoServer does
simplifyGeometry = simplifyMethod.methodType() == QgsSimplifyMethod::OptimizeForRendering;

query += QString( "%1(%5(%2%3,%6),'%4')" )
.arg( P->mConnectionRO->majorVersion() < 2 ? "asbinary" : "st_asbinary" )
@@ -368,6 +369,17 @@ bool QgsPostgresFeatureIterator::declareCursor( const QString& whereClause )
query += delim + P->mConnectionRO->fieldExpression( P->field( idx ) );
}

// query BBOX of geometries to redefine the geometries collapsed by ST_Simplify()
if ( simplifyGeometry && !( P->mConnectionRO->majorVersion() >= 2 && P->mConnectionRO->minorVersion() >= 1 ) )
{
query += QString( ",%1(%5(%2)%3,'%4')" )
.arg( P->mConnectionRO->majorVersion() < 2 ? "asbinary" : "st_asbinary" )
.arg( P->quotedIdentifier( P->mGeometryColumn ) )
.arg( P->mSpatialColType == sctGeography ? "::geometry" : "" )
.arg( P->endianString() )
.arg( P->mConnectionRO->majorVersion() < 2 ? "envelope" : "st_envelope" );
}

query += " FROM " + P->mQuery;

if ( !whereClause.isEmpty() )
@@ -591,6 +603,43 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
getFeatureAttribute( idx, queryResult, row, col, feature );
}

// fix collapsed geometries by ST_Simplify() using the BBOX fetched from the current query
const QgsSimplifyMethod& simplifyMethod = mRequest.simplifyMethod();
if ( mFetchGeometry && !simplifyMethod.forceLocalOptimization() && simplifyMethod.methodType() == QgsSimplifyMethod::OptimizeForRendering )
{
QgsGeometry* geometry = feature.geometry();

if ( !( P->mConnectionRO->majorVersion() >= 2 && P->mConnectionRO->minorVersion() >= 1 ) && ( !geometry || geometry->length() == 0 ) )
{
int returnedLength = ::PQgetlength( queryResult.result(), row, col );

if ( returnedLength > 0 )
{
unsigned char *featureGeom = new unsigned char[returnedLength + 1];
memcpy( featureGeom, PQgetvalue( queryResult.result(), row, col ), returnedLength );
memset( featureGeom + returnedLength, 0, 1 );

QgsGeometry *envelope = new QgsGeometry();
envelope->fromWkb( featureGeom, returnedLength + 1 );

if ( QGis::flatType( QGis::singleType( P->geometryType() ) ) == QGis::WKBPolygon )
{
feature.setGeometry( envelope );
}
else
{
QgsPolyline polyline;
polyline.append( envelope->vertexAt( 0 ) );
polyline.append( envelope->vertexAt( 2 ) );
delete envelope;

geometry = QgsGeometry::fromPolyline( polyline );
feature.setGeometry( geometry );
}
}
}
}

return true;
}
catch ( QgsPostgresProvider::PGFieldNotFound )

14 comments on commit 8b53001

@palmerj

This comment has been minimized.

Copy link
Contributor

@palmerj palmerj replied Jan 25, 2014

When activating "simplify on provider side if possible" on PostGIS layer I get an image output that is not drawing a few pixels. This is because this patch uses ST_Simplify which will drop vertices that are important for a for a complete image. Why not use ST_SnapToGrid instead?

See result with ST_Simplify (see missing white pixels in parcel network):

st_simplify

vs: ST_SnapToGrid:

st_snaptogrid

Data can be downloaded from http://data.linz.govt.nz/layer/772-nz-primary-parcels/

Note: I'm using simplification threshold of 1.00 pixel. Also I don't see any render issues when using not using provider side simplification.

@nyalldawson

This comment has been minimized.

Copy link
Collaborator

@nyalldawson nyalldawson replied Jan 25, 2014

I'd say the current behavior is correct - st_snaptogrid won't help with speeding up the rendering as the original number of vertices will still be returned from postgis. St_simplify results in a speedup because it removes unnecessary vertices from the geometry.

@palmerj

This comment has been minimized.

Copy link
Contributor

@palmerj palmerj replied Jan 25, 2014

If the right parameter is passed map to pixel value is passed to ST_SnapToGrid I would have thought the number of vertices would be reduced as it removes consecutive points falling on the same cell.

@nyalldawson

This comment has been minimized.

Copy link
Collaborator

@nyalldawson nyalldawson replied Jan 29, 2014

@palmerj interesting -- I didn't realise st_snaptogrid behaved this way. I still think st_simplify is a better method because it reduces vertices on segments which would appear straight at a given scale (eg, imagine a complex wiggly boundary which can be reduced to a two point line when zoomed out far enough. In this case using st_snaptogrid to snap points to pixels will still result in many vertices along this segment).

That said, @ahuarte47 I've just encountered this same issue myself. It seems to happen on polygon layers when a whole feature is reduced to less than the size of a single pixel (using a postgis layer with provider simplification enabled).

@jef-n

This comment has been minimized.

Copy link
Member

@jef-n jef-n replied Jan 29, 2014

select st_astext(st_snaptogrid(st_geomfromtext('LINESTRING(0 0,1 0,2 0,3 0,4 0,5 0,6 0)'),3));
        st_astext
-------------------------
 LINESTRING(0 0,3 0,6 0)
@ahuarte47

This comment has been minimized.

Copy link
Contributor

@ahuarte47 ahuarte47 replied Jan 29, 2014

Hi @palmerj, I would test the "NZ Primary Parcels" layer, what CRS should I download?
May be EPSG:4167 ?

@palmerj

This comment has been minimized.

Copy link
Contributor

@palmerj palmerj replied Jan 29, 2014

@ahuarte47 Yes EPSG:4167 is best.

@nyalldawson

This comment has been minimized.

Copy link
Collaborator

@nyalldawson nyalldawson replied Jan 29, 2014

@jef-n Are you suggesting st_snaptogrid is the way to go? Because your test case is reduced to just two vertices with st_simplify.

@jef-n

This comment has been minimized.

Copy link
Member

@jef-n jef-n replied Jan 29, 2014

I just verified that st_snaptogrid also reduces the number of vertices. I didn't check what either function does - perhaps even combining both could make sense.

@ahuarte47

This comment has been minimized.

Copy link
Contributor

@ahuarte47 ahuarte47 replied Jan 30, 2014

Hi @palmerj, I have tested your data in postgis (PostgreSQL 9.2) and I have not white pixels :-(

image1

image2

image3

image4

You are right, I use ST_simplify, but to avoid geometries collapsed I use a trick, when them are collapsed I use the BBOX of the original geometry ( 8b53001#diff-85b8cc237feeed878a569af636cd9d88R373 )

About white pixels, what can I do?
Best Regards

Alvaro

@palmerj

This comment has been minimized.

Copy link
Contributor

@palmerj palmerj replied Jan 30, 2014

Hmm interesting. Did you tick the box for "simplify on provider side if possible"? Using latest Ubuntu dev build from last night I still get the white pixels.

@palmerj

This comment has been minimized.

Copy link
Contributor

@palmerj palmerj replied Jan 30, 2014

I'm also getting the problem with qgis master on win32.

Note I'm running PG 9.1 and PostGIS 2

@ahuarte47

This comment has been minimized.

Copy link
Contributor

@ahuarte47 ahuarte47 replied Jan 30, 2014

Hi @palmerj, then I must install PostgreSQL 9.1 + PostGIS 2
Best regards

@ahuarte47

This comment has been minimized.

Copy link
Contributor

@ahuarte47 ahuarte47 replied Jan 31, 2014

Hi @palmerj, I have tested your layers in (PostgreSQL 9.1.11 + Postgis 2.0.3) and (PostgreSQL 9.3.02 + Postgis 2.1.1) in Windows XP SP3 32bits and It works fine, but using Postgis 2.1.1 I found this error on two of my layers :-)

I have done many tests and ST_Simplify not seem reliable (Sometimes it returns NULL, sometimes returns EMPTY):
http://lists.osgeo.org/pipermail/postgis-devel/2012-December/023152.html
http://trac.osgeo.org/postgis/ticket/1987

Using SnapToGrid it works fine!
Can you test this pull ( #1131 ) please ?

SnapToGrid may return more vertices, but the result is safer:
http://blog.cartodb.com/post/20163722809/speeding-up-tiles-rendering

Alvaro

Please sign in to comment.
You can’t perform that action at this time.