This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
commit 2fc21218f5b3041ca7e191133fb7d7efd9dfbcb9
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Thu Aug 12 01:14:53 2021 +0200
When reading multi-pixels packed images, restrict sub-region to element
boundaries (typically byte boundaries).
It allows the `SelfConsistencyCheck` test to pass for random sub-regions.
---
.../java/org/apache/sis/util/ArgumentChecks.java | 23 +++++++++++--
.../java/org/apache/sis/util/resources/Errors.java | 4 +--
.../apache/sis/util/resources/Errors.properties | 2 +-
.../apache/sis/util/resources/Errors_fr.properties | 2 +-
.../apache/sis/internal/geotiff/CopyFromBytes.java | 20 ++++++++---
.../org/apache/sis/internal/geotiff/Inflater.java | 23 ++++++++++++-
.../sis/storage/geotiff/CompressedSubset.java | 12 +++----
.../org/apache/sis/storage/geotiff/DataSubset.java | 3 --
.../sis/internal/storage/TiledGridResource.java | 39 ++++++++++++++++++++--
9 files changed, 104 insertions(+), 24 deletions(-)
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/util/ArgumentChecks.java
b/core/sis-utility/src/main/java/org/apache/sis/util/ArgumentChecks.java
index a855838..8acf005 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/util/ArgumentChecks.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/util/ArgumentChecks.java
@@ -703,7 +703,7 @@ public final class ArgumentChecks extends Static {
*
* @param name name of the argument for the divisor value. Used only
if an exception is thrown.
* @param number the number to be divided.
- * @param divisor the divisor to verify.
+ * @param divisor the value to verify.
* @throws IllegalArgumentException if {@code (number % divisor) != 0}.
* @throws ArithmeticException if {@code divisor == 0}.
*
@@ -711,7 +711,26 @@ public final class ArgumentChecks extends Static {
*/
public static void ensureDivisor(final String name, final int number,
final int divisor) {
if ((number % divisor) != 0) {
- throw new
IllegalArgumentException(Errors.format(Errors.Keys.NotADivisor_3, name, number,
divisor));
+ throw new
IllegalArgumentException(Errors.format(Errors.Keys.NotADivisorOrMultiple_4,
name, 0, number, divisor));
+ }
+ }
+
+ /**
+ * Ensures that a given value is a multiple of specified number.
+ * This method verifies that {@code (multiple % number) == 0}.
+ * If above condition is not met, the value considered to be wrong is the
multiple.
+ *
+ * @param name name of the argument for the multiple value. Used
only if an exception is thrown.
+ * @param number the number to be multiplied.
+ * @param multiple the value to verify.
+ * @throws IllegalArgumentException if {@code (multiple % number) != 0}.
+ * @throws ArithmeticException if {@code number == 0}.
+ *
+ * @since 1.1
+ */
+ public static void ensureMultiple(final String name, final int number,
final int multiple) {
+ if ((multiple % number) != 0) {
+ throw new
IllegalArgumentException(Errors.format(Errors.Keys.NotADivisorOrMultiple_4,
name, 1, number, multiple));
}
}
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.java
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.java
index 312eb45..cfc3af0 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.java
@@ -700,9 +700,9 @@ public final class Errors extends IndexedResourceBundle {
public static final short NotABackwardReference_1 = 108;
/**
- * Value of ‘{0}’ shall be a divisor of {1} but the provided value {2}
is not.
+ * Value of ‘{0}’ shall be a {1,choice,0#divisor|1#multiple} of {2}
but the given value is {3}.
*/
- public static final short NotADivisor_3 = 193;
+ public static final short NotADivisorOrMultiple_4 = 193;
/**
* “{0}” is not a key-value pair.
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.properties
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.properties
index 8c2cbf2..eee7216 100644
---
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.properties
+++
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors.properties
@@ -149,7 +149,7 @@ NonTemporalUnit_1 = \u201c{0}\u201d is not
a time unit.
NonSystemUnit_1 = \u201c{0}\u201d is not a fundamental or
derived unit.
NonRatioUnit_1 = The scale of measurement for
\u201c{0}\u201d unit is not a ratio scale.
NotABackwardReference_1 = No element for the \u201c{0}\u201d
identifier, or the identifier is a forward reference.
-NotADivisor_3 = Value of \u2018{0}\u2019 shall be a
divisor of {1} but the provided value {2} is not.
+NotADivisorOrMultiple_4 = Value of \u2018{0}\u2019 shall be a
{1,choice,0#divisor|1#multiple} of {2} but the given value is {3}.
NotAnInteger_1 = {0} is not an integer value.
NotAKeyValuePair_1 = \u201c{0}\u201d is not a key-value pair.
NotANumber_1 = Argument \u2018{0}\u2019 shall not be NaN
(Not-a-Number).
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors_fr.properties
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors_fr.properties
index ce9e5ed..e0ac843 100644
---
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors_fr.properties
+++
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Errors_fr.properties
@@ -146,7 +146,7 @@ NonTemporalUnit_1 =
\u00ab\u202f{0}\u202f\u00bb n\u2019est pas u
NonSystemUnit_1 = \u00ab\u202f{0}\u202f\u00bb n\u2019est pas
une unit\u00e9 fondamentale ou d\u00e9riv\u00e9e.
NonRatioUnit_1 = L\u2019\u00e9chelle de mesure de
l\u2019unit\u00e9 \u00ab\u202f{0}\u202f\u00bb n\u2019est pas une \u00e9chelle
de rapports.
NotABackwardReference_1 = Il n\u2019y a pas d\u2019\u00e9l\u00e9ment
pour l\u2019identifiant \u201c{0}\u201d, ou l\u2019identifiant est une
r\u00e9f\u00e9rence vers l\u2019avant.
-NotADivisor_3 = La valeur de \u2018{0}\u2019 doit
\u00eatre un diviseur de {1}, mais la valeur fournie {2} ne l\u2019est pas.
+NotADivisorOrMultiple_4 = La valeur de \u2018{0}\u2019 doit
\u00eatre un {1,choice,0#diviseur|1#multiple} de {2}, mais la valeur
donn\u00e9e est {3}.
NotAnInteger_1 = {0} n\u2019est pas un nombre entier.
NotAKeyValuePair_1 = \u00ab\u202f{0}\u202f\u00bb n\u2019est pas
une paire cl\u00e9-valeur.
NotANumber_1 = L\u2019argument \u2018{0}\u2019 ne doit
pas \u00eatre NaN (Not-a-Number).
diff --git
a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/CopyFromBytes.java
b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/CopyFromBytes.java
index b476a70..e18d8b6 100644
---
a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/CopyFromBytes.java
+++
b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/CopyFromBytes.java
@@ -128,6 +128,11 @@ abstract class CopyFromBytes extends Inflater {
* Skips the given amount of sample values without storing them.
* The given value is in units of sample values, not in bytes.
*
+ * <h4>Case of multi-pixels packed image</h4>
+ * It is caller's responsibility to ensure that <var>n</var> is a multiple
of {@link #samplesPerElement}
+ * if this method is not invoked for skipping all remaining values until
end of row.
+ * See {@link Inflater#skip(long)} for more information.
+ *
* @param n number of uncompressed sample values to ignore.
* @throws IOException if an error occurred while reading the input
channel.
*/
@@ -138,11 +143,16 @@ abstract class CopyFromBytes extends Inflater {
positionNeedsRefresh = false;
streamPosition = input.getStreamPosition();
}
- n *= bytesPerElement;
- if (n % samplesPerElement != 0) {
- throw new IllegalArgumentException(); // Condition should
have been verified before construction.
- }
- streamPosition = Math.addExact(streamPosition, n /
samplesPerElement);
+ /*
+ * Convert number of sample values to number of elements, then to
number of bytes.
+ * The number of sample values to skip shall be a multiple of
`samplesPerElement`,
+ * except when skipping all remaining values until end of row. We
do not verify
+ * because this method does not know when the row ends.
+ */
+ final boolean r = (n % samplesPerElement) > 0;
+ n /= samplesPerElement; if (r) n++; // Round after
division to next element boundary.
+ n *= bytesPerElement; // Must be
multiplied only after above rounding.
+ streamPosition = Math.addExact(streamPosition, n);
}
}
diff --git
a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/Inflater.java
b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/Inflater.java
index e5987c2..b158ed5 100644
---
a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/Inflater.java
+++
b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/geotiff/Inflater.java
@@ -107,7 +107,7 @@ public abstract class Inflater {
for (int i=0; i<skipAfterChunks.length; i++) {
final int s = skipAfterChunks[i];
ArgumentChecks.ensurePositive("skipAfterChunks", s);
- ArgumentChecks.ensureDivisor("samplesPerElement", s,
samplesPerElement);
+ ArgumentChecks.ensureMultiple("skipAfterChunks",
samplesPerElement, s);
skipAfterChunks[i] /= samplesPerElement;
}
}
@@ -198,6 +198,27 @@ public abstract class Inflater {
* Reads the given amount of sample values without storing them.
* The given value is in units of sample values, not in bytes.
*
+ * <h4>Case of multi-pixels packed image</h4>
+ * If there is more than one sample value per element, then this method
may round (at implementation choice)
+ * the stream position to the first element boundary after skipping
<var>n</var> sample values. For example
+ * a bilevel image packs 8 sample values per byte. Consequently a call to
{@code skip(10)} may skip 2 bytes:
+ * one byte for the first 8 bits, then 2 bits rounded to the next byte
boundary.
+ *
+ * <p>The exact rounding mode depends on the compression algorithm. To
avoid erratic behavior, callers shall
+ * restrict <var>n</var> to multiples of {@code samplesPerElement} before
to read the first pixel in a row.
+ * This restriction does not apply when skipping remaining data after the
last pixels in a row, because the
+ * next row will be realigned on an element boundary anyway. The way to
perform this realignment depends on
+ * the compression, which is the reason why the exact rounding mode may
vary with compression algorithms.</p>
+ *
+ * <p>Restricting <var>n</var> to multiples of {@code samplesPerElement}
implies the following restrictions:</p>
+ * <ul>
+ * <li>No subsampling on the <var>x</var> axis (however subsampling on
other axes is still allowed).</li>
+ * <li>No band subset if interleaved sample model, because the effect is
similar to subsampling.</li>
+ * <li>If {@link org.apache.sis.coverage.grid.GridDerivation} is used
for computing the sub-region to read,
+ * it should have been configured with {@code
chunkSize(samplesPerElement)}
+ * (unless chunk size is already used for tile size).</li>
+ * </ul>
+ *
* @param n number of uncompressed sample values to ignore.
* @throws IOException if an error occurred while reading the input
channel.
*/
diff --git
a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/CompressedSubset.java
b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/CompressedSubset.java
index 92b50bd..bf62d2d 100644
---
a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/CompressedSubset.java
+++
b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/CompressedSubset.java
@@ -128,7 +128,7 @@ final class CompressedSubset extends DataSubset {
// Number of sample values to skip after each band.
skips[i] = b - (b = includedBands[i]) - 1;
}
- beforeFirstBand = b;
+ beforeFirstBand = b; // After above
loop, `b` became the index of first band.
afterLastBand += skips[m]; // Add trailing
bands that were left unread.
skips[m] += between + beforeFirstBand; // Add pixels
skipped by subsampling and move to first band.
/*
@@ -225,13 +225,15 @@ final class CompressedSubset extends DataSubset {
final int samplesPerElement = typeSize / sampleSize; //
Always ≥ 1 and usually = 1.
if (typeSize % sampleSize != 0) {
throw new RasterFormatException(reader().errors().getString(
- Errors.Keys.NotADivisor_3, "BitsPerSample", typeSize,
sampleSize));
+ Errors.Keys.NotADivisorOrMultiple_4, "BitsPerSample",
0, typeSize, sampleSize));
}
- final Buffer bank = RasterFactory.createBuffer(type,
getBankCapacity(samplesPerElement));
+ // TiledGridResource shall ensure that following
`Inflater.skip(long)` restriction is met.
+ assert (head % samplesPerElement) == 0 : head;
/*
* Prepare the object which will perform the actual decompression
row-by-row,
* optionally skipping chunks if a subsampling is applied.
*/
+ final Buffer bank = RasterFactory.createBuffer(type,
getBankCapacity(samplesPerElement));
final Inflater algo = Inflater.create(compression, reader().input,
offsets[b], byteCounts[b],
getTileSize(0), chunksPerRow,
samplesPerChunk, skipAfterChunks,
samplesPerElement, bank);
@@ -240,9 +242,7 @@ final class CompressedSubset extends DataSubset {
Resources.Keys.UnsupportedCompressionMethod_1,
compression));
}
for (long y = lower[1]; --y >= 0;) {
- algo.skip(scanlineStride);
- // TODO: after we finished to implement decompression
algorithms,
- // revisit if we can safely replace the loop by a
multiplication.
+ algo.skip(scanlineStride); // `skip(…)` may round to
next element boundary.
}
for (int y = height; --y > 0;) { // (height - 1) iterations.
algo.skip(head);
diff --git
a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/DataSubset.java
b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/DataSubset.java
index 92d1624..9657b38 100644
---
a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/DataSubset.java
+++
b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/DataSubset.java
@@ -208,9 +208,6 @@ class DataSubset extends TiledGridCoverage implements
Localized {
if (model instanceof ComponentSampleModel) {
return DataBuffer.getDataTypeSize(model.getDataType());
}
- if (numBanks != 1) {
- return model.getSampleSize(bank);
- }
int size = 0;
for (int b = model.getNumBands(); --b >= 0;) {
size += model.getSampleSize(b);
diff --git
a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridResource.java
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridResource.java
index 0c171cf..45df226 100644
---
a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridResource.java
+++
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridResource.java
@@ -18,8 +18,10 @@ package org.apache.sis.internal.storage;
import java.util.List;
import java.util.Arrays;
+import java.awt.image.DataBuffer;
import java.awt.image.ColorModel;
import java.awt.image.SampleModel;
+import java.awt.image.ComponentSampleModel;
import java.awt.image.RenderedImage;
import java.awt.image.WritableRaster;
import java.awt.image.RasterFormatException;
@@ -133,11 +135,42 @@ public abstract class TiledGridResource extends
AbstractGridResource {
* Returns the size of tiles in this resource.
* The length of the returned array is the number of dimensions.
*
- * @return the size of tiles in this resource.
+ * @return the size of tiles (in pixels) in this resource.
*/
protected abstract int[] getTileSize();
/**
+ * Returns the number of sample values in an indivisible element of a tile.
+ * An element is a primitive type such as {@code byte}, {@code int} or
{@code float}.
+ * This value is usually 1 because each sample value is usually stored in
a separated element.
+ * However in multi-pixels packed sample model (e.g. bilevel image with 8
pixels per byte),
+ * it is difficult to start reading an image at <var>x</var> location
other than a byte boundary.
+ * By declaring an "atom" size of 8 sample values in dimension 0, the
{@link Subset} constructor
+ * will ensure than the sub-region to read starts at a byte boundary when
reading a bilevel image.
+ *
+ * @param dimension the dimension for which to get the atom size.
+ * This is in units of sample values (may be bits, bytes, floats,
<i>etc</i>).
+ * @return indivisible amount of sample values to read in the specified
dimension. Must be ≥ 1.
+ * @throws DataStoreException if an error occurred while fetching the
sample model.
+ */
+ private int getAtomSize(final int dimension) throws DataStoreException {
+ if (dimension == 0) {
+ final SampleModel model = getSampleModel();
+ if (model != null && !(model instanceof ComponentSampleModel)) {
+ int size = 1;
+ for (int b = model.getNumBands(); --b >= 0;) {
+ size = Math.max(size, model.getSampleSize(b));
+ }
+ final int samplesPerElement =
DataBuffer.getDataTypeSize(model.getDataType()) / size;
+ if (samplesPerElement >= 1) {
+ return samplesPerElement;
+ }
+ }
+ }
+ return 1;
+ }
+
+ /**
* Returns the Java2D sample model describing pixel type and layout for
all bands.
* The raster size is the {@linkplain #getTileSize() tile size} as stored
in the resource.
*
@@ -295,8 +328,8 @@ public abstract class TiledGridResource extends
AbstractGridResource {
*/
int tileWidth = tileSize[0];
int tileHeight = tileSize[1];
- if (tileWidth >= sourceExtent.getSize(0)) {tileWidth = 1;
sharedCache = false;}
- if (tileHeight >= sourceExtent.getSize(1)) {tileHeight = 1;
sharedCache = false;}
+ if (tileWidth >= sourceExtent.getSize(0)) {tileWidth =
getAtomSize(0); sharedCache = false;}
+ if (tileHeight >= sourceExtent.getSize(1)) {tileHeight =
getAtomSize(1); sharedCache = false;}
final GridDerivation target =
gridGeometry.derive().chunkSize(tileWidth, tileHeight)
.rounding(GridRoundingMode.ENCLOSING).subgrid(domain);
domain = target.build();
