Opened 11 years ago

Closed 10 years ago

#297 closed task (fixed)

[wktraster] Implement ST_DumpAsPolygons function for raster

Reported by: mloskot Owned by: jorgearevalo
Priority: high Milestone: WKTRaster 0.1.6
Component: raster Version: master
Keywords: Cc:

Description

Function ST_AsPolygon is part of Objective 0.1.6e and shall be implemented:

ST_AsPolygon(raster) -> polygon geometry set

This algorithm is very useful since it ensure that ST_AsRaster(ST_AsPolygon(raster)) should returns the same raster as the original (because it is based on the centroid of the pixel).

Open questions:

  • Pierre: Can we directly use the GDAL function by copying the raster data into a GDAL structure or should we just copy/adapt the algorythm?

References:

Attachments (1)

pele.zip (147.8 KB) - added by robe 11 years ago.

Download all attachments as: .zip

Change History (37)

comment:1 Changed 11 years ago by mloskot

Pierre,

Technically, we can use GDAL function, namely GDALPolygonize.

However, we have to keep in mind what steps it would require on various levels of use and deployment. It is, roughly:

  • GDAL requirement, compile-time and run-time
  • Translation of WKT Raster WKB to GDALDataset with bands (likely using GDAL MEM driver)
  • Call to GDALPolygonize to generate OGRLayer
  • Translation of OGRGeometry objects attached to OGRLayer to PostGIS geometry

comment:2 Changed 11 years ago by jorgearevalo

Owner: changed from pracine to jorgearevalo

comment:3 Changed 11 years ago by mloskot

Summary: Implement ST_AsPolygon function for raster[wktraster] Implement ST_AsPolygon function for raster

comment:4 Changed 11 years ago by pracine

Type: defecttask

comment:5 Changed 11 years ago by pracine

Version: trunk

comment:6 Changed 11 years ago by pracine

Priority: mediumhigh

comment:7 Changed 11 years ago by jorgearevalo

Status: newassigned

comment:8 in reply to:  1 Changed 11 years ago by pracine

Replying to mloskot:

Pierre,

Technically, we can use GDAL function, namely GDALPolygonize.

However, we have to keep in mind what steps it would require on various levels of use and deployment. It is, roughly:

  • GDAL requirement, compile-time and run-time
  • Translation of WKT Raster WKB to GDALDataset with bands (likely using GDAL MEM driver)
  • Call to GDALPolygonize to generate OGRLayer
  • Translation of OGRGeometry objects attached to OGRLayer to PostGIS geometry

Sounds good!

comment:9 Changed 11 years ago by jorgearevalo

We should take into account that GDALPolygonize uses a signed 32bit integer buffer (Int32) for the band data. Floating point values will be truncated before processing. This is a limitation for complex bands.

comment:10 Changed 11 years ago by jorgearevalo

rt_raster_as_wktpolygon implemented and tested in r5449 with this one-band raster: http://trac.osgeo.org/postgis/attachment/wiki/WKTRaster/SpecificationWorking01/WKTRasterEnvelopeConvexHullAndShape.gif

Comments welcome

comment:11 Changed 11 years ago by jorgearevalo

Bug solved: Now only the polygons with stored value distinct from nodata value for the band are return. In r5456.

comment:12 Changed 11 years ago by robe

Pierre et al. We'll probably have a lot of questions and bug reports coming up since we are working on the WKT Raster chapter of book.

For the above -- is this exposed as an SQL function yet. Didn't see it. It also seems to rely on GDAL so not sure if that is an additional dependency we need to compile in to expose this or not.

Thanks, Regina

comment:13 in reply to:  12 Changed 11 years ago by jorgearevalo

Replying to robe:

Pierre et al. We'll probably have a lot of questions and bug reports coming up since we are working on the WKT Raster chapter of book.

For the above -- is this exposed as an SQL function yet. Didn't see it. It also seems to rely on GDAL so not sure if that is an additional dependency we need to compile in to expose this or not.

Thanks, Regina

Hi Regina,

We don't have a SQL function yet, only a core C function. I'll create a SQL function ASAP. Sorry for delay, but I was involved in a different project (and studying) until 2 days ago. And the C function relys on GDAL, yes, but only if you specify the "--enable-development" option during configure. If not, a "dummy" function that only returns 0 is compiled. We may change this in the future.

Best regards, Jorge

comment:14 Changed 11 years ago by jorgearevalo

Created ST_DumpASPolygons in r5516. The base function, called RASTER_dumpAsWKTPolygons, is under conditional compilation, because still doesn't work properly. A default "dummy" version returning NULL has been created if no '--enable-development' option is passed to configure script. Any comments welcome.

comment:15 Changed 11 years ago by pracine

What is the purpose of the dummy function?

comment:16 Changed 11 years ago by pracine

Could you document the compilation process now including GDAL in section 2 of the documentation. What, how do we compile...

http://trac.osgeo.org/postgis/wiki/WKTRaster/Documentation01

comment:17 in reply to:  15 Changed 11 years ago by jorgearevalo

Replying to pracine:

What is the purpose of the dummy function?

Simply to have a function to compile when you don't specify --enable-development option. I thought in this option as a way to activate "experimental" or "under development" features. If you don't specify it, you have a function to compile (a needed function, actually), but a dummy function. For a short time.

comment:18 Changed 11 years ago by jorgearevalo

Implemented in r5582. Needs more testing. So, I leave the ticket open by now. Related ticket #494.

comment:19 Changed 11 years ago by jorgearevalo

Minor bug solved in r5583.

comment:20 Changed 11 years ago by robe

Jorge,

I believe you called this function ST_Polygon?

When I run against this:

SELECT ST_Polygon(rast,1)
FROM pele

I get error ERROR: lwline_deserialize: attempt to deserialize a line which is really a Polygon.

Anyrate haven't upgraded to 1.5.2SVN yet. Same dataset as #494.

I'll attach the dataset to this ticket.

Changed 11 years ago by robe

Attachment: pele.zip added

comment:22 Changed 11 years ago by robe

Ah okay. This ticket is confusing. So I guess this ticket is redundant and I should have opened another ticket for ST_Polygon. I forgot that ST_AsPolygon is what we now call ST_DumpAsPolygons.

Anyrate ST_Polygon should be the same as ST_Union(ST_DumpAsPolygons) -- correct?

comment:23 Changed 11 years ago by jorgearevalo

Don't worry, my fault. I should have remarked the ticket title was confusing when we decided to change the name of the function.

And about ST_Union, tecnically yes. But I'd like to implement a C function, similar to the one implemented for ST_DumpAsPolygons, to make it faster. Anyway, I suppose we could work with ST_Union(ST_DumpAsPolygons) by now...

comment:24 Changed 11 years ago by pracine

Summary: [wktraster] Implement ST_AsPolygon function for raster[wktraster] Implement ST_DumpAsPolygons function for raster

comment:25 Changed 11 years ago by jorgearevalo

Thanks Mateusz! How could I forgot to change the name... Stupid me :-)

comment:26 Changed 11 years ago by jorgearevalo

Function ST_Polygon, that collects all the single geometries returned by st_dumpaspolygons, implemented in r5605

comment:27 Changed 11 years ago by jorgearevalo

BTW, I'm going to delete the conditional compilation of ST_DumpAsPolygons function. This is important, because now we force the existence of GDAL in the system. Any vote against it?

comment:28 in reply to:  27 Changed 11 years ago by pracine

Replying to jorgearevalo:

BTW, I'm going to delete the conditional compilation of ST_DumpAsPolygons function. This is important, because now we force the existence of GDAL in the system. Any vote against it?

Is GDAL easy to compile for both Windows and Linux users? Is the process well documented? We should ensure this before.

comment:29 Changed 11 years ago by jorgearevalo

I think GDAL shouldn't be more difficult to compile on Windows than GEOS, for example. And GEOS is required. But I'm not a Windows developer. I can't ensure it. And ok... GEOS may be easier to compile, but the point is if we're going to compile WKT Raster against GDAL anyway...

About Linux, I think most users will be able to compile the library, at least if they could compile PostGIS, or GEOS.

The process is well documented here: http://trac.osgeo.org/gdal/wiki/BuildHints

comment:30 Changed 11 years ago by pracine

Regina what do you suggest? Are we ready to force our users to compile and link with GDAL?

comment:31 Changed 11 years ago by robe

I would go for it. libgdal seemed easier to compile than GEOS actually and compiles faster. So I don't consider it a major obstacle.

If you are doing a minimalist compile which is all that is needed to enable ST_DumpAsPolygons and I assume the others, I don't see it as a major barrier.

Most windows users will just download binaries anyway as the PostGIS compile process under windows is a bare to begin with. I imagine compiling WKT Raster even with GDAL is even easier under Linux, though I haven't tried yet.

comment:32 Changed 11 years ago by pracine

Then +1

comment:33 Changed 11 years ago by robe

Jorge,

For your r5605 -- may I be so bold as to suggest you use ST_Union instead of ST_Collect. ST_Collect may be faster but I think you will end up with many invalid polygons. Though in this example ST_Union is faster (I guess because of the new cascaded union feature)

For example

-- Takes 4.2 seconds

SELECT rid,ST_IsValid(ST_Collect(geom)) As geom
FROM (SELECT rid, (ST_DumpAsPolygons(rast,1)).*
FROM ch13.usdem WHERE rid = 1) As foo
WHERE val <> 0
GROUP BY rid

Gives false

-- Takes 3.8 seconds

SELECT rid,ST_IsValid(ST_Union(geom)) As geom
FROM (SELECT rid, (ST_DumpAsPolygons(rast,1)).*
FROM ch13.usdem WHERE rid = 1) As foo
WHERE val <> 0
GROUP BY rid


Gives true

to union the whole set above takes about 5 minutes. It looks about right though and didn't give any errors.

comment:34 Changed 11 years ago by jorgearevalo

I forgot to anwer this. I changed it. Now, we're using st_union instead of st_collect. Thanks Regina!

comment:35 Changed 10 years ago by jorgearevalo

I think this ticket may be closed. Working on Linux and Windows. I hope working on Mac...

comment:36 Changed 10 years ago by jorgearevalo

Resolution: fixed
Status: assignedclosed

The function is implemented. So, I close this ticket for the time being.

Note: See TracTickets for help on using tickets.