You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2021/08/11 23:32:43 UTC

[sis] branch geoapi-4.0 updated (3b223ec -> 3bc831a)

This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a change to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git.


    from 3b223ec  Fix a case where `GridDerivation` was not expanding the extent to an integer amount of tiles. Fix a `NullPointerException` when `gridToCRS` is null (i.e. when data have no known location).
     new 022a1f3  Consolidation: - Safer rounding of subsampling values. - Add test and clarifications in Javadoc.
     new 719cbff  Renamed `selectedBands` as `includedBands` because a band may be loaded despite not being selected by user.
     new 2fc2121  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.
     new 3bc831a  Group together the display of statistics about execution time.

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/sis/coverage/grid/GridDerivation.java   | 40 ++++++++---
 .../sis/coverage/grid/GridDerivationTest.java      | 24 +++++++
 .../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      | 22 +++---
 .../org/apache/sis/storage/geotiff/DataSubset.java | 27 ++++---
 .../org/apache/sis/storage/geotiff/GeoTIFF.java    |  2 +-
 .../apache/sis/storage/geotiff/GeoTiffStore.java   |  2 +-
 .../sis/internal/storage/AbstractResource.java     |  7 ++
 .../sis/internal/storage/TiledGridCoverage.java    | 12 ++--
 .../sis/internal/storage/TiledGridResource.java    | 82 ++++++++++++++++------
 .../apache/sis/storage/event/StoreListeners.java   |  6 +-
 .../sis/test/storage/CoverageReadConsistency.java  | 49 ++++++++++---
 17 files changed, 257 insertions(+), 90 deletions(-)

[sis] 02/04: Renamed `selectedBands` as `includedBands` because a band may be loaded despite not being selected by user.

Posted by de...@apache.org.
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 719cbff5a75b0a9bc88d7752e1d7401f35cb4c72
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Wed Aug 11 18:02:40 2021 +0200

    Renamed `selectedBands` as `includedBands` because a band may be loaded despite not being selected by user.
---
 .../sis/storage/geotiff/CompressedSubset.java      | 10 ++---
 .../org/apache/sis/storage/geotiff/DataSubset.java | 24 ++++++------
 .../sis/internal/storage/TiledGridCoverage.java    | 12 +++---
 .../sis/internal/storage/TiledGridResource.java    | 43 ++++++++++++----------
 4 files changed, 48 insertions(+), 41 deletions(-)

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 ef90088..92b50bd 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
@@ -94,7 +94,7 @@ final class CompressedSubset extends DataSubset {
      *   <li>1 if a chunk is a sample value.</li>
      *   <li>{@link #sourcePixelStride} if a chunk is a full pixel.</li>
      *   <li>Any intermediate value if some optimizations have been applied,
-     *       for example for taking advantage of consecutive indices in {@link #selectedBands}.</li>
+     *       for example for taking advantage of consecutive indices in {@link #includedBands}.</li>
      * </ul>
      *
      * This value shall always be a divisor of {@link #targetPixelStride}.
@@ -120,13 +120,13 @@ final class CompressedSubset extends DataSubset {
         scanlineStride = multiplyFull(getTileSize(0), sourcePixelStride);
         final int between = sourcePixelStride * (getSubsampling(0) - 1);
         int afterLastBand = sourcePixelStride * (getTileSize(0) - 1);
-        if (selectedBands != null && sourcePixelStride > 1) {
-            final int[] skips = new int[selectedBands.length];
+        if (includedBands != null && sourcePixelStride > 1) {
+            final int[] skips = new int[includedBands.length];
             final int m = skips.length - 1;
             int b = sourcePixelStride;
             for (int i=m; i >= 0; --i) {
                 // Number of sample values to skip after each band.
-                skips[i] = b - (b = selectedBands[i]) - 1;
+                skips[i] = b - (b = includedBands[i]) - 1;
             }
             beforeFirstBand = b;
             afterLastBand  += skips[m];                     // Add trailing bands that were left unread.
@@ -139,7 +139,7 @@ final class CompressedSubset extends DataSubset {
              * then we can read those 3 bands as a "chunk" on 3 sample values instead of reading 3 chunks of 1 value.
              */
             if (m != 0 && startsWithZeros(skips, m)) {
-                samplesPerChunk = selectedBands.length;
+                samplesPerChunk = includedBands.length;
                 skipAfterChunks = new int[] {skips[m]};
             } else {
                 samplesPerChunk = 1;
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 25a5200..92d1624 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
@@ -105,7 +105,7 @@ class DataSubset extends TiledGridCoverage implements Localized {
      * Number of banks retained for the target image data buffer.
      * This is equal to the number of bands only for planar images, and 1 in all other cases.
      * If the user asked to read only a subset of the bands, then "number of bands" in above
-     * sentence is the {@linkplain #selectedBands subset} size.
+     * sentence is the {@linkplain #includedBands subset} size.
      */
     protected final int numBanks;
 
@@ -153,8 +153,8 @@ class DataSubset extends TiledGridCoverage implements Localized {
         if (model instanceof BandedSampleModel) {
             numBanks = model.getNumBands();
             sourcePixelStride = targetPixelStride = 1;
-            maxBank = (selectedBands != null) ? selectedBands[selectedBands.length - 1] : numBanks - 1;
-            // Note: `selectedBands` is in strictly increasing order, so taking the last value is okay.
+            maxBank = (includedBands != null) ? includedBands[includedBands.length - 1] : numBanks - 1;
+            // Note: `includedBands` is in strictly increasing order, so taking the last value is okay.
         } else {
             maxBank  = 0;
             numBanks = 1;
@@ -259,11 +259,11 @@ class DataSubset extends TiledGridCoverage implements Localized {
          *
          * @param iterator  the iterator for which to create a snapshot of its current position.
          */
-        Tile(final AOI domain, final Vector tileOffsets, final int[] selectedBanks, final int numTiles) {
+        Tile(final AOI domain, final Vector tileOffsets, final int[] includedBanks, final int numTiles) {
             super(domain);
             int p = indexInTileVector;
-            if (selectedBanks != null) {
-                p += selectedBanks[0] * numTiles;
+            if (includedBanks != null) {
+                p += includedBanks[0] * numTiles;
             }
             byteOffset = tileOffsets.longValue(p);
         }
@@ -277,9 +277,9 @@ class DataSubset extends TiledGridCoverage implements Localized {
          * @param  target    the array where to copy vector values.
          * @param  numTiles  value of {@link DataSubset#numTiles}.
          */
-        final void copyTileInfo(final Vector source, final long[] target, final int[] selectedBanks, final int numTiles) {
+        final void copyTileInfo(final Vector source, final long[] target, final int[] includedBanks, final int numTiles) {
             for (int j=0; j<target.length; j++) {
-                final int i = indexInTileVector + numTiles * (selectedBanks != null ? selectedBanks[j] : j);
+                final int i = indexInTileVector + numTiles * (includedBanks != null ? includedBanks[j] : j);
                 target[j] = source.longValue(i);
             }
         }
@@ -317,7 +317,7 @@ class DataSubset extends TiledGridCoverage implements Localized {
          * Each tile will either store all sample values in an interleaved fashion inside a single bank
          * (`sourcePixelStride` > 1) or use one separated bank per band (`sourcePixelStride` == 1).
          */
-        final int[] selectedBanks = (sourcePixelStride == 1) ? selectedBands : null;
+        final int[] includedBanks = (sourcePixelStride == 1) ? includedBands : null;
         final WritableRaster[] result = new WritableRaster[iterator.tileCountInQuery];
         final Tile[] missings = new Tile[iterator.tileCountInQuery];
         int numMissings = 0;
@@ -329,7 +329,7 @@ class DataSubset extends TiledGridCoverage implements Localized {
                     result[iterator.getIndexInResultArray()] = tile;
                 } else {
                     // Tile not yet loaded. Add to a queue of tiles to load later.
-                    missings[numMissings++] = new Tile(iterator, tileOffsets, selectedBanks, numTiles);
+                    missings[numMissings++] = new Tile(iterator, tileOffsets, includedBanks, numTiles);
                 }
             } while (iterator.next());
             Arrays.sort(missings, 0, numMissings);
@@ -352,8 +352,8 @@ class DataSubset extends TiledGridCoverage implements Localized {
                 if (tile.getRegionInsideTile(lower, upper, subsampling, BIDIMENSIONAL)) {
                     origin.x = tile.originX;
                     origin.y = tile.originY;
-                    tile.copyTileInfo(tileOffsets,    offsets,    selectedBanks, numTiles);
-                    tile.copyTileInfo(tileByteCounts, byteCounts, selectedBanks, numTiles);
+                    tile.copyTileInfo(tileOffsets,    offsets,    includedBanks, numTiles);
+                    tile.copyTileInfo(tileByteCounts, byteCounts, includedBanks, numTiles);
                     for (int b=0; b<offsets.length; b++) {
                         offsets[b] = addExact(offsets[b], reader().origin);
                     }
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridCoverage.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridCoverage.java
index 41169bd..6ceb19a 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridCoverage.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/TiledGridCoverage.java
@@ -127,13 +127,15 @@ public abstract class TiledGridCoverage extends GridCoverage {
     /**
      * Indices of {@link TiledGridResource} bands which have been retained for
      * inclusion in this {@code TiledGridCoverage}, in strictly increasing order.
-     * This is {@code null} if all bands shall be included.
+     * An "included" band is stored in memory but not necessarily visible to the user,
+     * because the {@link SampleModel} can be configured for ignoring some bands.
+     * This array is {@code null} if all bands shall be included.
      *
      * <p>If the user specified bands out of order, the change of band order is taken in account
-     * by the sample {@link #model}. This {@code selectedBands} array does not show any change
+     * by the sample {@link #model}. This {@code includedBands} array does not apply any change
      * of order for making sequential readings easier.</p>
      */
-    protected final int[] selectedBands;
+    protected final int[] includedBands;
 
     /**
      * Cache of rasters read by this {@code TiledGridCoverage}. This cache may be shared with other coverages
@@ -177,7 +179,7 @@ public abstract class TiledGridCoverage extends GridCoverage {
         readExtent          = subset.readExtent;
         subsampling         = subset.subsampling;
         subsamplingOffsets  = subset.subsamplingOffsets;
-        selectedBands       = subset.selectedBands;
+        includedBands       = subset.includedBands;
         rasters             = subset.cache;
         tileSize            = subset.tileSize;
         tmcOfFirstTile      = new long[dimension];
@@ -666,7 +668,7 @@ public abstract class TiledGridCoverage extends GridCoverage {
      * Creates the key to use for caching the tile at given index.
      */
     private TiledGridResource.CacheKey createCacheKey(final int indexInTileVector) {
-        return new TiledGridResource.CacheKey(indexInTileVector, selectedBands, subsampling, subsamplingOffsets);
+        return new TiledGridResource.CacheKey(indexInTileVector, includedBands, subsampling, subsamplingOffsets);
     }
 
     /**
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 212c1b4..0c171cf 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
@@ -51,19 +51,20 @@ import org.apache.sis.util.ArraysExt;
 public abstract class TiledGridResource extends AbstractGridResource {
     /**
      * A key in the {@link #rasters} cache of tiles.
+     * Each key shall be unique within its enclosing {@link TiledGridResource} instance.
      */
     static final class CacheKey {
         /** Index in a row-major array of tiles. */ private final int   indexInTileVector;
-        /** Bands in strictly increasing order.  */ private final int[] selectedBands;
+        /** Bands in strictly increasing order.  */ private final int[] includedBands;
         /** Subsampling factors at read time.    */ private final int[] subsampling;
         /** Remainder of subsampling divisions.  */ private final int[] subsamplingOffsets;
 
         /** Creates a key with given arrays hold be reference (no copy). */
-        CacheKey(final int indexInTileVector, final int[] selectedBands,
+        CacheKey(final int indexInTileVector, final int[] includedBands,
                  final int[] subsampling, final int[] subsamplingOffsets)
         {
             this.indexInTileVector  = indexInTileVector;
-            this.selectedBands      = selectedBands;
+            this.includedBands      = includedBands;
             this.subsampling        = subsampling;
             this.subsamplingOffsets = subsamplingOffsets;
         }
@@ -71,7 +72,7 @@ public abstract class TiledGridResource extends AbstractGridResource {
         /** Returns a hash-code value for this key. */
         @Override public int hashCode() {
             return indexInTileVector
-                    +   73 * Arrays.hashCode(selectedBands)
+                    +   73 * Arrays.hashCode(includedBands)
                     + 1063 * Arrays.hashCode(subsampling)
                     + 7919 * Arrays.hashCode(subsamplingOffsets);
         }
@@ -81,7 +82,7 @@ public abstract class TiledGridResource extends AbstractGridResource {
             if (obj instanceof CacheKey) {
                 final CacheKey other = (CacheKey) obj;
                 return indexInTileVector == other.indexInTileVector
-                        && Arrays.equals(selectedBands,      other.selectedBands)
+                        && Arrays.equals(includedBands,      other.includedBands)
                         && Arrays.equals(subsampling,        other.subsampling)
                         && Arrays.equals(subsamplingOffsets, other.subsamplingOffsets);
             }
@@ -202,13 +203,17 @@ public abstract class TiledGridResource extends AbstractGridResource {
         /**
          * Indices of {@link TiledGridResource} bands which have been retained for inclusion
          * in the {@link TiledGridCoverage} to construct, in strictly increasing order.
-         * This is {@code null} if all bands shall be included.
+         * An "included" band is stored in memory but not necessarily visible to the user,
+         * because the {@link SampleModel} can be configured for ignoring some bands.
+         * This array is {@code null} if all bands shall be included.
          *
          * <p>If the user specified bands out of order, the change of band order is taken in
-         * account by the {@link #modelForBandSubset}. This {@code selectedBands} array does
-         * not show any change of order for making sequential readings easier.</p>
+         * account by the {@link #modelForBandSubset}. This {@code includedBands} array does
+         * not apply any change of order for making sequential readings easier.</p>
+         *
+         * @see TiledGridCoverage#includedBands
          */
-        final int[] selectedBands;
+        final int[] includedBands;
 
         /**
          * Coordinate conversion from subsampled grid to the grid at full resolution.
@@ -302,24 +307,24 @@ public abstract class TiledGridResource extends AbstractGridResource {
             /*
              * Get the bands selected by user in strictly increasing order of source band index.
              * If user has specified bands in a different order, that change of band order will
-             * be handled by the `SampleModel`, not in `selectedBands` array.
+             * be handled by the `SampleModel`, not in `includedBands` array.
              */
-            int[] selectedBands = null;
+            int[] includedBands = null;
             if (!rangeIndices.isIdentity()) {
                 bands = Arrays.asList(rangeIndices.select(bands));
-                selectedBands = new int[rangeIndices.getNumBands()];
-                for (int i=0; i<selectedBands.length; i++) {
-                    selectedBands[i] = rangeIndices.getSourceIndex(i);
+                includedBands = new int[rangeIndices.getNumBands()];
+                for (int i=0; i<includedBands.length; i++) {
+                    includedBands[i] = rangeIndices.getSourceIndex(i);
                 }
-                assert ArraysExt.isSorted(selectedBands, true);
+                assert ArraysExt.isSorted(includedBands, true);
                 if (rangeIndices.hasAllBands) {
-                    assert ArraysExt.isRange(0, selectedBands);
-                    selectedBands = null;
+                    assert ArraysExt.isRange(0, includedBands);
+                    includedBands = null;
                 }
             }
             this.domain              = domain;
             this.ranges              = bands;
-            this.selectedBands       = selectedBands;
+            this.includedBands       = includedBands;
             this.modelForBandSubset  = rangeIndices.select(getSampleModel(), loadAllBands);
             this.colorsForBandSubset = rangeIndices.select(getColorModel());
             this.fillValue           = getFillValue();
@@ -348,7 +353,7 @@ public abstract class TiledGridResource extends AbstractGridResource {
          * @return {@code true} if only a subset of bands will be read or if bands will be read out of order.
          */
         public boolean hasBandSubset() {
-            return selectedBands != null;
+            return includedBands != null;
         }
     }
 

[sis] 03/04: 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.

Posted by de...@apache.org.
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 <ma...@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();

[sis] 04/04: Group together the display of statistics about execution time.

Posted by de...@apache.org.
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 3bc831a7be599823a4f98fa31f0623cae83c43ce
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Thu Aug 12 01:31:59 2021 +0200

    Group together the display of statistics about execution time.
---
 .../sis/test/storage/CoverageReadConsistency.java  | 49 +++++++++++++++++-----
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/storage/sis-storage/src/test/java/org/apache/sis/test/storage/CoverageReadConsistency.java b/storage/sis-storage/src/test/java/org/apache/sis/test/storage/CoverageReadConsistency.java
index 793bbf1..8ff76ea 100644
--- a/storage/sis-storage/src/test/java/org/apache/sis/test/storage/CoverageReadConsistency.java
+++ b/storage/sis-storage/src/test/java/org/apache/sis/test/storage/CoverageReadConsistency.java
@@ -16,6 +16,8 @@
  */
 package org.apache.sis.test.storage;
 
+import java.util.List;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Random;
 import java.awt.Point;
@@ -33,10 +35,12 @@ import org.apache.sis.internal.util.StandardDateFormat;
 import org.apache.sis.internal.util.Numerics;
 import org.apache.sis.image.PixelIterator;
 import org.apache.sis.math.Statistics;
+import org.apache.sis.math.StatisticsFormat;
 import org.apache.sis.util.ArraysExt;
 import org.apache.sis.test.DependsOnMethod;
 import org.apache.sis.test.TestUtilities;
 import org.apache.sis.test.TestCase;
+import org.junit.AfterClass;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -53,6 +57,9 @@ import static org.junit.Assert.*;
  * we consider that the code reading the full extent is usually simpler than the code reading
  * a subset of data.
  *
+ * <p>This class is not thread-safe. Only one instance should exist in the JVM at a given time
+ * (because of the use of static fields).</p>
+ *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
  * @since   1.1
@@ -110,6 +117,12 @@ public strictfp class CoverageReadConsistency extends TestCase {
     private final boolean failOnMismatch;
 
     /**
+     * Statistics about execution time.
+     * Created only in benchmark mode.
+     */
+    private static List<Statistics> statistics;
+
+    /**
      * Creates a new tester. This constructor reads immediately the coverage at full extent and full resolution.
      * That full coverage will be used as a reference for verifying the pixel values read in sub-domains.
      * Any mismatch in pixel values will cause immediate test failure.
@@ -154,7 +167,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
      */
     @Test
     public void testSubRegionAtOrigin() throws DataStoreException {
-        readAndCompareRandomRegions();
+        readAndCompareRandomRegions("Origin-R");
     }
 
     /**
@@ -167,7 +180,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
     @DependsOnMethod("testSubRegionAtOrigin")
     public void testSubRegionsAnywhere() throws DataStoreException {
         allowOffsets = true;
-        readAndCompareRandomRegions();
+        readAndCompareRandomRegions("Subregions");
     }
 
     /**
@@ -180,7 +193,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
     @DependsOnMethod("testSubRegionAtOrigin")
     public void testSubsamplingAtOrigin() throws DataStoreException {
         allowSubsamplings = true;
-        readAndCompareRandomRegions();
+        readAndCompareRandomRegions("Origin-S");
     }
 
     /**
@@ -194,7 +207,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
     public void testSubsamplingAnywhere() throws DataStoreException {
         allowOffsets      = true;
         allowSubsamplings = true;
-        readAndCompareRandomRegions();
+        readAndCompareRandomRegions("Subsampling");
     }
 
     /**
@@ -206,7 +219,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
     @DependsOnMethod("testSubRegionAtOrigin")
     public void testBandSubsetAtOrigin() throws DataStoreException {
         allowBandSubset = true;
-        readAndCompareRandomRegions();
+        readAndCompareRandomRegions("Origin-B");
     }
 
     /**
@@ -219,7 +232,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
     public void testBandSubsetAnywhere() throws DataStoreException {
         allowOffsets    = true;
         allowBandSubset = true;
-        readAndCompareRandomRegions();
+        readAndCompareRandomRegions("Bands");
     }
 
     /**
@@ -234,7 +247,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
         allowOffsets      = true;
         allowBandSubset   = true;
         allowSubsamplings = true;
-        readAndCompareRandomRegions();
+        readAndCompareRandomRegions("All");
     }
 
     /**
@@ -300,9 +313,10 @@ public strictfp class CoverageReadConsistency extends TestCase {
     /**
      * Implementation of methods testing reading in random sub-regions with random sub-samplings.
      *
+     * @param  label  a label for the test being run.
      * @throws DataStoreException if an error occurred while using the resource.
      */
-    private void readAndCompareRandomRegions() throws DataStoreException {
+    private void readAndCompareRandomRegions(final String label) throws DataStoreException {
         randomConfigureResource();
         final GridGeometry gg = resource.getGridGeometry();
         final int    dimension   = gg.getDimension();
@@ -315,7 +329,7 @@ public strictfp class CoverageReadConsistency extends TestCase {
          * We will collect statistics on execution time only if the
          * test is executed in a more verbose mode than the default.
          */
-        final Statistics durations = (VERBOSE || !failOnMismatch) ? new Statistics("time (ms)") : null;
+        final Statistics durations = (VERBOSE || !failOnMismatch) ? new Statistics(label) : null;
         int failuresCount = 0;
         for (int it=0; it < numIterations; it++) {
             final GridGeometry domain = randomDomain(gg, low, high, subsampling);
@@ -417,7 +431,10 @@ nextSlice:  for (;;) {
          * or if this `CoverageReadConsistency` is used for benchmark.
          */
         if (durations != null) {
-            out.print(durations);
+            if (statistics == null) {
+                statistics = new ArrayList<>();
+            }
+            statistics.add(durations);
             final int totalCount = durations.count();
             out.println("Number of failures: " + failuresCount + " / " + totalCount
                         + " (" + (failuresCount / (totalCount / 100f)) + "%)");
@@ -425,6 +442,18 @@ nextSlice:  for (;;) {
     }
 
     /**
+     * Prints statistics about execution time (in milliseconds) after all tests completed.
+     */
+    @AfterClass
+    public static void printDurations() {
+        if (statistics != null) {
+            // It is too late for using `TestCase.out`.
+            System.out.print(StatisticsFormat.getInstance().format(statistics.toArray(new Statistics[statistics.size()])));
+            statistics = null;
+        }
+    }
+
+    /**
      * Creates a pixel iterator for a sub-region in a slice of the specified coverage.
      * All coordinates given to this method are in the coordinate space of subsampled coverage subset.
      * This method returns {@code null} if the arguments are valid but the image can not be created

[sis] 01/04: Consolidation: - Safer rounding of subsampling values. - Add test and clarifications in Javadoc.

Posted by de...@apache.org.
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 022a1f3fffaf04cc3b2bdf75829287a678889a3a
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Wed Aug 11 15:46:36 2021 +0200

    Consolidation:
    - Safer rounding of subsampling values.
    - Add test and clarifications in Javadoc.
---
 .../apache/sis/coverage/grid/GridDerivation.java   | 40 ++++++++++++++++------
 .../sis/coverage/grid/GridDerivationTest.java      | 24 +++++++++++++
 .../org/apache/sis/storage/geotiff/GeoTIFF.java    |  2 +-
 .../apache/sis/storage/geotiff/GeoTiffStore.java   |  2 +-
 .../sis/internal/storage/AbstractResource.java     |  7 ++++
 .../apache/sis/storage/event/StoreListeners.java   |  6 ++--
 6 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java
index 445e2ff..0c9c26a 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java
@@ -1277,7 +1277,7 @@ public class GridDerivation {
     /**
      * Rounds a subsampling value according the current {@link RoundingMode}.
      * If a {@link #chunkSize} has been specified, then the subsampling will be a divisor of that size.
-     * This is necessary for avoiding a drift in subsampled pixel coordinates computed from tile coordinates.
+     * This is necessary for avoiding a drift of subsampled pixel coordinates computed from tile coordinates.
      *
      * <div class="note"><b>Drift example:</b>
      * if the tile size is 16 pixels and the subsampling is 3, then the subsampled tile size is ⌊16/3⌋ = 5 pixels.
@@ -1306,8 +1306,8 @@ public class GridDerivation {
         switch (rounding) {
             default:        throw new AssertionError(rounding);
             case NEAREST:   subsampling = (int) Math.min(Math.round(scale), Integer.MAX_VALUE); break;
-            case CONTAINED: // Assume user wants more data in source (ENCLOSING) or target (CONTAINED) grid.
-            case ENCLOSING: subsampling = (int) Math.nextUp(scale); break;
+            case CONTAINED: subsampling = (int) Math.ceil(scale - tolerance(dimension)); break;
+            case ENCLOSING: subsampling = (int) (scale + tolerance(dimension)); break;
         }
         if (subsampling <= 1) {
             return 1;
@@ -1321,24 +1321,42 @@ public class GridDerivation {
                 /*
                  * `binarySearch(…)` should never find an exact match, otherwise (size % r) would have been zero.
                  * Furthermore `i` should never be 0 because divisors[0] = 1, which can not be selected if r > 1.
-                 * We nevertheless check for (i > 0) as a paranoiac safety.
+                 * We do not check `if (i > 0)` "as a safety" because client code such as `TiledGridCoverage`
+                 * will behave erratically if this method does not fulfill its contract (i.e. find a divisor).
+                 * It is better to know now if there is any problem here.
                  */
-                if (i > 0) {
-                    int s = divisors[i-1];
-                    if (rounding == GridRoundingMode.NEAREST && i < divisors.length) {
-                        final int above = divisors[i];
-                        if (above - r < r - s) {
-                            s = above;
+                int s = divisors[i-1];
+                if (i < divisors.length) {
+                    switch (rounding) {
+                        case CONTAINED: {
+                            s = divisors[i];
+                            break;
+                        }
+                        case NEAREST: {
+                            final int above = divisors[i];
+                            if (above - r < r - s) {
+                                s = above;
+                            }
+                            break;
                         }
                     }
-                    return s + (subsampling - r);
                 }
+                return s + (subsampling - r);
             }
         }
         return subsampling;
     }
 
     /**
+     * Returns a tolerance factor for comparing scale factors in the given dimension.
+     * The tolerance is such that the errors of pixel coordinates computed using the
+     * scale factor should not be greater than 0.5 pixel.
+     */
+    private double tolerance(final int dimension) {
+        return (base.extent != null) ? 0.5 / base.extent.getSize(dimension, false) : 0;
+    }
+
+    /**
      * Returns the offsets to be subtracted from pixel coordinates before subsampling.
      * In a conversion from <em>derived</em> grid to {@linkplain #base} grid coordinates
      * (the opposite direction of subsampling), the offset is the value to add after
diff --git a/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridDerivationTest.java b/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridDerivationTest.java
index 3573c30..28887b0 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridDerivationTest.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridDerivationTest.java
@@ -374,6 +374,30 @@ public final strictfp class GridDerivationTest extends TestCase {
     }
 
     /**
+     * Tests {@link GridDerivation#subgrid(GridExtent)} with a null "grid to CRS" transform.
+     */
+    @Test
+    public void testSubgridWithoutTransform() {
+        GridExtent   base   = new GridExtent(null, new long[] {100, 200}, new long[] {300, 350}, true);
+        GridExtent   query  = new GridExtent(null, new long[] {120, 180}, new long[] {280, 360}, true);
+        GridGeometry result = new GridGeometry(base, null, null, null).derive().subgrid(query).build();
+        assertExtentEquals(new long[] {120, 200}, new long[] {280, 350}, result.getExtent());
+        assertFalse(result.isDefined(GridGeometry.GRID_TO_CRS));
+        assertFalse(result.isDefined(GridGeometry.CRS));
+        assertFalse(result.isDefined(GridGeometry.RESOLUTION));
+        /*
+         * Try again with a subsampling. We can get the subsampling information back as the resolution.
+         * Note that there is no way with current API to get the subsampling offsets.
+         */
+        result = new GridGeometry(base, null, null, null).derive().subgrid(query, 3, 5).build();
+        assertExtentEquals(new long[] {40, 40}, new long[] {93, 70}, result.getExtent());
+        assertFalse(result.isDefined(GridGeometry.GRID_TO_CRS));
+        assertFalse(result.isDefined(GridGeometry.CRS));
+        assertTrue (result.isDefined(GridGeometry.RESOLUTION));
+        assertArrayEquals(new double[] {3, 5}, result.getResolution(false), STRICT);
+    }
+
+    /**
      * Tests {@link GridDerivation#slice(DirectPosition)}.
      */
     @Test
diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTIFF.java b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTIFF.java
index 3f9d7da..baa5a9e 100644
--- a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTIFF.java
+++ b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTIFF.java
@@ -49,7 +49,7 @@ abstract class GeoTIFF implements Closeable {
      * The locale to use for parsers or formatter. This is <strong>not</strong> the locale
      * for warnings or other messages emitted to the users.
      */
-    static final Locale LOCALE = Locale.US;
+    private static final Locale LOCALE = Locale.US;
 
     /**
      * The magic number for big-endian TIFF files or little-endian TIFF files.
diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java
index 4a234d1..98dd0ed 100644
--- a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java
+++ b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java
@@ -228,7 +228,7 @@ public class GeoTiffStore extends DataStore implements Aggregate {
             /*
              * Add the filename as an identifier only if the input was something convertible to URI (URL, File or Path),
              * otherwise reader.input.filename may not be useful; it may be just the InputStream classname. If the TIFF
-             * file did not specified any ImageDescription tag, then we will had the filename as a title instead than an
+             * file did not specified any ImageDescription tag, then we will add the filename as a title instead than an
              * identifier because the title is mandatory in ISO 19115 metadata.
              */
             getIdentifier().ifPresent((id) -> builder.addTitleOrIdentifier(id.toString(), MetadataBuilder.Scope.ALL));
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/AbstractResource.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/AbstractResource.java
index a1c8eba..c2def88 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/AbstractResource.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/AbstractResource.java
@@ -76,6 +76,12 @@ public class AbstractResource extends StoreListeners implements Resource {
      * Returns the resource persistent identifier if available.
      * The default implementation returns an empty value.
      * Subclasses are strongly encouraged to override if they can provide a value.
+     *
+     * <p>Note that the default implementation of {@link #createMetadata(MetadataBuilder)} uses this identifier
+     * for initializing the {@code metadata/identificationInfo/citation/title} property. So it is generally not
+     * useful to fallback on metadata if the identifier is empty.</p>
+     *
+     * @see org.apache.sis.internal.storage.StoreUtilities#getLabel(Resource)
      */
     @Override
     public Optional<GenericName> getIdentifier() throws DataStoreException {
@@ -120,6 +126,7 @@ public class AbstractResource extends StoreListeners implements Resource {
      * @throws DataStoreException if an error occurred while reading metadata from the data store.
      */
     protected void createMetadata(final MetadataBuilder metadata) throws DataStoreException {
+        // Note: title is mandatory in ISO metadata, contrarily to the identifier.
         getIdentifier().ifPresent((name) -> metadata.addTitle(new Sentence(name)));
         getEnvelope().ifPresent((envelope) -> {
             try {
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java
index 8c0819a..55d3d69 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java
@@ -31,6 +31,7 @@ import org.apache.sis.util.logging.Logging;
 import org.apache.sis.util.resources.Vocabulary;
 import org.apache.sis.internal.storage.StoreResource;
 import org.apache.sis.internal.storage.StoreUtilities;
+import org.apache.sis.internal.storage.AbstractResource;
 import org.apache.sis.storage.DataStoreProvider;
 import org.apache.sis.storage.DataStore;
 import org.apache.sis.storage.Resource;
@@ -84,6 +85,7 @@ public class StoreListeners implements Localized {
     /**
      * The declared source of events. This is not necessarily the real source,
      * but this is the source that the implementer wants to declare as public API.
+     * Never {@code null} but may be {@code this}.
      */
     private final Resource source;
 
@@ -214,7 +216,7 @@ public class StoreListeners implements Localized {
          * This is used as a convenience by AbstractResource internal class. We need this hack
          * because subclasses can not reference `this` before super-class constructor completed.
          */
-        if (source == null && this instanceof Resource) {
+        if (source == null && this instanceof AbstractResource) {
             source = (Resource) this;
         } else {
             ArgumentChecks.ensureNonNull("source", source);
@@ -226,7 +228,7 @@ public class StoreListeners implements Localized {
     /**
      * Returns the source of events. This value is specified at construction time.
      *
-     * @return the source of events.
+     * @return the source of events. Never {@code null} but may be {@code this}.
      */
     public Resource getSource() {
         return source;