[postgis-tickets] [SCM] PostGIS branch main updated. 3.1.0rc1-294-g7b2bc9a
git at osgeo.org
git at osgeo.org
Fri Jul 9 05:48:29 PDT 2021
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "PostGIS".
The branch, main has been updated
via 7b2bc9a07b5bbcc422231fa6bd526c7e4ad4ea54 (commit)
from c6d0af446c1c88ca369b03eab461b6267a7496f5 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 7b2bc9a07b5bbcc422231fa6bd526c7e4ad4ea54
Author: Sandro Santilli <strk at kbt.io>
Date: Thu Jul 8 23:49:27 2021 +0200
ValidateTopology check for faces with multiple shells
Closes #4945
Includes regress test
diff --git a/NEWS b/NEWS
index e15db67..3de871f 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ PostGIS 3.2.0
- #4933, topology.GetFaceByPoint will not work with topologies having invalid edge linking.
* Enhancements *
+ - #4945, Multi-shell face check in ValidateTopology (Sandro Santilli)
- #4944, Side-location conflict check in ValidateTopology (Sandro Santilli)
- #3042, ValidateTopology check for edge linking (Sandro Santilli)
- #3276, ValidateTopology check for face's mbr (Sandro Santilli)
diff --git a/topology/sql/manage/ValidateTopology.sql.in b/topology/sql/manage/ValidateTopology.sql.in
index 9c813e8..4592159 100644
--- a/topology/sql/manage/ValidateTopology.sql.in
+++ b/topology/sql/manage/ValidateTopology.sql.in
@@ -211,7 +211,7 @@ LANGUAGE 'plpgsql' VOLATILE;
--
-- NOTE: assumes search_path was set before calling this function
--
-CREATE OR REPLACE FUNCTION topology._ValidateTopologySideLabeling(bbox geometry DEFAULT NULL)
+CREATE OR REPLACE FUNCTION topology._ValidateTopologyRings(bbox geometry DEFAULT NULL)
RETURNS SETOF topology.ValidateTopology_ReturnType
AS --{
$BODY$
@@ -220,13 +220,11 @@ DECLARE
nextEdge INT;
rec RECORD;
affected_rows integer;
+ ring_poly GEOMETRY;
+ is_shell BOOLEAN;
BEGIN
RAISE DEBUG 'Gathering edges for side faces labeling';
- -- NOTE: this check relies on correct edge linking,
- -- if those are not correct the results
- -- of this check do not make much sense.
-
-- Pick all edges ending on any node in the bbox
-- Those are the only edges we want to consider
CREATE TEMP TABLE side_label_check_edge AS
@@ -249,7 +247,14 @@ BEGIN
CREATE INDEX ON side_label_check_edge (edge_id);
- RAISE DEBUG 'Checking edge side faces labeling';
+ CREATE TEMP TABLE ring_check (
+ ring_id int,
+ ring_geom geometry, -- polygon for shell, line for hole
+ is_shell boolean,
+ bound_face int
+ );
+
+ RAISE DEBUG 'Checking edge rings';
-- Find all rings that can be formed on both sides
-- of selected edges
@@ -258,6 +263,8 @@ BEGIN
-- Fetch next unvisited edge
SELECT edge_id
FROM pg_temp.side_label_check_edge
+ -- TODO: order by <-> from -inf/-inf to ensure catching
+ -- shells before holes
ORDER BY edge_id
LIMIT 1
INTO nextEdge;
@@ -268,66 +275,72 @@ BEGIN
affected_rows := affected_rows + 1;
+ -- Build ring formed on nextEdge
-- Gather side faces for the ring formed on nextEdge
- -- NOTE: if edge linking is bogus the following query
- -- won't make much sense
- BEGIN
-
- WITH RECURSIVE
- edgering AS (
- SELECT
- nextEdge as signed_edge_id,
- edge_id,
- next_left_edge,
- next_right_edge,
- CASE WHEN nextEdge > 0 THEN
- left_face
- ELSE
- right_face
- END as side_face
- FROM edge_data
- WHERE edge_id = abs(nextEdge)
- UNION
- SELECT
- CASE WHEN p.signed_edge_id < 0
+ WITH RECURSIVE
+ edgering AS (
+ SELECT
+ nextEdge as signed_edge_id,
+ edge_id,
+ geom,
+ next_left_edge,
+ next_right_edge,
+ CASE WHEN nextEdge > 0 THEN
+ left_face
+ ELSE
+ right_face
+ END as side_face
+ FROM edge_data
+ WHERE edge_id = abs(nextEdge)
+ UNION
+ SELECT
+ CASE WHEN p.signed_edge_id < 0
+ THEN
+ p.next_right_edge
+ ELSE
+ p.next_left_edge
+ END, -- signed_edge_id
+ e.edge_id,
+ e.geom,
+ e.next_left_edge,
+ e.next_right_edge,
+ CASE WHEN p.signed_edge_id < 0
+ THEN
+ CASE WHEN p.next_right_edge > 0
THEN
- p.next_right_edge
+ e.left_face
ELSE
- p.next_left_edge
- END, -- signed_edge_id
- e.edge_id,
- e.next_left_edge,
- e.next_right_edge,
- CASE WHEN p.signed_edge_id < 0
+ e.right_face
+ END
+ ELSE
+ CASE WHEN p.next_left_edge > 0
THEN
- CASE WHEN p.next_right_edge > 0
- THEN
- e.left_face
- ELSE
- e.right_face
- END
+ e.left_face
ELSE
- CASE WHEN p.next_left_edge > 0
- THEN
- e.left_face
- ELSE
- e.right_face
- END
- END -- side_face
- FROM edge_data e, edgering p
- WHERE
- e.edge_id = CASE
- WHEN p.signed_edge_id < 0 THEN
- abs(p.next_right_edge)
- ELSE
- abs(p.next_left_edge)
- END
- )
- SELECT
- array_agg(signed_edge_id) edges,
- array_agg(DISTINCT side_face) side_faces
- FROM edgering
- INTO rec;
+ e.right_face
+ END
+ END -- side_face
+ FROM edge_data e, edgering p
+ WHERE
+ e.edge_id = CASE
+ WHEN p.signed_edge_id < 0 THEN
+ abs(p.next_right_edge)
+ ELSE
+ abs(p.next_left_edge)
+ END
+ )
+ SELECT
+ ST_MakeLine(
+ CASE WHEN signed_edge_id > 0 THEN
+ geom
+ ELSE
+ ST_Reverse(geom)
+ END
+ ) ring_geom,
+ array_agg(signed_edge_id) edges,
+ array_agg(DISTINCT side_face) side_faces
+ FROM edgering
+ INTO rec;
-- RAISE DEBUG 'Ring % - edges:[%], faces:[%]',
-- nextEdge,
@@ -335,36 +348,87 @@ BEGIN
-- array_to_string(rec.side_faces, ',')
-- ;
- -- Check that there's a single face advertised
- IF array_upper(rec.side_faces,1) != 1
- THEN
- RAISE DEBUG 'Side faces found on ring %: %', nextEdge, rec.side_faces;
- retrec.error = 'mixed face labeling in ring';
- retrec.id1 = nextEdge;
- retrec.id2 = NULL;
- RETURN NEXT retrec;
- END IF;
- EXCEPTION WHEN OTHERS THEN
- retrec.error = SQLERRM;
- retrec.id1 = nextEdge;
- retrec.id2 = NULL;
- RETURN NEXT retrec;
- -- Make sure this edge will be marked as visited
- SELECT ARRAY[nextEdge] as edges INTO rec;
- END;
+ -- Check that there's a single face advertised
+ IF array_upper(rec.side_faces,1) != 1
+ THEN --{
+
+ RAISE DEBUG 'Side faces found on ring %: %', nextEdge, rec.side_faces;
+ retrec.error = 'mixed face labeling in ring';
+ retrec.id1 = nextEdge;
+ retrec.id2 = NULL;
+ RETURN NEXT retrec;
+
+ ELSE --}{
+
+ -- Save ring that a single face is ever used
+ -- for each distinct CCW ring (shell)
+ -- NOTE: multiple CW rings (holes) can exist for a given face
+ --RAISE DEBUG 'Ring geom: %', ST_AsTexT(rec.ring_geom);
+ IF NOT ST_Equals(
+ ST_StartPoint(rec.ring_geom),
+ ST_EndPoint(rec.ring_geom)
+ )
+ THEN --{
+-- -- This should have been reported before,
+-- -- on the edge linking check
+-- retrec.error = 'non-closed ring';
+-- retrec.id1 = nextEdge;
+-- retrec.id2 = NULL;
+-- RETURN NEXT retrec;
+ ELSE --}{
+ is_shell := false;
+ IF ST_NPoints(rec.ring_geom) > 3 THEN
+ ring_poly := ST_MakePolygon(rec.ring_geom);
+ IF ST_IsPolygonCCW(ring_poly) THEN
+ is_shell := true;
+ END IF;
+ END IF;
+ INSERT INTO ring_check VALUES (
+ nextEdge,
+ CASE WHEN is_shell THEN
+ ring_poly
+ ELSE
+ rec.ring_geom
+ END,
+ is_shell,
+ rec.side_faces[1]
+ );
+ END IF; --}
+
+ END IF; --}
- -- TODO: Check that a single face is ever used
- -- for each distinct CCW ring (shell)
- -- NOTE: multiple CW rings (holes) can exist for a given face
DELETE FROM pg_temp.side_label_check_edge
WHERE edge_id = ANY (rec.edges);
END LOOP; --}
- RAISE DEBUG 'Completed checking % rings for side faces labeling', affected_rows;
+ -- Check if any face has multiple shells
+ RAISE DEBUG 'Checking multi-shell faces';
+ FOR rec IN
+ SELECT
+ rc.bound_face,
+ count(rc.ring_id)
+ FROM ring_check rc
+ WHERE rc.is_shell
+ GROUP BY rc.bound_face
+ HAVING count(rc.ring_id) > 1
+ LOOP
+ retrec.error = 'face has multiple shells';
+ retrec.id1 = rec.bound_face;
+ retrec.id2 = NULL;
+ RETURN NEXT retrec;
+ END LOOP;
+
+ -- TODO: Check that holes really belong to the advertised shell
+ -- NOTE: when limited by bounding box we might be missing the
+ -- shell belonging to the most external holes, so for
+ -- those we would need to
+
+ RAISE DEBUG 'Completed checking % rings', affected_rows;
DROP TABLE pg_temp.side_label_check_edge;
+ DROP TABLE pg_temp.ring_check;
END;
@@ -669,8 +733,8 @@ BEGIN
--- Validate edge linking
RETURN QUERY SELECT * FROM topology._ValidateTopologyEdgeLinking(bbox);
- --- Validate side-labeling
- RETURN QUERY SELECT * FROM topology._ValidateTopologySideLabeling(bbox);
+ --- Validate edge rings
+ RETURN QUERY SELECT * FROM topology._ValidateTopologyRings(bbox);
-- Now create a temporary table to construct all face geometries
diff --git a/topology/test/regress/validatetopology.sql b/topology/test/regress/validatetopology.sql
index b51b3c5..7c1c824 100644
--- a/topology/test/regress/validatetopology.sql
+++ b/topology/test/regress/validatetopology.sql
@@ -159,5 +159,25 @@ SELECT '#4944', '---', null, null
ORDER BY 1,2,3,4;
ROLLBACK;
+-- Test face with multiple shells is caught
+-- See https://trac.osgeo.org/postgis/ticket/4945
+BEGIN;
+UPDATE city_data.edge_data SET right_face = 3 WHERE edge_id IN (8, 17);
+UPDATE city_data.edge_data SET left_face = 3 WHERE edge_id IN (11, 15);
+-- To reduce the noise,
+-- set face 3 mbr to include the mbr of face 5
+-- and delete face 5
+UPDATE city_data.face SET mbr = (
+ SELECT ST_Envelope(ST_Collect(mbr))
+ FROM city_data.face
+ WHERE face_id IN ( 3, 5 )
+) WHERE face_id = 3;
+DELETE FROM city_data.face WHERE face_id = 5;
+SELECT '#4945', * FROM ValidateTopology('city_data')
+UNION
+SELECT '#4945', '---', null, null
+ORDER BY 1,2,3,4;
+ROLLBACK;
+
SELECT NULL FROM topology.DropTopology('city_data');
diff --git a/topology/test/regress/validatetopology_expected b/topology/test/regress/validatetopology_expected
index 340796f..e2e9256 100644
--- a/topology/test/regress/validatetopology_expected
+++ b/topology/test/regress/validatetopology_expected
@@ -46,3 +46,5 @@
#4944|---||
#4944|mixed face labeling in ring|-21|
#4944|mixed face labeling in ring|-19|
+#4945|---||
+#4945|face has multiple shells|3|
-----------------------------------------------------------------------
Summary of changes:
NEWS | 1 +
topology/sql/manage/ValidateTopology.sql.in | 232 +++++++++++++++---------
topology/test/regress/validatetopology.sql | 20 ++
topology/test/regress/validatetopology_expected | 2 +
4 files changed, 171 insertions(+), 84 deletions(-)
hooks/post-receive
--
PostGIS
More information about the postgis-tickets
mailing list