Opened 12 years ago
Closed 12 years ago
#4005 closed enhancement (fixed)
GDALPolygonize should also be able to read raster values into a 32bit float buffer
|Reported by:||pracine||Owned by:||warmerdam|
And even a 64bit double if possible.
This is necessary for PostGIS raster to be able to polygonize many elevation or temperature rasters.
Change History (10)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
|Status:||new → closed|
Implemented in r22476.
comment:3 by , 12 years ago
|Status:||closed → reopened|
you might want to revise the doxygen comment for the new function. The below paragraph, taken from GDALPolygonize(), seems to no longer apply.
* Note that currently the source pixel band values are read into a * signed 32bit integer buffer (Int32), so floating point or complex * bands will be implicitly truncated before processing.
It might also be good to add a link in the doxygen comment of GDALPolygonize() to point to GDALFPolygonize(), and vice versa.
I've some doubts about the implementation of the equals() function. "int aInt = *(int*)&A", A being a float seems risky because it will violate strict aliasing. With some compilers and some optimization options, it might generate a no-op and aInt could be filled with garbage, a float* being not a valid alias for a int*. Here's what g++ -O2 gives :
fpolygonize.cpp: In function ‘GBool equals(float, float)’: fpolygonize.cpp:58: warning: dereferencing type-punned pointer will break strict-aliasing rules fpolygonize.cpp:63: warning: dereferencing type-punned pointer will break strict-aliasing rules
I've been bitten by this in the past: see http://trac.osgeo.org/gdal/changeset/15097/trunk/gdal/ogr/ogrsf_frmts/xplane
I'd encourage you doing a memcpy(&aInt, &A, 4) instead.
I'm also wondering how portable this function is for a big-endian CPU ... It might need some byte-swapping but I'm not 100% sure
Might also be good to add a static keyword before the declaration of the equals() method, to avoid namespace collisions with other code...
comment:4 by , 12 years ago
comment:5 by , 12 years ago
Thanks for your comments Even. I'll commit the needed changes ASAP.
comment:6 by , 12 years ago
Quick question about Doxygen comments. If I want to reference another function in a different file, should I use @see and/or @file?
comment:7 by , 12 years ago
Apparently, just putting the function name, e.g. "GDALPolygonize()", is sufficient for doxygen to figure out it is a function and create the hyperlink.
comment:8 by , 12 years ago
Some changes in r22500:
- The "equals" name has been replaced by "GDALEqualsFloat". The function is not static because is used in 2 sources. I think the name change will avoid collisions. Anyway, it probably needs to be reviewed for portability
- Doxygen comments added in GDALPolygonize and GDALFPolygonize.
- Memcpy used. I forgot to mention in the commit message. Sorry for that.
comment:9 by , 12 years ago
comment:10 by , 12 years ago
|Status:||reopened → closed|
An alternative GDALPolygonize2 function may be a good idea, to avoid side effects.