[raster] Optimize ST_DumpAsWKTPolygons

[dzwarg]

I looked at the code for ST_DumpAsWKTPolygons(), and identified a good place for optimization after talking it over with Frank W at the code sprint this afternoon.

In rt_raster_dump_as_wktpolygons (see trunk/raster/rt_core/rt_api.c@6923#L1728​, after calling GDALPolygonize, there are notes regarding what to do with polygons that contain NODATA cell values. Instead of iterating over both loops (which both create and destroy all features in the memory layer -- copying and destroying them, effectively), it would probably be more performant to do the following steps after trunk/raster/rt_core/rt_api.c@6923#L2002​:

1. Add a sql-like filter to the layer to select the features that are +/- FLT_EPSILON.
2. Iterate over all those features, and delete them from the layer.
3. Remove the filter, and get all the remaining features in the memory layer.

This approach means that all features in the layer are read once, even though there are 2 loops. The first loop would cycle over the features to remove, and the second loop would cycle over the features to create and return.

[jorgearevalo]

Actually, there's no need of double loop. The first loop's intention was to count the valid features (polygons with a pixel value different from NODATA). But applying a spatial filter forces OGR_L_GetFeatureCount to take that filter into account, and OGR_L_GetNextFeature to return only valid features. We only need one loop.

Working on it...

[jorgearevalo]

I meant "applying an attributes filter", with OGR_L_SetAttributeFilter.

[jorgearevalo]

Implemented in r6975. I close the ticket.

Just for the record: I trust OGR SQL executor to compare floating point numbers using "!=". Check it and reopen the ticket if the comparison:

doubleValue != anotherDoubleValue

is not safe.

[pracine]

Did you get any performance improvement?

[pracine]

This commit generated three new warnings...

[jorgearevalo]

It works faster, because I changed two loops into only one. Working on those warnings...

[pracine]

I get the regress test rt_spatial_relationship.shp to fail after your change. Apparently the set of polygon returned is different than before. It should not.

[jorgearevalo]

Fixed some bugs. I'm going to have a look at spatial_relationship test right now.

[jorgearevalo]

Replying to jorgearevalo:

Fixed some bugs. I'm going to have a look at spatial_relationship test right now.

Forgot to say the release: r6980

[jorgearevalo]

rt_spatial_relationship is working for me at r6984. Do you still have problems with it?

[pracine]

rt_spatial_relationship works but create_rt_gist_test fail:

*** create_rt_gist_test_expected	Tue Mar 29 11:32:59 2011
--- /tmp/rtpgis_reg_5884/test_25_out	Tue Mar 29 11:35:18 2011
***************
*** 1,2 ****
! SELECT 100
! SELECT 9
--- 1,2 ----
! SELECT
! SELECT

[jorgearevalo]

Fixed in r6986. The problem is I was testing with PostgreSQL 9.0.3. With that version, the expected output is:

SELECT 100 SELECT 9

instead of

SELECT SELECT

This will probably require a new ticket.