[
https://issues.apache.org/jira/browse/IMAGING-266?focusedWorklogId=649521&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-649521
]
ASF GitHub Bot logged work on IMAGING-266:
------------------------------------------
Author: ASF GitHub Bot
Created on: 10/Sep/21 22:59
Start Date: 10/Sep/21 22:59
Worklog Time Spent: 10m
Work Description: kinow commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706486666
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/ImageDataReader.java
##########
@@ -519,8 +516,55 @@ protected void applyPredictorToBlock(final int width,
final int height, final in
return samples;
}
+ protected int[] unpackIntSamples(
+ final int width,
+ final int height,
+ final int scansize,
+ final byte[] bytes,
+ final int predictor,
+ final int bitsPerSample,
+ final ByteOrder byteOrder)
+ throws ImageReadException {
Review comment:
`ImageReadException` never thrown, I think we can remove it from here.
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/ImageDataReader.java
##########
@@ -519,8 +516,55 @@ protected void applyPredictorToBlock(final int width,
final int height, final in
return samples;
}
+ protected int[] unpackIntSamples(
Review comment:
Javadoc comment, documenting this new method?
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/DataReaderTiled.java
##########
@@ -321,19 +336,68 @@ public TiffRasterData readRasterData(final Rectangle
subImage)
final int tile = iRow * nColumnsOfTiles + iCol;
final byte[] compressed = imageData.tiles[tile].getData();
final byte[] decompressed = decompress(compressed, compression,
- bytesPerTile, tileWidth, tileLength);
+ bytesPerTile, tileWidth, tileLength);
final int x = iCol * tileWidth;
final int y = iRow * tileLength;
+
final int[] blockData = unpackFloatingPointSamples(
- tileWidth, tileLength, tileWidth,
- decompressed,
- predictor, bitsPerPixel, byteOrder);
+ tileWidth, tileLength, tileWidth,
+ decompressed,
+ predictor, bitsPerPixel, byteOrder);
transferBlockToRaster(x, y, tileWidth, tileLength, blockData,
- xRaster, yRaster, rasterWidth, rasterHeight, rasterData);
+ xRaster, yRaster, rasterWidth, rasterHeight,
rasterDataFloat);
}
}
-
- return new TiffRasterData(rasterWidth, rasterHeight, rasterData);
+ return new TiffRasterDataFloat(rasterWidth, rasterHeight,
rasterDataFloat);
}
+ private TiffRasterData readRasterDataInt(final Rectangle subImage)
+ throws ImageReadException, IOException {
+ final int bitsPerRow = tileWidth * bitsPerPixel;
+ final int bytesPerRow = (bitsPerRow + 7) / 8;
+ final int bytesPerTile = bytesPerRow * tileLength;
+ int xRaster;
+ int yRaster;
+ int rasterWidth;
+ int rasterHeight;
+ if (subImage != null) {
+ xRaster = subImage.x;
+ yRaster = subImage.y;
+ rasterWidth = subImage.width;
+ rasterHeight = subImage.height;
+ } else {
+ xRaster = 0;
+ yRaster = 0;
+ rasterWidth = width;
+ rasterHeight = height;
+ }
+ int[] rasterDataInt = new int[rasterWidth * rasterHeight];
+
+ // tileWidth is the width of the tile
+ // tileLength is the height of the tile
+ final int col0 = xRaster / tileWidth;
+ final int col1 = (xRaster + rasterWidth - 1) / tileWidth;
+ final int row0 = yRaster / tileLength;
+ final int row1 = (yRaster + rasterHeight - 1) / tileLength;
Review comment:
Unrelated to this change too, but `tileLength`... shouldn't it be
`tileHeight`?
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/DataReaderTiled.java
##########
@@ -286,7 +288,20 @@ public ImageBuilder readImageData(final Rectangle
subImageSpecification,
@Override
public TiffRasterData readRasterData(final Rectangle subImage)
- throws ImageReadException, IOException {
+ throws ImageReadException, IOException {
+ switch (sampleFormat) {
+ case TiffTagConstants.SAMPLE_FORMAT_VALUE_IEEE_FLOATING_POINT:
Review comment:
Code is excellent here @gwlucastrig , but out of curiosity, do you know
why we have `SAMPLE_FORMAT_VALUE_IEEE_FLOATING_POINT` and
`SAMPLE_FORMAT_VALUE_IEEE_FLOATING_POINT_1`? That's probably from the
specification, but since we don't have any javadocs in the constants I can't
recall the reason for it.
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/DataReaderStrips.java
##########
@@ -390,16 +405,69 @@ public TiffRasterData readRasterData(final Rectangle
subImage)
final byte[] compressed = imageData.getImageData(strip).getData();
final byte[] decompressed = decompress(compressed, compression,
- bytesPerStrip, width, rowsInThisStrip);
+ bytesPerStrip, width, rowsInThisStrip);
final int[] blockData = unpackFloatingPointSamples(
- width, (int) rowsInThisStrip, width,
- decompressed,
- predictor, bitsPerPixel, byteOrder);
+ width,
+ (int) rowsInThisStrip,
Review comment:
Unnecessary casting.
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/DataReaderStrips.java
##########
@@ -390,16 +405,69 @@ public TiffRasterData readRasterData(final Rectangle
subImage)
final byte[] compressed = imageData.getImageData(strip).getData();
final byte[] decompressed = decompress(compressed, compression,
- bytesPerStrip, width, rowsInThisStrip);
+ bytesPerStrip, width, rowsInThisStrip);
final int[] blockData = unpackFloatingPointSamples(
- width, (int) rowsInThisStrip, width,
- decompressed,
- predictor, bitsPerPixel, byteOrder);
+ width,
+ (int) rowsInThisStrip,
+ width,
+ decompressed,
+ predictor, bitsPerPixel, byteOrder);
transferBlockToRaster(0, yStrip, width, (int) rowsInThisStrip,
blockData,
- xRaster, yRaster, rasterWidth, rasterHeight, rasterData);
+ xRaster, yRaster, rasterWidth, rasterHeight,
rasterDataFloat);
}
- return new TiffRasterData(rasterWidth, rasterHeight, rasterData);
+ return new TiffRasterDataFloat(rasterWidth, rasterHeight,
rasterDataFloat);
}
+ private TiffRasterData readRasterDataInt(final Rectangle subImage)
+ throws ImageReadException, IOException {
+ int xRaster;
+ int yRaster;
+ int rasterWidth;
+ int rasterHeight;
+ if (subImage != null) {
+ xRaster = subImage.x;
+ yRaster = subImage.y;
+ rasterWidth = subImage.width;
+ rasterHeight = subImage.height;
+ } else {
+ xRaster = 0;
+ yRaster = 0;
+ rasterWidth = width;
+ rasterHeight = height;
+ }
+
+ int[] rasterDataInt = new int[rasterWidth * rasterHeight];
+
+ // the legacy code is optimized to the reading of whole
+ // strips (except for the last strip in the image, which can
+ // be a partial). So create a working image with compatible
+ // dimensions and read that. Later on, the working image
+ // will be sub-imaged to the proper size.
+ // strip0 and strip1 give the indices of the strips containing
+ // the first and last rows of pixels in the subimage
+ final int strip0 = yRaster / rowsPerStrip;
+ final int strip1 = (yRaster + rasterHeight - 1) / rowsPerStrip;
+
+ for (int strip = strip0; strip <= strip1; strip++) {
+ final int yStrip = strip * rowsPerStrip;
+ final int rowsRemaining = height - yStrip;
+ final int rowsInThisStrip = Math.min(rowsRemaining, rowsPerStrip);
+ final int bytesPerRow = (bitsPerPixel * width + 7) / 8;
+ final int bytesPerStrip = rowsInThisStrip * bytesPerRow;
+
+ final byte[] compressed = imageData.getImageData(strip).getData();
+ final byte[] decompressed = decompress(compressed, compression,
+ bytesPerStrip, width, rowsInThisStrip);
+ final int[] blockData = unpackIntSamples(
+ width,
+ (int) rowsInThisStrip,
Review comment:
Unnecessary casting.
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/ImageDataReader.java
##########
@@ -602,6 +646,87 @@ void transferBlockToRaster(final int xBlock, final int
yBlock,
}
}
+ /**
+ * Transfer samples obtained from the TIFF file to an integer raster.
+ * @param xBlock coordinate of block relative to source data
+ * @param yBlock coordinate of block relative to source data
+ * @param blockWidth width of block, in pixels
+ * @param blockHeight height of block in pixels
+ * @param blockData the data for the block
+ * @param xRaster coordinate of raster relative to source data
+ * @param yRaster coordinate of raster relative to source data
+ * @param rasterWidth width of the raster (always smaller than source data)
+ * @param rasterHeight height of the raster (always smaller than source
+ * data)
+ * @param rasterData the raster data.
+ */
+ void transferBlockToRaster(final int xBlock, final int yBlock,
+ final int blockWidth, final int blockHeight, final int[] blockData,
+ final int xRaster, final int yRaster,
+ final int rasterWidth, final int rasterHeight, final int[] rasterData)
{
+
+ // xR0, yR0 are the coordinates within the raster (upper-left corner)
+ // xR1, yR1 are ONE PAST the coordinates of the lower-right corner
+ int xR0 = xBlock - xRaster; // xR0, yR0 coordinates relative to
+ int yR0 = yBlock - yRaster; // the raster
+ int xR1 = xR0 + blockWidth;
+ int yR1 = yR0 + blockHeight;
+ if (xR0 < 0) {
+ xR0 = 0;
+ }
+ if (yR0 < 0) {
+ yR0 = 0;
+ }
+ if (xR1 > rasterWidth) {
+ xR1 = rasterWidth;
+ }
+ if (yR1 > rasterHeight) {
+ yR1 = rasterHeight;
+ }
+
+ // Recall that the above logic may have adjusted xR0, xY0 so that
+ // they are not necessarily point to the source pixel at xRaster,
yRaster
+ // we compute xSource = xR0+xRaster.
+ // xOffset = xSource-xBlock
+ // since the block cannot be accessed with a negative offset,
+ // we check for negatives and adjust xR0, yR0 upward as necessary
+ int xB0 = xR0 + xRaster - xBlock;
+ int yB0 = yR0 + yRaster - yBlock;
+ if (xB0 < 0) {
+ xR0 -= xB0;
+ xB0 = 0;
+ }
+ if (yB0 < 0) {
+ yR0 -= yB0;
+ yB0 = 0;
+ }
+
+ int w = xR1 - xR0;
+ int h = yR1 - yR0;
+ if (w <= 0 || h <= 0) {
+ // The call to this method put the block outside the
Review comment:
s/put/puts?
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/DataReaderStrips.java
##########
@@ -390,16 +405,69 @@ public TiffRasterData readRasterData(final Rectangle
subImage)
final byte[] compressed = imageData.getImageData(strip).getData();
final byte[] decompressed = decompress(compressed, compression,
- bytesPerStrip, width, rowsInThisStrip);
+ bytesPerStrip, width, rowsInThisStrip);
final int[] blockData = unpackFloatingPointSamples(
- width, (int) rowsInThisStrip, width,
- decompressed,
- predictor, bitsPerPixel, byteOrder);
+ width,
+ (int) rowsInThisStrip,
+ width,
+ decompressed,
+ predictor, bitsPerPixel, byteOrder);
transferBlockToRaster(0, yStrip, width, (int) rowsInThisStrip,
blockData,
Review comment:
Also unnecessary casting, but not in this PR.
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/DataReaderStrips.java
##########
@@ -390,16 +405,69 @@ public TiffRasterData readRasterData(final Rectangle
subImage)
final byte[] compressed = imageData.getImageData(strip).getData();
final byte[] decompressed = decompress(compressed, compression,
- bytesPerStrip, width, rowsInThisStrip);
+ bytesPerStrip, width, rowsInThisStrip);
final int[] blockData = unpackFloatingPointSamples(
- width, (int) rowsInThisStrip, width,
- decompressed,
- predictor, bitsPerPixel, byteOrder);
+ width,
+ (int) rowsInThisStrip,
+ width,
+ decompressed,
+ predictor, bitsPerPixel, byteOrder);
transferBlockToRaster(0, yStrip, width, (int) rowsInThisStrip,
blockData,
- xRaster, yRaster, rasterWidth, rasterHeight, rasterData);
+ xRaster, yRaster, rasterWidth, rasterHeight,
rasterDataFloat);
}
- return new TiffRasterData(rasterWidth, rasterHeight, rasterData);
+ return new TiffRasterDataFloat(rasterWidth, rasterHeight,
rasterDataFloat);
}
+ private TiffRasterData readRasterDataInt(final Rectangle subImage)
+ throws ImageReadException, IOException {
+ int xRaster;
+ int yRaster;
+ int rasterWidth;
+ int rasterHeight;
+ if (subImage != null) {
+ xRaster = subImage.x;
+ yRaster = subImage.y;
+ rasterWidth = subImage.width;
+ rasterHeight = subImage.height;
+ } else {
+ xRaster = 0;
+ yRaster = 0;
+ rasterWidth = width;
+ rasterHeight = height;
+ }
+
+ int[] rasterDataInt = new int[rasterWidth * rasterHeight];
+
+ // the legacy code is optimized to the reading of whole
+ // strips (except for the last strip in the image, which can
+ // be a partial). So create a working image with compatible
+ // dimensions and read that. Later on, the working image
+ // will be sub-imaged to the proper size.
+ // strip0 and strip1 give the indices of the strips containing
+ // the first and last rows of pixels in the subimage
+ final int strip0 = yRaster / rowsPerStrip;
+ final int strip1 = (yRaster + rasterHeight - 1) / rowsPerStrip;
+
+ for (int strip = strip0; strip <= strip1; strip++) {
+ final int yStrip = strip * rowsPerStrip;
+ final int rowsRemaining = height - yStrip;
+ final int rowsInThisStrip = Math.min(rowsRemaining, rowsPerStrip);
+ final int bytesPerRow = (bitsPerPixel * width + 7) / 8;
+ final int bytesPerStrip = rowsInThisStrip * bytesPerRow;
+
+ final byte[] compressed = imageData.getImageData(strip).getData();
+ final byte[] decompressed = decompress(compressed, compression,
+ bytesPerStrip, width, rowsInThisStrip);
+ final int[] blockData = unpackIntSamples(
+ width,
+ (int) rowsInThisStrip,
+ width,
+ decompressed,
+ predictor, bitsPerPixel, byteOrder);
+ transferBlockToRaster(0, yStrip, width, (int) rowsInThisStrip,
blockData,
Review comment:
Unnecessary casting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 649521)
Time Spent: 2h 20m (was: 2h 10m)
> Read integer data from GeoTIFFS
> --------------------------------
>
> Key: IMAGING-266
> URL: https://issues.apache.org/jira/browse/IMAGING-266
> Project: Commons Imaging
> Issue Type: New Feature
> Components: Format: TIFF
> Affects Versions: 1.0-alpha3
> Reporter: Gary Lucas
> Priority: Major
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> I recently discovered that there is a large amount of digital elevation data
> available in the form of 16-bit integer coded data in GeoTIFF files (TIFF
> files with geographic tags). I propose to enhance the Commons Imaging API to
> read these files. This work will be similar to the work I did for reading
> floating-point raster data under ISSUE-251.
> Available data include the nearly-global coverage of one-second of arc
> elevation data produced from the Shuttle Radar Topography Mission (SRTM) and
> other sources. These products give grids of elevation data with a 30 meter
> cell spacing for most of the world's land masses. They are available at NASA
> Earthdata and Japan Space Systems websites, see
> [https://asterweb.jpl.nasa.gov/gdem.asp|https://asterweb.jpl.nasa.gov/gdem.asp]
> There is also a ocean bathymetry data set available in this format at
> [http://www.shadedrelief.com/blue-earth/]
> This new feature will continue to expand the usefulness of the Commons
> Imaging API in accessing GeoTIFF products.
> Request for Feedback
> So far, the data products I've found (ASTER and Blue Earth Bathymetry) give
> elevation and ocean depth data in meters recorded as a short integer. I
> haven't found an example of where the 32-bit integer format is used. For
> now, I am planning on only coding the 16-bit integer variation. Does anyone
> know if the 32-bit version is worth supporting? My criteria for determining
> that would be based on whether there is a significant number of projects
> using that format (life is too short to chase rarely used data formats).
> Currently, one of the code-analysis operations conducted by the Commons
> Imaging build process is coverage by JUnit tests. Lacking any test data for
> the 32-bit case, I am reluctant to include it in the code base because it
> would mean putting uncovered code into the distribution.
> Also, I am wondering about the best design for the access API. The current
> TiffImageParser class has a method called getFloatingPointRasterData() that
> returns an instance of TiffRasterData. TiffRasterData is pretty much
> hard-wired to floating point data. I am thinking of creating a new method
> called getIntegerRasterData() that would return an instance of a new class
> called TiffIntegerRasterData. Does this seem reasonable? I considered trying
> to combine both kinds of results into a unified class and method, but it
> seems more unwieldy than useful.
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
