Skip to content

Commit 2ea4718

Browse files
WeatherGodtdrwenski
authored andcommitted
Fix geotiff support for unsigned/signed data
* Expand unit tests for geotiff writing * Reduce code duplication
1 parent acd010f commit 2ea4718

4 files changed

Lines changed: 365 additions & 32 deletions

File tree

cdm/misc/src/main/java/ucar/nc2/geotiff/GeoTiff.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.nio.ByteOrder;
1616
import java.nio.FloatBuffer;
1717
import java.nio.IntBuffer;
18+
import java.nio.ShortBuffer;
1819
import java.nio.channels.FileChannel;
1920
import java.nio.charset.StandardCharsets;
2021
import java.util.ArrayList;
@@ -192,6 +193,30 @@ int writeData(byte[] data, int imageNumber) throws IOException {
192193
return nextOverflowData;
193194
}
194195

196+
int writeData(short[] data, int imageNumber) throws IOException {
197+
if (file == null)
198+
init();
199+
200+
if (imageNumber == 1)
201+
channel.position(headerSize);
202+
else
203+
channel.position(nextOverflowData);
204+
205+
// no way around making a copy
206+
ByteBuffer direct = ByteBuffer.allocateDirect(2 * data.length);
207+
ShortBuffer buffer = direct.asShortBuffer();
208+
buffer.put(data);
209+
// buffer.flip();
210+
channel.write(direct);
211+
212+
if (imageNumber == 1)
213+
firstIFD = headerSize + 2 * data.length;
214+
else
215+
firstIFD = 2 * data.length + nextOverflowData;
216+
217+
return nextOverflowData;
218+
}
219+
195220
int writeData(int[] data, int imageNumber) throws IOException {
196221
if (file == null)
197222
init();

cdm/misc/src/main/java/ucar/nc2/geotiff/GeotiffWriter.java

Lines changed: 104 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import ucar.ma2.ArrayByte;
1515
import ucar.ma2.ArrayInt;
1616
import ucar.ma2.ArrayFloat;
17+
import ucar.ma2.ArrayShort;
1718
import ucar.ma2.DataType;
1819
import ucar.ma2.Index;
1920
import ucar.ma2.IndexIterator;
@@ -211,19 +212,15 @@ void writeGrid(GridDatatype grid, Array data, boolean greyScale, double xStart,
211212
// write the data first
212213
MAMath.MinMax dataMinMax = grid.getMinMaxSkipMissingData(data);
213214
if (greyScale) {
214-
ArrayByte result = replaceMissingValuesAndScale(grid, data, dataMinMax);
215-
nextStart = geotiff.writeData((byte[]) result.getStorage(), imageNumber);
215+
data = replaceMissingValuesAndScale(grid, data, dataMinMax);
216+
nextStart = writeData(data, DataType.UBYTE);
217+
} else if (dtype == DataType.FLOAT) {
218+
// Backwards compatibility shim
219+
data = replaceMissingValues(grid, data, dataMinMax);
220+
nextStart = writeData(data, dtype);
216221
} else {
217-
if (dtype == DataType.BYTE || dtype == DataType.UBYTE) {
218-
ArrayByte result = coerceByte(data);
219-
nextStart = geotiff.writeData((byte[]) result.getStorage(), imageNumber);
220-
} else if (dtype.isIntegral()) {
221-
ArrayInt result = coerceInt(data);
222-
nextStart = geotiff.writeData((int[]) result.getStorage(), imageNumber);
223-
} else {
224-
ArrayFloat result = replaceMissingValues(grid, data, dataMinMax);
225-
nextStart = geotiff.writeData((float[]) result.getStorage(), imageNumber);
226-
}
222+
data = coerceData(data, dtype);
223+
nextStart = writeData(data, dtype);
227224
}
228225

229226
// set the width and the height
@@ -472,8 +469,16 @@ public static HashMap<Integer, Color> createColorMap(int[] flag_values, String[]
472469
return colorMap;
473470
}
474471

475-
private ArrayByte coerceByte(Array data) {
476-
ArrayByte array = (ArrayByte) Array.factory(DataType.BYTE, data.getShape());
472+
/**
473+
* Coerce a given data array into an array of bytes.
474+
* Always returns a copy. No data safety check is performed.
475+
*
476+
* @param data input data array (of any data type)
477+
* @param isUnsigned coerce to unsigned bytes
478+
* @return integer data array
479+
*/
480+
static ArrayByte coerceByte(Array data, boolean isUnsigned) {
481+
ArrayByte array = (ArrayByte) Array.factory(isUnsigned ? DataType.UBYTE : DataType.BYTE, data.getShape());
477482
IndexIterator dataIter = data.getIndexIterator();
478483
IndexIterator resultIter = array.getIndexIterator();
479484

@@ -484,8 +489,56 @@ private ArrayByte coerceByte(Array data) {
484489
return array;
485490
}
486491

487-
private ArrayInt coerceInt(Array data) {
488-
ArrayInt array = (ArrayInt) Array.factory(DataType.INT, data.getShape());
492+
/**
493+
* Coerce a given data array into an array of 16-bit integers.
494+
* Always returns a copy. No data safety check is performed.
495+
*
496+
* @param data input data array (of any data type)
497+
* @param isUnsigned coerce to unsigned integers
498+
* @return integer data array
499+
*/
500+
static ArrayShort coerceShort(Array data, boolean isUnsigned) {
501+
ArrayShort array = (ArrayShort) Array.factory(isUnsigned ? DataType.USHORT : DataType.SHORT, data.getShape());
502+
IndexIterator dataIter = data.getIndexIterator();
503+
IndexIterator resultIter = array.getIndexIterator();
504+
505+
while (dataIter.hasNext()) {
506+
resultIter.setIntNext(dataIter.getIntNext());
507+
}
508+
509+
return array;
510+
}
511+
512+
513+
/**
514+
* Coerce a given data array into an array of 32-bit integers.
515+
* Always returns a copy. No data safety check is performed.
516+
*
517+
* @param data input data array (of any data type)
518+
* @param isUnsigned coerce to unsigned integers
519+
* @return integer data array
520+
*/
521+
static ArrayInt coerceInt(Array data, boolean isUnsigned) {
522+
ArrayInt array = (ArrayInt) Array.factory(isUnsigned ? DataType.UINT : DataType.INT, data.getShape());
523+
IndexIterator dataIter = data.getIndexIterator();
524+
IndexIterator resultIter = array.getIndexIterator();
525+
526+
while (dataIter.hasNext()) {
527+
resultIter.setIntNext(dataIter.getIntNext());
528+
}
529+
530+
return array;
531+
}
532+
533+
/**
534+
* Coerce a given data array into an array of 32-bit floats.
535+
* Always returns a copy. No data safety check is performed.
536+
*
537+
* @param data input data array (of any data type)
538+
* @return float data array
539+
*/
540+
static ArrayFloat coerceFloat(Array data) {
541+
ArrayFloat array = (ArrayFloat) Array.factory(DataType.FLOAT, data.getShape());
489542
IndexIterator dataIter = data.getIndexIterator();
490543
IndexIterator resultIter = array.getIndexIterator();
491544

@@ -845,19 +898,15 @@ public void writeGrid(GeoReferencedArray array, boolean greyScale, DataType dtyp
845898
int nextStart;
846899
MAMath.MinMax dataMinMax = MAMath.getMinMaxSkipMissingData(data, array);
847900
if (greyScale) {
848-
ArrayByte result = replaceMissingValuesAndScale(array, data, dataMinMax);
849-
nextStart = geotiff.writeData((byte[]) result.getStorage(), pageNumber);
901+
data = replaceMissingValuesAndScale(array, data, dataMinMax);
902+
nextStart = writeData(data, DataType.UBYTE);
903+
} else if (dtype == DataType.FLOAT) {
904+
// Backwards compatibility shim
905+
data = replaceMissingValues(array, data, dataMinMax);
906+
nextStart = writeData(data, dtype);
850907
} else {
851-
if (dtype == DataType.BYTE || dtype == DataType.UBYTE) {
852-
ArrayByte result = coerceByte(data);
853-
nextStart = geotiff.writeData((byte[]) result.getStorage(), pageNumber);
854-
} else if (dtype.isIntegral()) {
855-
ArrayInt result = coerceInt(data);
856-
nextStart = geotiff.writeData((int[]) result.getStorage(), pageNumber);
857-
} else {
858-
ArrayFloat result = replaceMissingValues(array, data, dataMinMax);
859-
nextStart = geotiff.writeData((float[]) result.getStorage(), pageNumber);
860-
}
908+
data = coerceData(data, dtype);
909+
nextStart = writeData(data, dtype);
861910
}
862911

863912
// set the width and the height
@@ -867,5 +916,32 @@ public void writeGrid(GeoReferencedArray array, boolean greyScale, DataType dtyp
867916
writeMetadata(greyScale, xStart, yStart, xInc, yInc, height, width, pageNumber, nextStart, dataMinMax, proj, dtype);
868917
pageNumber++;
869918
}
919+
920+
static Array coerceData(Array data, DataType dtype) {
921+
if (dtype == DataType.BYTE || dtype == DataType.UBYTE) {
922+
data = coerceByte(data, dtype.isUnsigned());
923+
} else if (dtype == DataType.SHORT || dtype == DataType.USHORT) {
924+
data = coerceShort(data, dtype.isUnsigned());
925+
} else if (dtype == DataType.INT || dtype == DataType.UINT) {
926+
data = coerceInt(data, dtype.isUnsigned());
927+
} else if (dtype.isFloatingPoint()) {
928+
data = coerceFloat(data);
929+
}
930+
return data;
931+
}
932+
933+
private int writeData(Array data, DataType dtype) throws IOException {
934+
int nextStart;
935+
if (dtype == DataType.BYTE || dtype == DataType.UBYTE) {
936+
nextStart = geotiff.writeData((byte[]) data.getStorage(), pageNumber);
937+
} else if (dtype == DataType.SHORT || dtype == DataType.USHORT) {
938+
nextStart = geotiff.writeData((short[]) data.getStorage(), pageNumber);
939+
} else if (dtype == DataType.INT || dtype == DataType.UINT) {
940+
nextStart = geotiff.writeData((int[]) data.getStorage(), pageNumber);
941+
} else {
942+
nextStart = geotiff.writeData((float[]) data.getStorage(), pageNumber);
943+
}
944+
return nextStart;
945+
}
870946
}
871947

cdm/misc/src/test/java/ucar/nc2/geotiff/TestGeoTiffWriter.java

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
import java.util.List;
3535

3636
/**
37-
* GeoTiffWriter2 writing geotiffs
37+
* GeotiffWriter writing geotiffs
3838
*
3939
* @author caron
4040
* @since 7/31/2014
@@ -65,10 +65,12 @@ public static List<Object[]> getTestParameters() {
6565
result.add(new Object[] {TestDir.cdmUnitTestDir + "ft/coverage/testCFwriter.nc", FeatureType.GRID, "Temperature",
6666
greyscale == 1});
6767

68-
// This file is unique in that it is lambert conformal with yaxis flipped.
68+
// This file is unique in that it is lambert conformal with yaxis flipped (South to North).
69+
// Also, this drought data is stored as integers (the other test data above are floats).
70+
// This will check that the compatibility shim is still forcing the data
71+
// to floats when greyscale is false and not specifying the dtype.
6972
result.add(
7073
new Object[] {"src/test/data/ucar/nc2/geotiff/categorical.nc", FeatureType.GRID, "drought", greyscale == 1});
71-
7274
}
7375

7476
return result;
@@ -93,6 +95,15 @@ public void testWriteCoverage() throws IOException, InvalidRangeException {
9395
Array dtArray;
9496
float missingVal;
9597
MAMath.MinMax minmax;
98+
99+
// When not explicitly specifying the dtype in the writeGrid() call,
100+
// the code will fall back to old behavior and cast the data as floats
101+
// when greyscale is false. greyscale==true always produces unsigned bytes.
102+
FieldType expectedFieldType = greyscale ? FieldType.BYTE : FieldType.FLOAT;
103+
104+
// Float, signed, unsigned (future-proofing for when we expand expected field types)
105+
int expectedSampleFormat = expectedFieldType.code >= 6 ? expectedFieldType.code == 11 ? 3 : 2 : 1;
106+
96107
try (GridDataset gds = GridDataset.open(filename)) {
97108
GridDatatype grid = gds.findGridByName(field);
98109
assert grid != null;
@@ -143,8 +154,16 @@ public void testWriteCoverage() throws IOException, InvalidRangeException {
143154
Assert.assertEquals(minmax.max, (float) sMaxTag.valueD[0], 0.0);
144155

145156
IFDEntry noDataTag = geotiff.findTag(Tag.GDALNoData);
157+
// Technically, it would be ambiguous what was intended due to the
158+
// backwards compatibility shim, so FLOATS still get NoData tags.
146159
Assert.assertNotNull(noDataTag);
147160
Assert.assertEquals(String.valueOf(missingVal), noDataTag.valueS);
161+
162+
// Right now, this is the only case where the SampleFormat tag is written.
163+
// There is no reason why this has to be the case.
164+
IFDEntry sTypeTag = geotiff.findTag(Tag.SampleFormat);
165+
Assert.assertNotNull(sTypeTag);
166+
Assert.assertEquals(expectedSampleFormat, sTypeTag.value[0]);
148167
} else {
149168
IFDEntry sMinTag = geotiff.findTag(Tag.SMinSampleValue);
150169
Assert.assertNull(sMinTag);
@@ -156,6 +175,10 @@ public void testWriteCoverage() throws IOException, InvalidRangeException {
156175
Assert.assertNull(noDataTag);
157176
}
158177

178+
IFDEntry sSizeTag = geotiff.findTag(Tag.BitsPerSample);
179+
Assert.assertNotNull(sSizeTag);
180+
Assert.assertEquals(expectedFieldType.size * 8, sSizeTag.value[0]);
181+
159182
String gridOut2 = tempFolder.newFile().getAbsolutePath();
160183
logger.debug("geotiff2 read coverage {} write {} greyscale {}", filename, gridOut2, greyscale);
161184

@@ -169,11 +192,11 @@ public void testWriteCoverage() throws IOException, InvalidRangeException {
169192
String covName = (pos > 0) ? field.substring(pos + 1) : field;
170193

171194
Coverage coverage = gcd.findCoverage(covName);
195+
Assert.assertNotNull(covName, coverage);
172196
CoverageCoordAxis1D z = (CoverageCoordAxis1D) coverage.getCoordSys().getZAxis();
173197
SubsetParams params = new SubsetParams().set(SubsetParams.timePresent, true);
174198
if (z != null)
175199
params.set(SubsetParams.vertCoord, z.getCoordMidpoint(0));
176-
Assert.assertNotNull(covName, coverage);
177200
covArray = coverage.readData(params);
178201

179202
try (GeotiffWriter writer = new GeotiffWriter(gridOut2)) {

0 commit comments

Comments
 (0)