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 03a6a48e266ed8bea63814567801ce20a5055a95 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Mon Apr 3 17:09:48 2023 +0200 `BandAggregateImage` should share references to data arrays when possible. It avoids copying the sample values. --- .../org/apache/sis/image/BandAggregateImage.java | 312 +++++++++++++++++++-- .../apache/sis/image/BandedSampleConverter.java | 2 +- .../org/apache/sis/image/CombinedImageLayout.java | 74 +++-- .../java/org/apache/sis/image/ComputedImage.java | 15 +- .../java/org/apache/sis/image/ImageProcessor.java | 2 +- .../java/org/apache/sis/image/PlanarImage.java | 2 +- .../java/org/apache/sis/image/Visualization.java | 2 +- .../sis/internal/coverage/j2d/ImageLayout.java | 14 +- .../sis/internal/coverage/j2d/RasterFactory.java | 30 +- .../apache/sis/image/BandAggregateImageTest.java | 114 +++++++- .../org/apache/sis/image/ImageProcessorTest.java | 31 ++ 11 files changed, 539 insertions(+), 59 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 fd7aaa5758..e1e437b86f 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 @@ -20,11 +20,19 @@ import java.util.Arrays; import java.util.Objects; import java.awt.Rectangle; import java.awt.image.ColorModel; +import java.awt.image.SampleModel; +import java.awt.image.ComponentSampleModel; +import java.awt.image.DataBuffer; +import java.awt.image.DataBufferByte; +import java.awt.image.DataBufferShort; +import java.awt.image.DataBufferUShort; +import java.awt.image.DataBufferInt; +import java.awt.image.DataBufferFloat; +import java.awt.image.DataBufferDouble; import java.awt.image.Raster; import java.awt.image.RenderedImage; import java.awt.image.WritableRaster; import org.apache.sis.util.ArraysExt; -import org.apache.sis.util.Workaround; import org.apache.sis.internal.coverage.j2d.ImageUtilities; @@ -70,21 +78,28 @@ final class BandAggregateImage extends ComputedImage { */ private final int minTileX, minTileY; + /** + * Whether all sources have tiles at the same locations and use the same scanline stride. + * In such case, it is possible to share references to data arrays without copying them. + */ + private final boolean allowSharing; + /** * Creates a new aggregation of bands. - * This static method is a workaround for RFE #4093999 - * ("Relax constraint on placement of this()/super() call in constructors"). * - * @param sources images to combine, in order. - * @param bandsPerSource bands to use for each source image, in order. May contain {@code null} elements. - * @param colorizer provider of color model to use for this image, or {@code null} for automatic. + * @param sources images to combine, in order. + * @param bandsPerSource bands to use for each source image, in order. May contain {@code null} elements. + * @param colorizer provider of color model to use for this image, or {@code null} for automatic. + * @param allowSharing whether to allow the sharing of data buffers (instead of copying) if possible. * @throws IllegalArgumentException if there is an incompatibility between some source images * or if some band indices are duplicated or outside their range of validity. * @return the band aggregate image. */ - @Workaround(library="JDK", version="1.8") - static RenderedImage create(RenderedImage[] sources, int[][] bandsPerSource, Colorizer colorizer) { - var image = new BandAggregateImage(CombinedImageLayout.create(sources, bandsPerSource), colorizer); + static RenderedImage create(final RenderedImage[] sources, final int[][] bandsPerSource, + final Colorizer colorizer, final boolean allowSharing) + { + final var layout = CombinedImageLayout.create(sources, bandsPerSource, allowSharing); + final var image = new BandAggregateImage(layout, colorizer); if (image.filteredSources.length == 1) { final RenderedImage c = image.filteredSources[0]; if (image.colorModel == null) { @@ -95,7 +110,7 @@ final class BandAggregateImage extends ComputedImage { return c; } } - return ImageProcessor.unique(image); + return image; } /** @@ -113,6 +128,7 @@ final class BandAggregateImage extends ComputedImage { height = r.height; minTileX = layout.minTileX; minTileY = layout.minTileY; + allowSharing = layout.allowSharing; filteredSources = layout.getFilteredSources(); colorModel = layout.createColorModel(colorizer); ensureCompatible(colorModel); @@ -128,16 +144,33 @@ final class BandAggregateImage extends ComputedImage { @Override public int getMinTileY() {return minTileY;} /** - * Creates a raster sharing containing a copy of the selected bands in source images. + * Creates a raster containing the selected bands of source images. * - * @param tileX the column index of the tile to compute. - * @param tileY the row index of the tile to compute. - * @param previous the previous tile, reused if non-null. - * - * @todo Share data arrays instead of copying when possible. + * @param tileX the column index of the tile to compute. + * @param tileY the row index of the tile to compute. + * @param tile the previous tile, reused if non-null. */ @Override protected Raster computeTile(final int tileX, final int tileY, WritableRaster tile) { + /* + * If we are allowed to share the data arrays, try that first. + */ + if (allowSharing) { + final Sharing sharing = Sharing.create(sampleModel.getDataType(), sampleModel.getNumBands()); + if (sharing != null) { + final DataBuffer buffer = sharing.createDataBuffer( + Math.multiplyFull(tileX - minTileX, getTileWidth()) + minX, + Math.multiplyFull(tileY - minTileY, getTileHeight()) + minY, + filteredSources); + if (buffer != null) { + return Raster.createRaster(sampleModel, buffer, computeTileLocation(tileX, tileY)); + } + } + } + /* + * Fallback when the data arrays can not be shared. + * This code copies all sample values in new arrays. + */ if (tile == null) { tile = createTile(tileX, tileY); } @@ -155,6 +188,253 @@ final class BandAggregateImage extends ComputedImage { return tile; } + /** + * A builder of data buffers sharing arrays of source images. + * There is a subclass for each supported data type. + */ + private abstract static class Sharing { + /** + * The offsets of the first valid element into each bank array. + * Will be computed with the assumption that all offsets are zero + * in the target {@link java.awt.image.BandedSampleModel}. + */ + protected final int[] offsets; + + /** + * For subclass constructors. + */ + protected Sharing(final int numBands) { + offsets = new int[numBands]; + } + + /** + * Creates a new builder. + * + * @param dataType the data type as one of {@link DataBuffer} constants. + * @param numBands number of banks of the data buffer to create. + * @return the data buffer, or {@code null} if the dat type is not recognized. + */ + static Sharing create(final int dataType, final int numBands) { + switch (dataType) { + case DataBuffer.TYPE_BYTE: return new Bytes (numBands); + case DataBuffer.TYPE_SHORT: return new Shorts (numBands); + case DataBuffer.TYPE_USHORT: return new UShorts (numBands); + case DataBuffer.TYPE_INT: return new Integers(numBands); + case DataBuffer.TYPE_FLOAT: return new Floats (numBands); + case DataBuffer.TYPE_DOUBLE: return new Doubles (numBands); + } + return null; + } + + /** + * Creates a data buffer sharing the arrays of all given sources, in order. + * This method assumes a target {@link java.awt.image.BandedSampleModel} where + * all band offsets are zero and where bank indices define an identity mapping. + * + * @param x <var>x</var> pixel coordinate of the tile. + * @param y <var>y</var> pixel coordinate of the tile. + * @param sources the sources for which to aggregate all bands. + * @return a data buffer containing the aggregation of all bands, or {@code null} if it can not be created. + */ + final DataBuffer createDataBuffer(final long x, final long y, final RenderedImage[] sources) { + int band = 0; + int size = Integer.MAX_VALUE; + for (final RenderedImage source : sources) { + final int tileWidth = source.getTileWidth(); + final int tileHeight = source.getTileHeight(); + long tileX = x - source.getTileGridXOffset(); + long tileY = y - source.getTileGridYOffset(); + if (((tileX % tileWidth) | (tileY % tileHeight)) != 0) { + return null; // Source tile not aligned on target tile. + } + tileX /= tileWidth; + tileY /= tileHeight; + final Raster raster = source.getTile(Math.toIntExact(tileX), Math.toIntExact(tileY)); + final SampleModel c = raster.getSampleModel(); + if (!(c instanceof ComponentSampleModel)) { + return null; // Should never happen if `BandAggregateImage.allowSharing` is true. + } + final var sm = (ComponentSampleModel) c; + final var buffer = raster.getDataBuffer(); + final int[] offsets1 = buffer.getOffsets(); + final int[] offsets2 = sm.getBandOffsets(); + final int[] indices = sm.getBankIndices(); + for (int i=0; i<indices.length; i++) { + final int b = indices[i]; + takeReference(buffer, b, band); + offsets[band] = offsets1[b] + offsets2[i]; // Assume zero offset in target `BandedSampleModel`. + band++; + } + size = Math.min(size, buffer.getSize()); + } + final DataBuffer buffer = build(size); + assert buffer.getNumBanks() == band; + return buffer; + } + + /** + * Takes a reference to an array in the given data buffer. + * + * @param source the data buffer from which to take a reference to an array. + * @param src bank index of the reference to take. + * @param dst band index where to store the reference. + */ + abstract void takeReference(DataBuffer source, int src, int dst); + + /** + * Builds the data buffer after all references have been taken. + * The data buffer shall specify {@link #offsets} to the buffer constructor. + * + * @param size number of elements in the data buffer. + * @return the new data buffer. + */ + abstract DataBuffer build(int size); + } + + /** + * A builder of data buffer of {@link DataBuffer#TYPE_BYTE}. + */ + private static final class Bytes extends Sharing { + /** The shared arrays. */ + private final byte[][] data; + + /** Creates a new builder. */ + Bytes(final int numBands) { + super(numBands); + data = new byte[numBands][]; + } + + /** Takes a reference to an array in the given data buffer. */ + @Override void takeReference(DataBuffer buffer, int src, int dst) { + data[dst] = ((DataBufferByte) buffer).getData(src); + } + + /** Builds the data buffer after all references have been taken. */ + @Override DataBuffer build(int size) { + return new DataBufferByte(data, size, offsets); + } + } + + /** + * A builder of data buffer of {@link DataBuffer#TYPE_SHORT}. + */ + private static final class Shorts extends Sharing { + /** The shared arrays. */ + private final short[][] data; + + /** Creates a new builder. */ + Shorts(final int numBands) { + super(numBands); + data = new short[numBands][]; + } + + /** Takes a reference to an array in the given data buffer. */ + @Override void takeReference(DataBuffer buffer, int src, int dst) { + data[dst] = ((DataBufferShort) buffer).getData(src); + } + + /** Builds the data buffer after all references have been taken. */ + @Override DataBuffer build(int size) { + return new DataBufferShort(data, size, offsets); + } + } + + /** + * A builder of data buffer of {@link DataBuffer#TYPE_USHORT}. + */ + private static final class UShorts extends Sharing { + /** The shared arrays. */ + private final short[][] data; + + /** Creates a new builder. */ + UShorts(final int numBands) { + super(numBands); + data = new short[numBands][]; + } + + /** Takes a reference to an array in the given data buffer. */ + @Override void takeReference(DataBuffer buffer, int src, int dst) { + data[dst] = ((DataBufferUShort) buffer).getData(src); + } + + /** Builds the data buffer after all references have been taken. */ + @Override DataBuffer build(int size) { + return new DataBufferUShort(data, size, offsets); + } + } + + /** + * A builder of data buffer of {@link DataBuffer#TYPE_INT}. + */ + private static final class Integers extends Sharing { + /** The shared arrays. */ + private final int[][] data; + + /** Creates a new builder. */ + Integers(final int numBands) { + super(numBands); + data = new int[numBands][]; + } + + /** Takes a reference to an array in the given data buffer. */ + @Override void takeReference(DataBuffer buffer, int src, int dst) { + data[dst] = ((DataBufferInt) buffer).getData(src); + } + + /** Builds the data buffer after all references have been taken. */ + @Override DataBuffer build(int size) { + return new DataBufferInt(data, size, offsets); + } + } + + /** + * A builder of data buffer of {@link DataBuffer#TYPE_FLOAT}. + */ + private static final class Floats extends Sharing { + /** The shared arrays. */ + private final float[][] data; + + /** Creates a new builder. */ + Floats(final int numBands) { + super(numBands); + data = new float[numBands][]; + } + + /** Takes a reference to an array in the given data buffer. */ + @Override void takeReference(DataBuffer buffer, int src, int dst) { + data[dst] = ((DataBufferFloat) buffer).getData(src); + } + + /** Builds the data buffer after all references have been taken. */ + @Override DataBuffer build(int size) { + return new DataBufferFloat(data, size, offsets); + } + } + + /** + * A builder of data buffer of {@link DataBuffer#TYPE_DOUBLE}. + */ + private static final class Doubles extends Sharing { + /** The shared arrays. */ + private final double[][] data; + + /** Creates a new builder. */ + Doubles(final int numBands) { + super(numBands); + data = new double[numBands][]; + } + + /** Takes a reference to an array in the given data buffer. */ + @Override void takeReference(DataBuffer buffer, int src, int dst) { + data[dst] = ((DataBufferDouble) buffer).getData(src); + } + + /** Builds the data buffer after all references have been taken. */ + @Override DataBuffer build(int size) { + return new DataBufferDouble(data, size, offsets); + } + } + /** * Returns a hash code value for this image. */ diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/BandedSampleConverter.java b/core/sis-feature/src/main/java/org/apache/sis/image/BandedSampleConverter.java index 3ba4de609c..22a6a95c71 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/image/BandedSampleConverter.java +++ b/core/sis-feature/src/main/java/org/apache/sis/image/BandedSampleConverter.java @@ -220,7 +220,7 @@ class BandedSampleConverter extends ComputedImage { source = ((RecoloredImage) source).source; } final int numBands = converters.length; - final BandedSampleModel sampleModel = layout.createBandedSampleModel(targetType, numBands, source, null); + final BandedSampleModel sampleModel = layout.createBandedSampleModel(targetType, numBands, source, null, 0); final SampleDimension[] sampleDimensions = SampleDimensions.IMAGE_PROCESSOR_ARGUMENT.get(); final int visibleBand = ImageUtilities.getVisibleBand(source); ColorModel colorModel = ColorModelBuilder.NULL_COLOR_MODEL; diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/CombinedImageLayout.java b/core/sis-feature/src/main/java/org/apache/sis/image/CombinedImageLayout.java index 2ab7c0ef27..0edd294a15 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/image/CombinedImageLayout.java +++ b/core/sis-feature/src/main/java/org/apache/sis/image/CombinedImageLayout.java @@ -24,6 +24,8 @@ import java.awt.image.ColorModel; import java.awt.image.DataBuffer; import java.awt.image.RenderedImage; import java.awt.image.SampleModel; +import java.awt.image.BandedSampleModel; +import java.awt.image.ComponentSampleModel; import org.apache.sis.util.Workaround; import org.apache.sis.util.collection.FrequencySortedSet; import org.apache.sis.internal.feature.Resources; @@ -68,8 +70,11 @@ final class CombinedImageLayout extends ImageLayout { /** * The sample model of the combined image. + * All {@linkplain BandedSampleModel#getBandOffsets() band offsets} are zero and + * all {@linkplain BandedSampleModel#getBankIndices() bank indices} are identity mapping. + * This simplicity is needed by current implementation of {@link BandAggregateImage}. */ - final SampleModel sampleModel; + final BandedSampleModel sampleModel; /** * The domain of pixel coordinates in the combined image. All sources images are assumed to use @@ -96,6 +101,12 @@ final class CombinedImageLayout extends ImageLayout { */ private final boolean exactTileSize; + /** + * Whether all sources have tiles at the same locations and use the same scanline stride. + * In such case, it is possible to share references to data arrays without copying them. + */ + final boolean allowSharing; + /** * Computes the layout of an image combining all the specified source images. * The optional {@code bandsPerSource} argument specifies the bands to select in each source images. @@ -109,41 +120,62 @@ final class CombinedImageLayout extends ImageLayout { * * @param sources images to combine, in order. * @param bandsPerSource bands to use for each source image, in order. May contain {@code null} elements. + * @param allowSharing whether to allow the sharing of data buffers (instead of copying) if possible. * @throws IllegalArgumentException if there is an incompatibility between some source images * or if some band indices are duplicated or outside their range of validity. */ @Workaround(library="JDK", version="1.8") - static CombinedImageLayout create(RenderedImage[] sources, int[][] bandsPerSource) { + static CombinedImageLayout create(RenderedImage[] sources, int[][] bandsPerSource, boolean allowSharing) { final var aggregate = new MultiSourcesArgument<RenderedImage>(sources, bandsPerSource); aggregate.identityAsNull(); aggregate.validate(ImageUtilities::getNumBands); sources = aggregate.sources(); bandsPerSource = aggregate.bandsPerSource(); - Rectangle domain = null; + Rectangle domain = null; // Nullity check used for telling when the first image is processed. + int scanlineStride = 0; + int tileWidth = 0; + int tileHeight = 0; + int tileAlignX = 0; + int tileAlignY = 0; int commonDataType = DataBuffer.TYPE_UNDEFINED; for (final RenderedImage source : sources) { /* - * Ensure that all images use the same data type. - * Get the domain of the combined image to create. - * - * TODO: current implementation computes the intersection of all sources. - * But a future version should allow users to specify if they want intersection, - * union or strict mode instead. A "strict" mode would prevent the combination of - * images using different domains (i.e. raise an error if domains are not the same). + * Ensure that all images use the same data type. This is mandatory. + * If in addition all images use the same pixel and scanline stride, + * we may be able to share their buffers instead of copying values. */ - final int dataType = source.getSampleModel().getDataType(); + final SampleModel sm = source.getSampleModel(); + if (allowSharing && (allowSharing = (sm instanceof ComponentSampleModel))) { + final ComponentSampleModel csm = (ComponentSampleModel) sm; + if (allowSharing = (csm.getPixelStride() == 1)) { + allowSharing &= scanlineStride == (scanlineStride = csm.getScanlineStride()); + allowSharing &= tileWidth == (tileWidth = source.getTileWidth()); + allowSharing &= tileHeight == (tileHeight = source.getTileHeight()); + allowSharing &= tileAlignX == (tileAlignX = Math.floorMod(source.getTileGridXOffset(), tileWidth)); + allowSharing &= tileAlignY == (tileAlignY = Math.floorMod(source.getTileGridYOffset(), tileHeight)); + allowSharing |= (domain == null); + } + } + final int dataType = sm.getDataType(); if (domain == null) { domain = ImageUtilities.getBounds(source); commonDataType = dataType; } else { + if (dataType != commonDataType) { + throw new IllegalArgumentException(Resources.format(Resources.Keys.MismatchedDataType)); + } + /* + * Get the domain of the combined image to create. + * TODO: current implementation computes the intersection of all sources. + * But a future version should allow users to specify if they want intersection, + * union or strict mode instead. A "strict" mode would prevent the combination of + * images using different domains (i.e. raise an error if domains are not the same). + */ ImageUtilities.clipBounds(source, domain); if (domain.isEmpty()) { throw new DisjointExtentException(Resources.format(Resources.Keys.SourceImagesDoNotIntersect)); } - if (dataType != commonDataType) { - throw new IllegalArgumentException(Resources.format(Resources.Keys.MismatchedDataType)); - } } } if (domain == null) { @@ -153,7 +185,7 @@ final class CombinedImageLayout extends ImageLayout { /* * Tile size is chosen after the domain has been computed, because we prefer a tile size which * is a divisor of the combined image size. Tile sizes of existing source images are preferred, - * especially when the tiles are aligned, for increasing the chances that computation a tile of + * especially when the tiles are aligned, for increasing the chances that computing a tile of * 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. @@ -168,10 +200,11 @@ final class CombinedImageLayout extends ImageLayout { } final var preferredTileSize = new Dimension((int) cx, (int) cy); final boolean exactTileSize = ((cx | cy) >>> Integer.SIZE) == 0; + allowSharing &= exactTileSize; return new CombinedImageLayout(sources, bandsPerSource, domain, preferredTileSize, exactTileSize, chooseMinTile(tileGridXOffset, domain.x, preferredTileSize.width), chooseMinTile(tileGridYOffset, domain.y, preferredTileSize.height), - commonDataType, aggregate.numBands()); + commonDataType, aggregate.numBands(), allowSharing ? scanlineStride : 0); } /** @@ -182,11 +215,13 @@ final class CombinedImageLayout extends ImageLayout { * @param domain bounds of the image to create. * @param preferredTileSize the preferred tile size. * @param commonDataType data type of the combined image. + * @param scanlineStride common scanline stride if data buffers will be shared, or 0 if no sharing. * @param numBands number of bands of the image to create. */ private CombinedImageLayout(final RenderedImage[] sources, final int[][] bandsPerSource, final Rectangle domain, final Dimension preferredTileSize, final boolean exactTileSize, - final int minTileX, final int minTileY, final int commonDataType, final int numBands) + final int minTileX, final int minTileY, final int commonDataType, final int numBands, + final int scanlineStride) { super(preferredTileSize, false); this.exactTileSize = exactTileSize; @@ -195,7 +230,8 @@ final class CombinedImageLayout extends ImageLayout { this.domain = domain; this.minTileX = minTileX; this.minTileY = minTileY; - this.sampleModel = createBandedSampleModel(commonDataType, numBands, null, domain); + this.allowSharing = (scanlineStride > 0); + this.sampleModel = createBandedSampleModel(commonDataType, numBands, null, domain, scanlineStride); // Sample model must be last (all other fields must be initialized before). } @@ -218,7 +254,7 @@ final class CombinedImageLayout extends ImageLayout { */ private static long chooseTileSize(final long current, final int tileSize, final int imageSize, final int offset) { if ((imageSize % tileSize) == 0) { - long c = Math.abs(offset % tileSize); // How close the grid are aligned (ideal would be zero). + long c = Math.floorMod(offset, tileSize); // How close the grid are aligned (ideal would be zero). c <<= Integer.SIZE; // Pack grid offset in higher bits. c |= tileSize; // Pack tile size in lower bits. /* 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 859cb95883..a9033a25de 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 @@ -260,7 +260,7 @@ public abstract class ComputedImage extends PlanarImage implements Disposable { } /** - * Ensures that a user-supplied color model is compatible. + * Ensures that a user-supplied color model is compatible with the sample model. * This is a helper method for argument validation in sub-classes constructors. * * @param colors the color model to validate. Can be {@code null}. @@ -586,10 +586,21 @@ public abstract class ComputedImage extends PlanarImage implements Disposable { * @return initially empty tile for the given indices (cannot be null). */ protected WritableRaster createTile(final int tileX, final int tileY) { + return WritableRaster.createWritableRaster(getSampleModel(), computeTileLocation(tileX, tileY)); + } + + /** + * Returns the location of the tile to create for the given tile indices. + * + * @param tileX the column index of the tile to create. + * @param tileY the row index of the tile to create. + * @return location of the tile to create. + */ + final Point computeTileLocation(final int tileX, final int tileY) { // A temporary `int` overflow may occur before the final addition. final int x = Math.toIntExact((((long) tileX) - getMinTileX()) * getTileWidth() + getMinX()); final int y = Math.toIntExact((((long) tileY) - getMinTileY()) * getTileHeight() + getMinY()); - return WritableRaster.createWritableRaster(getSampleModel(), new Point(x,y)); + return new Point(x,y); } /** 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 1e71d8569d..656b267a52 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 @@ -987,7 +987,7 @@ public class ImageProcessor implements Cloneable { synchronized (this) { colorizer = this.colorizer; } - return BandAggregateImage.create(sources, bandsPerSource, colorizer); + return unique(BandAggregateImage.create(sources, bandsPerSource, colorizer, true)); } /** diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java b/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java index de86487842..0141edf318 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java +++ b/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java @@ -541,8 +541,8 @@ public abstract class PlanarImage implements RenderedImage { */ static String verifyCompatibility(final SampleModel sm, final ColorModel cm) { if (cm == null || cm.isCompatibleSampleModel(sm)) return null; - if (cm.getNumComponents() != sm.getNumBands()) return "numComponents"; if (cm.getTransferType() != sm.getTransferType()) return "transferType"; + if (cm.getNumComponents() != sm.getNumBands()) return "numComponents"; return ""; } diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java b/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java index a741724ca9..ee080d49dd 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java +++ b/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java @@ -282,7 +282,7 @@ final class Visualization extends ResampledImage { * The sample model is a mandatory argument before we invoke user-supplied colorizer, * which must be done before to build the color model. */ - sampleModel = layout.createBandedSampleModel(ColorModelBuilder.TYPE_COMPACT, NUM_BANDS, source, bounds); + sampleModel = layout.createBandedSampleModel(ColorModelBuilder.TYPE_COMPACT, NUM_BANDS, source, bounds, 0); final Target target = new Target(sampleModel, VISIBLE_BAND, visibleSD != null); if (colorizer != null) { colorModel = colorizer.apply(target).orElse(null); diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ImageLayout.java b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ImageLayout.java index a985bc2251..3783305f0a 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ImageLayout.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ImageLayout.java @@ -27,6 +27,7 @@ import java.awt.image.SampleModel; import java.awt.image.BandedSampleModel; import org.apache.sis.math.MathFunctions; import org.apache.sis.image.ComputedImage; +import org.apache.sis.util.ArraysExt; import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.resources.Errors; import org.apache.sis.internal.util.Strings; @@ -324,17 +325,26 @@ public class ImageLayout { * This method uses the {@linkplain #suggestTileSize(RenderedImage, Rectangle, boolean) * suggested tile size} for the given image and bounds. * + * <p>This method constructs the simplest possible banded sample model: + * All {@linkplain BandedSampleModel#getBandOffsets() band offsets} are zero and + * all {@linkplain BandedSampleModel#getBankIndices() bank indices} are identity mapping.</p> + * * @param dataType desired data type as a {@link java.awt.image.DataBuffer} constant. * @param numBands desired number of bands. * @param image the image which will be the source of the image for which a sample model is created. * @param bounds the bounds of the image to create, or {@code null} if same as {@code image}. + * @param scanlineStride the line stride of the of the image data, or ≤ 0 for automatic. * @return a banded sample model of the given type with the given number of bands. */ public BandedSampleModel createBandedSampleModel(final int dataType, final int numBands, - final RenderedImage image, final Rectangle bounds) + final RenderedImage image, final Rectangle bounds, int scanlineStride) { final Dimension tileSize = suggestTileSize(image, bounds, isBoundsAdjustmentAllowed); - return RasterFactory.unique(new BandedSampleModel(dataType, tileSize.width, tileSize.height, numBands)); + if (scanlineStride <= 0) { + scanlineStride = tileSize.width; + } + return RasterFactory.unique(new BandedSampleModel(dataType, tileSize.width, tileSize.height, + scanlineStride, ArraysExt.range(0, numBands), new int[numBands])); } /** diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/RasterFactory.java b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/RasterFactory.java index 083501ea6e..c8a31f3b97 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/RasterFactory.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/RasterFactory.java @@ -53,7 +53,7 @@ import org.apache.sis.util.collection.WeakHashSet; * creating {@link BufferedImage} since that kind of images wraps a single raster. * * @author Martin Desruisseaux (IRD, Geomatys) - * @version 1.1 + * @version 1.4 * @since 1.0 */ public final class RasterFactory extends Static { @@ -223,6 +223,34 @@ public final class RasterFactory extends Static { } } + /** + * Creates a NIO buffer wrapping an existing Java2D buffer. + * The buffer will be a view over the valid portion of the data array. + * The buffer position is zero. The capacity and limit are the number of valid elements. + * + * @param data the Java2D buffer to wrap. + * @param bank bank index of the data array to wrap. + * @return buffer wrapping the data array of the specified bank. + */ + public static Buffer createBuffer(final DataBuffer data, final int bank) { + Buffer buffer; + switch (data.getDataType()) { + case DataBuffer.TYPE_BYTE: buffer = ByteBuffer .wrap(((DataBufferByte) data).getData(bank)); break; + case DataBuffer.TYPE_USHORT: buffer = ShortBuffer .wrap(((DataBufferUShort) data).getData(bank)); break; + case DataBuffer.TYPE_SHORT: buffer = ShortBuffer .wrap(((DataBufferShort) data).getData(bank)); break; + case DataBuffer.TYPE_INT: buffer = IntBuffer .wrap(((DataBufferInt) data).getData(bank)); break; + case DataBuffer.TYPE_FLOAT: buffer = FloatBuffer .wrap(((DataBufferFloat) data).getData(bank)); break; + case DataBuffer.TYPE_DOUBLE: buffer = DoubleBuffer.wrap(((DataBufferDouble) data).getData(bank)); break; + default: throw new AssertionError(); + } + final int lower = data.getOffsets()[bank]; + final int upper = lower + data.getSize(); + if (lower != 0 || upper != buffer.capacity()) { + buffer.position(lower).limit(upper).slice(); // TODO: use slice(lower, length) with JDK13. + } + return buffer; + } + /** * Wraps the backing arrays of given NIO buffers into Java2D buffers. * This method wraps the underlying array of primitive types; data are not copied. diff --git a/core/sis-feature/src/test/java/org/apache/sis/image/BandAggregateImageTest.java b/core/sis-feature/src/test/java/org/apache/sis/image/BandAggregateImageTest.java index 2de3cd9135..0cb29939fa 100644 --- a/core/sis-feature/src/test/java/org/apache/sis/image/BandAggregateImageTest.java +++ b/core/sis-feature/src/test/java/org/apache/sis/image/BandAggregateImageTest.java @@ -16,12 +16,16 @@ */ package org.apache.sis.image; +import java.util.HashSet; +import java.util.stream.IntStream; +import java.util.function.Consumer; import java.awt.Rectangle; +import java.awt.image.BandedSampleModel; import java.awt.image.BufferedImage; import java.awt.image.DataBuffer; import java.awt.image.Raster; import java.awt.image.RenderedImage; -import java.util.stream.IntStream; +import org.apache.sis.internal.coverage.j2d.RasterFactory; import org.apache.sis.util.ArraysExt; import org.apache.sis.test.TestCase; import org.apache.sis.test.DependsOnMethod; @@ -40,22 +44,34 @@ import static org.junit.Assert.*; */ public final class BandAggregateImageTest extends TestCase { /** - * The image processor to use for testing band aggregations. + * Whether to allow the sharing of data arrays. + * If {@code false}, tests will force copies. */ - private final ImageProcessor processor; + private boolean allowSharing; /** * Creates a new test case. */ public BandAggregateImageTest() { - processor = new ImageProcessor(); + allowSharing = true; // This is the default mode of `ImageProcessor`. } /** - * Tests the aggregation of two untiled images having the same bounds and only one band. + * Tests the aggregation of two untiled images with forced copy of sample values. * This is the simplest case in this test class. */ @Test + public void copyUntiledImages() { + allowSharing = false; + aggregateUntiledImages(); + } + + /** + * Tests the aggregation of two untiled images having the same bounds and only one band. + * Sample values should not be copied unless forced to. + */ + @Test + @DependsOnMethod("copyUntiledImages") public void aggregateUntiledImages() { final int width = 3; final int height = 4; @@ -64,7 +80,7 @@ public final class BandAggregateImageTest extends TestCase { im1.getRaster().setSamples(0, 0, width, height, 0, IntStream.range(0, width*height).map(s -> s + 1).toArray()); im2.getRaster().setSamples(0, 0, width, height, 0, IntStream.range(0, width*height).map(s -> s * 2).toArray()); - final RenderedImage result = processor.aggregateBands(im1, im2); + final RenderedImage result = BandAggregateImage.create(new RenderedImage[] {im1, im2}, null, null, allowSharing); assertNotNull(result); assertEquals(0, result.getMinTileX()); assertEquals(0, result.getMinTileY()); @@ -83,19 +99,23 @@ public final class BandAggregateImageTest extends TestCase { }, tile.getPixels(0, 0, width, height, (int[]) null) ); + verifySharing(result, allowSharing); } /** * Tests the aggregation of two tiled images having the same tile matrix. * The same test is executed many times with different but equivalent classes of sample models. + * Bands may be copied or references, depending on the sample models. */ @Test @DependsOnMethod("aggregateUntiledImages") public void aggregateSimilarlyTiledImages() { - aggregateSimilarlyTiledImages(true, true); - aggregateSimilarlyTiledImages(false, false); - aggregateSimilarlyTiledImages(true, false); - aggregateSimilarlyTiledImages(false, true); + do { + aggregateSimilarlyTiledImages(true, true); + aggregateSimilarlyTiledImages(false, false); + aggregateSimilarlyTiledImages(true, false); + aggregateSimilarlyTiledImages(false, true); + } while ((allowSharing = !allowSharing) == false); // Loop executed exactly twice. } /** @@ -114,7 +134,7 @@ public final class BandAggregateImageTest extends TestCase { final TiledImageMock im2 = new TiledImageMock(DataBuffer.TYPE_USHORT, 2, minX, minY, width, height, 3, 3, 3, 4, secondBanded); initializeAllTiles(im1, im2); - RenderedImage result = processor.aggregateBands(im1, im2); + RenderedImage result = BandAggregateImage.create(new RenderedImage[] {im1, im2}, null, null, allowSharing); assertNotNull(result); assertEquals(minX, result.getMinX()); assertEquals(minY, result.getMinY()); @@ -148,15 +168,16 @@ public final class BandAggregateImageTest extends TestCase { }, raster.getPixels(minX, minY, width, height, (int[]) null) ); + verifySharing(result, allowSharing && allowSharing(im1, im2)); /* * Repeat the test with a custom band selection. * One of the source images is used twice, but with a different selection of bands. */ - result = processor.aggregateBands(new RenderedImage[] {im1, im2, im1}, new int[][] { + result = BandAggregateImage.create(new RenderedImage[] {im1, im2, im1}, new int[][] { new int[] {1}, // Take second band of image 1. null, // Take all bands of image 2. new int[] {0} // Take first band of image 1. - }); + }, null, allowSharing); assertNotNull(result); assertEquals(minX, result.getMinX()); assertEquals(minY, result.getMinY()); @@ -189,10 +210,15 @@ public final class BandAggregateImageTest extends TestCase { }, raster.getPixels(minX, minY, width, height, (int[]) null) ); + /* + * Do not invoke `verifySharing(result)` because this test + * references the same `DataBuffer` more than once. + */ } /** * Tests the aggregation of three tiled images having different tile matrices. + * A copy of sample values can not be avoided in this case. */ @Test @DependsOnMethod("aggregateSimilarlyTiledImages") @@ -211,7 +237,9 @@ public final class BandAggregateImageTest extends TestCase { final TiledImageMock oneTile = new TiledImageMock(DataBuffer.TYPE_FLOAT, 1, minX, minY, width, height, 8, 4, 5, 6, true); initializeAllTiles(tiled2x2, tiled4x1, oneTile); - final RenderedImage result = processor.aggregateBands(tiled2x2, tiled4x1, oneTile); + final RenderedImage result = BandAggregateImage.create( + new RenderedImage[] {tiled2x2, tiled4x1, oneTile}, null, null, allowSharing); + assertNotNull(result); assertEquals(minX, result.getMinX()); assertEquals(minY, result.getMinY()); @@ -236,10 +264,12 @@ public final class BandAggregateImageTest extends TestCase { }, raster.getPixels(minX, minY, width, height, (int[]) null) ); + verifySharing(result, false); } /** * Tests the aggregation of three tiled images having different extents and different tile matrices. + * A copy of sample values can not be avoided in this case. */ @Test @DependsOnMethod("aggregateImagesUsingSameExtentButDifferentTileSizes") @@ -258,7 +288,9 @@ public final class BandAggregateImageTest extends TestCase { final TiledImageMock tiled6x6 = new TiledImageMock(DataBuffer.TYPE_SHORT, 1, 2, 0, 12, 6, 6, 6, 0, 0, true); initializeAllTiles(untiled, tiled2x2, tiled4x4, tiled6x6); - final RenderedImage result = processor.aggregateBands(untiled, tiled2x2, tiled4x4, tiled6x6); + final RenderedImage result = BandAggregateImage.create( + new RenderedImage[] {untiled, tiled2x2, tiled4x4, tiled6x6}, null, null, allowSharing); + assertNotNull(result); assertEquals(4, result.getMinX()); assertEquals(2, result.getMinY()); @@ -284,6 +316,7 @@ public final class BandAggregateImageTest extends TestCase { }, raster.getPixels(4, 2, 8, 4, (int[]) null) ); + verifySharing(result, false); } /** @@ -305,4 +338,55 @@ public final class BandAggregateImageTest extends TestCase { band += numBands; } } + + /** + * Returns {@code true} if the sample model used by the given sources makes possible to share + * the internal data arrays. This method should be invoked for {@link TiledImageMock} having + * more than 1 band, because their sample model is selected randomly. + */ + private static boolean allowSharing(final RenderedImage... sources) { + for (final RenderedImage source : sources) { + if (!(source.getSampleModel() instanceof BandedSampleModel)) { + return false; + } + } + return true; + } + + /** + * Verifies if the given image reuses the data arrays of all its source. + * + * @param result the result of band aggregation. + * @param sharing whether the caller expects the result to share data arrays. + */ + private static void verifySharing(final RenderedImage result, final boolean sharing) { + final var arrays = new HashSet<Object>(); + for (final RenderedImage source : result.getSources()) { + forAllDataArrays(source, (data) -> assertTrue("Found two references to the same array.", arrays.add(data))); + } + final String message = sharing + ? "Expected the target image to reference an existing array." + : "Expected only copies, no references to existing arrays."; + forAllDataArrays(result, (data) -> assertEquals(message, sharing, arrays.remove(data))); + assertEquals("Expected sharing of either all arrays or none of them.", sharing, arrays.isEmpty()); + } + + /** + * Performs the given action on data arrays for all bands of all tiles of the given image. + * + * @param source the image for which to get data arrays. + * @param action the action to execute for each data arrays. + */ + private static void forAllDataArrays(final RenderedImage source, final Consumer<Object> action) { + for (int x = source.getNumXTiles(); --x >= 0;) { + final int tileX = source.getMinTileX() + x; + for (int y = source.getNumYTiles(); --y >= 0;) { + final int tileY = source.getMinTileY() + y; + final DataBuffer buffer = source.getTile(tileX, tileY).getDataBuffer(); + for (int b = buffer.getNumBanks(); --b >= 0;) { + action.accept(RasterFactory.createBuffer(buffer, b).array()); + } + } + } + } } diff --git a/core/sis-feature/src/test/java/org/apache/sis/image/ImageProcessorTest.java b/core/sis-feature/src/test/java/org/apache/sis/image/ImageProcessorTest.java index cf5c1a23bb..92b865cf01 100644 --- a/core/sis-feature/src/test/java/org/apache/sis/image/ImageProcessorTest.java +++ b/core/sis-feature/src/test/java/org/apache/sis/image/ImageProcessorTest.java @@ -17,7 +17,10 @@ package org.apache.sis.image; import java.util.Map; +import java.util.stream.IntStream; import java.awt.Shape; +import java.awt.Rectangle; +import java.awt.image.Raster; import java.awt.image.BufferedImage; import java.awt.image.RenderedImage; import org.apache.sis.internal.processing.isoline.IsolinesTest; @@ -51,6 +54,34 @@ public final class ImageProcessorTest extends TestCase { processor = new ImageProcessor(); } + /** + * Tests {@link ImageProcessor#aggregateBands(RenderedImage...)}. + * + * @see BandAggregateImageTest + */ + @Test + public void testBandAggregate() { + final int width = 3; + final int height = 4; + final BufferedImage im1 = new BufferedImage(width, height, BufferedImage.TYPE_BYTE_GRAY); + final BufferedImage im2 = new BufferedImage(width, height, BufferedImage.TYPE_BYTE_GRAY); + im1.getRaster().setSamples(0, 0, width, height, 0, IntStream.range(0, width*height).map(s -> s + 10).toArray()); + im2.getRaster().setSamples(0, 0, width, height, 0, IntStream.range(0, width*height).map(s -> s + 100).toArray()); + + final Raster data = processor.aggregateBands(im1, im2).getData(); + assertEquals(new Rectangle(0, 0, width, height), data.getBounds()); + assertEquals(2, data.getNumBands()); + assertArrayEquals( + new int[] { + 10, 100, 11, 101, 12, 102, + 13, 103, 14, 104, 15, 105, + 16, 106, 17, 107, 18, 108, + 19, 109, 20, 110, 21, 111 + }, + data.getPixels(0, 0, width, height, (int[]) null) + ); + } + /** * Tests {@link ImageProcessor#addUserProperties(RenderedImage, Map)}. */