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 3906ad83eb828893f5c0ad93335a517078195cf2 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Sun Apr 16 17:11:33 2023 +0200 The check for `MemoryGridResource` in `BandAggregateGridResource.create(…)` should not be an "all or nothing" operation. With this commit, shortcut is used even if only some of the resources to aggregate are `MemoryGridResource` instances. --- .../sis/coverage/grid/GridCoverageProcessor.java | 1 + .../sis/internal/coverage/MultiSourceArgument.java | 32 +++++- .../sis/internal/storage/MemoryGridResource.java | 45 ++++----- .../aggregate/BandAggregateGridResource.java | 112 +++++++++++++-------- .../aggregate/BandAggregateGridResourceTest.java | 42 ++++++-- .../sis/storage/aggregate/OpaqueGridResource.java | 46 +++++++++ 6 files changed, 203 insertions(+), 75 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageProcessor.java b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageProcessor.java index b952560d91..c38ec7d876 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageProcessor.java +++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageProcessor.java @@ -790,6 +790,7 @@ public class GridCoverageProcessor implements Cloneable { final var aggregate = new MultiSourceArgument<>(sources, bandsPerSource); aggregate.unwrap(BandAggregateGridCoverage::unwrap); aggregate.validate(GridCoverage::getSampleDimensions); + aggregate.mergeConsecutiveSources(); if (aggregate.isIdentity()) { return aggregate.sources()[0]; } 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 8044bd7d2c..6cab63d7ed 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 @@ -407,12 +407,37 @@ next: for (int i=0; i<sources.length; i++) { // `sources.length` may } /** - * If the same sources are repeated many times, merges them in a single reference. + * For each source which is repeated in consecutive positions, merges the repetition in a single reference. + * This method does the same work than {@link #mergeDuplicatedSources()}, except that it is restricted to + * repetitions in consecutive positions. Because of this restriction, the band order is never modified by + * this method call. + */ + public void mergeConsecutiveSources() { + checkValidationState(true); + for (int i=1; i < validatedSourceCount;) { + if (sources[i] == sources[i-1]) { + bandsPerSource[i-1] = ArraysExt.concatenate(bandsPerSource[i-1], bandsPerSource[i]); + final int remaining = --validatedSourceCount - i; + System.arraycopy(sources, i+1, sources, i, remaining); + System.arraycopy(bandsPerSource, i+1, bandsPerSource, i, remaining); + System.arraycopy(numBandsPerSource, i+1, numBandsPerSource, i, remaining); + } else { + i++; + } + } + } + + /** + * If the same sources are repeated many times, merges each repetition in a single reference. * The {@link #sources()} and {@link #bandsPerSource(boolean)} values are modified in-place. * The bands associated to each source reference are merged together, but not necessarily in the same order. * Caller must perform a "band select" operation using the array returned by this method * in order to reconstitute the band order specified by the user. * + * <p>This method does the same work than {@link #mergeConsecutiveSources()} except that this method can merge + * sources that are not necessarily at consecutive positions. The sources can be repeated at random positions. + * But the cost of this flexibility is the possible modification of band order.</p> + * * <h4>Use cases</h4> * {@code BandAggregateImage.subset(…)} and * {@code BandAggregateGridResource.read(…)} @@ -468,9 +493,8 @@ next: for (int i=0; i<sources.length; i++) { // `sources.length` may */ for (int j=0; j < tuples.length; j++) { final long t = tuples[j]; - final int sourceBand = (int) (t >>> Integer.SIZE); - reordered[(int) t] = sourceBand + targetBand; - selected[j] = sourceBand; + reordered[(int) t] = targetBand + j; + selected[j] = (int) (t >>> Integer.SIZE); } targetBand += tuples.length; numBandsPerSource[validatedSourceCount] = numBandsPerSource[i]; diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MemoryGridResource.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MemoryGridResource.java index 32a1eda795..f96563a553 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MemoryGridResource.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MemoryGridResource.java @@ -23,12 +23,10 @@ import org.opengis.referencing.datum.PixelInCell; import org.apache.sis.coverage.SampleDimension; import org.apache.sis.coverage.grid.GridCoverage; import org.apache.sis.coverage.grid.GridCoverageBuilder; +import org.apache.sis.coverage.grid.GridCoverageProcessor; import org.apache.sis.coverage.grid.GridExtent; import org.apache.sis.coverage.grid.GridGeometry; import org.apache.sis.coverage.grid.GridRoundingMode; -import org.apache.sis.image.ImageProcessor; -import org.apache.sis.internal.coverage.RangeArgument; -import org.apache.sis.internal.util.UnmodifiableArrayList; import org.apache.sis.storage.AbstractGridCoverageResource; import org.apache.sis.storage.event.StoreListeners; import org.apache.sis.util.ArgumentChecks; @@ -41,10 +39,10 @@ import org.apache.sis.util.ArgumentChecks; * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 1.1 */ -public class MemoryGridResource extends AbstractGridCoverageResource { +public final class MemoryGridResource extends AbstractGridCoverageResource { /** * The grid coverage specified at construction time. */ @@ -94,8 +92,16 @@ public class MemoryGridResource extends AbstractGridCoverageResource { */ @Override public GridCoverage read(GridGeometry domain, final int... ranges) { - List<SampleDimension> bands = coverage.getSampleDimensions(); - final RangeArgument rangeIndices = RangeArgument.validate(bands.size(), ranges, listeners); + /* + * If the user requested a subset of the bands, select the bands on the grid coverage. + * It is slightly more expensive than selecting the bands on the image produced at the + * end of this method, except if the coverage is a `BandAggregateGridCoverage` because + * the latter will do rendering only on the selected coverage components. + */ + GridCoverage subset = coverage; + if (ranges != null && ranges.length != 0) { + subset = new GridCoverageProcessor().selectSampleDimensions(subset, ranges); + } /* * The given `domain` may use arbitrary `gridToCRS` and `CRS` properties. * For this simple implementation we need the same `gridToCRS` and `CRS` @@ -107,7 +113,7 @@ public class MemoryGridResource extends AbstractGridCoverageResource { * by adjusting the pixel stride in SampleModel. */ GridExtent intersection = null; - final GridGeometry source = coverage.getGridGeometry(); + final GridGeometry source = subset.getGridGeometry(); if (domain == null) { domain = source; } else { @@ -122,9 +128,8 @@ public class MemoryGridResource extends AbstractGridCoverageResource { /* * Quick check before to invoke the potentially costly `coverage.render(…)` method. */ - final boolean sameBands = rangeIndices.isIdentity(); - if (sameBands && intersection == null) { - return coverage; + if (intersection == null) { + return subset; } /* * After `render(…)` execution, the (minX, minY) image coordinates are the differences between @@ -132,7 +137,7 @@ public class MemoryGridResource extends AbstractGridCoverageResource { * we need to translate the `GridExtent` in order to make it matches what we got. But before to * apply that translation, we adjust the grid size (because it may add another translation). */ - RenderedImage data = coverage.render(intersection); + RenderedImage data = subset.render(intersection); if (intersection != null) { final int[] sd = intersection.getSubspaceDimensions(2); final int dimX = sd[0]; @@ -157,26 +162,20 @@ public class MemoryGridResource extends AbstractGridCoverageResource { intersection = intersection.translate(changes); /* * If the result is the same intersection than the source coverage, - * we may be able to return that coverage directly. + * we can return that coverage directly. */ if (intersection.equals(source.getExtent())) { - if (sameBands) { - return coverage; - } - domain = source; + return subset; } else { domain = new GridGeometry(intersection, PixelInCell.CELL_CORNER, source.getGridToCRS(PixelInCell.CELL_CORNER), source.getCoordinateReferenceSystem()); } } - if (!sameBands) { - data = new ImageProcessor().selectBands(data, ranges); - bands = UnmodifiableArrayList.wrap(rangeIndices.select(bands)); - } return new GridCoverageBuilder() .setValues(data) - .setRanges(bands) - .setDomain(domain).build(); + .setDomain(domain) + .setRanges(subset.getSampleDimensions()) + .build(); } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/BandAggregateGridResource.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/BandAggregateGridResource.java index 301f28a259..5487b2cea2 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/BandAggregateGridResource.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/aggregate/BandAggregateGridResource.java @@ -38,6 +38,8 @@ import org.apache.sis.storage.DataStoreException; import org.apache.sis.storage.event.StoreListeners; import org.apache.sis.internal.util.UnmodifiableArrayList; import org.apache.sis.util.collection.BackingStoreException; +import org.apache.sis.util.ArgumentChecks; +import org.apache.sis.util.ArraysExt; /** @@ -120,6 +122,29 @@ final class BandAggregateGridResource extends AbstractGridCoverageResource imple * The {@linkplain #getSampleDimensions() list of sample dimensions} of the aggregated resource * will be the concatenation of the lists of all sources, or a subset of this concatenation. * + * @param parentListeners listeners of the parent resource, or {@code null} if none. + * @param aggregate sources to aggregate together with the bands to select for each source. + * @param processor the processor to use for creating grid coverages. + * @throws BackingStoreException if an error occurred while fetching the grid geometry or sample dimensions from a resource. + * @throws IllegalGridGeometryException if a grid geometry is not compatible with the others. + * @throws IllegalArgumentException if some band indices are duplicated or outside their range of validity. + */ + private BandAggregateGridResource(final StoreListeners parentListeners, + final MultiSourceArgument<GridCoverageResource> aggregate, + final GridCoverageProcessor processor) + { + super(parentListeners, false); + this.sources = aggregate.sources(); + this.gridGeometry = aggregate.domain(BandAggregateGridResource::domain); + this.sampleDimensions = List.copyOf(aggregate.ranges()); + this.bandsPerSource = aggregate.bandsPerSource(false); + this.processor = processor; + } + + /** + * Creates a new range aggregation of grid coverage resources, + * potentially unwrapping in-memory resources for efficiency. + * * <p>The {@code bandsPerSource} argument specifies the bands to select in each resource. * That array can be {@code null} for selecting all bands in all resources, * or may contain {@code null} elements for selecting all bands of the corresponding resource. @@ -141,24 +166,63 @@ final class BandAggregateGridResource extends AbstractGridCoverageResource imple * @param sources resources whose bands shall be aggregated, in order. At least one resource must be provided. * @param bandsPerSource sample dimensions for each source. May be {@code null} or may contain {@code null} elements. * @param processor the processor to use for creating grid coverages. + * @return the band aggregated grid resource. * @throws DataStoreException if an error occurred while fetching the grid geometry or sample dimensions from a resource. * @throws IllegalGridGeometryException if a grid geometry is not compatible with the others. * @throws IllegalArgumentException if some band indices are duplicated or outside their range of validity. */ - BandAggregateGridResource(final StoreListeners parentListeners, - final GridCoverageResource[] sources, final int[][] bandsPerSource, + static GridCoverageResource create(final StoreListeners parentListeners, + GridCoverageResource[] sources, int[][] bandsPerSource, final GridCoverageProcessor processor) throws DataStoreException { - super(parentListeners, false); + var coverages = new GridCoverage[sources.length]; + var coverageBands = new int[sources.length][]; + int firstBand = 0, count = 0; + for (int i=0; i<sources.length; i++) { + final GridCoverageResource source = sources[i]; + ArgumentChecks.ensureNonNullElement("sources", i, source); + if (source instanceof MemoryGridResource) { + if (count == 0) { + sources = sources.clone(); // Clone when first needed. + bandsPerSource = (bandsPerSource != null) ? bandsPerSource.clone() : new int[sources.length][]; + } + final int[] bands = bandsPerSource[i]; + final int numBands = (bands != null) ? bands.length : source.getSampleDimensions().size(); + coverages[count] = ((MemoryGridResource) source).coverage; + coverageBands[count] = bands; + bandsPerSource[i] = ArraysExt.range(firstBand, firstBand + numBands); + sources[i] = null; // To be replaced by the aggregated coverage. + firstBand += numBands; + count++; + } + } + /* + * If at least one `MemoryGridResource` has been found, apply the aggregation directly + * on the grid coverage, then build a single `MemoryGridResource` with the result. + */ + if (count != 0) { + coverages = ArraysExt.resize(coverages, count); + coverageBands = ArraysExt.resize(coverageBands, count); + var aggregate = new MemoryGridResource(parentListeners, processor.aggregateRanges(coverages, coverageBands)); + for (int i=0; i<sources.length; i++) { + if (sources[i] == null) { + sources[i] = aggregate; + } + } + } + /* + * If the same source appears two or more times consecutively, `MultiSourceArgument` will merge them + * in a single reference to that source. Consequently the arrays of sources may become shorter. + */ try { final var aggregate = new MultiSourceArgument<GridCoverageResource>(sources, bandsPerSource); aggregate.unwrap(BandAggregateGridResource::unwrap); aggregate.validate(BandAggregateGridResource::range); - this.sources = aggregate.sources(); - this.gridGeometry = aggregate.domain(BandAggregateGridResource::domain); - this.sampleDimensions = List.copyOf(aggregate.ranges()); - this.bandsPerSource = aggregate.bandsPerSource(false); - this.processor = processor; + aggregate.mergeConsecutiveSources(); + if (aggregate.isIdentity()) { + return aggregate.sources()[0]; + } + return new BandAggregateGridResource(parentListeners, aggregate, processor); } catch (BackingStoreException e) { throw e.unwrapOrRethrow(DataStoreException.class); } @@ -202,38 +266,6 @@ final class BandAggregateGridResource extends AbstractGridCoverageResource imple } } - /** - * Creates a new range aggregation of grid coverage resources, - * potentially unwrapping in-memory resources for efficiency. - * - * @param parentListeners listeners of the parent resource, or {@code null} if none. - * @param sources resources whose bands shall be aggregated, in order. At least one resource must be provided. - * @param bandsPerSource sample dimensions for each source. May be {@code null} or may contain {@code null} elements. - * @param processor the processor to use for creating grid coverages. - * @return the band aggregated grid resource. - * @throws DataStoreException if an error occurred while fetching the grid geometry or sample dimensions from a resource. - * @throws IllegalGridGeometryException if a grid geometry is not compatible with the others. - * @throws IllegalArgumentException if some band indices are duplicated or outside their range of validity. - */ - static GridCoverageResource create(final StoreListeners parentListeners, - final GridCoverageResource[] sources, final int[][] bandsPerSource, - final GridCoverageProcessor processor) throws DataStoreException - { - GridCoverageResource source = sources[0]; - if (source instanceof MemoryGridResource) { - final var coverages = new GridCoverage[sources.length]; - int i = 0; - do { - coverages[i] = ((MemoryGridResource) source).coverage; - if (++i >= sources.length) { - GridCoverage coverage = processor.aggregateRanges(coverages, bandsPerSource); - return new MemoryGridResource(parentListeners, coverage); - } - } while ((source = sources[i]) instanceof MemoryGridResource); - } - return new BandAggregateGridResource(parentListeners, sources, bandsPerSource, processor); - } - /** Not applicable to this implementation. */ @Override public Resource apply(MergeStrategy strategy) {return this;} diff --git a/storage/sis-storage/src/test/java/org/apache/sis/storage/aggregate/BandAggregateGridResourceTest.java b/storage/sis-storage/src/test/java/org/apache/sis/storage/aggregate/BandAggregateGridResourceTest.java index 9af75669e8..95ef5dc7bb 100644 --- a/storage/sis-storage/src/test/java/org/apache/sis/storage/aggregate/BandAggregateGridResourceTest.java +++ b/storage/sis-storage/src/test/java/org/apache/sis/storage/aggregate/BandAggregateGridResourceTest.java @@ -65,10 +65,16 @@ public final class BandAggregateGridResourceTest extends TestCase { */ private final GridCoverageProcessor processor; + /** + * Whether to hide {@link MemoryGridResource} in order to disable optimizations. + */ + private boolean opaque; + /** * Creates a new test case. */ public BandAggregateGridResourceTest() { + opaque = true; processor = new GridCoverageProcessor(); domain = new GridGeometry(new GridExtent(WIDTH, HEIGHT), PixelInCell.CELL_CENTER, MathTransforms.identity(2), HardCodedCRS.WGS84); @@ -82,8 +88,8 @@ public final class BandAggregateGridResourceTest extends TestCase { * @throws DataStoreException if an error occurred while fetching the grid geometry or sample dimensions from a resource. * @throws IllegalGridGeometryException if a grid geometry is not compatible with the others. */ - private BandAggregateGridResource create(final GridCoverageResource... sources) throws DataStoreException { - return new BandAggregateGridResource(null, sources, null, processor); + private GridCoverageResource create(final GridCoverageResource... sources) throws DataStoreException { + return BandAggregateGridResource.create(null, sources, null, processor); } /** @@ -125,12 +131,13 @@ public final class BandAggregateGridResourceTest extends TestCase { * In addition, band order in one of the 3 coverages is modified. */ final LocalName testName = Names.createLocalName(null, null, "test-name"); - aggregation = new BandAggregateGridResource(null, + aggregation = BandAggregateGridResource.create(null, new GridCoverageResource[] {firstAndSecondBands, thirdAndFourthBands, fifthAndSixthBands}, new int[][] {null, new int[] {1, 0}, new int[] {1}}, processor); - - aggregation.setIdentifier(testName); - assertEquals(testName, aggregation.getIdentifier().orElse(null)); + if (opaque) { + ((BandAggregateGridResource) aggregation).setIdentifier(testName); + assertEquals(testName, aggregation.getIdentifier().orElse(null)); + } assertAllPixelsEqual(aggregation.read(null), 101, 102, 104, 103, 106); assertAllPixelsEqual(aggregation.read(null, 2, 4), 104, 106); } @@ -150,12 +157,30 @@ public final class BandAggregateGridResourceTest extends TestCase { * If the result is not an instance of `MemoryGridResource`, * this is a bug in `BandAggregateGridResource.create(…)`. */ - final var aggregation = (MemoryGridResource) aggregator.build(null); + final LocalName testName = Names.createLocalName(null, null, "test-name"); + final var aggregation = (GridCoverageResource) aggregator.build(testName); + if (opaque) { + assertEquals(testName, aggregation.getIdentifier().orElse(null)); + } assertAllPixelsEqual(aggregation.read(null), 17, 23); assertAllPixelsEqual(aggregation.read(null, 0), 17); assertAllPixelsEqual(aggregation.read(null, 1), 23); } + /** + * Tests the shortcut for {@link MemoryGridResource} instances. + * {@link BandAggregateGridResource} should apply aggregations directly on the underlying grid coverages. + * + * @throws DataStoreException if an error occurred while reading a resource. + */ + @Test + public void testMemoryGridResourceShortcut() throws DataStoreException { + opaque = false; + aggregateBandsFromSingleBandSources(); + aggregateBandsFromMultiBandSources(); + usingCoverageAggregator(); + } + /** * Creates a new grid coverage resource with bands having the given values. * The length of the {@code bandValues} array is the number of bands to create. @@ -176,7 +201,8 @@ public final class BandAggregateGridResourceTest extends TestCase { System.arraycopy(bandValues, 0, data, i, numBands); } final var values = new DataBufferInt(data, data.length); - return new MemoryGridResource(null, new BufferedGridCoverage(domain, samples, values)); + final var r = new MemoryGridResource(null, new BufferedGridCoverage(domain, samples, values)); + return opaque ? new OpaqueGridResource(r) : r; } /** diff --git a/storage/sis-storage/src/test/java/org/apache/sis/storage/aggregate/OpaqueGridResource.java b/storage/sis-storage/src/test/java/org/apache/sis/storage/aggregate/OpaqueGridResource.java new file mode 100644 index 0000000000..1e1193813d --- /dev/null +++ b/storage/sis-storage/src/test/java/org/apache/sis/storage/aggregate/OpaqueGridResource.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sis.storage.aggregate; + +import org.apache.sis.internal.storage.GridResourceWrapper; +import org.apache.sis.storage.GridCoverageResource; + + +/** + * Wraps a grid coverage resource without changing anything. This is used for disabling optimizations, + * in order to test different code paths that what would be used with {@code MemoryGridResource}. + * + * @author Martin Desruisseaux (Geomatys) + * @version 1.4 + * @since 1.4 + */ +final class OpaqueGridResource extends GridResourceWrapper { + /** + * The resource to hide. + */ + private final GridCoverageResource source; + + /** + * Creates a new wrapper. + */ + OpaqueGridResource(final GridCoverageResource source) { + this.source = source; + } + + @Override protected Object getSynchronizationLock() {return source;} + @Override protected GridCoverageResource createSource() {return source;} +}