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 6a22d7e5e93ce0d60f1219b199fb2288052ef7b5
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Mon Dec 16 19:51:45 2024 +0100

    If an image does not fit the requirement of the TIFF specification,
    reformat the image before to write it in a TIFF file.
---
 .../sis/coverage/privy/SampleModelFactory.java     | 63 ++++++++++++++++++++--
 .../apache/sis/storage/geotiff/FormatModifier.java | 14 ++++-
 .../org/apache/sis/storage/geotiff/Writer.java     | 10 +++-
 .../storage/geotiff/writer/ReformattedImage.java   | 58 +++++++++++++++++---
 .../org/apache/sis/storage/geotiff/WriterTest.java | 12 +++--
 .../org/apache/sis/storage/DataStoreRegistry.java  | 35 ++++++------
 6 files changed, 157 insertions(+), 35 deletions(-)

diff --git 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/SampleModelFactory.java
 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/SampleModelFactory.java
index b3af1f0cc7..b842cc5e11 100644
--- 
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/SampleModelFactory.java
+++ 
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/privy/SampleModelFactory.java
@@ -76,6 +76,7 @@ public final class SampleModelFactory {
 
     /**
      * The bit masks for all bands, or {@code null} if not applicable.
+     * This field should be non-null only with {@link 
SinglePixelPackedSampleModel}.
      */
     private int[] bitMasks;
 
@@ -85,8 +86,8 @@ public final class SampleModelFactory {
     private int dataBitOffset;
 
     /**
-     * Number of bits per pixel, or 0 if not applicable.
-     * Also known as "pixel bit stride".
+     * Number of bits per pixel, or 0 if not applicable. Also known as "pixel 
bit stride".
+     * This field should be non-zero only with {@link 
MultiPixelPackedSampleModel}.
      */
     private int numberOfBits;
 
@@ -162,7 +163,7 @@ public final class SampleModelFactory {
         numBands = model.getNumBands();
         dataType = model.getDataType();
         if (model instanceof ComponentSampleModel) {
-            final ComponentSampleModel cm = (ComponentSampleModel) model;
+            final var cm = (ComponentSampleModel) model;
             bankIndices    = cm.getBankIndices();
             bandOffsets    = cm.getBandOffsets();
             scanlineStride = cm.getScanlineStride();
@@ -172,12 +173,12 @@ public final class SampleModelFactory {
             }
             bankIndices = null;     // PixelInterleavedSampleModel
         } else if (model instanceof SinglePixelPackedSampleModel) {
-            final SinglePixelPackedSampleModel cm = 
(SinglePixelPackedSampleModel) model;
+            final var cm = (SinglePixelPackedSampleModel) model;
             bitMasks       = cm.getBitMasks();
             scanlineStride = cm.getScanlineStride();
             pixelStride    = 1;
         } else if (model instanceof MultiPixelPackedSampleModel) {
-            final MultiPixelPackedSampleModel cm = 
(MultiPixelPackedSampleModel) model;
+            final var cm = (MultiPixelPackedSampleModel) model;
             numberOfBits   = cm.getPixelBitStride();
             dataBitOffset  = cm.getDataBitOffset();
             scanlineStride = cm.getScanlineStride();
@@ -267,6 +268,58 @@ public final class SampleModelFactory {
         return indices;
     }
 
+    /**
+     * Replaces the sample model with a variant where all components are 
stored in separated data elements.
+     * This method replaces {@link SinglePixelPackedSampleModel} and {@link 
MultiPixelPackedSampleModel} by
+     * a {@link ComponentSampleModel}.
+     *
+     * @param  banded  whether a {@link BandedSampleModel} is preferred. This 
hint may be ignored.
+     * @return whether the sample model changed as a result of this method 
call.
+     */
+    public boolean unpack(final boolean banded) {
+        if (bitMasks != null) {
+            /*
+             * SinglePixelPackedSampleModel: find the number of bits needed by 
the widest component.
+             * The `numberOfBits` field is temporarily set for computation 
purpose and cleared later.
+             */
+            int max = 0;
+            for (int i=0; i<numBands; i++) {
+                int mask = bitMasks[i];
+                mask >>>= Integer.numberOfTrailingZeros(mask);
+                if (mask > max) max = mask;
+            }
+            numberOfBits = Integer.SIZE - Integer.numberOfLeadingZeros(max);
+            bitMasks = null;
+        } else if (numberOfBits == 0) {
+            // Already a `ComponentSampleModel`.
+            return false;
+        }
+        /*
+         * Find the smallest data type capable to store the component values 
after unpacking.
+         * Then, reinitialize the strides and offsets to the simplest values 
they would have
+         * for a new component sample model.
+         */
+        if (numberOfBits <= Byte.SIZE) {
+            dataType = DataBuffer.TYPE_BYTE;
+        } else if (numberOfBits > Short.SIZE) {
+            dataType = DataBuffer.TYPE_INT;
+        } else if (dataType > DataBuffer.TYPE_SHORT) {    // Do not change the 
sign of current data type.
+            dataType = DataBuffer.TYPE_USHORT;
+        }
+        numberOfBits   = 0;
+        pixelStride    = numBands;
+        scanlineStride = Math.multiplyExact(width, numBands);
+        bandOffsets    = ArraysExt.range(0, numBands);
+        bankIndices    = null;
+        if (banded) {
+            pixelStride    = 1;
+            scanlineStride = width;
+            bankIndices    = bandOffsets;
+            bandOffsets    = new int[numBands];
+        }
+        return true;
+    }
+
     /**
      * Builds a sample model based on current factory configuration.
      * The factory is still valid after this method call.
diff --git 
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/FormatModifier.java
 
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/FormatModifier.java
index dc3aafcafa..8a6b93a2e2 100644
--- 
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/FormatModifier.java
+++ 
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/FormatModifier.java
@@ -53,10 +53,22 @@ public enum FormatModifier {
      * When the {@code BIG_TIFF} modifier is present, the addressable space of 
64-bits integers is used.
      * The BigTIFF format is non-standard and files written with this option 
may not be read by all TIFF readers.
      */
-    BIG_TIFF;
+    BIG_TIFF,
 
     // TODO: COG, SPARSE.
 
+    /**
+     * Whether to allow the writing of tiles of any size.
+     * The TIFF specification requires tile sizes to be multiples of 16 pixels.
+     * At reading time, Apache <abbr>SIS</abbr> always accept tiles of any 
size and this option is ignored.
+     * At writing time, by default Apache <abbr>SIS</abbr> checks the tile 
size and, if not compliant with
+     * <abbr>TIFF</abbr> requirement, reorganizes the pixel values in a new 
tiling before to write image.
+     * This reorganization may have a performance cost and consumes more disk 
space than needed.
+     * This option allows to disable this reorganization,
+     * which may result in smaller but non-standard TIFF files.
+     */
+    ANY_TILE_SIZE;
+
     /**
      * The key for declaring GeoTIFF format modifiers at store creation time.
      * See class Javadoc for usage example.
diff --git 
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java
 
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java
index ac69b69b37..43f7aa3d52 100644
--- 
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java
+++ 
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java
@@ -134,6 +134,11 @@ final class Writer extends IOBase implements Flushable {
      */
     private final boolean isBigTIFF;
 
+    /**
+     * Whether to disable the <abbr>TIFF</abbr> requirement that tile sizes 
are multiple of 16 pixels.
+     */
+    private final boolean anyTileSize;
+
     /**
      * Index of the image to write. This information is not needed by the 
writer, but is
      * needed by {@link WritableStore} for determining the "effectively added 
resource".
@@ -182,6 +187,7 @@ final class Writer extends IOBase implements Flushable {
         super(store);
         this.output = output;
         isBigTIFF   = ArraysExt.contains(options, FormatModifier.BIG_TIFF);
+        anyTileSize = ArraysExt.contains(options, 
FormatModifier.ANY_TILE_SIZE);
         /*
          * Write the TIFF file header before first IFD. Stream position matter 
and must start at zero.
          * Note that it does not necessarily mean that the stream has no bytes 
before current position.
@@ -212,6 +218,7 @@ final class Writer extends IOBase implements Flushable {
     Writer(final Reader reader) throws IOException, DataStoreException {
         super(reader.store);
         isBigTIFF = (reader.intSizeExpansion != 0);
+        anyTileSize = false;
         try {
             output = new ChannelDataOutput(reader.input);
         } catch (ClassCastException e) {
@@ -287,9 +294,10 @@ final class Writer extends IOBase implements Flushable {
     public final long append(final RenderedImage image, final GridGeometry 
grid, final Metadata metadata)
             throws IOException, DataStoreException
     {
+        final var exportable = new ReformattedImage(image, this::processor, 
anyTileSize);
         final TileMatrix tiles;
         try {
-            tiles = writeImageFileDirectory(new ReformattedImage(image, 
this::processor), grid, metadata, false);
+            tiles = writeImageFileDirectory(exportable, grid, metadata, false);
         } finally {
             largeTagData.clear();       // For making sure that there is no 
memory retention.
         }
diff --git 
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ReformattedImage.java
 
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ReformattedImage.java
index c9cbff5c26..2e48d5d57e 100644
--- 
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ReformattedImage.java
+++ 
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ReformattedImage.java
@@ -17,6 +17,7 @@
 package org.apache.sis.storage.geotiff.writer;
 
 import java.util.function.Supplier;
+import java.awt.Dimension;
 import java.awt.color.ColorSpace;
 import java.awt.image.ColorModel;
 import java.awt.image.IndexColorModel;
@@ -25,9 +26,12 @@ import java.awt.image.SampleModel;
 import static javax.imageio.plugins.tiff.BaselineTIFFTagSet.*;
 import org.apache.sis.util.ArraysExt;
 import org.apache.sis.math.Statistics;
+import org.apache.sis.pending.jdk.JDK18;
 import org.apache.sis.image.PlanarImage;
 import org.apache.sis.image.ImageProcessor;
+import org.apache.sis.coverage.privy.ImageLayout;
 import org.apache.sis.coverage.privy.ImageUtilities;
+import org.apache.sis.coverage.privy.SampleModelFactory;
 import org.apache.sis.io.stream.HyperRectangleWriter;
 
 
@@ -35,11 +39,16 @@ import org.apache.sis.io.stream.HyperRectangleWriter;
  * An image prepared for writing in a TIFF file. The TIFF specification puts 
some restrictions on tile size
  * (must be a multiple of 16), and {@code HyperRectangleWriter} has some other 
restrictions on data layout.
  *
- * @todo Force tile size to multiple of 16.
- *
  * @author  Martin Desruisseaux (Geomatys)
  */
 public final class ReformattedImage {
+    /**
+     * Divisor of tile sizes mandated by the TIFF specification.
+     * All tile sizes must be a multiple of this value.
+     * This constant must be a power of 2.
+     */
+    private static final int TILE_DIVISOR = 16;
+
     /**
      * Number of color bands before the extra bands. This number does not 
include the alpha channel,
      * which is considered an extra band in TIFF. This number does not apply 
to gray scale images,
@@ -71,14 +80,16 @@ public final class ReformattedImage {
      * The visible image will have at most 3 bands and should have no alpha 
channel.
      * If no change is needed, then the given image is used unchanged.
      *
-     * @param  image      the image to separate into visible, alpha and extra 
bands.
-     * @param  processor  supplier of the image processor to use if the image 
must be transformed.
+     * @param  image        the image to separate into visible, alpha and 
extra bands.
+     * @param  processor    supplier of the image processor to use if the 
image must be transformed.
+     * @param  anyTileSize  whether to disable the <abbr>TIFF</abbr> 
requirement that tile sizes are multiple of 16 pixels.
      */
-    public ReformattedImage(RenderedImage image, final 
Supplier<ImageProcessor> processor) {
+    public ReformattedImage(RenderedImage image, final 
Supplier<ImageProcessor> processor, final boolean anyTileSize) {
         int alphaBand = -1;
         boolean isAlphaPremultiplied = false;
         final int numBands = ImageUtilities.getNumBands(image);
         final int visibleBand = ImageUtilities.getVisibleBand(image);
+        final boolean banded;   // Whether to prefer `BandedSampleModel`.
         if (visibleBand >= 0) {
             /*
              * Indexed color model or gray scale image. This is conceptually a 
single band.
@@ -86,6 +97,7 @@ public final class ReformattedImage {
              * where only one band is shown. We need to order the visible band 
first.
              * All other bands will be extra samples.
              */
+            banded = true;
             singleBand = true;
             if (visibleBand != 0) {
                 final int[] bands = ArraysExt.range(0, numBands);
@@ -107,7 +119,9 @@ public final class ReformattedImage {
                     alphaBand = -1;
                 }
             }
-            singleBand = numBands < (alphaBand >= 0 ? 1 + NUM_COLOR_BANDS : 
NUM_COLOR_BANDS);
+            final int expected = (alphaBand < 0 ? NUM_COLOR_BANDS : 
NUM_COLOR_BANDS + 1);
+            banded = (numBands != expected);
+            singleBand = numBands < expected;
         }
         /*
          * Check if there is any extra samples. If yes, prepare an 
`extraSamples` array with all
@@ -126,8 +140,36 @@ public final class ReformattedImage {
          * If the image cannot be written directly, reformat. Because this 
operation
          * forces the copy of pixel values, it should be executed in last 
resort.
          */
-        if (!HyperRectangleWriter.Builder.isSupported(image.getSampleModel())) 
{
-            // TODO: reformat the image here.
+        boolean reformat = false;
+        SampleModel sm = image.getSampleModel();
+        if (!HyperRectangleWriter.Builder.isSupported(sm)) {
+            final var factory = new SampleModelFactory(sm);
+            if (factory.unpack(banded)) {
+                sm = factory.build();
+                reformat = true;
+            }
+        }
+        /*
+         * Force the image tile size to multiple of 16 pixels. This is a 
requirement of the TIFF specification.
+         * We can ignore this requirement when the image is untiled on the X 
axis, because in such case the SIS
+         * writer will write the image as strips instead of as tiles.
+         */
+        if (!anyTileSize && image.getNumXTiles() != 1) {
+            var tileSize = new Dimension(image.getTileWidth(), 
image.getTileHeight());
+            if (((tileSize.width | tileSize.height) & (TILE_DIVISOR - 1)) != 
0) {
+                final int width  = JDK18.ceilDiv(image.getWidth(),  
TILE_DIVISOR);
+                final int height = JDK18.ceilDiv(image.getHeight(), 
TILE_DIVISOR);
+                tileSize.width   = JDK18.ceilDiv(tileSize.width,    
TILE_DIVISOR);
+                tileSize.height  = JDK18.ceilDiv(tileSize.height,   
TILE_DIVISOR);
+                tileSize = new ImageLayout(tileSize, 
false).suggestTileSize(width, height, true);
+                tileSize.width  *= TILE_DIVISOR;
+                tileSize.height *= TILE_DIVISOR;
+                sm = sm.createCompatibleSampleModel(tileSize.width, 
tileSize.height);
+                reformat = true;
+            }
+        }
+        if (reformat) {
+            image = processor.get().reformat(image, sm);
         }
         exportable = image;
     }
diff --git 
a/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java
 
b/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java
index 0f33859697..0a1f93a1ff 100644
--- 
a/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java
+++ 
b/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java
@@ -181,7 +181,8 @@ public final class WriterTest extends TestCase {
      */
     @Test
     public void testUntiledGrayScale() throws IOException, DataStoreException {
-        initialize(DataType.BYTE, ByteOrder.BIG_ENDIAN, false, 1, 1, 1);
+        initialize(DataType.BYTE, ByteOrder.BIG_ENDIAN, false, 1, 1, 1,
+                   FormatModifier.ANY_TILE_SIZE);
         writeImage();
         verifyHeader(false, IOBase.BIG_ENDIAN);
         verifyImageFileDirectory(Writer.COMMON_NUMBER_OF_TAGS - 1,             
 // One less tag because stripped layout.
@@ -199,7 +200,8 @@ public final class WriterTest extends TestCase {
      */
     @Test
     public void testUntiledBigTIFF() throws IOException, DataStoreException {
-        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 1, 1, 1, 
FormatModifier.BIG_TIFF);
+        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 1, 1, 1,
+                   FormatModifier.ANY_TILE_SIZE, FormatModifier.BIG_TIFF);
         writeImage();
         verifyHeader(true, IOBase.LITTLE_ENDIAN);
         verifyImageFileDirectory(Writer.COMMON_NUMBER_OF_TAGS - 1,          // 
One less tag because stripped layout.
@@ -218,7 +220,7 @@ public final class WriterTest extends TestCase {
      */
     @Test
     public void testTiledGrayScale() throws IOException, DataStoreException {
-        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 1, 3, 4);
+        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 1, 3, 4, 
FormatModifier.ANY_TILE_SIZE);
         writeImage();
         verifyHeader(false, IOBase.LITTLE_ENDIAN);
         verifyImageFileDirectory(Writer.COMMON_NUMBER_OF_TAGS,
@@ -236,7 +238,7 @@ public final class WriterTest extends TestCase {
      */
     @Test
     public void testUntiledRGB() throws IOException, DataStoreException {
-        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 3, 1, 1);
+        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 3, 1, 1, 
FormatModifier.ANY_TILE_SIZE);
         image.setColorModel(new 
ColorModelBuilder().createRGB(image.getSampleModel()));
         writeImage();
         verifyHeader(false, IOBase.LITTLE_ENDIAN);
@@ -255,7 +257,7 @@ public final class WriterTest extends TestCase {
      */
     @Test
     public void testGeoTIFF() throws IOException, DataStoreException {
-        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 1, 1, 1);
+        initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 1, 1, 1, 
FormatModifier.ANY_TILE_SIZE);
         createGridGeometry();
         writeImage();
         verifyHeader(false, IOBase.LITTLE_ENDIAN);
diff --git 
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/DataStoreRegistry.java
 
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/DataStoreRegistry.java
index cd117b737b..38d856b488 100644
--- 
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/DataStoreRegistry.java
+++ 
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/DataStoreRegistry.java
@@ -190,7 +190,7 @@ final class DataStoreRegistry extends 
LazySet<DataStoreProvider> {
     }
 
     /**
-     * Implementation of {@link #probeContentType(Object)} and {@link 
#open(Object, Capability, String)}.
+     * Implementation of {@link #probeContentType(Object)} and {@link 
#open(Object, Capability, Function)}.
      *
      * @param  storage     the input/output object as a URL, file, image input 
stream, <i>etc.</i>.
      * @param  capability  the capability that the data store must have (read, 
write, create).
@@ -208,26 +208,31 @@ final class DataStoreRegistry extends 
LazySet<DataStoreProvider> {
             Predicate<DataStoreProvider> preferred, final boolean open)
             throws DataStoreException
     {
+        final boolean writable;
         StorageConnector connector;                 // Will be reset to `null` 
if it shall not be closed.
         if (storage instanceof StorageConnector) {
             connector = (StorageConnector) storage;
-            final var p = 
connector.getOption(InternalOptionKey.PREFERRED_PROVIDERS);
-            if (preferred == null) {
-                preferred = p;
-            } else {
-                preferred = preferred.and(p);
-                connector.setOption(InternalOptionKey.PREFERRED_PROVIDERS, 
preferred);
+            writable = (capability == Capability.WRITE) && 
connector.getOption(OptionKey.OPEN_OPTIONS) == null;
+            final var filter = 
connector.getOption(InternalOptionKey.PREFERRED_PROVIDERS);
+            if (filter != null) {
+                preferred = (preferred != null) ? preferred.and(filter) : 
filter;
             }
         } else {
             connector = new StorageConnector(storage);
-            if (capability == Capability.WRITE) {
-                connector.setOption(OptionKey.OPEN_OPTIONS, new 
StandardOpenOption[] {
-                    StandardOpenOption.CREATE, StandardOpenOption.WRITE
-                });
-            }
-            if (preferred != null) {
-                connector.setOption(InternalOptionKey.PREFERRED_PROVIDERS, 
preferred);
-            }
+            writable = (capability == Capability.WRITE);
+        }
+        /*
+         * If this method is invoked by `DataStores.openWritable(…)`, add NIO 
open options if not already present.
+         * Note that this code may modify a user provided storage connector. 
It should be okay considering that
+         * each `StorageConnector` instance should be short-lived and used 
only once.
+         */
+        if (writable) {
+            connector.setOption(OptionKey.OPEN_OPTIONS, new 
StandardOpenOption[] {
+                StandardOpenOption.CREATE, StandardOpenOption.WRITE
+            });
+        }
+        if (preferred != null) {
+            connector.setOption(InternalOptionKey.PREFERRED_PROVIDERS, 
preferred);
         }
         /*
          * If we can get a filename extension from the given storage (file, 
URL, etc.), we may perform two times

Reply via email to