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;