You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/09/09 02:05:23 UTC
[GitHub] [commons-imaging] gwlucastrig opened a new pull request #165: IMAGING-266 read numeric data from GeoTIFFs
gwlucastrig opened a new pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165
Updates to the TIFF API to support access to numeric data from TIFF files. This change addresses JIRA item IMAGING-266.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-915723586
[](https://coveralls.io/builds/42806845)
Coverage increased (+0.1%) to 76.837% when pulling **1c829621843e7dfb6a82080a859171407f401a7a on gwlucastrig:IMAGING-266a** into **6cf514e43cfdd77e5eb8293248990828dd17fb68 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig edited a comment on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig edited a comment on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917501066
I just pushed the squashed version of the code.
Please go ahead and rename scansize to scanSize. I definitely like that name better.
I am really looking forward to going onto the JIRA page and posting a notice about our progress on ISSUE 266 (including the picture of Auckland). Commons Imaging is shaping up to be one of the most useful TIFF solutions out there... Certainly, it's my favorite Java solution.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706662852
##########
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:
done
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r705470722
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
+ assertEquals(index, test, "Set/get int value test failed");
+ }
+ }
+ }
+
+ /**
+ * Test of getValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetValue() {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + x;
+ int test = (int) raster.getValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ test = (int) raster.getIntValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ }
+ }
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_0args() {
+
+ final TiffRasterStatistics result = raster.getSimpleStatistics();
+ assertEquals(0, result.getMinValue(), "Min value failure");
+ assertEquals(width * height - 1, result.getMaxValue(), "Max value failure");
+ assertEquals(meanValue, result.getMeanValue(), "Mean value failure");
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_float() {
+ // exclude the maximum value (width*height-1). This will result
+ // in a max value of width*height-2
+ final TiffRasterStatistics result = raster.getSimpleStatistics(width * height - 1);
+ assertEquals(width * height - 2, result.getMaxValue(), "Max value failure");
+ }
+
+ /**
+ * Test of getWidth method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetWidth() {
+ assertEquals(width, raster.getWidth(), "Improper width stored");
+ }
+
+ /**
+ * Test of getHeight method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetHeight() {
+ assertEquals(width, raster.getWidth(), "Improper height stored");
+ }
+
+ /**
+ * Test of getData method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetData() {
+ final float[] result = raster.getData();
+ for(int i=0; i<result.length; i++){
+ assertEquals((int)result[i], data[i]);
+ }
+ final int []iResult = raster.getIntData();
+ assertArrayEquals(data, iResult);
Review comment:
will look for these.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706663344
##########
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:
done
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-915723586
[](https://coveralls.io/builds/42745670)
Coverage decreased (-0.04%) to 76.689% when pulling **f122b890fb36f55d12c19238e8cac293346dd21d on gwlucastrig:IMAGING-266a** into **32e1a3b839eb75b38c47531cb2dd6e0012bb3bf5 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917504233
@gwlucastrig here's what I've done in the last commit.
1. Renamed `scansize` to `scanSize` in `ImageDataReader.java`
2. `git commit -am 'f'` (just so I know I need to squash it)
3. `git rebase -i HEAD~2` and I replace `pick f` by `f f`
4. Added an entry in `changes.xml`. This file is used to generate the changelog and in the site and in the `RELEASE-NOTES.txt` file (I think, although can't recall if we use JIRA export tool in release process?). I added an entry with myself as dev, and you as due-to. The dev is always an ASF committer, and due-to the person responsible for work (it's not always present). I didn't put your user as dev as you are still not a committer (only pending you accept & submit the form :)
5. Pushed force a commit with the squashed commit and the `changes.xml` (I can push to your branch as you allowed it when creating the PR)
6. Waited for CI to confirm I didn't mess it up, then merged the PR
7. Updated the JIRA (fix version, status, etc).
This is the whole process for a ticket in Imaging, but applies to pretty much every Apache Commons projects too.
Cheers
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-915723586
[](https://coveralls.io/builds/42782694)
Coverage increased (+0.1%) to 76.837% when pulling **add3ee4345c3391a36977f2f42ceff0ad622da97 on gwlucastrig:IMAGING-266a** into **32e1a3b839eb75b38c47531cb2dd6e0012bb3bf5 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706662956
##########
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:
done
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706663375
##########
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:
Done
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-916759213
Made changes for your review comments and added a few more JUnit tests to address the Coverage issues.
Unfortunately, I accidentally picked up two files that had tabs in them when I replaced tabs with spaces. Other than replacing tabs with spaces, these files did not change.
\test\java\org\apache\commons\imaging\TestImageReadException.java
\test\java\org\apache\commons\imaging\TestImageWriteException.java
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917501066
I just pushed the squashed version of the code.
Please go ahead and rename scansize to scanSize. I definitely like that name better.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-916017529
Found a bug in TiffRasterInt, getDataType returns FLOAT instead of INTEGER. I will fix it when I address the other review comments.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-916857602
I think that's everything... Thanks for your excellent review.
Github is reporting that more changes are requested, but searching through this discussion, I don't see anything that's unresolved.
Please let me know if you find additional items that need attention.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-915723586
[](https://coveralls.io/builds/42806882)
Coverage increased (+0.1%) to 76.837% when pulling **507b6f20f336ff963d94c16400c47e0b633fd085 on gwlucastrig:IMAGING-266a** into **6cf514e43cfdd77e5eb8293248990828dd17fb68 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917502804
>I am really looking forward to going onto the JIRA page and posting a notice about our progress on ISSUE 266 (including the picture of Auckland). Commons Imaging is shaping up to be one of the most useful TIFF solutions out there... Certainly, it's my favorite Java solution.
And props go to **you** for improving the API and code so much. Really appreciate all your work in reviewing issues, sending new code (with the extra work of chasing test images), and in educating other devs like myself :)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917472391
I looked up rebasing and squashing commits and I wanted to check with you before proceeding.
I have 6 commits that I just pushed as part of this PR. So would my command be the following?
> git rebase -i HEAD~6
But as I was researching this, I found a comment:
> If you have previously pushed your code to a remote branch, you will need to force push.
>
> git push origin branchName --force
Do I need to do the force when I push? And, if so, would my command be the following?
> git push origin IMAGING-266a --force
where IMAGING-266a is the branch name for these changes?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917478355
Hi @gwlucastrig !
Great job fixing the code comments and trying to rebase.
You have done everything correctly. And you will have indeed to push force.
I always take a look at the remote branch in GitHub, and compare with my local branch (using gitk).
If I'm happy that my local branch is the way I want my remote to look, then I push force.
Note that you should avoid push-force to certain repos in case of existijg etiquette. E.g. linux project forbids push force to their main branches. Apache projects normally discourage too.
But you are pushing force to your branch, to your fork. So it doesn't matter in this case. It's only to make merging the code easier :)
I will take a look at the code latert oday gain, if nothing wrong will include one commit later with changes.xml and merge it! 👏
p.s.: a git rebase squash is an advanced git topic, great job! It is also part of the process for fixing conflicts, so you got most of what's needed to fix conflicts in the future
p.p.s: I forgot to add a comment about scansize. Is it OK if I edit your commit when merging, and rename it to scanSize?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917247770
Hope you and your wife stay healthy.
On Fri, Sep 10, 2021, 5:38 PM Bruno P. Kinoshita ***@***.***>
wrote:
> Thanks @gwlucastrig <https://github.com/gwlucastrig> !
>
> Github is reporting that more changes are requested, but searching through
> this discussion, I don't see anything that's unresolved.
>
> It's because when I submitted my review, I had marked it as request
> changes. It will remain with that status until I review it again. We are in
> lockdown in Auckland due to the delta variant, so quite sure I'll be able
> to finish reviewing it over this weekend.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/165#issuecomment-917227883>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYKARSB6OHMRODCUNKTUBJ3FZANCNFSM5DWCCJSA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r704981060
##########
File path: src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java
##########
@@ -762,12 +763,12 @@ public void writeImage(final BufferedImage src, final OutputStream os, final Map
}
/**
- * Reads the content of a TIFF file that contains floating-point data
- * samples.
+ * Reads the content of a TIFF file that contains numerical data samples
+ * rather than image-related pixels.
* <p>
- * If desired, sub-image data can be read from the file by using a Java Map
- * instance to specify the subsection of the image that is required. The
- * following code illustrates the approach:
+ * If desired, sub-image data can be read from the file by using a Java
+ * {@code Map} instance to specify the subsection of the image that
+ * isrequired. The following code illustrates the approach:
Review comment:
s/isrequired/is required?
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffShortIntRoundTripTest.java
##########
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.awt.image.BufferedImage;
Review comment:
Not used.
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
Review comment:
Unnecessary casting.
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
Review comment:
Unnecessary casting.
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
+ assertEquals(index, test, "Set/get int value test failed");
+ }
+ }
+ }
+
+ /**
+ * Test of getValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetValue() {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + x;
+ int test = (int) raster.getValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ test = (int) raster.getIntValue(x, y);
Review comment:
Unnecessary casting.
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
+ assertEquals(index, test, "Set/get int value test failed");
+ }
+ }
+ }
+
+ /**
+ * Test of getValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetValue() {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + x;
+ int test = (int) raster.getValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ test = (int) raster.getIntValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ }
+ }
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_0args() {
+
+ final TiffRasterStatistics result = raster.getSimpleStatistics();
+ assertEquals(0, result.getMinValue(), "Min value failure");
+ assertEquals(width * height - 1, result.getMaxValue(), "Max value failure");
+ assertEquals(meanValue, result.getMeanValue(), "Mean value failure");
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_float() {
+ // exclude the maximum value (width*height-1). This will result
+ // in a max value of width*height-2
+ final TiffRasterStatistics result = raster.getSimpleStatistics(width * height - 1);
+ assertEquals(width * height - 2, result.getMaxValue(), "Max value failure");
+ }
+
+ /**
+ * Test of getWidth method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetWidth() {
+ assertEquals(width, raster.getWidth(), "Improper width stored");
+ }
+
+ /**
+ * Test of getHeight method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetHeight() {
+ assertEquals(width, raster.getWidth(), "Improper height stored");
+ }
+
+ /**
+ * Test of getData method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetData() {
+ final float[] result = raster.getData();
+ for(int i=0; i<result.length; i++){
+ assertEquals((int)result[i], data[i]);
+ }
+ final int []iResult = raster.getIntData();
+ assertArrayEquals(data, iResult);
+ }
+
+
+ /**
+ * Test of constructors with bad arguments, of class TiffRasterData.
+ */
+ @Test
+ public void testBadConstructor() {
+ try{
+ final TiffRasterData raster = new TiffRasterDataInt(-1, 10);
+ fail("Constructor did not detect bad width");
+ }catch(final IllegalArgumentException illArgEx){
+ // success!
+ }
Review comment:
@gwlucastrig I believe the tests that are using this pattern `try { something; fail(); } catch {}` is JUnit 4. But for Junit 5 we can use:
```java
assertThrows(IllegalArgumentException.class, () -> new TiffRasterDataInt(-1, 10));
```
There are other places in this PR where this can be applied, you can look at GitHub's UI to search for this "fail(" text if you'd like.
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
+ assertEquals(index, test, "Set/get int value test failed");
+ }
+ }
+ }
+
+ /**
+ * Test of getValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetValue() {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + x;
+ int test = (int) raster.getValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ test = (int) raster.getIntValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ }
+ }
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_0args() {
+
+ final TiffRasterStatistics result = raster.getSimpleStatistics();
+ assertEquals(0, result.getMinValue(), "Min value failure");
+ assertEquals(width * height - 1, result.getMaxValue(), "Max value failure");
+ assertEquals(meanValue, result.getMeanValue(), "Mean value failure");
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_float() {
+ // exclude the maximum value (width*height-1). This will result
+ // in a max value of width*height-2
+ final TiffRasterStatistics result = raster.getSimpleStatistics(width * height - 1);
+ assertEquals(width * height - 2, result.getMaxValue(), "Max value failure");
+ }
+
+ /**
+ * Test of getWidth method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetWidth() {
+ assertEquals(width, raster.getWidth(), "Improper width stored");
+ }
+
+ /**
+ * Test of getHeight method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetHeight() {
+ assertEquals(width, raster.getWidth(), "Improper height stored");
+ }
+
+ /**
+ * Test of getData method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetData() {
+ final float[] result = raster.getData();
+ for(int i=0; i<result.length; i++){
+ assertEquals((int)result[i], data[i]);
+ }
+ final int []iResult = raster.getIntData();
+ assertArrayEquals(data, iResult);
Review comment:
Tabs and spaces here @gwlucastrig :point_up: (better confirm other files don't have tabs, only spotted it here due to the lines not aligned due to mix spaces/tabs, but we could have in the other files).
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706662776
##########
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:
Actually, tile length is what they call it in the TIFFF specification. Not sure they used that term, though it might be a holdover from the earlier terminology strip-length.
In either case, I think we should leave this one as is...
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-915708190
Sorry, I forgot about the Spotbugs test (did checkstyle, pmd, and findbugs, but not Spotbugs). So I just submitted changes for Spotbugs.
For reasons of efficiency, the TiffRasterData classes expose arrays directly through the API. This approach is reasonable because the raster data classes are really just simple containers for transferring image read results to the calling application. Also, since many TIFF rasters consist of literally 100s of millions of raster cells, we wish to avoid making redundant copies of data in memory. This rationale is described in the Javadoc for the relevant methods with appropriate caveats for developers.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706663369
##########
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:
Done
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917502626
@gwlucastrig great job! GitHUb UI is already showing 1 commit, so you have successfully rebased and squashed your commits :-)
I had marked during the previous review all files reviewed as "Viewed". Now, when I go to Files Changed, @gwlucastrig , GitHub keeps the files that did not change closed. And it expands the ones that suffered modifications since I left reviewed it.
This saves a lot of time since I don't need to re-read the code to compare with the previous version.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r705470722
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
+ assertEquals(index, test, "Set/get int value test failed");
+ }
+ }
+ }
+
+ /**
+ * Test of getValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetValue() {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + x;
+ int test = (int) raster.getValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ test = (int) raster.getIntValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ }
+ }
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_0args() {
+
+ final TiffRasterStatistics result = raster.getSimpleStatistics();
+ assertEquals(0, result.getMinValue(), "Min value failure");
+ assertEquals(width * height - 1, result.getMaxValue(), "Max value failure");
+ assertEquals(meanValue, result.getMeanValue(), "Mean value failure");
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_float() {
+ // exclude the maximum value (width*height-1). This will result
+ // in a max value of width*height-2
+ final TiffRasterStatistics result = raster.getSimpleStatistics(width * height - 1);
+ assertEquals(width * height - 2, result.getMaxValue(), "Max value failure");
+ }
+
+ /**
+ * Test of getWidth method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetWidth() {
+ assertEquals(width, raster.getWidth(), "Improper width stored");
+ }
+
+ /**
+ * Test of getHeight method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetHeight() {
+ assertEquals(width, raster.getWidth(), "Improper height stored");
+ }
+
+ /**
+ * Test of getData method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetData() {
+ final float[] result = raster.getData();
+ for(int i=0; i<result.length; i++){
+ assertEquals((int)result[i], data[i]);
+ }
+ final int []iResult = raster.getIntData();
+ assertArrayEquals(data, iResult);
Review comment:
Did a search through the code space. These are the files that contain tabs
I will fix TiffRasterDataIntTest.java as part of the PR.
\test\java\org\apache\commons\imaging\formats\icns\IcnsReadTest.java (20 hits)
\test\java\org\apache\commons\imaging\formats\png\PngWithInvalidPngChunkSizeTest.java (20 hits)
\test\java\org\apache\commons\imaging\formats\tiff\TiffRasterDataIntTest.java (13 hits)
\test\java\org\apache\commons\imaging\TestImageReadException.java (18 hits)
\test\java\org\apache\commons\imaging\TestImageWriteException.java (80 hits)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow merged pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
kinow merged pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706663275
##########
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:
done
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706662585
##########
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:
It took a bit of searching to find what this is about.
public static final int SAMPLE_FORMAT_VALUE_COMPLEX_INTEGER = 5;
public static final int SAMPLE_FORMAT_VALUE_IEEE_FLOATING_POINT_1 = 6;
Sample formats 5 (complex integer) and 6 (complex float) are not actually part of the TIFF spec, but seem to have been introduced by LibTIFF at some point. There is no support for complex-conjugate-pairs in Commons Imaging at this time. Neither of these values are used elsewhere in the the code.
At the very least, I will rename FLOATING_POINT_1 to be COMPLEX_FLOAT.
You can read more at
[Extension TIFF Tag Sample Format](https://www.awaresystems.be/imaging/tiff/tifftags/sampleformat.html)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-917227883
Thanks @gwlucastrig !
>Github is reporting that more changes are requested, but searching through this discussion, I don't see anything that's unresolved.
It's because when I submitted my review, I had marked it as request changes. It will remain with that status until I review it again. We are in lockdown in Auckland due to the delta variant, so quite sure I'll be able to finish reviewing it over this weekend.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#issuecomment-915723586
[](https://coveralls.io/builds/42806305)
Coverage increased (+0.1%) to 76.837% when pulling **8c6b2cc879cf7d9918e7db4052d7417c5c951229 on gwlucastrig:IMAGING-266a** into **32e1a3b839eb75b38c47531cb2dd6e0012bb3bf5 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #165: IMAGING-266 read numeric data from GeoTIFFs
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #165:
URL: https://github.com/apache/commons-imaging/pull/165#discussion_r706026440
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffShortIntRoundTripTest.java
##########
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.awt.image.BufferedImage;
Review comment:
added a new test that uses it.
##########
File path: src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java
##########
@@ -762,12 +763,12 @@ public void writeImage(final BufferedImage src, final OutputStream os, final Map
}
/**
- * Reads the content of a TIFF file that contains floating-point data
- * samples.
+ * Reads the content of a TIFF file that contains numerical data samples
+ * rather than image-related pixels.
* <p>
- * If desired, sub-image data can be read from the file by using a Java Map
- * instance to specify the subsection of the image that is required. The
- * following code illustrates the approach:
+ * If desired, sub-image data can be read from the file by using a Java
+ * {@code Map} instance to specify the subsection of the image that
+ * isrequired. The following code illustrates the approach:
Review comment:
done
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
Review comment:
done
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
Review comment:
done
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
+ assertEquals(index, test, "Set/get int value test failed");
+ }
+ }
+ }
+
+ /**
+ * Test of getValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetValue() {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + x;
+ int test = (int) raster.getValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ test = (int) raster.getIntValue(x, y);
Review comment:
done
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffRasterDataIntTest.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.tiff;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Provides unit test for the raster-data class.
+ */
+public class TiffRasterDataIntTest {
+
+ int width = 11;
+ int height = 10;
+ int[] data;
+ TiffRasterData raster;
+ float meanValue;
+
+ public TiffRasterDataIntTest() {
+ double sum = 0;
+ data = new int[width * height];
+ int k = 0;
+ for (int i = 0; i < width; i++) {
+ for (int j = 0; j < height; j++) {
+ data[k] = (int)k;
+ sum += k;
+ k++;
+ }
+ }
+ raster = new TiffRasterDataInt(width, height, data);
+ meanValue = (float) (sum / k);
+ }
+
+ /**
+ * Test of setValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testSetValue() {
+ final TiffRasterData instance = new TiffRasterDataInt(width, height);
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + height;
+ instance.setValue(x, y, index+0.4f);
+ int test = (int) instance.getValue(x, y);
+ assertEquals(index, test, "Set/get value test failed");
+ instance.setIntValue(x, y, index);
+ test = (int) instance.getIntValue(x, y);
+ assertEquals(index, test, "Set/get int value test failed");
+ }
+ }
+ }
+
+ /**
+ * Test of getValue method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetValue() {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
+ final int index = y * width + x;
+ int test = (int) raster.getValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ test = (int) raster.getIntValue(x, y);
+ assertEquals(index, test, "Get into source data test failed at (" + x + "," + y + ")");
+ }
+ }
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_0args() {
+
+ final TiffRasterStatistics result = raster.getSimpleStatistics();
+ assertEquals(0, result.getMinValue(), "Min value failure");
+ assertEquals(width * height - 1, result.getMaxValue(), "Max value failure");
+ assertEquals(meanValue, result.getMeanValue(), "Mean value failure");
+ }
+
+ /**
+ * Test of getSimpleStatistics method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetSimpleStatistics_float() {
+ // exclude the maximum value (width*height-1). This will result
+ // in a max value of width*height-2
+ final TiffRasterStatistics result = raster.getSimpleStatistics(width * height - 1);
+ assertEquals(width * height - 2, result.getMaxValue(), "Max value failure");
+ }
+
+ /**
+ * Test of getWidth method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetWidth() {
+ assertEquals(width, raster.getWidth(), "Improper width stored");
+ }
+
+ /**
+ * Test of getHeight method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetHeight() {
+ assertEquals(width, raster.getWidth(), "Improper height stored");
+ }
+
+ /**
+ * Test of getData method, of class TiffRasterData.
+ */
+ @Test
+ public void testGetData() {
+ final float[] result = raster.getData();
+ for(int i=0; i<result.length; i++){
+ assertEquals((int)result[i], data[i]);
+ }
+ final int []iResult = raster.getIntData();
+ assertArrayEquals(data, iResult);
+ }
+
+
+ /**
+ * Test of constructors with bad arguments, of class TiffRasterData.
+ */
+ @Test
+ public void testBadConstructor() {
+ try{
+ final TiffRasterData raster = new TiffRasterDataInt(-1, 10);
+ fail("Constructor did not detect bad width");
+ }catch(final IllegalArgumentException illArgEx){
+ // success!
+ }
Review comment:
done
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org