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 fc7868cc4a Adjust the verifications of tile size.
fc7868cc4a is described below

commit fc7868cc4ab688749238b0b5998c15e88ef8593b
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Sun Sep 15 23:56:14 2024 +0200

    Adjust the verifications of tile size.
---
 .../org/apache/sis/coverage/privy/ImageLayout.java | 38 +++++++++++++++++-----
 .../apache/sis/coverage/privy/ImageUtilities.java  | 15 ---------
 .../org/apache/sis/image/MultiSourceLayout.java    |  2 +-
 .../main/org/apache/sis/image/Transferer.java      |  2 +-
 .../apache/sis/coverage/privy/ImageLayoutTest.java |  8 +++++
 .../sis/coverage/privy/ImageUtilitiesTest.java     |  8 -----
 .../main/org/apache/sis/storage/gdal/Band.java     | 12 +++++--
 .../org/apache/sis/storage/gdal/TiledResource.java | 24 ++++++++++----
 .../org/apache/sis/gui/coverage/GridTileCache.java |  4 +--
 9 files changed, 69 insertions(+), 44 deletions(-)

diff --git 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageLayout.java
 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageLayout.java
index 171739d0dd..719e6ed9de 100644
--- 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageLayout.java
+++ 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageLayout.java
@@ -43,21 +43,41 @@ import org.apache.sis.system.Configuration;
  */
 public class ImageLayout {
     /**
-     * The minimum tile size. The {@link #toTileSize(int, int, boolean)} 
method will not suggest tiles
-     * smaller than this size. This size must be smaller than {@link 
ImageUtilities#DEFAULT_TILE_SIZE}.
+     * The minimum tile width or height. The {@link #toTileSize(int, int, 
boolean)} method will not
+     * suggest tiles smaller than this size. This size must be smaller than 
{@link #DEFAULT_TILE_SIZE}.
      *
      * <p>Tiles of 180×180 pixels consume about 127 kB, assuming 4 bytes per 
pixel. This is about half
      * the consumption of tiles of 256×256 pixels. We select a size which is a 
multiple of 90 because
      * images are often used with a resolution of e.g. ½° per pixel.</p>
      *
-     * @see ImageUtilities#DEFAULT_TILE_SIZE
+     * @see #DEFAULT_TILE_SIZE
      */
     @Configuration
     private static final int MIN_TILE_SIZE = 180;
 
     /**
-     * The default instance which will target {@value 
ImageUtilities#DEFAULT_TILE_SIZE} pixels as tile
-     * width and height.
+     * Arbitrary number of pixels for considering a tile as too large.
+     * Tile larger than this size should be divided in smaller tiles.
+     */
+    @Configuration
+    public static final int LARGE_TILE_SIZE = 1024 * 1024 * 16;
+
+    /**
+     * Default width and height of tiles, in pixels.
+     */
+    @Configuration
+    public static final int DEFAULT_TILE_SIZE = 256;
+
+    /**
+     * Suggested size for a tile cache in number of tiles. This value can be 
used for very simple caching mechanism,
+     * keeping the most recently used tiles up to 10 Mb of memory. This is not 
for sophisticated caching mechanism;
+     * instead the "real" caching should be done by {@link 
org.apache.sis.image.ComputedImage}.
+     */
+    @Configuration
+    public static final int SUGGESTED_TILE_CACHE_SIZE = 10 * (1024 * 1024) / 
(DEFAULT_TILE_SIZE * DEFAULT_TILE_SIZE);
+
+    /**
+     * The default instance which will target {@value #DEFAULT_TILE_SIZE} 
pixels as tile width and height.
      */
     public static final ImageLayout DEFAULT = new ImageLayout(null, false);
 
@@ -69,7 +89,7 @@ public class ImageLayout {
     /**
      * Preferred size for tiles.
      *
-     * @see ImageUtilities#DEFAULT_TILE_SIZE
+     * @see #DEFAULT_TILE_SIZE
      */
     protected final int preferredTileWidth, preferredTileHeight;
 
@@ -95,8 +115,8 @@ public class ImageLayout {
             preferredTileWidth  = Math.max(1, preferredTileSize.width);
             preferredTileHeight = Math.max(1, preferredTileSize.height);
         } else {
-            preferredTileWidth  = ImageUtilities.DEFAULT_TILE_SIZE;
-            preferredTileHeight = ImageUtilities.DEFAULT_TILE_SIZE;
+            preferredTileWidth  = DEFAULT_TILE_SIZE;
+            preferredTileHeight = DEFAULT_TILE_SIZE;
         }
         this.isBoundsAdjustmentAllowed = isBoundsAdjustmentAllowed;
     }
@@ -193,7 +213,7 @@ public class ImageLayout {
      * method returns a size that minimize the number of empty pixels in the 
last tile.
      *
      * @param  imageSize          the image size (width or height).
-     * @param  preferredTileSize  the preferred tile size, which is often 
{@value ImageUtilities#DEFAULT_TILE_SIZE}.
+     * @param  preferredTileSize  the preferred tile size, which is often 
{@value #DEFAULT_TILE_SIZE}.
      * @param  allowPartialTiles  whether to allow tiles that are only 
partially filled.
      * @return the suggested tile size, or {@code imageSize} if none.
      */
diff --git 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageUtilities.java
 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageUtilities.java
index 527cf5148e..362490293a 100644
--- 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageUtilities.java
+++ 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/ImageUtilities.java
@@ -36,7 +36,6 @@ import static java.lang.Math.floorDiv;
 import static java.lang.Math.toIntExact;
 import static java.lang.Math.multiplyFull;
 import org.apache.sis.feature.internal.Resources;
-import org.apache.sis.system.Configuration;
 import org.apache.sis.system.Modules;
 import org.apache.sis.util.Numbers;
 import org.apache.sis.util.Static;
@@ -53,20 +52,6 @@ import static 
org.apache.sis.util.privy.Numerics.COMPARISON_THRESHOLD;
  * @author  Martin Desruisseaux (Geomatys)
  */
 public final class ImageUtilities extends Static {
-    /**
-     * Default width and height of tiles, in pixels.
-     */
-    @Configuration
-    public static final int DEFAULT_TILE_SIZE = 256;
-
-    /**
-     * Suggested size for a tile cache in number of tiles. This value can be 
used for very simple caching mechanism,
-     * keeping the most recently used tiles up to 10 Mb of memory. This is not 
for sophisticated caching mechanism;
-     * instead the "real" caching should be done by {@link 
org.apache.sis.image.ComputedImage}.
-     */
-    @Configuration
-    public static final int SUGGESTED_TILE_CACHE_SIZE = 10 * (1024 * 1024) / 
(DEFAULT_TILE_SIZE * DEFAULT_TILE_SIZE);
-
     /**
      * The logger for operations on images and rasters.
      */
diff --git 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/MultiSourceLayout.java
 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/MultiSourceLayout.java
index fbf924197e..884e620780 100644
--- 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/MultiSourceLayout.java
+++ 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/MultiSourceLayout.java
@@ -199,7 +199,7 @@ final class MultiSourceLayout extends ImageLayout {
          * the combined image causes the computation of a single tile of each 
source image.
          */
         long cx, cy;        // A combination of tile size with alignment on 
the tile matrix grid.
-        cx = cy = (((long) Integer.MAX_VALUE) << Integer.SIZE) | 
ImageUtilities.DEFAULT_TILE_SIZE;
+        cx = cy = (((long) Integer.MAX_VALUE) << Integer.SIZE) | 
DEFAULT_TILE_SIZE;
         final var tileGridXOffset = new FrequencySortedSet<Integer>(true);
         final var tileGridYOffset = new FrequencySortedSet<Integer>(true);
         for (final RenderedImage source : sources) {
diff --git 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/Transferer.java 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/Transferer.java
index cf84950982..7350792c05 100644
--- 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/Transferer.java
+++ 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/image/Transferer.java
@@ -59,7 +59,7 @@ abstract class Transferer {
      * @see #prepareTransferRegion(Rectangle, int)
      */
     @Configuration
-    private static final int BUFFER_SIZE = 32 * 
ImageUtilities.DEFAULT_TILE_SIZE * Byte.SIZE;
+    private static final int BUFFER_SIZE = 32 * ImageLayout.DEFAULT_TILE_SIZE 
* Byte.SIZE;
 
     /**
      * The image tile from which to read sample values.
diff --git 
a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageLayoutTest.java
 
b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageLayoutTest.java
index fadd11dd61..f7be8ad7e1 100644
--- 
a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageLayoutTest.java
+++ 
b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageLayoutTest.java
@@ -36,6 +36,14 @@ public final class ImageLayoutTest extends TestCase {
     public ImageLayoutTest() {
     }
 
+    /**
+     * Verifies that {@link ImageLayout#SUGGESTED_TILE_CACHE_SIZE} is strictly 
positive.
+     */
+    @Test
+    public void verifySuggestedTileCacheSize() {
+        assertTrue(ImageLayout.SUGGESTED_TILE_CACHE_SIZE >= 1);
+    }
+
     /**
      * Tests {@link ImageLayout#suggestTileSize(int, int, boolean)}.
      */
diff --git 
a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageUtilitiesTest.java
 
b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageUtilitiesTest.java
index b0fa08aad0..7934f1b83e 100644
--- 
a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageUtilitiesTest.java
+++ 
b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/privy/ImageUtilitiesTest.java
@@ -47,14 +47,6 @@ public final class ImageUtilitiesTest extends TestCase {
     public ImageUtilitiesTest() {
     }
 
-    /**
-     * Verifies that {@link ImageUtilities#SUGGESTED_TILE_CACHE_SIZE} is 
strictly positive.
-     */
-    @Test
-    public void verifySuggestedTileCacheSize() {
-        assertTrue(ImageUtilities.SUGGESTED_TILE_CACHE_SIZE >= 1);
-    }
-
     /**
      * Tests {@link ImageUtilities#getBounds(RenderedImage)} and {@link 
ImageUtilities#clipBounds(RenderedImage, Rectangle)}.
      */
diff --git 
a/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/Band.java
 
b/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/Band.java
index 6da315a03f..e6e252243f 100644
--- 
a/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/Band.java
+++ 
b/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/Band.java
@@ -283,15 +283,23 @@ final class Band {
         final var   dataBuffer  = raster.getDataBuffer();
         final int   dataSize    = 
DataBuffer.getDataTypeSize(dataBuffer.getDataType()) / Byte.SIZE;
         final int[] bankIndices = sampleModel.getBankIndices();
+        /*
+         * The following assertions are critical: if those conditions are not 
true, it may crash the JVM.
+         * For that reason, we test them unconditionally instead of using the 
`assert` statement.
+         */
+        if (!raster.getBounds().contains(rasterBounds)) {
+            throw new AssertionError(rasterBounds);
+        }
+        if (transferBuffer.byteSize() < Math.multiplyFull(rasterBounds.width, 
rasterBounds.height) * dataSize) {
+            throw new AssertionError(rasterBounds);
+        }
         for (int i=0; i < selectedBands.length; i++) {
-            assert raster.getBounds().contains(rasterBounds) : rasterBounds;
             final Buffer buffer = RasterFactory.wrapAsBuffer(dataBuffer, 
bankIndices[i])
                     .position(sampleModel.getOffset(
                             rasterBounds.x - raster.getSampleModelTranslateX(),
                             rasterBounds.y - 
raster.getSampleModelTranslateY(), i));
             final int err;
             try {
-                assert transferBuffer.byteSize() >= 
Math.multiplyFull(rasterBounds.width, rasterBounds.height) * dataSize;
                 err = (int) gdal.rasterIO.invokeExact(selectedBands[i].handle, 
readWriteFlags,
                         resourceBounds.x, resourceBounds.y, 
resourceBounds.width, resourceBounds.height,
                         transferBuffer,
diff --git 
a/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/TiledResource.java
 
b/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/TiledResource.java
index dc9316db6f..22935058ed 100644
--- 
a/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/TiledResource.java
+++ 
b/incubator/src/org.apache.sis.storage.gdal/main/org/apache/sis/storage/gdal/TiledResource.java
@@ -75,12 +75,14 @@ final class TiledResource extends TiledGridResource {
     private GridGeometry geometry;
 
     /**
-     * The image size in pixels.
+     * The image size in pixels. Considered as unsigned integers.
      */
     private final int width, height;
 
     /**
-     * The tile size in pixels.
+     * The tile size in pixels. This is the <abbr>GDAL</abbr> block size,
+     * unless the block size was too large in which case <abbr>SIS</abbr>
+     * selects a smaller size.
      *
      * @see #getTileSize()
      */
@@ -174,11 +176,21 @@ final class TiledResource extends TiledGridResource {
          * when the file format is not tiled. But that default can consume a 
lot of memory.
          */
         Dimension tileSize() {
-            if (tileWidth < width || tileHeight > 1) {
-                return new Dimension(tileWidth, tileHeight);
-            } else {
-                return ImageLayout.DEFAULT.suggestTileSize(width, height, 
true);
+            int w, h;
+            if ((w = tileWidth)  < 1 || Integer.compareUnsigned(w, width)  >= 
0 ||
+                (h = tileHeight) < 1 || Integer.compareUnsigned(h, height) >= 
0)
+            {
+                /*
+                 * If the tile size seems invalid or overflows the capacity of 
32 bits integers,
+                 * we will derive a tile size from the image size. If the 
image size overflows,
+                 * use a default tile size.
+                 */
+                if ((w = width)  < 0) w = ImageLayout.DEFAULT_TILE_SIZE;
+                if ((h = height) < 0) h = ImageLayout.DEFAULT_TILE_SIZE;
+            } else if (Math.multiplyFull(w, h) <= ImageLayout.LARGE_TILE_SIZE) 
{
+                return new Dimension(w, h);
             }
+            return ImageLayout.DEFAULT.suggestTileSize(w, h, true);
         }
     }
 
diff --git 
a/optional/src/org.apache.sis.gui/main/org/apache/sis/gui/coverage/GridTileCache.java
 
b/optional/src/org.apache.sis.gui/main/org/apache/sis/gui/coverage/GridTileCache.java
index 5520ef48d1..68a7bd14cf 100644
--- 
a/optional/src/org.apache.sis.gui/main/org/apache/sis/gui/coverage/GridTileCache.java
+++ 
b/optional/src/org.apache.sis.gui/main/org/apache/sis/gui/coverage/GridTileCache.java
@@ -18,7 +18,7 @@ package org.apache.sis.gui.coverage;
 
 import java.util.Map;
 import java.util.LinkedHashMap;
-import org.apache.sis.coverage.privy.ImageUtilities;
+import org.apache.sis.coverage.privy.ImageLayout;
 
 
 /**
@@ -45,7 +45,7 @@ final class GridTileCache extends 
LinkedHashMap<GridTile,GridTile> {
      */
     @Override
     protected boolean removeEldestEntry(final Map.Entry<GridTile,GridTile> 
entry) {
-        if (size() > ImageUtilities.SUGGESTED_TILE_CACHE_SIZE) {
+        if (size() > ImageLayout.SUGGESTED_TILE_CACHE_SIZE) {
             return entry.getValue().clearTile();
         }
         return false;

Reply via email to