[postgis-tickets] [SCM] PostGIS branch master updated. 3.4.0rc1-669-g83fbad6d5

git at osgeo.org git at osgeo.org
Tue Oct 10 02:48:46 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, master has been updated
       via  83fbad6d5213208f4af25a07c9ac2ed20e334ea9 (commit)
      from  e7343702a4f796e4e508211863d3a63918edf726 (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 83fbad6d5213208f4af25a07c9ac2ed20e334ea9
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 master branch (3.5.0dev)

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:
 liblwgeom/lwgeom_topo.c                            | 57 ++++++++++++----------
 topology/test/regress/topogeo_addlinestring.sql    | 12 +++++
 .../test/regress/topogeo_addlinestring_expected    |  2 +
 3 files changed, 44 insertions(+), 27 deletions(-)


hooks/post-receive
-- 
PostGIS


More information about the postgis-tickets mailing list