Skip to content

Commit 4e04d02

Browse files
nyalldawsonnyalldawson
committed
By default, validity check should treat ring self intersections as invalid
We use the OGC definition of validity to ensure consistent results with PostGIS, GDAL, etc Fixes #16418, fixes #21336
1 parent 35f613d commit 4e04d02

14 files changed

+213
-17
lines changed

‎python/core/auto_generated/geometry/qgsgeometry.sip.in

+2-4
Original file line numberDiff line numberDiff line change
@@ -1803,14 +1803,12 @@ True if the location available from :py:func:`where` is valid.
18031803
ValidatorGeos,
18041804
};
18051805

1806-
void validateGeometry( QVector<QgsGeometry::Error> &errors /Out/, ValidationMethod method = ValidatorQgisInternal ) const;
1806+
void validateGeometry( QVector<QgsGeometry::Error> &errors /Out/, ValidationMethod method = ValidatorQgisInternal, QgsGeometry::ValidityFlags flags = 0 ) const;
18071807
%Docstring
18081808
Validates geometry and produces a list of geometry errors.
18091809
The ``method`` argument dictates which validator to utilize.
18101810

1811-
.. note::
1812-
1813-
Available in Python bindings since QGIS 1.6
1811+
The ``flags`` parameter indicates optional flags which control the type of validity checking performed.
18141812

18151813
.. versionadded:: 1.5
18161814
%End

‎python/plugins/processing/tests/testdata/custom/circular_strings.gpkg

0 Bytes
Binary file not shown.

‎python/plugins/processing/tests/testdata/custom/poly_ring_self_intersection.gml

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<ogr:FeatureCollection
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://ogr.maptools.org/ poly_ring_self_intersection.xsd"
5+
xmlns:ogr="http://ogr.maptools.org/"
6+
xmlns:gml="http://www.opengis.net/gml">
7+
<gml:boundedBy>
8+
<gml:Box>
9+
<gml:coord><gml:X>200</gml:X><gml:Y>200</gml:Y></gml:coord>
10+
<gml:coord><gml:X>400</gml:X><gml:Y>400</gml:Y></gml:coord>
11+
</gml:Box>
12+
</gml:boundedBy>
13+
14+
<gml:featureMember>
15+
<ogr:poly_ring_self_intersection fid="poly_ring_self_intersection.0">
16+
<ogr:geometryProperty><gml:Polygon srsName="EPSG:28356"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>200,400 400,400 400,200 300,200 350,250 250,250 300,200 200,200 200,400</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
17+
</ogr:poly_ring_self_intersection>
18+
</gml:featureMember>
19+
</ogr:FeatureCollection>

‎python/plugins/processing/tests/testdata/custom/poly_ring_self_intersection.xsd

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<xs:schema targetNamespace="http://ogr.maptools.org/" xmlns:ogr="http://ogr.maptools.org/" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:gml="http://www.opengis.net/gml" elementFormDefault="qualified" version="1.0">
3+
<xs:import namespace="http://www.opengis.net/gml" schemaLocation="http://schemas.opengis.net/gml/2.1.2/feature.xsd"/>
4+
<xs:element name="FeatureCollection" type="ogr:FeatureCollectionType" substitutionGroup="gml:_FeatureCollection"/>
5+
<xs:complexType name="FeatureCollectionType">
6+
<xs:complexContent>
7+
<xs:extension base="gml:AbstractFeatureCollectionType">
8+
<xs:attribute name="lockId" type="xs:string" use="optional"/>
9+
<xs:attribute name="scope" type="xs:string" use="optional"/>
10+
</xs:extension>
11+
</xs:complexContent>
12+
</xs:complexType>
13+
<xs:element name="poly_ring_self_intersection" type="ogr:poly_ring_self_intersection_Type" substitutionGroup="gml:_Feature"/>
14+
<xs:complexType name="poly_ring_self_intersection_Type">
15+
<xs:complexContent>
16+
<xs:extension base="gml:AbstractFeatureType">
17+
<xs:sequence>
18+
<xs:element name="geometryProperty" type="gml:PolygonPropertyType" nillable="true" minOccurs="0" maxOccurs="1"/>
19+
</xs:sequence>
20+
</xs:extension>
21+
</xs:complexContent>
22+
</xs:complexType>
23+
</xs:schema>

‎python/plugins/processing/tests/testdata/expected/poly_ring_self_intersection_error.gml

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<ogr:FeatureCollection
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://ogr.maptools.org/ poly_ring_self_intersection_error.xsd"
5+
xmlns:ogr="http://ogr.maptools.org/"
6+
xmlns:gml="http://www.opengis.net/gml">
7+
<gml:boundedBy>
8+
<gml:Box>
9+
<gml:coord><gml:X>300</gml:X><gml:Y>200</gml:Y></gml:coord>
10+
<gml:coord><gml:X>300</gml:X><gml:Y>200</gml:Y></gml:coord>
11+
</gml:Box>
12+
</gml:boundedBy>
13+
14+
<gml:featureMember>
15+
<ogr:poly_ring_self_intersection_error fid="poly_ring_self_intersection_error.0">
16+
<ogr:geometryProperty><gml:Point srsName="EPSG:28356"><gml:coordinates>300,200</gml:coordinates></gml:Point></ogr:geometryProperty>
17+
<ogr:message>Ring self-intersection</ogr:message>
18+
</ogr:poly_ring_self_intersection_error>
19+
</gml:featureMember>
20+
</ogr:FeatureCollection>

‎python/plugins/processing/tests/testdata/expected/poly_ring_self_intersection_error.xsd

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<xs:schema targetNamespace="http://ogr.maptools.org/" xmlns:ogr="http://ogr.maptools.org/" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:gml="http://www.opengis.net/gml" elementFormDefault="qualified" version="1.0">
3+
<xs:import namespace="http://www.opengis.net/gml" schemaLocation="http://schemas.opengis.net/gml/2.1.2/feature.xsd"/>
4+
<xs:element name="FeatureCollection" type="ogr:FeatureCollectionType" substitutionGroup="gml:_FeatureCollection"/>
5+
<xs:complexType name="FeatureCollectionType">
6+
<xs:complexContent>
7+
<xs:extension base="gml:AbstractFeatureCollectionType">
8+
<xs:attribute name="lockId" type="xs:string" use="optional"/>
9+
<xs:attribute name="scope" type="xs:string" use="optional"/>
10+
</xs:extension>
11+
</xs:complexContent>
12+
</xs:complexType>
13+
<xs:element name="poly_ring_self_intersection_error" type="ogr:poly_ring_self_intersection_error_Type" substitutionGroup="gml:_Feature"/>
14+
<xs:complexType name="poly_ring_self_intersection_error_Type">
15+
<xs:complexContent>
16+
<xs:extension base="gml:AbstractFeatureType">
17+
<xs:sequence>
18+
<xs:element name="geometryProperty" type="gml:PointPropertyType" nillable="true" minOccurs="0" maxOccurs="1"/>
19+
<xs:element name="message" nillable="true" minOccurs="0" maxOccurs="1">
20+
<xs:simpleType>
21+
<xs:restriction base="xs:string">
22+
<xs:maxLength value="255"/>
23+
</xs:restriction>
24+
</xs:simpleType>
25+
</xs:element>
26+
</xs:sequence>
27+
</xs:extension>
28+
</xs:complexContent>
29+
</xs:complexType>
30+
</xs:schema>

‎python/plugins/processing/tests/testdata/expected/poly_ring_self_intersection_invalid.gml

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<ogr:FeatureCollection
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://ogr.maptools.org/ poly_ring_self_intersection_invalid.xsd"
5+
xmlns:ogr="http://ogr.maptools.org/"
6+
xmlns:gml="http://www.opengis.net/gml">
7+
<gml:boundedBy>
8+
<gml:Box>
9+
<gml:coord><gml:X>200</gml:X><gml:Y>200</gml:Y></gml:coord>
10+
<gml:coord><gml:X>400</gml:X><gml:Y>400</gml:Y></gml:coord>
11+
</gml:Box>
12+
</gml:boundedBy>
13+
14+
<gml:featureMember>
15+
<ogr:poly_ring_self_intersection_invalid fid="poly_ring_self_intersection.0">
16+
<ogr:geometryProperty><gml:Polygon srsName="EPSG:28356"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>200,400 400,400 400,200 300,200 350,250 250,250 300,200 200,200 200,400</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
17+
<ogr:_errors>Ring self-intersection</ogr:_errors>
18+
</ogr:poly_ring_self_intersection_invalid>
19+
</gml:featureMember>
20+
</ogr:FeatureCollection>

‎python/plugins/processing/tests/testdata/expected/poly_ring_self_intersection_invalid.xsd

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<xs:schema targetNamespace="http://ogr.maptools.org/" xmlns:ogr="http://ogr.maptools.org/" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:gml="http://www.opengis.net/gml" elementFormDefault="qualified" version="1.0">
3+
<xs:import namespace="http://www.opengis.net/gml" schemaLocation="http://schemas.opengis.net/gml/2.1.2/feature.xsd"/>
4+
<xs:element name="FeatureCollection" type="ogr:FeatureCollectionType" substitutionGroup="gml:_FeatureCollection"/>
5+
<xs:complexType name="FeatureCollectionType">
6+
<xs:complexContent>
7+
<xs:extension base="gml:AbstractFeatureCollectionType">
8+
<xs:attribute name="lockId" type="xs:string" use="optional"/>
9+
<xs:attribute name="scope" type="xs:string" use="optional"/>
10+
</xs:extension>
11+
</xs:complexContent>
12+
</xs:complexType>
13+
<xs:element name="poly_ring_self_intersection_invalid" type="ogr:poly_ring_self_intersection_invalid_Type" substitutionGroup="gml:_Feature"/>
14+
<xs:complexType name="poly_ring_self_intersection_invalid_Type">
15+
<xs:complexContent>
16+
<xs:extension base="gml:AbstractFeatureType">
17+
<xs:sequence>
18+
<xs:element name="geometryProperty" type="gml:PolygonPropertyType" nillable="true" minOccurs="0" maxOccurs="1"/>
19+
<xs:element name="_errors" nillable="true" minOccurs="0" maxOccurs="1">
20+
<xs:simpleType>
21+
<xs:restriction base="xs:string">
22+
<xs:maxLength value="255"/>
23+
</xs:restriction>
24+
</xs:simpleType>
25+
</xs:element>
26+
</xs:sequence>
27+
</xs:extension>
28+
</xs:complexContent>
29+
</xs:complexType>
30+
</xs:schema>

‎python/plugins/processing/tests/testdata/expected/poly_ring_self_intersection_valid.gml

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<ogr:FeatureCollection
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://ogr.maptools.org/ poly_ring_self_intersection_valid.xsd"
5+
xmlns:ogr="http://ogr.maptools.org/"
6+
xmlns:gml="http://www.opengis.net/gml">
7+
<gml:boundedBy><gml:null>missing</gml:null></gml:boundedBy>
8+
9+
</ogr:FeatureCollection>

‎python/plugins/processing/tests/testdata/expected/poly_ring_self_intersection_valid.xsd

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<xs:schema targetNamespace="http://ogr.maptools.org/" xmlns:ogr="http://ogr.maptools.org/" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:gml="http://www.opengis.net/gml" elementFormDefault="qualified" version="1.0">
3+
<xs:import namespace="http://www.opengis.net/gml" schemaLocation="http://schemas.opengis.net/gml/2.1.2/feature.xsd"/>
4+
<xs:element name="FeatureCollection" type="ogr:FeatureCollectionType" substitutionGroup="gml:_FeatureCollection"/>
5+
<xs:complexType name="FeatureCollectionType">
6+
<xs:complexContent>
7+
<xs:extension base="gml:AbstractFeatureCollectionType">
8+
<xs:attribute name="lockId" type="xs:string" use="optional"/>
9+
<xs:attribute name="scope" type="xs:string" use="optional"/>
10+
</xs:extension>
11+
</xs:complexContent>
12+
</xs:complexType>
13+
<xs:element name="poly_ring_self_intersection_valid" type="ogr:poly_ring_self_intersection_valid_Type" substitutionGroup="gml:_Feature"/>
14+
<xs:complexType name="poly_ring_self_intersection_valid_Type">
15+
<xs:complexContent>
16+
<xs:extension base="gml:AbstractFeatureType">
17+
<xs:sequence>
18+
<xs:element name="geometryProperty" type="gml:PolygonPropertyType" nillable="true" minOccurs="0" maxOccurs="1"/>
19+
</xs:sequence>
20+
</xs:extension>
21+
</xs:complexContent>
22+
</xs:complexType>
23+
</xs:schema>

‎python/plugins/processing/tests/testdata/qgis_algorithm_tests.yaml

+18
Original file line numberDiff line numberDiff line change
@@ -3746,6 +3746,24 @@ tests:
37463746
name: expected/valid.gml
37473747
type: vector
37483748

3749+
- algorithm: qgis:checkvalidity
3750+
name: Check validity polygon ring self intersection
3751+
params:
3752+
INPUT_LAYER:
3753+
name: custom/poly_ring_self_intersection.gml|layername=poly_ring_self_intersection
3754+
type: vector
3755+
METHOD: 2
3756+
results:
3757+
ERROR_OUTPUT:
3758+
name: expected/poly_ring_self_intersection_error.gml
3759+
type: vector
3760+
INVALID_OUTPUT:
3761+
name: expected/poly_ring_self_intersection_invalid.gml
3762+
type: vector
3763+
VALID_OUTPUT:
3764+
name: expected/poly_ring_self_intersection_valid.gml
3765+
type: vector
3766+
37493767
- algorithm: qgis:polygonize
37503768
name: Polygonize
37513769
params:

‎src/core/geometry/qgsgeometry.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -2457,7 +2457,7 @@ QgsGeometry QgsGeometry::forceRHR() const
24572457
}
24582458

24592459

2460-
void QgsGeometry::validateGeometry( QVector<QgsGeometry::Error> &errors, ValidationMethod method ) const
2460+
void QgsGeometry::validateGeometry( QVector<QgsGeometry::Error> &errors, const ValidationMethod method, const QgsGeometry::ValidityFlags flags ) const
24612461
{
24622462
errors.clear();
24632463
if ( !d->geometry )
@@ -2480,7 +2480,7 @@ void QgsGeometry::validateGeometry( QVector<QgsGeometry::Error> &errors, Validat
24802480
QgsGeos geos( d->geometry.get() );
24812481
QString error;
24822482
QgsGeometry errorLoc;
2483-
if ( !geos.isValid( &error, true, &errorLoc ) )
2483+
if ( !geos.isValid( &error, flags & FlagAllowSelfTouchingHoles, &errorLoc ) )
24842484
{
24852485
if ( errorLoc.isNull() )
24862486
{

‎src/core/geometry/qgsgeometry.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -1875,10 +1875,12 @@ class CORE_EXPORT QgsGeometry
18751875
/**
18761876
* Validates geometry and produces a list of geometry errors.
18771877
* The \a method argument dictates which validator to utilize.
1878-
* \note Available in Python bindings since QGIS 1.6
1878+
*
1879+
* The \a flags parameter indicates optional flags which control the type of validity checking performed.
1880+
*
18791881
* \since QGIS 1.5
18801882
*/
1881-
void validateGeometry( QVector<QgsGeometry::Error> &errors SIP_OUT, ValidationMethod method = ValidatorQgisInternal ) const;
1883+
void validateGeometry( QVector<QgsGeometry::Error> &errors SIP_OUT, ValidationMethod method = ValidatorQgisInternal, QgsGeometry::ValidityFlags flags = nullptr ) const;
18821884

18831885
/**
18841886
* Compute the unary union on a list of \a geometries. May be faster than an iterative union on a set of geometries.

‎tests/src/python/test_qgsgeometry.py

+13-9
Original file line numberDiff line numberDiff line change
@@ -5064,23 +5064,27 @@ def testIsGeosValid(self):
50645064

50655065
def testValidateGeometry(self):
50665066
tests = [
5067-
["", []],
5068-
["Point (100 100)", [], []],
5069-
["MultiPoint (100 100, 100 200)", [], []],
5070-
["LINESTRING (0 0, 0 100, 100 100)", [], []],
5071-
["POLYGON((-1 -1, 4 0, 4 2, 0 2, -1 -1))", [], []],
5072-
["MULTIPOLYGON(Polygon((-1 -1, 4 0, 4 2, 0 2, -1 -1)),Polygon((100 100, 200 100, 200 200, 100 200, 100 100)))", [], []],
5073-
['MultiPolygon (((159865.14786298031685874 6768656.31838363595306873, 159858.97975336571107619 6769211.44824895076453686, 160486.07089751763851382 6769211.44824895076453686, 160481.95882444124436006 6768658.37442017439752817, 160163.27316101978067309 6768658.37442017439752817, 160222.89822062765597366 6769116.87056819349527359, 160132.43261294672265649 6769120.98264127038419247, 160163.27316101978067309 6768658.37442017439752817, 159865.14786298031685874 6768656.31838363595306873)))', [], []],
5074-
['Polygon((0 3, 3 0, 3 3, 0 0, 0 3))', [QgsGeometry.Error('Self-intersection', QgsPointXY(1.5, 1.5))], [QgsGeometry.Error('segments 0 and 2 of line 0 intersect at 1.5, 1.5', QgsPointXY(1.5, 1.5))]],
5067+
["", [], [], []],
5068+
["Point (100 100)", [], [], []],
5069+
["MultiPoint (100 100, 100 200)", [], [], []],
5070+
["LINESTRING (0 0, 0 100, 100 100)", [], [], []],
5071+
["POLYGON((-1 -1, 4 0, 4 2, 0 2, -1 -1))", [], [], []],
5072+
["MULTIPOLYGON(Polygon((-1 -1, 4 0, 4 2, 0 2, -1 -1)),Polygon((100 100, 200 100, 200 200, 100 200, 100 100)))", [], [], []],
5073+
['POLYGON ((200 400, 400 400, 400 200, 300 200, 350 250, 250 250, 300 200, 200 200, 200 400))', [QgsGeometry.Error('Ring self-intersection', QgsPointXY(300, 200))], [], []],
5074+
['MultiPolygon (((159865.14786298031685874 6768656.31838363595306873, 159858.97975336571107619 6769211.44824895076453686, 160486.07089751763851382 6769211.44824895076453686, 160481.95882444124436006 6768658.37442017439752817, 160163.27316101978067309 6768658.37442017439752817, 160222.89822062765597366 6769116.87056819349527359, 160132.43261294672265649 6769120.98264127038419247, 160163.27316101978067309 6768658.37442017439752817, 159865.14786298031685874 6768656.31838363595306873)))', [QgsGeometry.Error('Ring self-intersection', QgsPointXY(160163.27316101978067309, 6768658.37442017439752817))], [], []],
5075+
['Polygon((0 3, 3 0, 3 3, 0 0, 0 3))', [QgsGeometry.Error('Self-intersection', QgsPointXY(1.5, 1.5))], [QgsGeometry.Error('Self-intersection', QgsPointXY(1.5, 1.5))], [QgsGeometry.Error('segments 0 and 2 of line 0 intersect at 1.5, 1.5', QgsPointXY(1.5, 1.5))]],
50755076
]
50765077
for t in tests:
50775078
g1 = QgsGeometry.fromWkt(t[0])
50785079
res = g1.validateGeometry(QgsGeometry.ValidatorGeos)
50795080
self.assertEqual(res, t[1],
50805081
"mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[1], res[0].where() if res else ''))
5081-
res = g1.validateGeometry(QgsGeometry.ValidatorQgisInternal)
5082+
res = g1.validateGeometry(QgsGeometry.ValidatorGeos, QgsGeometry.FlagAllowSelfTouchingHoles)
50825083
self.assertEqual(res, t[2],
50835084
"mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[2], res[0].where() if res else ''))
5085+
res = g1.validateGeometry(QgsGeometry.ValidatorQgisInternal)
5086+
self.assertEqual(res, t[3],
5087+
"mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[3], res[0].where() if res else ''))
50845088

50855089
def renderGeometry(self, geom, use_pen, as_polygon=False, as_painter_path=False):
50865090
image = QImage(200, 200, QImage.Format_RGB32)

0 commit comments

Comments
 (0)