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/16 16:51:04 UTC
[sis] 01/02: 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.
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 <ma...@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;}
+}