[postgis-tickets] [SCM] PostGIS branch stable-3.4 updated. 3.4.0-30-gbfab1e9b7
git at osgeo.org
git at osgeo.org
Tue Oct 10 02:57:59 PDT 2023
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, stable-3.4 has been updated
via bfab1e9b74bc196d5e7e5d1a8a0faea46877a24a (commit)
from 05ea0b3557949bc81adfeaa5ac88abd034cabb72 (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 bfab1e9b74bc196d5e7e5d1a8a0faea46877a24a
Author: Sandro Santilli <strk at kbt.io>
Date: Mon Oct 9 12:09:38 2023 +0200
Fix robustness issue in topology face split handling
Stop using interpolation to find edges to update in _lwt_AddFaceSplit.
Includes regress test.
References #5568 in stable-3.4 branch (3.4.1dev)
diff --git a/NEWS b/NEWS
index 12b99a950..c4b60eb10 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ To take advantage of all SFCGAL featurs, SFCGAL 1.4.1+ is needed.
* Bug Fixes *
+ - #5568, Improve robustness of topology face split handling (Sandro Santilli)
- #5548, Fix box-filtered validity check of topologies with edge-less faces
(Sandro Santilli)
- #5485, Fix postgis script on OpenBSD (Sandro Santilli)
diff --git a/liblwgeom/lwgeom_topo.c b/liblwgeom/lwgeom_topo.c
index df573fdde..a8b0a2e50 100644
--- a/liblwgeom/lwgeom_topo.c
+++ b/liblwgeom/lwgeom_topo.c
@@ -1908,7 +1908,12 @@ _lwt_AddFaceSplit( LWT_TOPOLOGY* topo,
LWDEBUGF(1, "Edge %" LWTFMT_ELEMID " split face %" LWTFMT_ELEMID " (mbr_only:%d)",
sedge, face, mbr_only);
- /* Construct a polygon using edges of the ring */
+ /*
+ * Construct a polygon using edges of the ring
+ *
+ * NOTE: this possibily includes dangling edges
+ *
+ */
LWPOLY *shell = _lwt_MakeRingShell(topo, signed_edge_ids,
num_signed_edge_ids);
if ( ! shell ) {
@@ -2055,7 +2060,7 @@ _lwt_AddFaceSplit( LWT_TOPOLOGY* topo,
lwerror("Backend error: %s", lwt_be_lastErrorMessage(topo->be_iface));
return -2;
}
- LWDEBUGF(1, "_lwt_AddFaceSplit: lwt_be_getEdgeByFace returned %d edges", numfaceedges);
+ LWDEBUGF(1, "_lwt_AddFaceSplit: lwt_be_getEdgeByFace(%d) returned %d edges", face, numfaceedges);
if ( numfaceedges )
{
@@ -2072,70 +2077,68 @@ _lwt_AddFaceSplit( LWT_TOPOLOGY* topo,
int contains;
POINT2D ep;
- /* (2.1) skip edges whose ID is in the list of boundary edges ? */
+ /* (2.1) skip edges whose ID is in the list of boundary edges */
for ( j=0; j<num_signed_edge_ids; ++j )
{
- int seid = signed_edge_ids[j];
+ int seid = signed_edge_ids[j]; /* signed ring edge id */
if ( seid == e->edge_id )
{
- /* IDEA: remove entry from signed_edge_ids ? */
- LWDEBUGF(1, "Edge %d is a forward edge of the new ring", e->edge_id);
+ /* IDEA: remove entry from signed_edge_ids, to speed next loop ? */
+ LWDEBUGF(1, "Edge %d is a known forward edge of the new ring", e->edge_id);
forward_edges[forward_edges_count].edge_id = e->edge_id;
forward_edges[forward_edges_count++].face_left = newface.face_id;
found++;
- if ( found == 2 ) break;
+ if ( found == 2 ) break; /* both edge sides are found on the ring */
}
else if ( -seid == e->edge_id )
{
- /* IDEA: remove entry from signed_edge_ids ? */
- LWDEBUGF(1, "Edge %d is a backward edge of the new ring", e->edge_id);
+ /* IDEA: remove entry from signed_edge_ids, to speed next loop ? */
+ LWDEBUGF(1, "Edge %d is a known backward edge of the new ring", e->edge_id);
backward_edges[backward_edges_count].edge_id = e->edge_id;
backward_edges[backward_edges_count++].face_right = newface.face_id;
found++;
- if ( found == 2 ) break;
+ if ( found == 2 ) break; /* both edge sides are found on the ring */
}
}
if ( found ) continue;
+ LWDEBUGF(1, "Edge %d is not a known edge of the new ring", e->edge_id);
- /* We need to check only a single point
- * (to avoid collapsed elements of the shell polygon
- * giving false positive).
- * The point but must not be an endpoint.
- */
- if ( ! _lwt_GetInteriorEdgePoint(e->geom, &ep) )
+ /* Check if the edge is now binding a different face */
+
+ if ( ! getPoint2d_p(e->geom->points, 0, &ep) )
{
lwfree(signed_edge_ids);
lwpoly_free(shell);
lwfree(forward_edges); /* contents owned by edges */
lwfree(backward_edges); /* contents owned by edges */
_lwt_release_edges(edges, numfaceedges);
- lwerror("Could not find interior point for edge %d: %s",
- e->edge_id, lwgeom_geos_errmsg);
+ lwerror("Edge %d is empty", e->edge_id);
return -2;
}
/* IDEA: check that bounding box shortcut is taken, or use
* shellbox to do it here */
- contains = ptarray_contains_point(pa, &ep) == LW_INSIDE;
- if ( contains == 2 )
- LWDEBUGF(1, "Edge %d %scontained in new ring", e->edge_id,
- (contains?"":"not "));
+ contains = ptarray_contains_point(pa, &ep);
- /* (2.2) skip edges (NOT, if newface_outside) contained in shell */
+ LWDEBUGF(1, "Edge %d first point %s new ring",
+ e->edge_id, (contains == LW_INSIDE ? "inside" :
+ contains == LW_OUTSIDE ? "outside" : "on boundary of"));
+
+ /* (2.2) skip edges (NOT, if newface_outside) contained in ring */
if ( newface_outside )
{
- if ( contains )
+ if ( contains != LW_OUTSIDE )
{
- LWDEBUGF(1, "Edge %d contained in an hole of the new face",
+ LWDEBUGF(1, "Edge %d not outside of the new ring, not updating it",
e->edge_id);
continue;
}
}
else
{
- if ( ! contains )
+ if ( contains != LW_INSIDE )
{
- LWDEBUGF(1, "Edge %d not contained in the face shell",
+ LWDEBUGF(1, "Edge %d not inside the new ring, not updating it",
e->edge_id);
continue;
}
diff --git a/topology/test/regress/topogeo_addlinestring.sql b/topology/test/regress/topogeo_addlinestring.sql
index 84fa46a16..8ddf5c914 100644
--- a/topology/test/regress/topogeo_addlinestring.sql
+++ b/topology/test/regress/topogeo_addlinestring.sql
@@ -501,3 +501,15 @@ SELECT 'b5234.1', count(*) FROM
TopoGeo_addLinestring('b5234',
'LINESTRING(10 10 3, 5 0 0.8)', 0);
ROLLBACK;
+
+-- See https://trac.osgeo.org/postgis/ticket/5568
+BEGIN;
+SELECT NULL FROM topology.CreateTopology ('t5568');
+SELECT NULL FROM topology.TopoGeo_addLinestring('t5568','0102000000030000007A6873CA73782440DE38B01D9CEE4D40438AD46B73782440FEAF5D499CEE4D40588FC2F528782440106D5B2FB5EE4D40');
+SELECT NULL FROM topology.TopoGeo_addLinestring('t5568','010200000002000000588FC2F528782440106D5B2FB5EE4D4089058AA63E7824401D9FCAB6B0EE4D40');
+SELECT NULL FROM topology.TopoGeo_addLinestring('t5568','01020000000300000089058AA63E7824401D9FCAB6B0EE4D4040A8CCBF35782440B80E7F8CB2EE4D40588FC2F528782440106D5B2FB5EE4D40');
+SELECT NULL FROM topology.TopoGeo_addLinestring('t5568','01020000000200000089058AA63E7824401D9FCAB6B0EE4D4084CBFA5C7A7824405CC705259CEE4D40');
+SELECT 't5568', 'invalidities at start', array_agg((v)) FROM validatetopology('t5568') v;
+SELECT NULL FROM topology.TopoGeo_addLinestring('t5568','01020000000200000084CBFA5C7A7824405CC705259CEE4D407A6873CA73782440DE38B01D9CEE4D40');
+SELECT 't5568', 'invalidities at end', array_agg(error) FROM validatetopology('t5568') v;
+ROLLBACK;
diff --git a/topology/test/regress/topogeo_addlinestring_expected b/topology/test/regress/topogeo_addlinestring_expected
index de9b8d8fe..6b433feae 100644
--- a/topology/test/regress/topogeo_addlinestring_expected
+++ b/topology/test/regress/topogeo_addlinestring_expected
@@ -225,3 +225,5 @@ b5081|dangling_edges|1
b5081|faces|1
b5234.0|1
b5234.1|1
+t5568|invalidities at start|
+t5568|invalidities at end|
-----------------------------------------------------------------------
Summary of changes:
NEWS | 1 +
liblwgeom/lwgeom_topo.c | 57 ++++++++++++----------
topology/test/regress/topogeo_addlinestring.sql | 12 +++++
.../test/regress/topogeo_addlinestring_expected | 2 +
4 files changed, 45 insertions(+), 27 deletions(-)
hooks/post-receive
--
PostGIS
More information about the postgis-tickets
mailing list