Merge pull request #4037 from wonder-sk/union-fixes

[processing] Union: fix output of features that do not instersect layer A
qgis · Feb 10, 2017 · 274f61f · 274f61f
2 parents fe2b34d + 78396dc
commit 274f61f
Show file tree

Hide file tree

Showing 6 changed files with 198 additions and 39 deletions.
diff --git a/python/plugins/processing/algs/qgis/Union.py b/python/plugins/processing/algs/qgis/Union.py
@@ -123,6 +123,11 @@ def processAlgorithm(self, progress):
                         else:
                             int_geom = QgsGeometry(int_geom)
 
+                        # TODO: the result may have a different dimension (e.g. intersection of two polygons may result in a single point)
+                        # or the result may be a collection of geometries (e.g. intersection of two polygons results in three polygons and one linestring).
+                        # We need to filter out all acceptable geometries into a single (possibly multi-part) geometry - and we need
+                        # to do it consistently also in the code further below
+
                         if int_geom.wkbType() == QGis.WKBUnknown or QgsWKBTypes.flatType(int_geom.geometry().wkbType()) == QgsWKBTypes.GeometryCollection:
                             # Intersection produced different geomety types
                             temp_list = int_geom.asGeometryCollection()
@@ -160,6 +165,7 @@ def processAlgorithm(self, progress):
                         ProcessingLog.addToLog(ProcessingLog.LOG_ERROR,
                                                self.tr('GEOS geoprocessing error: One or more input features have invalid geometry.'))
 
+                # TODO: correctly handly different output geometry types (see todo above)
                 if diff_geom.wkbType() == 0 or QgsWKBTypes.flatType(diff_geom.geometry().wkbType()) == QgsWKBTypes.GeometryCollection:
                     temp_list = diff_geom.asGeometryCollection()
                     for i in temp_list:
@@ -182,51 +188,38 @@ def processAlgorithm(self, progress):
             progress.setPercentage(nElement / float(nFeat) * 100)
             add = False
             geom = QgsGeometry(inFeatA.geometry())
-            diff_geom = QgsGeometry(geom)
             atMap = [None] * length
             atMap.extend(inFeatA.attributes())
             intersects = indexB.intersects(geom.boundingBox())
+            lstIntersectingA = []
 
-            if len(intersects) < 1:
-                try:
-                    outFeat.setGeometry(geom)
-                    outFeat.setAttributes(atMap)
-                    writer.addFeature(outFeat)
-                except:
-                    ProcessingLog.addToLog(ProcessingLog.LOG_INFO,
-                                           self.tr('Feature geometry error: One or more output features ignored due to invalid geometry.'))
-            else:
-                for id in intersects:
-                    request = QgsFeatureRequest().setFilterFid(id)
-                    inFeatB = vlayerA.getFeatures(request).next()
-                    atMapB = inFeatB.attributes()
-                    tmpGeom = QgsGeometry(inFeatB.geometry())
+            for id in intersects:
+                request = QgsFeatureRequest().setFilterFid(id)
+                inFeatB = vlayerA.getFeatures(request).next()
+                atMapB = inFeatB.attributes()
+                tmpGeom = QgsGeometry(inFeatB.geometry())
 
-                    if diff_geom.intersects(tmpGeom):
-                        add = True
-                        diff_geom = QgsGeometry(diff_geom.difference(tmpGeom))
-                        if diff_geom.isGeosEmpty() or not diff_geom.isGeosValid():
-                            ProcessingLog.addToLog(ProcessingLog.LOG_ERROR,
-                                                   self.tr('GEOS geoprocessing error: One or more input features have invalid geometry.'))
-                    else:
-                        try:
-                            # Ihis only happends if the bounding box
-                            # intersects, but the geometry doesn't
-                            outFeat.setGeometry(diff_geom)
-                            outFeat.setAttributes(atMap)
-                            writer.addFeature(outFeat)
-                        except:
-                            ProcessingLog.addToLog(ProcessingLog.LOG_INFO,
-                                                   self.tr('Feature geometry error: One or more output features ignored due to invalid geometry.'))
+                if geom.intersects(tmpGeom):
+                    lstIntersectingA.append(tmpGeom)
 
-            if add:
-                try:
-                    outFeat.setGeometry(diff_geom)
-                    outFeat.setAttributes(atMap)
-                    writer.addFeature(outFeat)
-                except:
-                    ProcessingLog.addToLog(ProcessingLog.LOG_INFO,
-                                           self.tr('Feature geometry error: One or more output features ignored due to invalid geometry.'))
+            if len(lstIntersectingA) == 0:
+                res_geom = geom
+            else:
+                intA = QgsGeometry.unaryUnion(lstIntersectingA)
+                res_geom = geom.difference(intA)
+                if res_geom.isGeosEmpty() or not res_geom.isGeosValid():
+                    ProcessingLog.addToLog(ProcessingLog.LOG_ERROR,
+                                           self.tr('GEOS geoprocessing error: One or more input features have invalid geometry.'))
+
+            # TODO: correctly handly different output geometry types (see todo above)
+
+            try:
+                outFeat.setGeometry(res_geom)
+                outFeat.setAttributes(atMap)
+                writer.addFeature(outFeat)
+            except:
+                ProcessingLog.addToLog(ProcessingLog.LOG_INFO,
+                                       self.tr('Feature geometry error: One or more output features ignored due to invalid geometry.'))
             nElement += 1
 
         del writer
diff --git a/python/plugins/processing/tests/testdata/custom/union1_a.geojson b/python/plugins/processing/tests/testdata/custom/union1_a.geojson
@@ -0,0 +1,10 @@
+{
+"type": "FeatureCollection",
+"crs": { "type": "name", "properties": { "name": "urn:ogc:def:crs:EPSG::3857" } },
+"features": [
+{ "type": "Feature", "properties": { "id_a": "A1" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 1.0, 3.0 ], [ 2.0, 3.0 ], [ 2.0, 10.0 ], [ 8.0, 10.0 ], [ 8.0, 11.0 ], [ 1.0, 11.0 ], [ 1.0, 3.0 ] ] ] } },
+{ "type": "Feature", "properties": { "id_a": "A4" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 3.0, 3.0 ], [ 3.0, 4.0 ], [ 4.0, 4.0 ], [ 4.0, 3.0 ], [ 3.0, 3.0 ] ] ] } },
+{ "type": "Feature", "properties": { "id_a": "A2" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 3.0, 5.0 ], [ 6.0, 5.0 ], [ 6.0, 6.0 ], [ 3.0, 6.0 ], [ 3.0, 5.0 ] ] ] } },
+{ "type": "Feature", "properties": { "id_a": "A3" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 5.0, 7.0 ], [ 9.0, 7.0 ], [ 9.0, 8.0 ], [ 5.0, 8.0 ], [ 5.0, 7.0 ] ] ] } }
+]
+}
diff --git a/python/plugins/processing/tests/testdata/custom/union1_b.geojson b/python/plugins/processing/tests/testdata/custom/union1_b.geojson
@@ -0,0 +1,10 @@
+{
+"type": "FeatureCollection",
+"crs": { "type": "name", "properties": { "name": "urn:ogc:def:crs:EPSG::3857" } },
+"features": [
+{ "type": "Feature", "properties": { "id_b": "B1" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 1.0, 1.0 ], [ 8.0, 1.0 ], [ 8.0, 9.0 ], [ 7.0, 9.0 ], [ 7.0, 2.0 ], [ 1.0, 2.0 ], [ 1.0, 1.0 ] ] ] } },
+{ "type": "Feature", "properties": { "id_b": "B4" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 5.0, 3.0 ], [ 6.0, 3.0 ], [ 6.0, 4.0 ], [ 5.0, 4.0 ], [ 5.0, 3.0 ] ] ] } },
+{ "type": "Feature", "properties": { "id_b": "B2" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 0.0, 5.0 ], [ 4.0, 5.0 ], [ 4.0, 6.0 ], [ 0.0, 6.0 ], [ 0.0, 5.0 ] ] ] } },
+{ "type": "Feature", "properties": { "id_b": "B3" }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 3.0, 7.0 ], [ 6.0, 7.0 ], [ 6.0, 8.0 ], [ 3.0, 8.0 ], [ 3.0, 7.0 ] ] ] } }
+]
+}
diff --git a/python/plugins/processing/tests/testdata/expected/union1.gml b/python/plugins/processing/tests/testdata/expected/union1.gml
@@ -0,0 +1,90 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<ogr:FeatureCollection
+     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+     xsi:schemaLocation="http://ogr.maptools.org/ union1.xsd"
+     xmlns:ogr="http://ogr.maptools.org/"
+     xmlns:gml="http://www.opengis.net/gml">
+  <gml:boundedBy>
+    <gml:Box>
+      <gml:coord><gml:X>0</gml:X><gml:Y>1</gml:Y></gml:coord>
+      <gml:coord><gml:X>9</gml:X><gml:Y>11</gml:Y></gml:coord>
+    </gml:Box>
+  </gml:boundedBy>
+
+  <gml:featureMember>
+    <ogr:union1 fid="union1.0">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>2,6 2,5 1,5 1,6 2,6</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_a>A1</ogr:id_a>
+      <ogr:id_b>B2</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.1">
+      <ogr:geometryProperty><gml:MultiPolygon srsName="EPSG:3857"><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>2,5 2,3 1,3 1,5 2,5</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>1,6 1,11 8,11 8,10 2,10 2,6 1,6</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember></gml:MultiPolygon></ogr:geometryProperty>
+      <ogr:id_a>A1</ogr:id_a>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.2">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>3,3 3,4 4,4 4,3 3,3</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_a>A4</ogr:id_a>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.3">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>4,5 3,5 3,6 4,6 4,5</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_a>A2</ogr:id_a>
+      <ogr:id_b>B2</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.4">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>4,6 6,6 6,5 4,5 4,6</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_a>A2</ogr:id_a>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.5">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>8,7 7,7 7,8 8,8 8,7</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_a>A3</ogr:id_a>
+      <ogr:id_b>B1</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.6">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>6,7 5,7 5,8 6,8 6,7</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_a>A3</ogr:id_a>
+      <ogr:id_b>B3</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.7">
+      <ogr:geometryProperty><gml:MultiPolygon srsName="EPSG:3857"><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>7,7 6,7 6,8 7,8 7,7</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>8,8 9,8 9,7 8,7 8,8</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember></gml:MultiPolygon></ogr:geometryProperty>
+      <ogr:id_a>A3</ogr:id_a>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.8">
+      <ogr:geometryProperty><gml:MultiPolygon srsName="EPSG:3857"><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>8,7 8,1 1,1 1,2 7,2 7,7 8,7</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>7,8 7,9 8,9 8,8 7,8</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember></gml:MultiPolygon></ogr:geometryProperty>
+      <ogr:id_b>B1</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.9">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>5,3 6,3 6,4 5,4 5,3</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_b>B4</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.10">
+      <ogr:geometryProperty><gml:MultiPolygon srsName="EPSG:3857"><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>1,5 0,5 0,6 1,6 1,5</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember><gml:polygonMember><gml:Polygon><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>3,5 2,5 2,6 3,6 3,5</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></gml:polygonMember></gml:MultiPolygon></ogr:geometryProperty>
+      <ogr:id_b>B2</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+  <gml:featureMember>
+    <ogr:union1 fid="union1.11">
+      <ogr:geometryProperty><gml:Polygon srsName="EPSG:3857"><gml:outerBoundaryIs><gml:LinearRing><gml:coordinates>5,7 3,7 3,8 5,8 5,7</gml:coordinates></gml:LinearRing></gml:outerBoundaryIs></gml:Polygon></ogr:geometryProperty>
+      <ogr:id_b>B3</ogr:id_b>
+    </ogr:union1>
+  </gml:featureMember>
+</ogr:FeatureCollection>
diff --git a/python/plugins/processing/tests/testdata/expected/union1.xsd b/python/plugins/processing/tests/testdata/expected/union1.xsd
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<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">
+<xs:import namespace="http://www.opengis.net/gml" schemaLocation="http://schemas.opengis.net/gml/2.1.2/feature.xsd"/>
+<xs:element name="FeatureCollection" type="ogr:FeatureCollectionType" substitutionGroup="gml:_FeatureCollection"/>
+<xs:complexType name="FeatureCollectionType">
+  <xs:complexContent>
+    <xs:extension base="gml:AbstractFeatureCollectionType">
+      <xs:attribute name="lockId" type="xs:string" use="optional"/>
+      <xs:attribute name="scope" type="xs:string" use="optional"/>
+    </xs:extension>
+  </xs:complexContent>
+</xs:complexType>
+<xs:element name="union1" type="ogr:union1_Type" substitutionGroup="gml:_Feature"/>
+<xs:complexType name="union1_Type">
+  <xs:complexContent>
+    <xs:extension base="gml:AbstractFeatureType">
+      <xs:sequence>
+        <xs:element name="geometryProperty" type="gml:PolygonPropertyType" nillable="true" minOccurs="0" maxOccurs="1"/>
+        <xs:element name="id_a" nillable="true" minOccurs="0" maxOccurs="1">
+          <xs:simpleType>
+            <xs:restriction base="xs:string">
+              <xs:maxLength value="255"/>
+            </xs:restriction>
+          </xs:simpleType>
+        </xs:element>
+        <xs:element name="id_b" nillable="true" minOccurs="0" maxOccurs="1">
+          <xs:simpleType>
+            <xs:restriction base="xs:string">
+              <xs:maxLength value="255"/>
+            </xs:restriction>
+          </xs:simpleType>
+        </xs:element>
+      </xs:sequence>
+    </xs:extension>
+  </xs:complexContent>
+</xs:complexType>
+</xs:schema>
diff --git a/python/plugins/processing/tests/testdata/qgis_algorithm_tests.yaml b/python/plugins/processing/tests/testdata/qgis_algorithm_tests.yaml
@@ -583,3 +583,22 @@ tests:
       OUTPUT_LAYER:
         name: expected/point_on_line.gml
         type: vector
+
+  - algorithm: qgis:union
+    name: Test Union (basic)
+    params:
+      INPUT:
+        name: custom/union1_a.geojson
+        type: vector
+      INPUT2:
+        name: custom/union1_b.geojson
+        type: vector
+    results:
+      OUTPUT:
+        name: expected/union1.gml
+        type: vector
+        # TODO: may need improvements to comparison
+        # - unordered comparison
+        #   (features could be written in different order and still being correct output)
+        # - geometry equality comparison instead of WKT string comparison
+        #   (geometries could different order of coordinate but still being correct output)