Opened 13 years ago

Closed 13 years ago

#1586 closed defect (fixed)

[raster] st_dumpaspolygons makes not valid polygons

Reported by: jomarlla Owned by: Bborie Park
Priority: high Milestone: PostGIS 2.0.0
Component: raster Version: master
Keywords: Cc:

Description

Dear PostGIS team,

ST_Dumpaspolygons is building not valid polygons.

select count(*) from (select st_dumpaspolygons (rast) as vect from co1) as tabla where not st_isvalid ((vect).geom);

NOTICE: Ring Self-intersection at or near point 503582.96553112275 4420706.2469141362 NOTICE: Ring Self-intersection at or near point 505882.96553112275 4417206.2469141362 ...282 more errors.

Some cases:: POLYGON((462682.965531123 4376006.24691414,462682.965531123 4375706.24691414,462982.965531123 4375706.24691414,462982.965531123 4375606.24691414,463182.965531123 4375606.24691414,463182.965531123 4375706.24691414,463082.965531123 4375706.24691414,463082.965531123 4375806.24691414,463182.965531123 4375806.24691414,463182.965531123 4375706.24691414,463282.965531123 4375706.24691414,463282.965531123 4375806.24691414,463382.965531123 4375806.24691414,463482.965531123 4375806.24691414,463482.965531123 4375906.24691414,463282.965531123 4375906.24691414,463282.965531123 4376006.24691414,462682.965531123 4376006.24691414))

POLYGON((500282.965531123 4380006.24691414,500282.965531123 4379606.24691414,500382.965531123 4379606.24691414,500382.965531123 4379406.24691414,500582.965531123 4379406.24691414,500582.965531123 4379306.24691414,500682.965531123 4379306.24691414,500682.965531123 4379206.24691414,500782.965531123 4379206.24691414,500782.965531123 4379506.24691414,500582.965531123 4379506.24691414,500582.965531123 4379606.24691414,500482.965531123 4379606.24691414,500482.965531123 4379706.24691414,500382.965531123 4379706.24691414,500382.965531123 4379806.24691414,500482.965531123 4379806.24691414,500482.965531123 4379706.24691414,500582.965531123 4379706.24691414,500582.965531123 4379906.24691414,500382.965531123 4379906.24691414,500382.965531123 4380006.24691414,500282.965531123 4380006.24691414))

POLYGON((516082.965531123 4364206.24691414,516082.965531123 4363806.24691414,516382.965531123 4363806.24691414,516382.965531123 4363906.24691414,516282.965531123 4363906.24691414,516282.965531123 4364006.24691414,516182.965531123 4364006.24691414,516182.965531123 4364106.24691414,516282.965531123 4364106.24691414,516282.965531123 4364006.24691414,516382.965531123 4364006.24691414,516382.965531123 4363906.24691414,516582.965531123 4363906.24691414,516582.965531123 4364006.24691414,516682.965531123 4364006.24691414,516782.965531123 4364006.24691414,516782.965531123 4364206.24691414,516582.965531123 4364206.24691414,516582.965531123 4364106.24691414,516482.965531123 4364106.24691414,516482.965531123 4364206.24691414,516082.965531123 4364206.24691414))

2 notes:

ST_Makevalid fixes all of them and I think in a right way.

Any of the fixed polygons become a multipolygon after fixing. Regards,

Change History (11)

comment:1 by pramsey, 13 years ago

Component: postgisraster
Owner: changed from pramsey to pracine

comment:2 by pracine, 13 years ago

We could simply add a ST_MakeValid() call to the plpgsql st_dumpaspolygons function (at a little performance cost). Users can also easily do it with the geom part of the geomval.

any good reason why we would like to do it at the source?

What did the fact that the returned geometry were invalid prevented you to do?

comment:3 by pracine, 13 years ago

Summary: st_dumpaspolygons makes not valid polygons[raster] st_dumpaspolygons makes not valid polygons

comment:4 by jomarlla, 13 years ago

ST_Makevalid is making st_dumpaspolygons much slower. Dont know if it is worth it to do it at the source. I was wondering if the algorithm can be changed for not making invalid polygons instead of using st_makevalid without a lot of work.

I just dont like invalid polygons because they just bother me about any future spatial analysis about unpredictable results. As you said users can do it easily but in that case maybe the docs should point out that st_dumpaspolygons can return invalid geometries, then the user can take care about that.

Jose

in reply to:  4 comment:5 by pracine, 13 years ago

Replying to jomarlla:

ST_Makevalid is making st_dumpaspolygons much slower. Dont know if it is worth it to do it at the source. I was wondering if the algorithm can be changed for not making invalid polygons instead of using st_makevalid without a lot of work.

I guess the amount of work for producing a valid geometry from what GDAL is returning us would be equivalent to ST_MakeValid but I might be wrong.

I just dont like invalid polygons because they just bother me about any future spatial analysis about unpredictable results. As you said users can do it easily but in that case maybe the docs should point out that st_dumpaspolygons can return invalid geometries, then the user can take care about that.

If there is no simple low cost way to make them valid we will add a note. It's the first time someone is complaining about that so I guess the type of invalidity does not affect most analyses. Would be great if you could find an example of invalid post st_dumpaspolygon analysis.

comment:6 by Bborie Park, 13 years ago

After doing some thinking about GDALPolygonize() and GDALFPolygonize() (both used by ST_DumpAsPolygons), it is possible/probably for invalid polygons to be generated. From the GDAL API docs for GDALPolygonize:

This function creates vector polygons for all connected regions of pixels in the raster sharing a common pixel value

Two pixels with the same value with one common corner may cause a bow-tie...

   +--+
   |  |
   |  |
+--+--+
|  |
|  |
+--+

I don't see any way to make GDAL generate valid polygons but we could add a validation routine to the appropriate C code in rt_api.c.

comment:7 by pracine, 13 years ago

There is already a ticket (#637) to convert wkt geometries to wkb geometries. We could probably call some the right GEOS function to make it valid afterward. So first make it a true geometry second make it a valid one all in C. That should reduce the performance impact. I would set this one to 2.0.1.

comment:8 by Bborie Park, 13 years ago

Owner: changed from pracine to Bborie Park
Status: newassigned

comment:9 by Bborie Park, 13 years ago

So for a simple bow-tie situation, ST_DumpAsPolygons returns two polygons

WITH foo AS (
	SELECT
		ST_SetValue(
			ST_SetValue(
				ST_AddBand(
					ST_MakeEmptyRaster(2, 2, 0, 0, 1, 1, 0, 0, 0)
					, '8BUI'::text, 0, 0
				)
				, 1, 1, 1
			)
			, 2, 2, 1
		) AS rast
)
SELECT
	ST_AsText((ST_DumpAsPolygons(rast)).geom)
FROM foo

Need to find an example of a raster dumping invalid polygons.

comment:10 by Bborie Park, 13 years ago

Success! Found one...

WITH foo AS (
	SELECT (ST_DumpAsPolygons(
		ST_SetValue(
			ST_SetValue(
				ST_AddBand(
					ST_MakeEmptyRaster(4, 4, 0, 0, 1, 1, 0, 0, 0)
					, '8BUI'::text, 1, 0
				)
				, 2, 2, 0
			)
			, 3, 3, 0
		)
	)).geom AS geom
)
SELECT
	ST_IsValid(geom),
	ST_AsText(geom)
FROM foo

comment:11 by Bborie Park, 13 years ago

Resolution: fixed
Status: assignedclosed

Fixed in r9280

Note: See TracTickets for help on using tickets.