[postgis-tickets] r14968 - Fix wrapping of multigeometry elements becoming multi on wrap

Sandro Santilli strk at kbt.io
Thu Jun 16 07:04:27 PDT 2016


Author: strk
Date: 2016-06-16 07:04:27 -0700 (Thu, 16 Jun 2016)
New Revision: 14968

Modified:
   trunk/liblwgeom/cunit/cu_wrapx.c
   trunk/liblwgeom/lwgeom_wrapx.c
Log:
Fix wrapping of multigeometry elements becoming multi on wrap

Closes #454 again

Modified: trunk/liblwgeom/cunit/cu_wrapx.c
===================================================================
--- trunk/liblwgeom/cunit/cu_wrapx.c	2016-06-16 13:04:05 UTC (rev 14967)
+++ trunk/liblwgeom/cunit/cu_wrapx.c	2016-06-16 14:04:27 UTC (rev 14968)
@@ -24,9 +24,9 @@
 	geom = lwgeom_from_wkt(
 		"POLYGON EMPTY",
 		LW_PARSER_CHECK_NONE);
-	CU_ASSERT(geom != NULL);
+	CU_ASSERT_FATAL(geom != NULL);
 	ret = lwgeom_wrapx(geom, 0, 20);
-	CU_ASSERT(ret != NULL);
+	CU_ASSERT_FATAL(ret != NULL);
 	obt_wkt = lwgeom_to_ewkt(ret);
 	exp_wkt = "POLYGON EMPTY";
 	ASSERT_STRING_EQUAL(obt_wkt, exp_wkt);
@@ -37,9 +37,9 @@
 	geom = lwgeom_from_wkt(
 		"POINT(0 0)",
 		LW_PARSER_CHECK_NONE);
-	CU_ASSERT(geom != NULL);
+	CU_ASSERT_FATAL(geom != NULL);
 	ret = lwgeom_wrapx(geom, 2, 10);
-	CU_ASSERT(ret != NULL);
+	CU_ASSERT_FATAL(ret != NULL);
 	obt_wkt = lwgeom_to_ewkt(ret);
 	exp_wkt = "POINT(10 0)";
 	ASSERT_STRING_EQUAL(obt_wkt, exp_wkt);
@@ -50,9 +50,9 @@
 	geom = lwgeom_from_wkt(
 		"POINT(0 0)",
 		LW_PARSER_CHECK_NONE);
-	CU_ASSERT(geom != NULL);
+	CU_ASSERT_FATAL(geom != NULL);
 	ret = lwgeom_wrapx(geom, 0, 20);
-	CU_ASSERT(ret != NULL);
+	CU_ASSERT_FATAL(ret != NULL);
 	obt_wkt = lwgeom_to_ewkt(ret);
 	exp_wkt = "POINT(0 0)";
 	ASSERT_STRING_EQUAL(obt_wkt, exp_wkt);
@@ -63,9 +63,9 @@
 	geom = lwgeom_from_wkt(
 		"POINT(0 0)",
 		LW_PARSER_CHECK_NONE);
-	CU_ASSERT(geom != NULL);
+	CU_ASSERT_FATAL(geom != NULL);
 	ret = lwgeom_wrapx(geom, 0, -20);
-	CU_ASSERT(ret != NULL);
+	CU_ASSERT_FATAL(ret != NULL);
 	obt_wkt = lwgeom_to_ewkt(ret);
 	exp_wkt = "POINT(0 0)";
 	ASSERT_STRING_EQUAL(obt_wkt, exp_wkt);
@@ -76,9 +76,9 @@
 	geom = lwgeom_from_wkt(
 		"LINESTRING(0 0,10 0)",
 		LW_PARSER_CHECK_NONE);
-	CU_ASSERT(geom != NULL);
+	CU_ASSERT_FATAL(geom != NULL);
 	ret = lwgeom_wrapx(geom, 8, -10);
-	CU_ASSERT(ret != NULL);
+	CU_ASSERT_FATAL(ret != NULL);
 	obt_wkt = lwgeom_to_ewkt(ret);
 	exp_wkt = "MULTILINESTRING((0 0,8 0),(-2 0,0 0))";
 	ASSERT_STRING_EQUAL(obt_wkt, exp_wkt);
@@ -89,9 +89,9 @@
 	geom = lwgeom_from_wkt(
 		"MULTILINESTRING((-5 -2,0 0),(0 0,10 10))",
 		LW_PARSER_CHECK_NONE);
-	CU_ASSERT(geom != NULL);
+	CU_ASSERT_FATAL(geom != NULL);
 	ret = lwgeom_wrapx(geom, 0, 20);
-	CU_ASSERT(ret != NULL);
+	CU_ASSERT_FATAL(ret != NULL);
 	obt_wkt = lwgeom_to_ewkt(ret);
 	exp_wkt = "MULTILINESTRING((15 -2,20 0),(0 0,10 10))";
 	ASSERT_STRING_EQUAL(obt_wkt, exp_wkt);
@@ -100,15 +100,40 @@
 	lwgeom_free(geom);
 
 	geom = lwgeom_from_wkt(
+		"MULTIPOLYGON("
+		" ((0 0,10 0,10 10,0 10,0 0),(2 2,4 2,4 4,2 4,2 2)),"
+		" ((0 11,10 11,10 21,0 21,0 11),(2 13,4 13,4 15,2 15,2 13))"
+		")",
+		LW_PARSER_CHECK_NONE);
+	CU_ASSERT_FATAL(geom != NULL);
+	ret = lwgeom_wrapx(geom, 2, 20);
+	CU_ASSERT_FATAL(ret != NULL);
+	obt_wkt = lwgeom_to_ewkt(ret);
+	exp_wkt = "GEOMETRYCOLLECTION("
+						"MULTIPOLYGON("
+						"((22 0,20 0,20 10,22 10,22 4,22 2,22 0)),"
+						"((2 10,10 10,10 0,2 0,2 2,4 2,4 4,2 4,2 10))"
+						"),"
+						"MULTIPOLYGON("
+						"((22 11,20 11,20 21,22 21,22 15,22 13,22 11)),"
+						"((2 21,10 21,10 11,2 11,2 13,4 13,4 15,2 15,2 21))"
+						")"
+						")";
+	ASSERT_STRING_EQUAL(obt_wkt, exp_wkt);
+	lwfree(obt_wkt);
+	lwgeom_free(ret);
+	lwgeom_free(geom);
+
+	geom = lwgeom_from_wkt(
 		"GEOMETRYCOLLECTION("
 		" MULTILINESTRING((-5 -2,0 0),(0 0,10 10)),"
 		" POINT(-5 0),"
 		" POLYGON EMPTY"
 		")",
 		LW_PARSER_CHECK_NONE);
-	CU_ASSERT(geom != NULL);
+	CU_ASSERT_FATAL(geom != NULL);
 	ret = lwgeom_wrapx(geom, 0, 20);
-	CU_ASSERT(ret != NULL);
+	CU_ASSERT_FATAL(ret != NULL);
 	obt_wkt = lwgeom_to_ewkt(ret);
 	exp_wkt = "GEOMETRYCOLLECTION("
 						"MULTILINESTRING((15 -2,20 0),(0 0,10 10)),"

Modified: trunk/liblwgeom/lwgeom_wrapx.c
===================================================================
--- trunk/liblwgeom/lwgeom_wrapx.c	2016-06-16 13:04:05 UTC (rev 14967)
+++ trunk/liblwgeom/lwgeom_wrapx.c	2016-06-16 14:04:27 UTC (rev 14968)
@@ -18,7 +18,7 @@
  *
  **********************************************************************
  *
- * Copyright 2016 Sandro Santilli <strk at kbt.net>
+ * Copyright 2016 Sandro Santilli <strk at kbt.io>
  *
  **********************************************************************/
 
@@ -52,7 +52,7 @@
 	box_in = lwgeom_get_bbox(geom_in);
 	if ( ! box_in ) {
 		/* must be empty */
-		return lwgeom_clone(geom_in);
+		return lwgeom_clone_deep(geom_in);
 	}
 
 	LWDEBUGF(2, "BOX X range is %g..%g, cutx:%g, amount:%g", box_in->xmin, box_in->xmax, cutx, amount);
@@ -61,18 +61,17 @@
 	if ( ( amount < 0 && box_in->xmin >= cutx ) || ( amount > 0 && box_in->xmax <= cutx ) )
 	{
 		split = lwgeom_clone_deep(geom_in);
-		LWDEBUG(2, "returning the translated geometry");
 		lwgeom_affine(split, &affine);
+		LWDEBUGG(2, split, "returning the translated geometry");
 		return split;
 	}
 
-//DEBUG2: [lwgeom_wrapx.c:lwgeom_split_wrapx:58] BOX X range is 8..10, cutx:8, amount:-10
-
 	/* Check if geometry is fully on the side needing no shift */
 	if ( ( amount < 0 && box_in->xmax <= cutx ) || ( amount > 0 && box_in->xmin >= cutx ) )
 	{
-		LWDEBUG(2, "returning the cloned geometry");
-		return lwgeom_clone_deep(geom_in);
+		split = lwgeom_clone_deep(geom_in);
+		LWDEBUGG(2, split, "returning the cloned geometry");
+		return split;
 	}
 
 	/* We need splitting here */
@@ -91,19 +90,28 @@
 	/* split by blade */
 	split = lwgeom_split(geom_in, blade);
 	lwgeom_free(blade);
+	if ( ! split ) {
+		lwerror("%s:%d - lwgeom_split_wrapx:  %s", __FILE__, __LINE__, lwgeom_geos_errmsg);
+		return NULL;
+	}
+	LWDEBUGG(2, split, "split geometry");
 
+
 	/* iterate over components, translate if needed */
 	const LWCOLLECTION *col = lwgeom_as_lwcollection(split);
 	if ( ! col ) {
 		/* not split, this is unexpected */
 		lwnotice("WARNING: unexpected lack of split in lwgeom_split_wrapx");
-		return lwgeom_clone(geom_in);
+		return lwgeom_clone_deep(geom_in);
 	}
 	LWCOLLECTION *col_out = lwcollection_wrapx(col, cutx, amount);
 	lwgeom_free(split);
 
 	/* unary-union the result (homogenize too ?) */
 	LWGEOM* out = lwgeom_unaryunion(lwcollection_as_lwgeom(col_out));
+	LWDEBUGF(2, "col_out:%p, unaryunion_out:%p", col_out, out);
+	LWDEBUGG(2, out, "unary-unioned");
+
 	lwcollection_free(col_out);
 
 	return out;
@@ -112,9 +120,10 @@
 static LWCOLLECTION*
 lwcollection_wrapx(const LWCOLLECTION* lwcoll_in, double cutx, double amount)
 {
-	LWGEOM** wrap_geoms=NULL;
+	LWGEOM** wrap_geoms;
 	LWCOLLECTION* out;
 	size_t i;
+	int outtype = lwcoll_in->type;
 
 	wrap_geoms = lwalloc(lwcoll_in->ngeoms * sizeof(LWGEOM*));
 	if ( ! wrap_geoms )
@@ -125,18 +134,30 @@
 
 	for (i=0; i<lwcoll_in->ngeoms; ++i)
 	{
+		LWDEBUGF(3, "Wrapping collection element %d", i);
 		wrap_geoms[i] = lwgeom_wrapx(lwcoll_in->geoms[i], cutx, amount);
 		/* an exception should prevent this from ever returning NULL */
 		if ( ! wrap_geoms[i] ) {
-			while (--i>=0) lwgeom_free(wrap_geoms[i]);
+			lwnotice("Error wrapping geometry, cleaning up");
+			while ((--i)>=0) {
+				lwnotice("cleaning geometry %d (%p)", i, wrap_geoms[i]);
+				lwgeom_free(wrap_geoms[i]);
+			}
 			lwfree(wrap_geoms);
+			lwnotice("cleanup complete");
 			return NULL;
 		}
+	  if ( outtype != COLLECTIONTYPE ) {
+			if ( MULTITYPE[wrap_geoms[i]->type] != outtype )
+			{
+				outtype = COLLECTIONTYPE;
+			}
+		}
 	}
 
 	/* Now wrap_geoms has wrap_geoms_size geometries */
-	out = lwcollection_construct(lwcoll_in->type, lwcoll_in->srid,
-	                             NULL, lwcoll_in->ngeoms, wrap_geoms);
+	out = lwcollection_construct(outtype, lwcoll_in->srid, NULL,
+	                             lwcoll_in->ngeoms, wrap_geoms);
 
 	return out;
 }
@@ -146,15 +167,24 @@
 lwgeom_wrapx(const LWGEOM* lwgeom_in, double cutx, double amount)
 {
 	/* Nothing to wrap in an empty geom */
-	if ( lwgeom_is_empty(lwgeom_in) ) return lwgeom_clone(lwgeom_in);
+	if ( lwgeom_is_empty(lwgeom_in) )
+	{
+		LWDEBUG(2, "geom is empty, cloning");
+		return lwgeom_clone_deep(lwgeom_in);
+	}
 
 	/* Nothing to wrap if shift amount is zero */
-	if ( amount == 0 ) return lwgeom_clone(lwgeom_in);
+	if ( amount == 0 )
+	{
+		LWDEBUG(2, "amount is zero, cloning");
+		return lwgeom_clone_deep(lwgeom_in);
+	}
 
 	switch (lwgeom_in->type)
 	{
 	case LINETYPE:
 	case POLYGONTYPE:
+		LWDEBUG(2, "split-wrapping line or polygon");
 		return lwgeom_split_wrapx(lwgeom_in, cutx, amount);
 
 	case POINTTYPE:
@@ -177,6 +207,7 @@
 	case MULTIPOLYGONTYPE:
 	case MULTILINETYPE:
 	case COLLECTIONTYPE:
+		LWDEBUG(2, "collection-wrapping multi");
 		return lwcollection_as_lwgeom(
 						lwcollection_wrapx((const LWCOLLECTION*)lwgeom_in, cutx, amount)
 					 );



More information about the postgis-tickets mailing list