Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix bug #9360-revival: fix whole layer not rendered (when simplify geometry activated) #1087

Merged
merged 6 commits into from Jan 22, 2014
Merged

Fix bug #9360-revival: fix whole layer not rendered (when simplify geometry activated) #1087

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2e81934
#9360R: fix bad ogr simplification
ahuarte47 Jan 19, 2014
b7ee396
#9360R: remove ogr_geometry.h
ahuarte47 Jan 19, 2014
a2b8efa
#9360R: mix CPP/C code - experimental
ahuarte47 Jan 19, 2014
6c14b50
#9360R: enable ogr simplification on provider side only cpp
ahuarte47 Jan 19, 2014
0c99423
#9360R: disable ogr simplification while OGR_G_SetPoints does not exist
ahuarte47 Jan 20, 2014
f79dfa1
#9360R: enable ogr simplification for GDAL-OGR >= 1.11
ahuarte47 Jan 21, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
131 changes: 98 additions & 33 deletions src/providers/ogr/qgsogrgeometrysimplifier.cpp
View file
Open in desktop
Expand Up @@ -18,6 +18,26 @@
#include "qgsogrprovider.h"
#include "qgsapplication.h"

/***************************************************************************/
// Use OgrGeometry class to speed up simplification if possible

#ifdef __cplusplus
#define USE_OGR_GEOMETRY_CLASS
#endif

#ifdef USE_OGR_GEOMETRY_CLASS
#include <ogr_geometry.h>
#else
class OGRRawPoint
{
public:
double x;
double y;
};
#endif

/***************************************************************************/

QgsOgrAbstractGeometrySimplifier::~QgsOgrAbstractGeometrySimplifier()
{
}
Expand Down Expand Up @@ -61,20 +81,42 @@ bool QgsOgrTopologyPreservingSimplifier::simplifyGeometry( OGRGeometryH geometry

QgsOgrMapToPixelSimplifier::QgsOgrMapToPixelSimplifier( int simplifyFlags, double map2pixelTol )
: QgsMapToPixelSimplifier( simplifyFlags, map2pixelTol )
, mPointBufferPtr( NULL )
, mPointBufferCount( 0 )
{
}

QgsOgrMapToPixelSimplifier::~QgsOgrMapToPixelSimplifier()
{
if ( mPointBufferPtr )
{
OGRFree( mPointBufferPtr );
mPointBufferPtr = NULL;
}
}

//! Returns a point buffer of the specified size
OGRRawPoint* QgsOgrMapToPixelSimplifier::mallocPoints( int numPoints )
{
if ( mPointBufferPtr && mPointBufferCount < numPoints )
{
OGRFree( mPointBufferPtr );
mPointBufferPtr = NULL;
}
if ( !mPointBufferPtr )
{
mPointBufferCount = numPoints;
mPointBufferPtr = ( OGRRawPoint* )OGRMalloc( mPointBufferCount * sizeof( OGRRawPoint ) );
}
return mPointBufferPtr;
}

//////////////////////////////////////////////////////////////////////////////////////////////
// Helper simplification methods

//! Simplifies the OGR-geometry (Removing duplicated points) when is applied the specified map2pixel context
bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( QGis::GeometryType geometryType, const QgsRectangle& envelope, double *xptr, double *yptr, int pointCount, int& pointSimplifiedCount )
bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( QGis::GeometryType geometryType, double* xptr, int xStride, double* yptr, int yStride, int pointCount, int& pointSimplifiedCount )
{
Q_UNUSED( envelope )
bool canbeGeneralizable = ( mSimplifyFlags & QgsMapToPixelSimplifier::SimplifyGeometry );

pointSimplifiedCount = pointCount;
Expand All @@ -84,20 +126,20 @@ bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( QGis::GeometryType geometr
double map2pixelTol = mMapToPixelTol * mMapToPixelTol; //-> Use mappixelTol for 'LengthSquare' calculations.
double x, y, lastX = 0, lastY = 0;

double *xsourcePtr = xptr;
double *ysourcePtr = yptr;
double *xtargetPtr = xptr;
double *ytargetPtr = yptr;
char* xsourcePtr = ( char* )xptr;
char* ysourcePtr = ( char* )yptr;
char* xtargetPtr = ( char* )xptr;
char* ytargetPtr = ( char* )yptr;

for ( int i = 0, numPoints = geometryType == QGis::Polygon ? pointCount - 1 : pointCount; i < numPoints; ++i )
{
memcpy( &x, xsourcePtr++, sizeof( double ) );
memcpy( &y, ysourcePtr++, sizeof( double ) );
memcpy( &x, xsourcePtr, sizeof( double ) ); xsourcePtr += xStride;
memcpy( &y, ysourcePtr, sizeof( double ) ); ysourcePtr += yStride;

if ( i == 0 || !canbeGeneralizable || QgsMapToPixelSimplifier::calculateLengthSquared2D( x, y, lastX, lastY ) > map2pixelTol || ( geometryType == QGis::Line && ( i == 1 || i >= numPoints - 2 ) ) )
{
memcpy( xtargetPtr++, &x, sizeof( double ) ); lastX = x;
memcpy( ytargetPtr++, &y, sizeof( double ) ); lastY = y;
memcpy( xtargetPtr, &x, sizeof( double ) ); lastX = x; xtargetPtr += xStride;
memcpy( ytargetPtr, &y, sizeof( double ) ); lastY = y; ytargetPtr += yStride;
pointSimplifiedCount++;
}
}
Expand All @@ -107,7 +149,6 @@ bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( QGis::GeometryType geometr
memcpy( ytargetPtr, yptr, sizeof( double ) );
pointSimplifiedCount++;
}

return pointSimplifiedCount != pointCount;
}

Expand All @@ -120,7 +161,8 @@ bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( OGRGeometryH geometry, boo
if ( wkbGeometryType == wkbLineString )
{
int numPoints = OGR_G_GetPointCount( geometry );
if (( isaLinearRing && numPoints <= 5 ) || ( !isaLinearRing && numPoints <= 4 ) )

if (( isaLinearRing && numPoints <= 5 ) || ( !isaLinearRing && numPoints <= 4 ) )
return false;

OGREnvelope env;
Expand All @@ -130,25 +172,31 @@ bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( OGRGeometryH geometry, boo
// Can replace the geometry by its BBOX ?
if (( mSimplifyFlags & QgsMapToPixelSimplifier::SimplifyEnvelope ) && canbeGeneralizedByMapBoundingBox( envelope ) )
{
OGRRawPoint* points = NULL;

double x1 = envelope.xMinimum();
double y1 = envelope.yMinimum();
double x2 = envelope.xMaximum();
double y2 = envelope.yMaximum();

if ( isaLinearRing )
{
OGR_G_SetPoint( geometry, 0, x1, y1, 0.0 );
OGR_G_SetPoint( geometry, 1, x2, y1, 0.0 );
OGR_G_SetPoint( geometry, 2, x2, y2, 0.0 );
OGR_G_SetPoint( geometry, 3, x1, y2, 0.0 );
OGR_G_SetPoint( geometry, 4, x1, y1, 0.0 );
numPoints = 5;
points = mallocPoints( numPoints );
points[0].x = x1; points[0].y = y1;
points[1].x = x2; points[1].y = y1;
points[2].x = x2; points[2].y = y2;
points[3].x = x1; points[3].y = y2;
points[4].x = x1; points[4].y = y1;
}
else
{
OGR_G_SetPoint( geometry, 0, x1, y1, 0.0 );
OGR_G_SetPoint( geometry, 1, x2, y2, 0.0 );
numPoints = 2;
points = mallocPoints( numPoints );
points[0].x = x1; points[0].y = y1;
points[1].x = x2; points[1].y = y2;
}

setGeometryPoints( geometry, points, numPoints, isaLinearRing );
OGR_G_FlattenTo2D( geometry );

return true;
Expand All @@ -158,22 +206,16 @@ bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( OGRGeometryH geometry, boo
QGis::GeometryType geometryType = isaLinearRing ? QGis::Polygon : QGis::Line;
int numSimplifiedPoints = 0;

QVector<double> x( numPoints ), y( numPoints );
for ( int i = 0; i < numPoints; i++ )
{
double z;
OGR_G_GetPoint( geometry, i, &x[i], &y[i], &z );
}
OGRRawPoint* points = mallocPoints( numPoints );
double* xptr = ( double* )points;
double* yptr = xptr + 1;
OGR_G_GetPoints( geometry, xptr, 16, yptr, 16, NULL, 0 );

if ( simplifyOgrGeometry( geometryType, envelope, x.data(), y.data(), numPoints, numSimplifiedPoints ) )
if ( simplifyOgrGeometry( geometryType, xptr, 16, yptr, 16, numPoints, numSimplifiedPoints ) )
{
for ( int i = 0; i < numSimplifiedPoints; i++ )
{
OGR_G_SetPoint( geometry, i, x[i], y[i], 0.0 );
}
setGeometryPoints( geometry, points, numSimplifiedPoints, isaLinearRing );
OGR_G_FlattenTo2D( geometry );
}

return numSimplifiedPoints != numPoints;
}
}
Expand All @@ -195,18 +237,41 @@ bool QgsOgrMapToPixelSimplifier::simplifyOgrGeometry( OGRGeometryH geometry, boo
{
bool result = false;

for ( int i = 1, numGeometries = OGR_G_GetGeometryCount( geometry ); i < numGeometries; ++i )
for ( int i = 0, numGeometries = OGR_G_GetGeometryCount( geometry ); i < numGeometries; ++i )
{
result |= simplifyOgrGeometry( OGR_G_GetGeometryRef( geometry, i ), wkbGeometryType == wkbMultiPolygon );
}

if ( result )
OGR_G_FlattenTo2D( geometry );

return result;
}

return false;
}

//! Load a point array to the specified LineString geometry
void QgsOgrMapToPixelSimplifier::setGeometryPoints( OGRGeometryH geometry, OGRRawPoint* points, int numPoints, bool isaLinearRing )
{
#ifdef USE_OGR_GEOMETRY_CLASS
OGRLineString* lineString = ( OGRLineString* )geometry;
lineString->setPoints( numPoints, points );
#else
OGRGeometryH g = OGR_G_CreateGeometry( isaLinearRing ? wkbLinearRing : wkbLineString );

for (int i = 0; i < numPoints; i++, points++)
OGR_G_SetPoint( g, i, points->x, points->y, 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jef-n, I released an experimental commit, to show the problem: this 'OGR_G_SetPoint' is a terrible bottleneck because of it does a realloc of one point each new point.

The only solution that I see is disable the simplification on ogr provider side when "__cplusplus" is not enabled, I can do it because of the provider capabalities indicates if it can simplify.

What do you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using GEOS.... there is other option:

GEOSCoordSequence* seq = GEOSCoordSeq_create(unsigned int size, unsigned int dims);
...

GEOSGeometry * g = GEOSGeom_createLinearRing(GEOSCoordSequence* s);
char * wkb = GEOSGeomToWKT(const GEOSGeometry* g);

Do you agree ?

NOTE:
OGR provider has not references to GEOS, it is the above option better ? (Disable ogr simplification ifndef "__cplusplus" )


size_t wkbSize = OGR_G_WkbSize( g );
unsigned char *wkb = new unsigned char[ wkbSize ];
OGR_G_ExportToWkb( g, ( OGRwkbByteOrder ) QgsApplication::endian(), wkb );
OGR_G_ImportFromWkb( geometry, wkb, wkbSize );
delete [] wkb;
OGR_G_DestroyGeometry( g );
#endif
}

//////////////////////////////////////////////////////////////////////////////////////////////

//! Simplifies the specified geometry
Expand Down
15 changes: 14 additions & 1 deletion src/providers/ogr/qgsogrgeometrysimplifier.h
View file
Open in desktop
Expand Up @@ -20,6 +20,8 @@
#include "qgsmaptopixelgeometrysimplifier.h"
#include <ogr_api.h>

class OGRRawPoint;

/**
* Abstract base class for simplify OGR-geometries using a specific algorithm
*/
Expand Down Expand Up @@ -63,11 +65,22 @@ class QgsOgrMapToPixelSimplifier : public QgsOgrAbstractGeometrySimplifier, QgsM
virtual ~QgsOgrMapToPixelSimplifier();

private:
//! Point memory buffer for optimize the simplification process
OGRRawPoint* mPointBufferPtr;
//! Current Point memory buffer size
int mPointBufferCount;

//! Simplifies the OGR-geometry (Removing duplicated points) when is applied the specified map2pixel context
bool simplifyOgrGeometry( QGis::GeometryType geometryType, const QgsRectangle& envelope, double *xptr, double *yptr, int pointCount, int &pointSimplifiedCount );
bool simplifyOgrGeometry( QGis::GeometryType geometryType, double* xptr, int xStride, double* yptr, int yStride, int pointCount, int& pointSimplifiedCount );
//! Simplifies the OGR-geometry (Removing duplicated points) when is applied the specified map2pixel context
bool simplifyOgrGeometry( OGRGeometryH geometry, bool isaLinearRing );

//! Returns a point buffer of the specified size
OGRRawPoint* mallocPoints( int numPoints );

//! Load a point array to the specified LineString geometry
static void setGeometryPoints( OGRGeometryH geometry, OGRRawPoint* points, int numPoints, bool isaLinearRing );

public:
//! Simplifies the specified geometry
virtual bool simplifyGeometry( OGRGeometryH geometry );
Expand Down