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 7336b1cedac7632512316f0b18d16975e5a7a39d Author: Martin Desruisseaux <[email protected]> AuthorDate: Sun Mar 22 15:56:40 2020 +0100 Tune the checks for error conditions in GridCoverage and BufferedGridCoverage construction. --- .../org/apache/sis/coverage/grid/GridCoverage.java | 7 ++- .../apache/sis/coverage/grid/ImageRenderer.java | 8 +--- .../coverage/j2d/BufferedGridCoverage.java | 55 ++++++++++++++++------ .../org/apache/sis/internal/feature/Resources.java | 6 +++ .../sis/internal/feature/Resources.properties | 1 + .../sis/internal/feature/Resources_fr.properties | 1 + .../sis/coverage/grid/GridCoverageBuilderTest.java | 4 +- .../org/apache/sis/internal/map/CanvasContext.java | 2 +- .../gazetteer/MilitaryGridReferenceSystem.java | 3 +- .../java/org/apache/sis/internal/jdk9/JDK9.java | 11 +++++ .../main/java/org/apache/sis/math/Fraction.java | 14 +++--- 11 files changed, 79 insertions(+), 33 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java index d8b55d9..4c84fa1 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java +++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java @@ -98,12 +98,15 @@ public abstract class GridCoverage { * * @param domain the grid extent, CRS and conversion from cell indices to CRS. * @param range sample dimensions for each image band. + * @throws NullPointerException if an argument is {@code null} or if the list contains a null element. + * @throws IllegalArgumentException if the {@code range} list is empty. */ protected GridCoverage(final GridGeometry domain, final Collection<? extends SampleDimension> range) { - ArgumentChecks.ensureNonNull("domain", domain); - ArgumentChecks.ensureNonNull("range", range); + ArgumentChecks.ensureNonNull ("domain", domain); + ArgumentChecks.ensureNonEmpty("range", range); gridGeometry = domain; sampleDimensions = range.toArray(new SampleDimension[range.size()]); + ArgumentChecks.ensureNonEmpty("range", sampleDimensions); for (int i=0; i<sampleDimensions.length; i++) { ArgumentChecks.ensureNonNullElement("range", i, sampleDimensions[i]); } diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java index df4095d..52baa47 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java +++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java @@ -444,13 +444,9 @@ public class ImageRenderer { * Add the offset specified by the user (if any), or the default offset. The default is 0, 1, 2… * for interleaved sample model (all bands in one bank) and 0, 0, 0… for banded sample model. */ - if (bandOffsets != null) { + if (bandOffsets != null || isInterleaved) { for (int i=0; i<offsets.length; i++) { - offsets[i] = Math.addExact(offsets[i], bandOffsets[i]); - } - } else if (isInterleaved) { - for (int i=1; i<offsets.length; i++) { - offsets[i] = Math.addExact(offsets[i], i); + offsets[i] = Math.addExact(offsets[i], (bandOffsets != null) ? bandOffsets[i] : i); } } final Point location = new Point(imageX, imageY); diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BufferedGridCoverage.java b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BufferedGridCoverage.java index 0f1a68a..a6e643c 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BufferedGridCoverage.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BufferedGridCoverage.java @@ -16,6 +16,7 @@ */ package org.apache.sis.internal.coverage.j2d; +import java.util.Collection; import java.awt.image.DataBuffer; import java.awt.image.DataBufferByte; import java.awt.image.DataBufferDouble; @@ -25,15 +26,18 @@ import java.awt.image.DataBufferShort; import java.awt.image.DataBufferUShort; import java.awt.image.RasterFormatException; import java.awt.image.RenderedImage; -import java.util.Collection; import org.apache.sis.coverage.SampleDimension; import org.apache.sis.coverage.grid.GridCoverage; import org.apache.sis.coverage.grid.GridExtent; import org.apache.sis.coverage.grid.GridGeometry; import org.apache.sis.coverage.grid.IllegalGridGeometryException; import org.apache.sis.coverage.grid.ImageRenderer; +import org.apache.sis.internal.feature.Resources; import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.resources.Errors; + +// Branch-specific imports +import org.apache.sis.internal.jdk9.JDK9; import org.opengis.coverage.CannotEvaluateException; @@ -58,24 +62,45 @@ public class BufferedGridCoverage extends GridCoverage { * Constructs a grid coverage using the specified grid geometry, sample dimensions and data buffer. * This method stores the given buffer by reference (no copy). * - * @param grid the grid extent, CRS and conversion from cell indices to CRS. - * @param bands sample dimensions for each image band. - * @param data the sample values, potentially multi-banded. + * @param domain the grid extent, CRS and conversion from cell indices to CRS. + * @param range sample dimensions for each image band. + * @param data the sample values, potentially multi-banded. + * @throws NullPointerException if an argument is {@code null}. + * @throws IllegalArgumentException if the data buffer has an incompatible number of banks. + * @throws IllegalGridGeometryException if the grid extent is larger than the data buffer capacity. + * @throws ArithmeticException if the grid extent is larger than 64 bits integer capacity. */ - public BufferedGridCoverage(final GridGeometry grid, final Collection<? extends SampleDimension> bands, final DataBuffer data) { - super(grid, bands); + public BufferedGridCoverage(final GridGeometry domain, final Collection<? extends SampleDimension> range, final DataBuffer data) { + super(domain, range); this.data = data; ArgumentChecks.ensureNonNull("data", data); - - //verify buffer size - GridExtent extent = grid.getExtent(); - long expectedSize = extent.getSize(0) * bands.size(); - for (int i = 1; i <extent.getDimension(); i++) { - expectedSize *= extent.getSize(i); + /* + * The buffer shall either contain all values in a single bank (pixel interleaved sample model) + * or contain as many banks as bands (banded sample model). + */ + final int numBands = range.size(); + final int numBanks = data.getNumBanks(); + if (numBanks != 1 && numBands != numBands) { + throw new IllegalArgumentException(Resources.format(Resources.Keys.MismatchedBandCount_2, numBands, numBands)); + } + /* + * Verify that the buffer has enough elements for all cells in grid extent. + * Note that the buffer may have all elements in a single bank. + */ + long expectedSize = numBands; + final GridExtent extent = domain.getExtent(); + for (int i = extent.getDimension(); --i >= 0;) { + expectedSize = Math.multiplyExact(expectedSize, extent.getSize(i)); } - long buffersize = Math.multiplyExact(data.getSize(), data.getNumBanks()); - if (buffersize < expectedSize) { - throw new IllegalGridGeometryException("Expecting a buffer size of at least " + expectedSize + " to contain all samples from given grid geometry, but buffer is only " + buffersize); + final long bufferSize = JDK9.multiplyFull(data.getSize(), numBanks); + if (bufferSize < expectedSize) { + final StringBuilder b = new StringBuilder(); + for (int i=0; i < extent.getDimension(); i++) { + if (i != 0) b.append(" × "); + b.append(extent.getSize(i)); + } + throw new IllegalGridGeometryException(Resources.format( + Resources.Keys.InsufficientBufferCapacity_3, b, numBands, expectedSize - bufferSize)); } } diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java index a83ddec..315979f 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java @@ -222,6 +222,12 @@ public final class Resources extends IndexedResourceBundle { public static final short IncompatibleTile_2 = 35; /** + * Data buffer capacity is insufficient for a grid of {0} cells × {1} bands. Missing {2} + * elements. + */ + public static final short InsufficientBufferCapacity_3 = 71; + + /** * Invalid or unsupported “{1}” expression at index {0}. */ public static final short InvalidExpression_2 = 56; diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties index 4a69e74..ef7e9b3 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties @@ -51,6 +51,7 @@ ImageAllowsTransparency = Image allows transparency. ImageHasAlphaChannel = Image has alpha channel. ImageIsOpaque = Image is opaque. IncompatibleTile_2 = The ({0}, {1}) tile has an unexpected size, number of bands or sample layout. +InsufficientBufferCapacity_3 = Data buffer capacity is insufficient for a grid of {0} cells \u00d7 {1} bands. Missing {2} elements. InvalidExpression_2 = Invalid or unsupported \u201c{1}\u201d expression at index {0}. IterationIsFinished = Iteration is finished. IterationNotStarted = Iteration did not started. diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties index 2dfc07f..7eb8dc9 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties @@ -56,6 +56,7 @@ ImageAllowsTransparency = L\u2019image permet la transparence. ImageHasAlphaChannel = L\u2019image a un canal alpha. ImageIsOpaque = L\u2019image est opaque. IncompatibleTile_2 = La tuile ({0}, {1}) a une taille, un nombre de bandes ou une disposition des valeurs inattendu. +InsufficientBufferCapacity_3 = La capacit\u00e9 du buffer est insuffisante pour une grille de {0} cellules \u00d7 {1} bandes. Il lui manque {2} \u00e9l\u00e9ments. InvalidExpression_2 = Expression \u00ab\u202f{1}\u202f\u00bb invalide ou non-support\u00e9e \u00e0 l\u2019index {0}. IterationIsFinished = L\u2019it\u00e9ration est termin\u00e9e. IterationNotStarted = L\u2019it\u00e9ration n\u2019a pas commenc\u00e9e. diff --git a/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridCoverageBuilderTest.java b/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridCoverageBuilderTest.java index dae3155..3bed297 100644 --- a/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridCoverageBuilderTest.java +++ b/core/sis-feature/src/test/java/org/apache/sis/coverage/grid/GridCoverageBuilderTest.java @@ -43,7 +43,7 @@ import org.opengis.referencing.operation.MathTransform; public class GridCoverageBuilderTest extends TestCase { /** - * Tests {@link GridCoverageBuilder#setValues(Image)}. + * Tests {@link GridCoverageBuilder#setValues(RenderedImage)}. */ @Test public void createFromImageTest() { @@ -105,7 +105,7 @@ public class GridCoverageBuilderTest extends TestCase { } /** - * Tests {@link GridCoverageBuilder#setValues(Raster)}. + * Tests {@link GridCoverageBuilder#setValues(WritableRaster)}. */ @Test public void createFromRasterTest() { diff --git a/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/CanvasContext.java b/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/CanvasContext.java index 59ae628..6f76f0b 100644 --- a/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/CanvasContext.java +++ b/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/CanvasContext.java @@ -176,7 +176,7 @@ final class CanvasContext extends CoordinateOperationContext { /* * Estimate spatial resolution at the point of interest. The calculation is done in * (longitude, latitude, height) space where the height is optional. The angles are - * converted to meters using the authalic radius. + * converted to meters using the radius of conformal sphere. */ if (!(resolution > 0)) { final double[] poi = canvas.getObjectivePOI(); diff --git a/core/sis-referencing-by-identifiers/src/main/java/org/apache/sis/referencing/gazetteer/MilitaryGridReferenceSystem.java b/core/sis-referencing-by-identifiers/src/main/java/org/apache/sis/referencing/gazetteer/MilitaryGridReferenceSystem.java index b8cde89..1f9acd3 100644 --- a/core/sis-referencing-by-identifiers/src/main/java/org/apache/sis/referencing/gazetteer/MilitaryGridReferenceSystem.java +++ b/core/sis-referencing-by-identifiers/src/main/java/org/apache/sis/referencing/gazetteer/MilitaryGridReferenceSystem.java @@ -72,6 +72,7 @@ import org.apache.sis.measure.Longitude; import org.apache.sis.measure.Latitude; // Branch-dependent imports +import org.apache.sis.internal.jdk9.JDK9; import org.opengis.metadata.citation.Party; import org.opengis.referencing.gazetteer.Location; import org.opengis.referencing.gazetteer.LocationType; @@ -1150,7 +1151,7 @@ public class MilitaryGridReferenceSystem extends ReferencingByIdentifiers { */ @Override public long estimateSize() { - return (xEnd - (long) gridX) * Math.abs(yEnd - (long) yStart) / (step * (long) step); + return (xEnd - (long) gridX) * Math.abs(yEnd - (long) yStart) / JDK9.multiplyFull(step, step); } /** diff --git a/core/sis-utility/src/main/java/org/apache/sis/internal/jdk9/JDK9.java b/core/sis-utility/src/main/java/org/apache/sis/internal/jdk9/JDK9.java index f886b3f..1e3c7bb 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/internal/jdk9/JDK9.java +++ b/core/sis-utility/src/main/java/org/apache/sis/internal/jdk9/JDK9.java @@ -127,4 +127,15 @@ public final class JDK9 { name = (separator >= 1) ? name.substring(0, separator) : ""; return name; } + + /** + * Place holder for {@code Math.multiplyFull(int, int)}. + * + * @param x the first value. + * @param y the second value. + * @return Product of the two values. + */ + public static long multiplyFull(int x, int y) { + return ((long) x) * ((long) y); + } } diff --git a/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java b/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java index e15e2e2..99778b9 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java +++ b/core/sis-utility/src/main/java/org/apache/sis/math/Fraction.java @@ -21,6 +21,7 @@ import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.resources.Errors; import org.apache.sis.util.collection.WeakHashSet; import org.apache.sis.internal.util.Numerics; +import org.apache.sis.internal.jdk9.JDK9; /** @@ -216,7 +217,7 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri * However it should never happen. Even in the worst scenario:</p> * * {@prefomat java - * long n = Integer.MIN_VALUE * (long) Integer.MAX_VALUE; + * long n = Math.multiplyFull(Integer.MIN_VALUE, Integer.MAX_VALUE); * n += n; * } * @@ -319,8 +320,8 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri * @throws ArithmeticException if the result overflows. */ public Fraction multiply(final Fraction other) { - return simplify(this, numerator * (long) other.numerator, - denominator * (long) other.denominator); + return simplify(this, JDK9.multiplyFull(numerator, other.numerator), + JDK9.multiplyFull(denominator, other.denominator)); } /** @@ -331,8 +332,8 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri * @throws ArithmeticException if the result overflows. */ public Fraction divide(final Fraction other) { - return simplify(this, numerator * (long) other.denominator, - denominator * (long) other.numerator); + return simplify(this, JDK9.multiplyFull(numerator, other.denominator), + JDK9.multiplyFull(denominator, other.numerator)); } /** @@ -506,7 +507,8 @@ public final class Fraction extends Number implements Comparable<Fraction>, Seri */ @Override public int compareTo(final Fraction other) { - return Long.signum(numerator * (long) other.denominator - other.numerator * (long) denominator); + return Long.signum(JDK9.multiplyFull(numerator, other.denominator) + - JDK9.multiplyFull(other.numerator, denominator)); } /**
