Opened 17 years ago
Closed 17 years ago
#1732 closed defect (fixed)
Possible error with Imagine Palettized images (rounding vs truncating)
Reported by: | mrosen | Owned by: | warmerdam |
---|---|---|---|
Priority: | normal | Milestone: | 1.4.3 |
Component: | GDAL_Raster | Version: | unspecified |
Severity: | normal | Keywords: | hfa |
Cc: |
Description (last modified by )
I was chasing down an issue with GDAL's support for palettized Imagine files and think I found a defect in HFARasterBand. In the construtor we convert from a 0..1 double to a 0..255 short. But we do so by truncation:
sEntry.c1 = (short) (padfRed[iColor] * 255); sEntry.c2 = (short) (padfGreen[iColor] * 255); sEntry.c3 = (short) (padfBlue[iColor] * 255); sEntry.c4 = (short) (padfAlpha[iColor] * 255); poCT->SetColorEntry( iColor, &sEntry );
Currently, this code suffers from two difficulties: (a) it gives different answers on different platforms (b) rounding rather than truncation would seem to be the natural way to map from [0..1] to [0..255].
The first problem is probably unavoidable since floating point arithmetic is involved. Solving the second problem can be solved by something like "(short)((padfRed[iColor]*255+0.5))".
I tried this and found that the input table did seem more correct (true grey palette in repro below). However, then a subsquent call (from the application layer) to the band's GetMetadataItem("STATISTICS_MEAN") returns an empty string (where it did not prior to the change ... spurious?)
REPRO
I'm attaching a small repro image "grey.img." Despite the name, it contains an RGB lookup table. Under linux/gcc3.2 the lookup table is not quite grey; under win32/vc8 it creates slightly different not-quite grey lookup table. I've confirmed the differences are due to FP rounding. Looking at the values read from the table and the name of the file, the obvious intent is that the table be an RGB palette that implements gray scale. Unfortunately, I used application level printf() calls to dump the entries in the table.
A short demo of the FP rounding issue based on the second entry in the CLUT:
double df = 0.0039215686274509803; // taken from CLUT, entry #2. short s; s = (short)((df * 255)+0.5); printf ("df = %lf, s = %d\n", df, s); // linux/gcc32, s==0; win32/vc8 s==1 return 0;
I verified that ArcMap 9.2 will read this image and indicates a true greyscale palette!
Attachments (1)
Change History (3)
by , 17 years ago
comment:1 by , 17 years ago
Component: | default → GDAL_Raster |
---|---|
Description: | modified (diff) |
Keywords: | hfa added |
Milestone: | → 1.5.0 |
Status: | new → assigned |
comment:2 by , 17 years ago
Milestone: | 1.5.0 → 1.4.3 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Michael,
I have chosen to use the following conversion which gives a similar result for "normal" values, but also gives reasonably continuous values for various other circumstances.
Index: hfadataset.cpp =================================================================== --- hfadataset.cpp (revision 11816) +++ hfadataset.cpp (working copy) @@ -499,10 +499,14 @@ { GDALColorEntry sEntry; - sEntry.c1 = (short) (padfRed[iColor] * 255); - sEntry.c2 = (short) (padfGreen[iColor] * 255); - sEntry.c3 = (short) (padfBlue[iColor] * 255); - sEntry.c4 = (short) (padfAlpha[iColor] * 255); + // The following mapping assigns "equal sized" section of + // the [0...1] range to each possible output value and avoid + // rounding issues for the "normal" values generated using n/255. + // See bug #1732 for some discussion. + sEntry.c1 = MIN(255,(short) (padfRed[iColor] * 256)); + sEntry.c2 = MIN(255,(short) (padfGreen[iColor] * 256)); + sEntry.c3 = MIN(255,(short) (padfBlue[iColor] * 256)); + sEntry.c4 = MIN(255,(short) (padfAlpha[iColor] * 256)); poCT->SetColorEntry( iColor, &sEntry ); } }
Let me know if you see any problems with this. I am going to apply this patch in trunk, 1.4 branch and 1.4-esri branch.
repro image for 1732