Opened 3 months ago

Closed 8 weeks ago

Last modified 8 weeks ago

#5792 closed defect (fixed)

TopoGeo_addPoint silently corrupts topology

Reported by: strk Owned by: strk
Priority: high Milestone: PostGIS 3.5.1
Component: topology Version: 3.5.x
Keywords: Cc:

Description

With the new experimental code snapping existing edges to incoming nodes there are cases in which attracted edges swap position around a pre-existing node. The case at hand was submitted in https://trac.osgeo.org/postgis/ticket/5786#comment:4 making this a spin off of ticket #5786

For this ticket we want to either raise an exception OR keep the topology valid.

This only happens from PostGIS-3.5 because prior to that version we never snapped existing edges to incoming node other than a single one if found to contain the incoming node.

Change History (10)

comment:1 by strk, 3 months ago

The test:

select NULL FROM topology.DropTopology ('t5792');                                                                                                                                              
select NULL FROM topology.CreateTopology ('t5792');                                                                                                                                            
                                                                                                                                                                                               
SELECT NULL FROM topology.TopoGeo_addLinestring('t5792', 'LINESTRING(                                                                                                                          
11.812075769533624 59.77938755222866,                                                                                                                                                          
11.811862389533625 59.77938237222866                                                                                                                                                           
)');                                                                                                                                                                                           
                                                                                                                                                                                               
SELECT NULL FROM topology.TopoGeo_addLinestring('t5792', 'LINESTRING(                                                                                                                          
11.811862389533625 59.77938237222866,                                                                                                                                                          
11.811969079533624 59.77938496222866,                                                                                                                                                          
11.812075769533624 59.77938755222866                                                                                                                                                           
)');                                                                                                                                                                                           
                                                                                                                                                                                               
SELECT * FROM ValidateTopology('t5792');  -- topology is valid here                                                                                                                                                     
                                                                                                                                                                                               
BEGIN;                      

-- This breaks the topology                                                                                                                                                                   
SELECT NULL FROM topology.TopoGeo_addPoint('t5792',                                                                                                                                            
'POINT(11.812029186127067 59.7793864213727)');                                                                                                                                                 
                                                                                                                                                                                               
SELECT * FROM ValidateTopology('t5792'); -- topology is INVALID here  

The introduced invalidities are reported as:

             error              | id1 | id2 
--------------------------------+-----+-----
 universal face has shell rings |   0 |    
 face has wrong mbr             |   1 |    
 hole not in advertised face    |  -3 |    

comment:2 by strk, 3 months ago

https://git.osgeo.org/gitea/postgis/postgis/pulls/226 prevents the invalidation of topology replacing it with an error like:

ERROR:  Ring on the left of edge 1 would change winding by snapping to POINT(11.8120291861271 59.7793864213727)

comment:3 by Sandro Santilli <strk@…>, 2 months ago

In f02b6db/git:

Check motion range upon snap/splitting edge to existing node

References #5792 (fixing it)
References #5786 (not resolving the problem)
Includes regress test

comment:4 by Sandro Santilli <strk@…>, 2 months ago

Resolution: fixed
Status: newclosed

In 158efb88/git:

Check motion range upon snap/splitting edge to existing node

Fixes #5792 in 3.5 branch (3.5.1dev)
References #5786 (not resolving the problem)
Includes regress test

comment:5 by strk, 2 months ago

Resolution: fixed
Status: closedreopened

I'm reopening this because the new check seems to give different outcome with GEOS-3.8 than it gives with GEOS-3.9 and we're back to wonder who's right and who's wrong about the fact that snapping the edge to the new node moves it past the other edge :(

GEOS-3.8: https://github.com/postgis/postgis/actions/runs/11579339434/job/32235298841#step:4:2430

GEOS-3.9: https://github.com/postgis/postgis/actions/runs/11583139573/job/32247684590#step:4:2799

comment:6 by strk, 2 months ago

The GEOS-3.8 used in GHA is 3.8.2dev-CAPI-1.13.3 — current last and EOLed version is 3.8.4 — I'll try that

comment:7 by strk, 2 months ago

I don't get the failures with GEOS-3.8.4 locally, but I do get other failures which are ticketed as #5805

comment:8 by strk, 2 months ago

I was wrong, I DO get the failures with GEOS-3.8.4 too:

 topology/test/regress/topogeo_addpoint_merge_edges .. failed (diff expected obtained: /tmp/pgis_reg/test_1_diff)
-----------------------------------------------------------------------------
--- topology/test/regress/topogeo_addpoint_merge_edges_expected 2024-10-29 10:58:55.965585948 +0100
+++ /tmp/pgis_reg/test_1_out    2024-10-30 09:56:59.595821417 +0100
@@ -1,21 +1,21 @@
-sn.CW-back-merge|-checking-
-sn.CW-back-merge.ep-edges|-checking-
-sn.CW-back-merge.ep-edges-back|-checking-
+ERROR:  snapping edge 3 to new node moves it past edge 2
+ERROR:  snapping edge 3 to new node moves it past edge 2
+ERROR:  snapping edge 3 to new node moves it past edge 2
 sn.CCW-back-merge|-checking-
 sn.CCW-back-merge.ep-edges|-checking-
 sn.CCW-back-merge.ep-edges-back|-checking-
-sn.CW-forw-merge|-checking-
-sn.CW-forw-merge.ep-edges|-checking-
-sn.CW-forw-merge.ep-edges-back|-checking-
+ERROR:  snapping edge 3 to new node moves it past edge 4
+ERROR:  snapping edge 3 to new node moves it past edge 6
+ERROR:  snapping edge 3 to new node moves it past edge 6
 sn.CCW-forw-merge|-checking-
 sn.CCW-forw-merge.ep-edges|-checking-
 sn.CCW-forw-merge.ep-edges-back|-checking-
-en.CW-back-merge|-checking-
-en.CW-back-merge.ep-edges|-checking-
-en.CW-back-merge.ep-edges-back|-checking-
-en.CW-forw-merge|-checking-
-en.CW-forw-merge.ep-edges|-checking-
-en.CW-forw-merge.ep-edges-back|-checking-
+ERROR:  snapping edge 3 to new node moves it past edge 4
+ERROR:  snapping edge 3 to new node moves it past edge 6
+ERROR:  snapping edge 3 to new node moves it past edge 6
+ERROR:  snapping edge 3 to new node moves it past edge 2
+ERROR:  snapping edge 3 to new node moves it past edge 2
+ERROR:  snapping edge 3 to new node moves it past edge 2
 snap-CW|-checking-
 snap-CW-newface-sp|-checking-
 snap-CW-newface-ep|-checking-
-----------------------------------------------------------------------------

comment:9 by Sandro Santilli <strk@…>, 8 weeks ago

Resolution: fixed
Status: reopenedclosed

In 6366bd6/git:

Check motion range upon snap/splitting edge to existing node

This time make the polygon valid before using Covers on it

Closes #5792 in master branch (3.6.0dev)
References #5786

comment:10 by Sandro Santilli <strk@…>, 8 weeks ago

In 58d45d6/git:

Check motion range upon snap/splitting edge to existing node

This time make the polygon valid before using Covers on it

Closes #5792 in 3.5 branch (3.5.1dev)
References #5786

Note: See TracTickets for help on using tickets.