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 2023/04/09 20:56:21 UTC

[sis] branch geoapi-4.0 updated: When doing an aggregation of "band select", verify if the operations cancel each other.

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 ed74a09dc9 When doing an aggregation of "band select", verify if the operations cancel each other.
ed74a09dc9 is described below

commit ed74a09dc99377677e4a1b0d6c3ccb4f0e29e991
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Sun Apr 9 22:54:58 2023 +0200

    When doing an aggregation of "band select", verify if the operations cancel each other.
---
 .../org/apache/sis/image/BandAggregateImage.java   |  22 ++--
 .../java/org/apache/sis/image/BandSelectImage.java |  28 +++++-
 .../java/org/apache/sis/image/ComputedImage.java   |  18 ++++
 .../java/org/apache/sis/image/ImageProcessor.java  |   4 +-
 .../org/apache/sis/image/MultiSourceImage.java     |  19 +---
 .../org/apache/sis/image/MultiSourceLayout.java    |   5 +-
 .../sis/internal/coverage/MultiSourceArgument.java | 111 +++++++++++++--------
 7 files changed, 136 insertions(+), 71 deletions(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/BandAggregateImage.java b/core/sis-feature/src/main/java/org/apache/sis/image/BandAggregateImage.java
index 7ffbd60648..2e672a1bb1 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/BandAggregateImage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/BandAggregateImage.java
@@ -73,8 +73,8 @@ class BandAggregateImage extends MultiSourceImage {
         } else {
             image = new BandAggregateImage(layout, colorizer, allowSharing, parallel);
         }
-        if (image.filteredSources.length == 1) {
-            final RenderedImage c = image.filteredSources[0];
+        if (image.getNumSources() == 1) {
+            final RenderedImage c = image.getSource();
             if (image.colorModel == null) {
                 return c;
             }
@@ -92,8 +92,8 @@ class BandAggregateImage extends MultiSourceImage {
      * @param  layout     pixel and tile coordinate spaces of this image, together with sample model.
      * @param  colorizer  provider of color model to use for this image, or {@code null} for automatic.
      */
-    BandAggregateImage(final MultiSourceLayout layout, final Colorizer colorizer,
-                       final boolean allowSharing, final boolean parallel)
+    private BandAggregateImage(final MultiSourceLayout layout, final Colorizer colorizer,
+                               final boolean allowSharing, final boolean parallel)
     {
         super(layout, colorizer, parallel);
         this.allowSharing = allowSharing;
@@ -120,7 +120,7 @@ class BandAggregateImage extends MultiSourceImage {
         if (allowSharing) {
             final BandSharing sharing = BandSharing.create((BandedSampleModel) sampleModel);
             if (sharing != null) {
-                tile = shared = sharing.createRaster(tileToPixel(tileX, tileY), filteredSources);
+                tile = shared = sharing.createRaster(tileToPixel(tileX, tileY), getSourceArray());
             }
         }
         /*
@@ -131,8 +131,9 @@ class BandAggregateImage extends MultiSourceImage {
             tile = createTile(tileX, tileY);
         }
         int band = 0;
-        for (int i=0; i < filteredSources.length; i++) {
-            final RenderedImage source = filteredSources[i];
+        final int n = getNumSources();
+        for (int i=0; i<n; i++) {
+            final RenderedImage source = getSource(i);
             final int numBands = ImageUtilities.getNumBands(source);
             if (shared == null || shared.needCopy(i)) {
                 final Rectangle aoi = tile.getBounds();
@@ -172,7 +173,7 @@ class BandAggregateImage extends MultiSourceImage {
         public WritableRaster getWritableTile(final int tileX, final int tileY) {
             final WritableRaster tile = (WritableRaster) getTile(tileX, tileY);
             if (tile instanceof BandSharedRaster) {
-                ((BandSharedRaster) tile).acquireWritableTiles(filteredSources);
+                ((BandSharedRaster) tile).acquireWritableTiles(getSourceArray());
             }
             try {
                 markTileWritable(tileX, tileY, true);
@@ -210,8 +211,9 @@ class BandAggregateImage extends MultiSourceImage {
         public void setData(final Raster tile) {
             final BandSharedRaster shared = (tile instanceof BandSharedRaster) ? (BandSharedRaster) tile : null;
             int band = 0;
-            for (int i=0; i < filteredSources.length; i++) {
-                final var target = (WritableRenderedImage) filteredSources[i];
+            final int n = getNumSources();
+            for (int i=0; i<n; i++) {
+                final var target = (WritableRenderedImage) getSource(i);
                 final int numBands = ImageUtilities.getNumBands(target);
                 if (shared == null || shared.needCopy(i)) {
                     final Rectangle aoi = tile.getBounds();
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/BandSelectImage.java b/core/sis-feature/src/main/java/org/apache/sis/image/BandSelectImage.java
index 60fbb426ec..f8fb8bc318 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/BandSelectImage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/BandSelectImage.java
@@ -84,19 +84,41 @@ class BandSelectImage extends SourceAlignedImage {
         ensureCompatible(cm);
     }
 
+    /**
+     * If the given image is already a band select operation, returns the original source
+     * and updates the band indices. If there is no replacement, then the {@code image}
+     * argument is returned as-is and the {@code bands} array shall be unmodified.
+     *
+     * @param  image  the image to check.
+     * @param  bands  the band to select in the specified source.
+     *                Will be updated in-place if the source is replaced.
+     * @return the source of the image, or {@code image} if no replacement.
+     */
+    static RenderedImage unwrap(final RenderedImage image, final int[] bands) {
+        if (image instanceof BandSelectImage) {
+            final var select = (BandSelectImage) image;
+            for (int i=0; i<bands.length; i++) {
+                bands[i] = select.bands[bands[i]];
+            }
+            return select.getSource();
+        }
+        return image;
+    }
+
     /**
      * Creates a new "band select" operation for the given source.
      *
      * @param  source  the image in which to select bands.
-     * @param  bands   the bands to select. Should be a clone of user-specified argument
-     *                 for protection against user changes in the given array.
+     * @param  bands   the bands to select. Shall be a clone of user-specified argument
+     *                 because it may be modified in-place.
      */
-    static RenderedImage create(final RenderedImage source, final int... bands) {
+    static RenderedImage create(RenderedImage source, int... bands) {
         final int numBands = ImageUtilities.getNumBands(source);
         if (bands.length == numBands && ArraysExt.isRange(0, bands)) {
             return source;
         }
         ArgumentChecks.ensureNonEmptyBounded("bands", false, 0, numBands - 1, bands);
+        source = unwrap(source, bands);
         final ColorModel cm = ColorModelFactory.createSubset(source.getColorModel(), bands);
         /*
          * If the image is an instance of `BufferedImage`, create the subset immediately
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java b/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java
index c991a18cb4..792bb06d08 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java
@@ -337,6 +337,24 @@ public abstract class ComputedImage extends PlanarImage implements Disposable {
         return sources[0];
     }
 
+    /**
+     * Returns the sources as an array.
+     * A future version may remove this method if Java allows unmodifiable arrays.
+     */
+    final RenderedImage[] getSourceArray() {
+        return sources.clone();
+    }
+
+    /**
+     * Returns the number of sources, or 0 if none or unknown.
+     * This is the size of the vector returned by {@link #getSources()}.
+     *
+     * @return number of sources, or 0 if none or unknown.
+     */
+    final int getNumSources() {
+        return (sources != null) ? sources.length : 0;
+    }
+
     /**
      * Returns the source at the given index.
      *
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/ImageProcessor.java b/core/sis-feature/src/main/java/org/apache/sis/image/ImageProcessor.java
index 34d0e60ca3..eebd30af21 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/ImageProcessor.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/ImageProcessor.java
@@ -1245,7 +1245,7 @@ public class ImageProcessor implements Cloneable {
         if (source == null || source instanceof BufferedImage || source instanceof TiledImage) {
             return source;
         }
-        while (source instanceof PrefetchedImage) {
+        if (source instanceof PrefetchedImage) {
             source = ((PrefetchedImage) source).source;
         }
         final boolean parallel;
@@ -1254,7 +1254,7 @@ public class ImageProcessor implements Cloneable {
             parallel = parallel(source);
             errorListener = errorHandler;
         }
-        final PrefetchedImage image = new PrefetchedImage(source, areaOfInterest, errorListener, parallel);
+        final var image = new PrefetchedImage(source, areaOfInterest, errorListener, parallel);
         return image.isEmpty() ? source : image;
     }
 
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceImage.java b/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceImage.java
index ed67b5446c..743e1580aa 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceImage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceImage.java
@@ -17,11 +17,9 @@
 package org.apache.sis.image;
 
 import java.awt.Point;
-import java.util.Arrays;
 import java.util.Objects;
 import java.awt.Rectangle;
 import java.awt.image.ColorModel;
-import java.awt.image.RenderedImage;
 import java.awt.image.WritableRenderedImage;
 import org.apache.sis.internal.coverage.j2d.ImageUtilities;
 import org.apache.sis.util.Disposable;
@@ -41,13 +39,6 @@ import org.apache.sis.util.Disposable;
  * @since   1.4
  */
 abstract class MultiSourceImage extends WritableComputedImage {
-    /**
-     * The source images, potentially with a preprocessing applied.
-     * Those sources may be different than {@link #getSources()} for example with the
-     * application of a "band select" operation for retaining only the bands needed.
-     */
-    protected final RenderedImage[] filteredSources;
-
     /**
      * Color model of this image.
      *
@@ -82,7 +73,7 @@ abstract class MultiSourceImage extends WritableComputedImage {
      * @param  parallel   whether parallel computation is allowed.
      */
     MultiSourceImage(final MultiSourceLayout layout, final Colorizer colorizer, final boolean parallel) {
-        super(layout.sampleModel, layout.sources);
+        super(layout.sampleModel, layout.filteredSources);
         final Rectangle r = layout.domain;
         minX            = r.x;
         minY            = r.y;
@@ -90,7 +81,6 @@ abstract class MultiSourceImage extends WritableComputedImage {
         height          = r.height;
         minTileX        = layout.minTileX;
         minTileY        = layout.minTileY;
-        filteredSources = layout.filteredSources;
         colorModel      = layout.createColorModel(colorizer);
         ensureCompatible(colorModel);
         this.parallel = parallel;
@@ -129,7 +119,7 @@ abstract class MultiSourceImage extends WritableComputedImage {
          * tile indices for each source because the tile numbering may not be the same.
          */
         final Rectangle aoi = ImageUtilities.tilesToPixels(this, tiles);
-        return new MultiSourcePrefetch(filteredSources, aoi).run(parallel);
+        return new MultiSourcePrefetch(getSourceArray(), aoi).run(parallel);
     }
 
     /**
@@ -137,7 +127,7 @@ abstract class MultiSourceImage extends WritableComputedImage {
      */
     @Override
     public int hashCode() {
-        return hashCodeBase() + 37 * (Arrays.hashCode(filteredSources) + 31 * Objects.hashCode(colorModel));
+        return hashCodeBase() + 37 * Objects.hashCode(colorModel);
     }
 
     /**
@@ -151,8 +141,7 @@ abstract class MultiSourceImage extends WritableComputedImage {
                    minTileX == other.minTileX &&
                    minTileY == other.minTileY &&
                    getBounds().equals(other.getBounds()) &&
-                   Objects.equals(colorModel, other.colorModel) &&
-                   Arrays.equals(filteredSources, other.filteredSources);
+                   Objects.equals(colorModel, other.colorModel);
         }
         return false;
     }
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceLayout.java b/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceLayout.java
index afa30b76e5..f649382dfe 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceLayout.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/MultiSourceLayout.java
@@ -59,7 +59,7 @@ final class MultiSourceLayout extends ImageLayout {
      * The source images. This is a copy of the user-specified array,
      * except that images associated to an empty set of bands are discarded.
      */
-    final RenderedImage[] sources;
+    private final RenderedImage[] sources;
 
     /**
      * The source images with only the user-specified bands.
@@ -129,6 +129,7 @@ final class MultiSourceLayout extends ImageLayout {
     static MultiSourceLayout create(RenderedImage[] sources, int[][] bandsPerSource, boolean allowSharing) {
         final var aggregate = new MultiSourceArgument<RenderedImage>(sources, bandsPerSource);
         aggregate.identityAsNull();
+        aggregate.unwrap(BandSelectImage::unwrap);
         aggregate.validate(ImageUtilities::getNumBands);
 
         sources            = aggregate.sources();
@@ -241,7 +242,7 @@ final class MultiSourceLayout extends ImageLayout {
             RenderedImage source = sources[i];
             final int[] bands = bandsPerSource[i];
             if (bands != null) {
-                source = BandSelectImage.create(source, bands);
+                source = BandSelectImage.create(source, bands.clone());
             }
             filteredSources[i] = source;
         }
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/MultiSourceArgument.java b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/MultiSourceArgument.java
index 27d82af545..be9fa9b6fb 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/MultiSourceArgument.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/MultiSourceArgument.java
@@ -22,6 +22,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Objects;
 import java.util.function.Function;
+import java.util.function.BiFunction;
 import java.util.function.ToIntFunction;
 import org.apache.sis.coverage.SampleDimension;
 import org.apache.sis.coverage.grid.GridGeometry;
@@ -40,8 +41,9 @@ import org.apache.sis.util.ComparisonMode;
  * <p>Instances of this class should be short-lived.
  * They are used only the time needed for constructing an image or coverage operation.</p>
  *
- * @todo Verify if a source is itself an aggregated image or coverage,
- *       and provide a way to get a flattened view of such nested aggregations.
+ * <p>This class can optionally verify if a source is itself an aggregated image or coverage.
+ * This is done by an "unwrapper", which should be specified in order to provide a flattened
+ * view of nested aggregations.</p>
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.4
@@ -72,6 +74,13 @@ public final class MultiSourceArgument<S> {
      */
     private boolean identityAsNull;
 
+    /**
+     * A function which, given an (image, bands) pair, may return the source of the image.
+     * If the source is returned, then the bands array is updated with the indices in that
+     * source.
+     */
+    private BiFunction<S,int[],S> unwrapper;
+
     /**
      * Union of all selected bands in all specified sources, or {@code null} if not applicable.
      */
@@ -117,6 +126,19 @@ public final class MultiSourceArgument<S> {
         identityAsNull = true;
     }
 
+    /**
+     * Specifies a function which, given an (image, bands) pair, may return the source of the image.
+     * If the source is returned, then the bands array is updated with the indices in that source.
+     * The function shall modify the given {@code int[]} in-place and return the new source,
+     * or return the {@code S} value unchanged if no unwrapping has been done.
+     *
+     * @param  filter  the function to invoke for getting the source of an image or coverage.
+     */
+    public void unwrap(final BiFunction<S,int[],S> filter) {
+        if (validated) throw new IllegalStateException();
+        unwrapper = filter;
+    }
+
     /**
      * Clones and validates the arguments given to the constructor.
      *
@@ -147,8 +169,8 @@ public final class MultiSourceArgument<S> {
      *
      * <p>Exactly one of {@code getter} or {@code count} arguments shall be non-null.</p>
      *
-     * @param  getter          method to invoke for getting the list of sample dimensions.
-     * @param  counter         method to invoke for counting the number of bands in a source.
+     * @param  getter   method to invoke for getting the list of sample dimensions.
+     * @param  counter  method to invoke for counting the number of bands in a source.
      * @throws IllegalArgumentException if some band indices are duplicated or outside their range of validity.
      */
     private void validate(final Function<S, List<SampleDimension>> getter, final ToIntFunction<S> counter) {
@@ -183,56 +205,67 @@ public final class MultiSourceArgument<S> {
                 // Note that the source is allowed to be null in this particular case.
                 continue;
             }
-            final S source = sources[i];
-            sources[filteredCount] = source;
+            S source = sources[i];
             ArgumentChecks.ensureNonNullElement("sources", i, source);
             /*
              * Get the number of bands, or optionally the bands themselves.
              * This information is required before to validate arguments.
              */
-            final List<SampleDimension> bands;
-            final int n;
-            if (getter != null) {
-                bands = getter.apply(source);
-                n = bands.size();
-            } else {
-                bands = null;
-                n = counter.applyAsInt(source);
+            List<SampleDimension> sourceBands;
+            int numSourceBands;
+            RangeArgument range;
+            do {
+                if (getter != null) {
+                    sourceBands = getter.apply(source);
+                    numSourceBands = sourceBands.size();
+                } else {
+                    sourceBands = null;
+                    numSourceBands = counter.applyAsInt(source);
+                }
+                range = RangeArgument.validate(numSourceBands, selected, null);
+                selected = range.getSelectedBands();
+                /*
+                 * Verify if the source is a nested aggregation, in order to get a flattened view.
+                 * This replacement must be done before the optimization for consecutive images.
+                 */
+            } while (unwrapper != null && source != (source = unwrapper.apply(source, selected)));
+            /*
+             * Store now the sample dimensions before the `selected` array get modified.
+             */
+            if (ranges != null) {
+                for (int b : selected) {
+                    ranges.add(sourceBands.get(b));
+                }
             }
             /*
-             * If the next source is the same than the source in current iteration, merge the bands together.
+             * If the source in current iteration is the same than the previous source, merge the bands together.
              * The `BandAggregateGridResource.read(…)` implementation relies on that optimization.
              */
-            final int next = i+1;
-            if (next < sourceCount && sources[next] == source) {
-                final int[] nextBands = bandsPerSource[next];
-                ArgumentChecks.ensureNonNullElement("bandsPerSource", i,    selected);
-                ArgumentChecks.ensureNonNullElement("bandsPerSource", next, nextBands);
-                final int[] merged = Arrays.copyOf(selected, selected.length + nextBands.length);
-                System.arraycopy(nextBands, 0, merged, selected.length, nextBands.length);
-                bandsPerSource[next] = merged;
-                bandsPerSource[i] = ArraysExt.EMPTY_INT;
-                continue;
+            if (filteredCount > 0 && sources[filteredCount-1] == source) {
+                final int[] previous = bandsPerSource[--filteredCount];
+                ArgumentChecks.ensureNonNullElement("bandsPerSource", filteredCount,   previous);
+                ArgumentChecks.ensureNonNullElement("bandsPerSource", filteredCount+1, selected);
+                numBands -= previous.length;   // Rollback the value added in previous iteration.
+
+                final int[] merged = Arrays.copyOf(previous, previous.length + selected.length);
+                System.arraycopy(selected, 0, merged, previous.length, selected.length);
+                range = RangeArgument.validate(numSourceBands, merged, null);
+                selected = range.getSelectedBands();
             }
             /*
-             * Validate the `bandsPerSource` argument given at construction time.
-             * Then store a copy of that argument.
+             * Store a copy of the `bandsPerSource` argument given at construction time.
+             * Its validation has been done by `RangeArgument.validate(…)` above calls.
              */
-            final var range = RangeArgument.validate(n, selected, null);
             if (range.isIdentity()) {
-                selected = (pool != null) ? pool.computeIfAbsent(n, (k) -> ArraysExt.range(0, k)) : null;
-                if (ranges != null) {
-                    ranges.addAll(bands);
-                }
-            } else {
-                selected = range.getSelectedBands();
-                if (ranges != null) {
-                    for (int b : selected) {
-                        ranges.add(bands.get(b));
-                    }
+                if (pool != null) {
+                    int[] previous = pool.putIfAbsent(numSourceBands, selected);
+                    if (previous != null) selected = previous;
+                } else {
+                    selected = null;
                 }
             }
-            bandsPerSource[filteredCount++] = selected;
+            bandsPerSource[filteredCount] = selected;
+            sources[filteredCount++] = source;
             numBands += range.getNumBands();
         }
         sources = ArraysExt.resize(sources, filteredCount);