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/10/13 16:53:33 UTC

[sis] branch geoapi-4.0 updated: Fix erroneous tile indices when subsampling is larger than tile size.

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


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new c910ee0  Fix erroneous tile indices when subsampling is larger than tile size.
c910ee0 is described below

commit c910ee0db60f2b0b9a8bfedf9bb615c05581a687
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Wed Oct 13 18:53:06 2021 +0200

    Fix erroneous tile indices when subsampling is larger than tile size.
---
 .../apache/sis/coverage/grid/GridDerivation.java   |  7 +++++-
 .../java/org/apache/sis/image/PixelIterator.java   |  7 ++++--
 .../sis/internal/storage/TiledGridCoverage.java    | 25 +++++++++++++++++-----
 .../apache/sis/internal/storage/package-info.java  |  2 +-
 4 files changed, 32 insertions(+), 9 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 6d921de..35f1893 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
@@ -1261,8 +1261,13 @@ public class GridDerivation {
             return 1;
         }
         if (chunkSize != null) {
+            /*
+             * If the coverage is divided in tiles (or "chunk" in n-dimensional case), we want the subsampling
+             * to be a divisor of tile size. In the special case where the subsampling is larger than tile size,
+             * we can remove an integer amount of tiles because tiles can be fully skipped at read time.
+             */
             final int size = chunkSize[dimension];
-            final int r = subsampling % size;
+            final int r = subsampling % size;       // Reduced subsampling (with integer amont of tiles removed).
             if (r > 1 && (size % r) != 0) {
                 final int[] divisors = MathFunctions.divisors(size);
                 final int i = ~Arrays.binarySearch(divisors, r);
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java b/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java
index 4724b5b..64f0f30 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java
@@ -83,7 +83,7 @@ import static org.apache.sis.internal.util.Numerics.ceilDiv;
  * @author  Rémi Maréchal (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
  * @author  Johann Sorel (Geomatys)
- * @version 1.1
+ * @version 1.2
  * @since   1.0
  * @module
  */
@@ -723,7 +723,10 @@ public class PixelIterator {
                 releaseTile();                                      // Release current writable raster, if any.
                 tileX = tx;
                 tileY = ty;
-                if (fetchTile() > py || currentLowerX > px) {       // `fetchTile()` must be before `currentLowerX`.
+                int currentLowerY = fetchTile();                    // Also update `currentLowerX` field value.
+                if (currentLowerY > py || py >= currentUpperY ||
+                    currentLowerX > px || px >= currentUpperX)
+                {
                     throw new RasterFormatException(Resources.format(Resources.Keys.IncompatibleTile_2, tileX, tileY));
                 }
             }
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 7bf8026..bf12aa3 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
@@ -62,7 +62,7 @@ import static org.apache.sis.internal.util.Numerics.ceilDiv;
  * in order to avoid integer overflow.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
  * @since   1.1
  * @module
  */
@@ -460,7 +460,8 @@ public abstract class TiledGridCoverage extends GridCoverage {
         private final int[] tileUpper;
 
         /**
-         * Pixel coordinates to assign to the upper-left corner of the region to read, ignoring subsampling.
+         * Pixel coordinates to assign to the upper-left corner of the region to render, ignoring subsampling.
+         * This is the difference between the region requested by user and the region which will be rendered.
          */
         private final int[] offsetAOI;
 
@@ -570,11 +571,25 @@ public abstract class TiledGridCoverage extends GridCoverage {
 
         /**
          * Returns the origin to assign to the tile at current iterator position.
-         * Note that if there is more than one tile, then the subsampling should be
-         * a divisor of tile size, otherwise a drift in pixel coordinates will appear.
+         * Note that the subsampling should be a divisor of tile size,
+         * otherwise a drift in pixel coordinates will appear.
+         * There is two exceptions to this rule:
+         *
+         * <ul>
+         *   <li>If image is untiled (i.e. there is only one tile),
+         *       we allow to read a sub-region of the unique tile.</li>
+         *   <li>If subsampling is larger than tile size.</li>
+         * </ul>
          */
         final int getTileOrigin(final int dimension) {
-            return floorDiv(tileOffsetAOI[dimension], subsampling[dimension]);
+            /*
+             * We really need `ceilDiv(…)` below, not `floorDiv(…)`. It makes no difference in the usual
+             * case where the subsampling is a divisor of the tile size. But if we are in the case where
+             * subsampling is larger than tile size, we need the `ceilDiv(…)`. At first the results seem
+             * inconsistent, but after we discard the tiles where `getRegionInsideTile(…)` returns false
+             * it become consistent.
+             */
+            return ceilDiv(tileOffsetAOI[dimension], subsampling[dimension]);
         }
 
         /**
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/package-info.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/package-info.java
index b072411..d07eeac 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/package-info.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/package-info.java
@@ -25,7 +25,7 @@
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @author  Johann Sorel (Geomatys)
- * @version 1.1
+ * @version 1.2
  * @since   0.4
  * @module
  */