mirror of
https://github.com/qgis/QGIS.git
synced 2025-02-27 00:33:48 -05:00
Allow converting polygons with unclosed rings to GEOS
Forces ring close on conversion, fixing a regression from 2.8 (and 2.14). See #13635 Adds test for identifying invalid polygons, currently only testing for the unclosed-ring invalidity. The test was verified to fail without the fixes included in this same commit, and to pass in 2.14.
This commit is contained in:
parent
3fb87de1f8
commit
e92e7fe472
@ -1478,7 +1478,7 @@ bool QgsGeos::isEmpty( QString* errorMsg ) const
|
||||
CATCH_GEOS_WITH_ERRMSG( false );
|
||||
}
|
||||
|
||||
GEOSCoordSequence* QgsGeos::createCoordinateSequence( const QgsCurveV2* curve, double precision )
|
||||
GEOSCoordSequence* QgsGeos::createCoordinateSequence( const QgsCurveV2* curve, double precision, bool forceClose )
|
||||
{
|
||||
bool segmentize = false;
|
||||
const QgsLineStringV2* line = dynamic_cast<const QgsLineStringV2*>( curve );
|
||||
@ -1507,10 +1507,17 @@ GEOSCoordSequence* QgsGeos::createCoordinateSequence( const QgsCurveV2* curve, d
|
||||
}
|
||||
|
||||
int numPoints = line->numPoints();
|
||||
|
||||
int numOutPoints = numPoints;
|
||||
if ( forceClose && ( line->pointN( 0 ) != line->pointN( numPoints - 1 ) ) )
|
||||
{
|
||||
++numOutPoints;
|
||||
}
|
||||
|
||||
GEOSCoordSequence* coordSeq = nullptr;
|
||||
try
|
||||
{
|
||||
coordSeq = GEOSCoordSeq_create_r( geosinit.ctxt, numPoints, coordDims );
|
||||
coordSeq = GEOSCoordSeq_create_r( geosinit.ctxt, numOutPoints, coordDims );
|
||||
if ( !coordSeq )
|
||||
{
|
||||
QgsMessageLog::logMessage( QObject::tr( "Could not create coordinate sequence for %1 points in %2 dimensions" ).arg( numPoints ).arg( coordDims ), QObject::tr( "GEOS" ) );
|
||||
@ -1518,9 +1525,9 @@ GEOSCoordSequence* QgsGeos::createCoordinateSequence( const QgsCurveV2* curve, d
|
||||
}
|
||||
if ( precision > 0. )
|
||||
{
|
||||
for ( int i = 0; i < numPoints; ++i )
|
||||
for ( int i = 0; i < numOutPoints; ++i )
|
||||
{
|
||||
QgsPointV2 pt = line->pointN( i ); //todo: create method to get const point reference
|
||||
const QgsPointV2 &pt = line->pointN( i % numPoints ); //todo: create method to get const point reference
|
||||
GEOSCoordSeq_setX_r( geosinit.ctxt, coordSeq, i, qgsRound( pt.x() / precision ) * precision );
|
||||
GEOSCoordSeq_setY_r( geosinit.ctxt, coordSeq, i, qgsRound( pt.y() / precision ) * precision );
|
||||
if ( hasZ )
|
||||
@ -1535,9 +1542,9 @@ GEOSCoordSequence* QgsGeos::createCoordinateSequence( const QgsCurveV2* curve, d
|
||||
}
|
||||
else
|
||||
{
|
||||
for ( int i = 0; i < numPoints; ++i )
|
||||
for ( int i = 0; i < numOutPoints; ++i )
|
||||
{
|
||||
QgsPointV2 pt = line->pointN( i ); //todo: create method to get const point reference
|
||||
const QgsPointV2 &pt = line->pointN( i % numPoints ); //todo: create method to get const point reference
|
||||
GEOSCoordSeq_setX_r( geosinit.ctxt, coordSeq, i, pt.x() );
|
||||
GEOSCoordSeq_setY_r( geosinit.ctxt, coordSeq, i, pt.y() );
|
||||
if ( hasZ )
|
||||
@ -1640,7 +1647,7 @@ GEOSGeometry* QgsGeos::createGeosPolygon( const QgsAbstractGeometryV2* poly , do
|
||||
GEOSGeometry* geosPolygon = nullptr;
|
||||
try
|
||||
{
|
||||
GEOSGeometry* exteriorRingGeos = GEOSGeom_createLinearRing_r( geosinit.ctxt, createCoordinateSequence( exteriorRing, precision ) );
|
||||
GEOSGeometry* exteriorRingGeos = GEOSGeom_createLinearRing_r( geosinit.ctxt, createCoordinateSequence( exteriorRing, precision, true ) );
|
||||
|
||||
|
||||
int nHoles = polygon->numInteriorRings();
|
||||
@ -1653,7 +1660,7 @@ GEOSGeometry* QgsGeos::createGeosPolygon( const QgsAbstractGeometryV2* poly , do
|
||||
for ( int i = 0; i < nHoles; ++i )
|
||||
{
|
||||
const QgsCurveV2* interiorRing = polygon->interiorRing( i );
|
||||
holes[i] = GEOSGeom_createLinearRing_r( geosinit.ctxt, createCoordinateSequence( interiorRing, precision ) );
|
||||
holes[i] = GEOSGeom_createLinearRing_r( geosinit.ctxt, createCoordinateSequence( interiorRing, precision, true ) );
|
||||
}
|
||||
geosPolygon = GEOSGeom_createPolygon_r( geosinit.ctxt, exteriorRingGeos, holes, nHoles );
|
||||
delete[] holes;
|
||||
|
@ -137,7 +137,7 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
|
||||
void cacheGeos() const;
|
||||
QgsAbstractGeometryV2* overlay( const QgsAbstractGeometryV2& geom, Overlay op, QString* errorMsg = nullptr ) const;
|
||||
bool relation( const QgsAbstractGeometryV2& geom, Relation r, QString* errorMsg = nullptr ) const;
|
||||
static GEOSCoordSequence* createCoordinateSequence( const QgsCurveV2* curve , double precision );
|
||||
static GEOSCoordSequence* createCoordinateSequence( const QgsCurveV2* curve , double precision, bool forceClose = false );
|
||||
static QgsLineStringV2* sequenceToLinestring( const GEOSGeometry* geos, bool hasZ, bool hasM );
|
||||
static int numberOfGeometries( GEOSGeometry* g );
|
||||
static GEOSGeometry* nodeGeometries( const GEOSGeometry *splitLine, const GEOSGeometry *geom );
|
||||
|
@ -45,11 +45,36 @@ class TestQgsMapToolIdentifyAction : public QObject
|
||||
void areaCalculation(); //test calculation of derived area attribute
|
||||
void identifyRasterFloat32(); // test pixel identification and decimal precision
|
||||
void identifyRasterFloat64(); // test pixel identification and decimal precision
|
||||
void identifyInvalidPolygons(); // test selecting invalid polygons
|
||||
|
||||
private:
|
||||
QgsMapCanvas* canvas;
|
||||
|
||||
QString testIdentifyRaster( QgsRasterLayer* layer, double xGeoref, double yGeoref );
|
||||
QList<QgsMapToolIdentify::IdentifyResult> testIdentifyVector( QgsVectorLayer* layer, double xGeoref, double yGeoref );
|
||||
|
||||
// Release return with delete []
|
||||
unsigned char *
|
||||
hex2bytes( const char *hex, int *size )
|
||||
{
|
||||
QByteArray ba = QByteArray::fromHex( hex );
|
||||
unsigned char *out = new unsigned char[ba.size()];
|
||||
memcpy( out, ba.data(), ba.size() );
|
||||
*size = ba.size();
|
||||
return out;
|
||||
}
|
||||
|
||||
// TODO: make this a QgsGeometry member...
|
||||
QgsGeometry geomFromHexWKB( const char *hexwkb )
|
||||
{
|
||||
int wkbsize;
|
||||
unsigned char *wkb = hex2bytes( hexwkb, &wkbsize );
|
||||
QgsGeometry geom;
|
||||
// NOTE: QgsGeometry takes ownership of wkb
|
||||
geom.fromWkb( wkb, wkbsize );
|
||||
return geom;
|
||||
}
|
||||
|
||||
};
|
||||
|
||||
void TestQgsMapToolIdentifyAction::initTestCase()
|
||||
@ -248,6 +273,7 @@ void TestQgsMapToolIdentifyAction::areaCalculation()
|
||||
QVERIFY( qgsDoubleNear( area, 389.6117, 0.001 ) );
|
||||
}
|
||||
|
||||
// private
|
||||
QString TestQgsMapToolIdentifyAction::testIdentifyRaster( QgsRasterLayer* layer, double xGeoref, double yGeoref )
|
||||
{
|
||||
QScopedPointer< QgsMapToolIdentifyAction > action( new QgsMapToolIdentifyAction( canvas ) );
|
||||
@ -258,6 +284,16 @@ QString TestQgsMapToolIdentifyAction::testIdentifyRaster( QgsRasterLayer* layer,
|
||||
return result[0].mAttributes["Band 1"];
|
||||
}
|
||||
|
||||
// private
|
||||
QList<QgsMapToolIdentify::IdentifyResult>
|
||||
TestQgsMapToolIdentifyAction::testIdentifyVector( QgsVectorLayer* layer, double xGeoref, double yGeoref )
|
||||
{
|
||||
QScopedPointer< QgsMapToolIdentifyAction > action( new QgsMapToolIdentifyAction( canvas ) );
|
||||
QgsPoint mapPoint = canvas->getCoordinateTransform()->transform( xGeoref, yGeoref );
|
||||
QList<QgsMapToolIdentify::IdentifyResult> result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer*>() << layer );
|
||||
return result;
|
||||
}
|
||||
|
||||
void TestQgsMapToolIdentifyAction::identifyRasterFloat32()
|
||||
{
|
||||
//create a temporary layer
|
||||
@ -315,6 +351,31 @@ void TestQgsMapToolIdentifyAction::identifyRasterFloat64()
|
||||
QCOMPARE( testIdentifyRaster( tempLayer.data(), 6.5, 0.5 ), QString( "1.2345678901234" ) );
|
||||
}
|
||||
|
||||
void TestQgsMapToolIdentifyAction::identifyInvalidPolygons()
|
||||
{
|
||||
//create a temporary layer
|
||||
QScopedPointer< QgsVectorLayer > memoryLayer( new QgsVectorLayer( "Polygon?field=pk:int", "vl", "memory" ) );
|
||||
QVERIFY( memoryLayer->isValid() );
|
||||
QgsFeature f1( memoryLayer->dataProvider()->fields(), 1 );
|
||||
f1.setAttribute( "pk", 1 );
|
||||
f1.setGeometry( geomFromHexWKB(
|
||||
"010300000001000000030000000000000000000000000000000000000000000000000024400000000000000000000000000000244000000000000024400000000000000000"
|
||||
) );
|
||||
// TODO: check why we need the ->dataProvider() part, since
|
||||
// there's a QgsVectorLayer::addFeatures method too
|
||||
//memoryLayer->addFeatures( QgsFeatureList() << f1 );
|
||||
memoryLayer->dataProvider()->addFeatures( QgsFeatureList() << f1 );
|
||||
|
||||
canvas->setExtent( QgsRectangle( 0, 0, 10, 10 ) );
|
||||
QList<QgsMapToolIdentify::IdentifyResult> identified;
|
||||
identified = testIdentifyVector( memoryLayer.data(), 4, 6 );
|
||||
QCOMPARE( identified.length(), 0 );
|
||||
identified = testIdentifyVector( memoryLayer.data(), 6, 4 );
|
||||
QCOMPARE( identified.length(), 1 );
|
||||
QCOMPARE( identified[0].mFeature.attribute( "pk" ), QVariant( 1 ) );
|
||||
|
||||
}
|
||||
|
||||
|
||||
QTEST_MAIN( TestQgsMapToolIdentifyAction )
|
||||
#include "testqgsmaptoolidentifyaction.moc"
|
||||
|
Loading…
x
Reference in New Issue
Block a user