Hi Even, Thanks for your input
On Tue, Apr 9, 2013 at 3:34 PM, Even Rouault <even.roua...@mines-paris.org>wrote: > Le mardi 09 avril 2013 19:06:28, Etienne Tourigny a écrit : > > I have committed new warping methods average and mode to trunk, this will > > be part of gdal-1.10 > > Hi Etienne, > > It would be good if you could extend the autotest suite to add tests for > those > new warping methods. For that, you can likely take inspiration from the > first > tests of autotest/warp/warp.py. "Reference" images based on utmsmall.tif > reference image is a bit big, but you can likely start from a smaller > source > image like byte.tif instead that will produce reference images of > reasonable > size to be put in svn. > I will do that, thanks > > Regarding nAlgo == 2 (mode with foating point data), the allocations of > pafVals and panSums have the potential to fail if warping is done on a > large > image whose floating point values are rarely identical. So I think that > VSIRealloc shoud be used instead with a test on the result to fail > properly. > I'm also a bit doubtfull about the practical usefulness of this case on > real > data. There might also be a performance issue due to the loop "//Check > array > for existing entry" that is at the most inner level of the algorithm. > Unless > you have a practical use case, my personnal opinion would be to refuse to > run > mode resampling on floating point data, or implement it with a tolerance > parameter to avoid strict floating point comparison (if that makes sense), > but > even that wouldn't guarantee reasonable performance and memory consumption. > The logic was copied from existing code for overviews... It is rather inefficient when dealing with floating-point or even integer (16/32 bits). You are right that it is not really practical, but I decided to go ahead and implement it anyway. Mostly I implemented it to deal with 16 and 32 bit integers, and was lazy to implement an integer-only solution. So here is my suggestion : modify this to use 32-bit integers instead, and refuse to run with floating-point data. The same could be done to overviews, although that *might* cause some regressions (although as you point out, mode resampling doesn't make sense for floating-point data). Or another solution is to implement a new integer algorithm, and leave the floating-point one around but print a warning. > > As far as mode resampling is concerned, it would likely run correctly for > images with color tables, so I believe that the warning in gdalwarp.cpp at > line 1048 could be shut down in that case too (in addition to nearest > resampling) > I have tested with a color table, and mode resampling does work. I forgot to update gdalwarp for this case, thanks. > > > Best regards, > > Even >
_______________________________________________ gdal-dev mailing list gdal-dev@lists.osgeo.org http://lists.osgeo.org/mailman/listinfo/gdal-dev