[postgis-tickets] r16509 - GEOSNode is not robust, avoid it in MakeValid.
Darafei
komzpa at gmail.com
Fri Mar 30 12:05:57 PDT 2018
Author: komzpa
Date: 2018-03-30 12:05:56 -0700 (Fri, 30 Mar 2018)
New Revision: 16509
Modified:
trunk/liblwgeom/cunit/cu_geos.c
trunk/liblwgeom/lwgeom_geos_clean.c
Log:
GEOSNode is not robust, avoid it in MakeValid.
Thread: https://lists.osgeo.org/pipermail/postgis-devel/2018-March/027078.html
Closes #4601
Closes https://github.com/postgis/postgis/pull/235
Modified: trunk/liblwgeom/cunit/cu_geos.c
===================================================================
--- trunk/liblwgeom/cunit/cu_geos.c 2018-03-28 15:01:18 UTC (rev 16508)
+++ trunk/liblwgeom/cunit/cu_geos.c 2018-03-30 19:05:56 UTC (rev 16509)
@@ -109,6 +109,25 @@
lwgeom_free(geom2);
}
+static void
+test_geos_makevalid(void)
+{
+ uint8_t* wkb;
+ char* out_ewkt;
+ LWGEOM* geom1;
+ LWGEOM* geom2;
+
+ wkb = (uint8_t*) "\001\003\000\000\000\001\000\000\000\011\000\000\000b\020X9 }\366 at 7\211A\340\235I\034A\316\326t18}\366@\306g\347\323\230I\034Ay\351&18}\366@\331\316\367\323\230I\034A\372~j\274\370}\366@\315\314\314LpI\034A\343\245\233\304R}\366 at R\270\036\005?I\034A\315\314\314\314Z~\366@\343\245\233\304\007I\034A\004V\016-\242}\366@\252\361\322M\323H\034A\351&1\010\306{\366 at H\341z\0247I\034Ab\020X9 }\366 at 7\211A\340\235I\034A";
+ geom1 = lwgeom_from_wkb(wkb, 157, LW_PARSER_CHECK_NONE);
+ geom2 = lwgeom_make_valid(geom1);
+ out_ewkt = lwgeom_to_ewkt((LWGEOM*)geom2);
+ ASSERT_STRING_EQUAL(out_ewkt, "GEOMETRYCOLLECTION(POLYGON((92114.014 463463.469,92115.5120743 463462.206937,92115.512 463462.207,92127.546 463452.075,92117.173 463439.755,92133.675 463425.942,92122.136 463412.826,92092.377 463437.77,92114.014 463463.469)),MULTIPOINT(92115.5120743 463462.206937,92122.136 463412.826))");
+ lwfree(out_ewkt);
+ lwgeom_free(geom1);
+ lwgeom_free(geom2);
+}
+
+
static void test_geos_subdivide(void)
{
#if POSTGIS_GEOS_VERSION < 35
@@ -150,4 +169,5 @@
PG_ADD_TEST(suite, test_geos_subdivide);
PG_ADD_TEST(suite, test_geos_linemerge);
PG_ADD_TEST(suite, test_geos_offsetcurve);
+ PG_ADD_TEST(suite, test_geos_makevalid);
}
Modified: trunk/liblwgeom/lwgeom_geos_clean.c
===================================================================
--- trunk/liblwgeom/lwgeom_geos_clean.c 2018-03-28 15:01:18 UTC (rev 16508)
+++ trunk/liblwgeom/lwgeom_geos_clean.c 2018-03-30 19:05:56 UTC (rev 16509)
@@ -320,37 +320,21 @@
static GEOSGeometry*
LWGEOM_GEOS_nodeLines(const GEOSGeometry* lines)
{
- GEOSGeometry* noded;
+ /* GEOS3.7 GEOSNode fails on regression tests */
+ /* GEOS3.7 GEOSUnaryUnion fails on regression tests */
- /* first, try just to node the line */
- noded = GEOSNode(lines);
- if (!noded) noded = (GEOSGeometry *)lines;
-
- /* GEOS3.7 UnaryUnion fails on regression tests, cannot be used here */
-
- /* fall back to union of first point with geometry */
- if (!GEOSisValid(noded))
- {
- GEOSGeometry *unioned, *point;
- point = LWGEOM_GEOS_getPointN(lines, 0);
- if (!point) return NULL;
- unioned = GEOSUnion(noded, point);
- GEOSGeom_destroy(point);
- if (!unioned)
- return NULL;
- else
- {
- GEOSGeom_destroy(noded);
- return unioned;
- }
- }
- return noded;
+ /* union of first point with geometry */
+ GEOSGeometry *unioned, *point;
+ point = LWGEOM_GEOS_getPointN(lines, 0);
+ if (!point) return NULL;
+ unioned = GEOSUnion(lines, point);
+ GEOSGeom_destroy(point);
+ return unioned;
}
/*
* We expect initGEOS being called already.
* Will return NULL on error (expect error handler being called by then)
- *
*/
static GEOSGeometry*
LWGEOM_GEOS_makeValidPolygon(const GEOSGeometry* gin)
@@ -360,32 +344,16 @@
GEOSGeom geos_cut_edges, geos_area, collapse_points;
GEOSGeometry* vgeoms[3]; /* One for area, one for cut-edges */
unsigned int nvgeoms = 0;
-#if POSTGIS_DEBUG_LEVEL >= 3
- LWGEOM *geos_geom;
- char *geom_ewkt;
-#endif
assert(GEOSGeomTypeId(gin) == GEOS_POLYGON || GEOSGeomTypeId(gin) == GEOS_MULTIPOLYGON);
geos_bound = GEOSBoundary(gin);
- if (NULL == geos_bound) return NULL;
+ if (!geos_bound) return NULL;
-#if POSTGIS_DEBUG_LEVEL >= 3
- geos_geom = GEOS2LWGEOM(geos_bound, 0);
- geom_ewkt = lwgeom_to_ewkt(geos_geom);
- LWDEBUGF(3, "Boundaries: %s", geom_ewkt);
- lwgeom_free(geos_geom);
- lwfree(geom_ewkt);
-#endif
-
/* Use noded boundaries as initial "cut" edges */
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: noding lines");
-#endif
-
geos_cut_edges = LWGEOM_GEOS_nodeLines(geos_bound);
- if (NULL == geos_cut_edges)
+ if (!geos_cut_edges)
{
GEOSGeom_destroy(geos_bound);
lwnotice("LWGEOM_GEOS_nodeLines(): %s", lwgeom_geos_errmsg);
@@ -398,12 +366,8 @@
GEOSGeometry* pi;
GEOSGeometry* po;
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: extracting unique points from bounds");
-#endif
-
pi = GEOSGeom_extractUniquePoints(geos_bound);
- if (NULL == pi)
+ if (!pi)
{
GEOSGeom_destroy(geos_bound);
lwnotice("GEOSGeom_extractUniquePoints(): %s", lwgeom_geos_errmsg);
@@ -410,20 +374,8 @@
return NULL;
}
-#if POSTGIS_DEBUG_LEVEL >= 3
- geos_geom = GEOS2LWGEOM(pi, 0);
- geom_ewkt = lwgeom_to_ewkt(geos_geom);
- LWDEBUGF(3, "Boundaries input points %s", geom_ewkt);
- lwgeom_free(geos_geom);
- lwfree(geom_ewkt);
-#endif
-
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: extracting unique points from cut_edges");
-#endif
-
po = GEOSGeom_extractUniquePoints(geos_cut_edges);
- if (NULL == po)
+ if (!po)
{
GEOSGeom_destroy(geos_bound);
GEOSGeom_destroy(pi);
@@ -431,20 +383,8 @@
return NULL;
}
-#if POSTGIS_DEBUG_LEVEL >= 3
- geos_geom = GEOS2LWGEOM(po, 0);
- geom_ewkt = lwgeom_to_ewkt(geos_geom);
- LWDEBUGF(3, "Boundaries output points %s", geom_ewkt);
- lwgeom_free(geos_geom);
- lwfree(geom_ewkt);
-#endif
-
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: find collapse points");
-#endif
-
collapse_points = GEOSDifference(pi, po);
- if (NULL == collapse_points)
+ if (!collapse_points)
{
GEOSGeom_destroy(geos_bound);
GEOSGeom_destroy(pi);
@@ -453,31 +393,11 @@
return NULL;
}
-#if POSTGIS_DEBUG_LEVEL >= 3
- geos_geom = GEOS2LWGEOM(collapse_points, 0);
- geom_ewkt = lwgeom_to_ewkt(geos_geom);
- LWDEBUGF(3, "Collapse points: %s", geom_ewkt);
- lwgeom_free(geos_geom);
- lwfree(geom_ewkt);
-#endif
-
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: cleanup(1)");
-#endif
-
GEOSGeom_destroy(pi);
GEOSGeom_destroy(po);
}
GEOSGeom_destroy(geos_bound);
-#if POSTGIS_DEBUG_LEVEL >= 3
- geos_geom = GEOS2LWGEOM(geos_cut_edges, 0);
- geom_ewkt = lwgeom_to_ewkt(geos_geom);
- LWDEBUGF(3, "Noded Boundaries: %s", geom_ewkt);
- lwgeom_free(geos_geom);
- lwfree(geom_ewkt);
-#endif
-
/* And use an empty geometry as initial "area" */
geos_area = GEOSGeom_createEmptyPolygon();
if (!geos_area)
@@ -525,15 +445,7 @@
}
/*
- * We succeeded in building a ring !
- */
-
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: ring built with %d cut edges, saving boundaries",
- GEOSGetNumGeometries(geos_cut_edges));
-#endif
-
- /*
+ * We succeeded in building a ring!
* Save the new ring boundaries first (to compute
* further cut edges later)
*/
@@ -540,8 +452,7 @@
new_area_bound = GEOSBoundary(new_area);
if (!new_area_bound)
{
- /* We did check for empty area already so
- * this must be some other error */
+ /* We did check for empty area already so this must be some other error */
lwnotice("GEOSBoundary('%s') threw an error: %s",
lwgeom_to_ewkt(GEOS2LWGEOM(new_area, 0)),
lwgeom_geos_errmsg);
@@ -550,10 +461,6 @@
return NULL;
}
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: running SymDifference with new area");
-#endif
-
/*
* Now symdif new and old area
*/
@@ -584,10 +491,6 @@
*
*/
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: computing new cut_edges (GEOSDifference)");
-#endif
-
new_cut_edges = GEOSDifference(geos_cut_edges, new_area_bound);
GEOSGeom_destroy(new_area_bound);
if (!new_cut_edges) /* an exception ? */
@@ -595,8 +498,7 @@
/* cleanup and throw */
GEOSGeom_destroy(geos_cut_edges);
GEOSGeom_destroy(geos_area);
- /* TODO: Shouldn't this be an lwerror ? */
- lwnotice("GEOSDifference() threw an error: %s", lwgeom_geos_errmsg);
+ lwerror("GEOSDifference() threw an error: %s", lwgeom_geos_errmsg);
return NULL;
}
GEOSGeom_destroy(geos_cut_edges);
@@ -603,10 +505,6 @@
geos_cut_edges = new_cut_edges;
}
-#ifdef LWGEOM_PROFILE_MAKEVALID
- lwnotice("ST_MakeValid: final checks");
-#endif
-
if (!GEOSisEmpty(geos_area))
vgeoms[nvgeoms++] = geos_area;
else
@@ -634,8 +532,7 @@
if (!gout) /* an exception again */
{
/* cleanup and throw */
- /* TODO: Shouldn't this be an lwerror ? */
- lwnotice("GEOSGeom_createCollection() threw an error: %s", lwgeom_geos_errmsg);
+ lwerror("GEOSGeom_createCollection() threw an error: %s", lwgeom_geos_errmsg);
/* TODO: cleanup! */
return NULL;
}
More information about the postgis-tickets
mailing list