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 d6a7e59c93710aa7f002f68cc0a231e9dfdeed4e Author: Martin Desruisseaux <[email protected]> AuthorDate: Thu Oct 30 14:48:48 2025 +0100 Better detection of cases when an array of JTS coordinates has no Z or M values. --- .../wrapper/jts/PackedCoordinateSequence.java | 150 +++++++++++---------- .../jts/PackedCoordinateSequenceFactory.java | 24 ++-- .../sis/geometry/wrapper/GeometriesTestCase.java | 8 +- .../wrapper/jts/PackedCoordinateSequenceTest.java | 115 ++++++++++++++++ 4 files changed, 207 insertions(+), 90 deletions(-) diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequence.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequence.java index 695e8b4568..5d65f106ac 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequence.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequence.java @@ -39,29 +39,27 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa /** * For cross-version compatibility. */ - private static final long serialVersionUID = 6323915437380051705L; + private static final long serialVersionUID = -3374522920399590093L; /** * Number of dimensions for this coordinate sequence. + * Values are restricted to 2, 3 or 4. * * @see #getDimension() */ - protected final int dimension; + final byte dimension; /** - * Whether this coordinate sequence has <var>z</var> and/or <var>M</var> coordinate values. - * This is a combination of {@link #Z_MASK} and {@link #M_MASK} bit masks. - * - * @see #hasZ() - * @see #hasM() + * Whether this coordinate sequence has <var>z</var> coordinate values. + * If {@code true}, exactly one <var>z</var> value is present per coordinate tuple. */ - private final int hasZM; + final boolean hasZ; /** - * Bit to set to 1 in the {@link #hasZM} mask if this coordinate sequence - * has <var>z</var> and/or <var>M</var> coordinate values. + * Whether this coordinate sequence has <var>M</var> coordinate values. + * If {@code true}, one or more <var>M</var> values are present per coordinate tuple. */ - private static final int Z_MASK = 1, M_MASK = 2; // Z_MASK must be 1 for bit twiddling reason. + final boolean hasM; /** * Creates a new sequence initialized to a copy of the given sequence. @@ -69,7 +67,8 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa */ PackedCoordinateSequence(final PackedCoordinateSequence original) { dimension = original.dimension; - hasZM = original.hasZM; + hasZ = original.hasZ; + hasM = original.hasM; } /** @@ -79,28 +78,19 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa * @param measures number of <var>M</var> coordinates. */ PackedCoordinateSequence(final int dimension, final int measures) { - ArgumentChecks.ensurePositive("measures", measures); - ArgumentChecks.ensureBetween("dimension", Factory.BIDIMENSIONAL + measures, - Math.addExact(Factory.TRIDIMENSIONAL, measures), dimension); - this.dimension = dimension; - int hasZM = (measures == 0) ? 0 : M_MASK; - if ((dimension - measures) >= Factory.TRIDIMENSIONAL) { - hasZM |= Z_MASK; - } - this.hasZM = hasZM; + ArgumentChecks.ensureBetween("measures", 0, 100, measures); // Arbitrary upper limit. + ArgumentChecks.ensureBetween("dimension", + Factory.BIDIMENSIONAL + measures, + Factory.TRIDIMENSIONAL + measures, + dimension); + this.dimension = (byte) dimension; + this.hasM = (measures != 0); + this.hasZ = (dimension - measures) >= Factory.TRIDIMENSIONAL; } /** - * Returns the number of spatial dimensions, - * which is {@value Factory#BIDIMENSIONAL} or {@value Factory#TRIDIMENSIONAL}. - */ - private static int getSpatialDimension(final int hasZM) { - return Factory.BIDIMENSIONAL | (hasZM & Z_MASK); - } - - /** - * Returns the number of dimensions for all coordinates in this sequence, - * including {@linkplain #getMeasures() measures}. + * Returns the number of dimensions for all coordinates in this sequence. + * This value includes the number of {@linkplain #getMeasures() measures}. */ @Override public final int getDimension() { @@ -112,27 +102,32 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa */ @Override public final int getMeasures() { - return dimension - getSpatialDimension(hasZM); + return dimension - (hasZ ? Factory.TRIDIMENSIONAL : Factory.BIDIMENSIONAL); } /** * Returns whether this coordinate sequence has <var>z</var> coordinate values. + * If {@code true}, exactly one <var>z</var> value is present per coordinate tuple. */ @Override public final boolean hasZ() { - return (hasZM & Z_MASK) != 0; + return hasZ; } /** * Returns whether this coordinate sequence has <var>M</var> coordinate values. + * If {@code true}, one or more <var>M</var> values are present per coordinate tuple. */ @Override public final boolean hasM() { - return (hasZM & M_MASK) != 0; + return hasM; } /** * Returns the <var>x</var> coordinate value for the tuple at the given index. + * For performance reasons, this method does not check the validity of the {@code index} argument. + * + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override public final double getX(final int index) { @@ -141,6 +136,9 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa /** * Returns the <var>y</var> coordinate value for the tuple at the given index. + * For performance reasons, this method does not check the validity of the {@code index} argument. + * + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override public final double getY(final int index) { @@ -148,66 +146,65 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa } /** - * Returns the <var>z</var> coordinate value for the tuple at the given index, - * or {@link java.lang.Double.NaN} if this sequence has no <var>z</var> coordinates. + * Returns the <var>z</var> coordinate value for the tuple at the given index. + * If this sequence has no <var>z</var> coordinates, returns {@link java.lang.Double.NaN}. + * For performance reasons, this method does not check the validity of the {@code index} argument. + * + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override public final double getZ(final int index) { - return (hasZM & Z_MASK) != 0 ? coordinate(index * dimension + Z) : java.lang.Double.NaN; + return hasZ ? coordinate(index * dimension + Z) : java.lang.Double.NaN; } /** - * Returns the first <var>M</var> coordinate value for the tuple at the given index, - * or {@link java.lang.Double.NaN} if this sequence has no <var>M</var> coordinates. + * Returns the first <var>M</var> coordinate value for the tuple at the given index. + * If this sequence has no <var>M</var> coordinates, returns {@link java.lang.Double.NaN}. + * For performance reasons, this method does not check the validity of the {@code index} argument. + * + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override public final double getM(final int index) { - switch (hasZM) { - default: return java.lang.Double.NaN; - case M_MASK: return coordinate(index * dimension + Z); - case M_MASK | Z_MASK: return coordinate(index * dimension + M); - } + return hasM ? coordinate(index * dimension + (hasZ ? M : Z)) : java.lang.Double.NaN; } /** * Returns the coordinate tuple at the given index. + * For performance reasons, this method does not check the validity of the {@code index} argument. * * @param index index of the coordinate tuple. * @return coordinate tuple at the given index. - * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds. + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override public final Coordinate getCoordinate(int index) { index *= dimension; final double x = coordinate( index); final double y = coordinate(++index); - switch (hasZM) { - default: return new Coordinate (x,y); - case 0: return new CoordinateXY (x,y); - case Z_MASK: return new Coordinate (x,y, coordinate(++index)); - case M_MASK: return new CoordinateXYM (x,y, coordinate(++index)); - case Z_MASK | M_MASK: return new CoordinateXYZM(x,y, coordinate(++index), coordinate(++index)); + if (!(hasZ | hasM)) { + return new CoordinateXY(x, y); // Most common case. } + final double z = coordinate(++index); + if (!hasM) return new Coordinate (x, y, z); + if (!hasZ) return new CoordinateXYM(x, y, z); + return new CoordinateXYZM(x, y, z, coordinate(++index)); } /** * Copies the coordinate tuple at the given index into the specified target. * - * @param index index of the coordinate tuple. - * @param dest where to copy the coordinates. - * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds. + * @param index index of the coordinate tuple. + * @param dest where to copy the coordinates. + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override - @SuppressWarnings("fallthrough") public final void getCoordinate(int index, final Coordinate dest) { index *= dimension; dest.x = coordinate( index); dest.y = coordinate(++index); - switch (hasZM) { - case Z_MASK: dest.setZ(coordinate(++index)); break; - case Z_MASK | M_MASK: dest.setZ(coordinate(++index)); // Fall through - case M_MASK: dest.setM(coordinate(++index)); break; - } + if (hasZ) dest.setZ(coordinate(++index)); + if (hasM) dest.setM(coordinate(++index)); } /** @@ -215,7 +212,7 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa * * @param index index of the coordinate tuple. * @return coordinate tuple at the given index. - * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds. + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override public final Coordinate getCoordinateCopy(int index) { @@ -224,11 +221,12 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa /** * Returns a coordinate value from the coordinate tuple at the given index. - * For performance reasons, this method does not check {@code dim} validity. + * For performance reasons, this method does not check arguments validity. * * @param index index of the coordinate tuple. * @param dim index of the coordinate value in the tuple. * @return value of the specified value in the coordinate tuple. + * @throws ArrayIndexOutOfBoundsException if the given index is out of bounds (not always detected). */ @Override public final double getOrdinate(final int index, final int dim) { @@ -304,10 +302,14 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa /** Sets all coordinates in this sequence. */ @Override void setCoordinates(final Coordinate[] values) { int t = 0; + int skip = getMeasures(); + if (hasM) skip--; for (final Coordinate c : values) { - for (int i=0; i<dimension; i++) { - coordinates[t++] = c.getOrdinate(i); - } + /*always*/coordinates[t++] = c.getX(); + /*always*/coordinates[t++] = c.getY(); + if (hasZ) coordinates[t++] = c.getZ(); + if (hasM) coordinates[t++] = c.getM(); + t += skip; } assert t == coordinates.length; } @@ -392,10 +394,14 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa /** Sets all coordinates in this sequence. */ @Override void setCoordinates(final Coordinate[] values) { int t = 0; + int skip = getMeasures(); + if (hasM) skip--; for (final Coordinate c : values) { - for (int i=0; i<dimension; i++) { - coordinates[t++] = (float) c.getOrdinate(i); - } + /*always*/coordinates[t++] = (float) c.getX(); + /*always*/coordinates[t++] = (float) c.getY(); + if (hasZ) coordinates[t++] = (float) c.getZ(); + if (hasM) coordinates[t++] = (float) c.getM(); + t += skip; } assert t == coordinates.length; } @@ -438,7 +444,7 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa */ @Override public final Coordinate[] toCoordinateArray() { - final Coordinate[] coordinates = new Coordinate[size()]; + final var coordinates = new Coordinate[size()]; for (int i=0; i < coordinates.length; i++) { coordinates[i] = getCoordinate(i); } @@ -458,7 +464,7 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa */ @Override public int hashCode() { - return (37 * dimension) ^ hasZM; + return (37 * dimension) ^ (hasZ ? 3 : 7) ^ (hasM ? 11 : 17); } /** @@ -467,8 +473,8 @@ abstract class PackedCoordinateSequence implements CoordinateSequence, Serializa @Override public boolean equals(final Object obj) { if (obj != null && obj.getClass() == getClass()) { - final PackedCoordinateSequence other = (PackedCoordinateSequence) obj; - return other.dimension == dimension && other.hasZM == hasZM; + final var other = (PackedCoordinateSequence) obj; + return other.dimension == dimension && other.hasZ == hasZ && other.hasM == hasM; } return false; } diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequenceFactory.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequenceFactory.java index 22379b0713..e0761b46d3 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequenceFactory.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequenceFactory.java @@ -18,7 +18,6 @@ package org.apache.sis.geometry.wrapper.jts; import java.io.Serializable; import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.Coordinates; import org.locationtech.jts.geom.CoordinateSequence; import org.locationtech.jts.geom.CoordinateSequenceFactory; @@ -61,23 +60,20 @@ final class PackedCoordinateSequenceFactory implements CoordinateSequenceFactory */ @Override public CoordinateSequence create(final Coordinate[] coordinates) { - int dimension = Factory.TRIDIMENSIONAL; - int measures = 0; - int size = 0; - boolean first = true; + int size = 0; + boolean noZ = true; + boolean noM = true; if (coordinates != null) { size = coordinates.length; for (final Coordinate c : coordinates) { - final int m = Coordinates.measures(c); - dimension = Math.min(dimension, Coordinates.dimension(c) - m); - if (first | m < measures) { - measures = m; - first = false; - } + if (noZ) noZ = Double.isNaN(c.getZ()); + if (noM) noM = Double.isNaN(c.getM()); + if (noZ & noM) break; // Shortcut. } } - dimension += measures; - final PackedCoordinateSequence cs = create(size, dimension, measures); + int measures = noM ? 0 : 1; + int dimension = noZ ? Factory.BIDIMENSIONAL : Factory.TRIDIMENSIONAL; + final PackedCoordinateSequence cs = create(size, dimension + measures, measures); if (size != 0) { cs.setCoordinates(coordinates); } @@ -101,7 +97,7 @@ final class PackedCoordinateSequenceFactory implements CoordinateSequenceFactory measures = original.getMeasures(); size = original.size(); } else { - dimension = Factory.TRIDIMENSIONAL; + dimension = Factory.BIDIMENSIONAL; measures = 0; size = 0; } diff --git a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/geometry/wrapper/GeometriesTestCase.java b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/geometry/wrapper/GeometriesTestCase.java index cd969c0a88..7cfd96a132 100644 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/geometry/wrapper/GeometriesTestCase.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/geometry/wrapper/GeometriesTestCase.java @@ -36,7 +36,7 @@ import org.apache.sis.referencing.crs.HardCodedCRS; /** - * Base class of Java2D, ESRI and JTS implementation tests. + * Base class of tests for Java2D, <abbr>ESRI</abbr> and <abbr>JTS</abbr> implementations. * * @author Martin Desruisseaux (Geomatys) * @author Alexis Manin (Geomatys) @@ -180,7 +180,7 @@ public abstract class GeometriesTestCase extends TestCase { */ @Test public void testToGeometry2D() { - final GeneralEnvelope envelope = new GeneralEnvelope(HardCodedCRS.WGS84); + final var envelope = new GeneralEnvelope(HardCodedCRS.WGS84); envelope.setRange(0, -30, 24); envelope.setRange(1, -60, -51); final double[] expected = {-30, -60, -30, -51, 24, -51, 24, -60, -30, -60}; @@ -196,7 +196,7 @@ public abstract class GeometriesTestCase extends TestCase { */ @Test public void testToGeometryWraparound() { - final GeneralEnvelope e = new GeneralEnvelope(HardCodedCRS.WGS84); + final var e = new GeneralEnvelope(HardCodedCRS.WGS84); e.setRange(0, 165, -170); e.setRange(1, 32, 33); assertToGeometryEquals(e, WraparoundMethod.NONE, 165, 32, 165, 33, -170, 33, -170, 32, 165, 32); @@ -223,7 +223,7 @@ public abstract class GeometriesTestCase extends TestCase { */ @Test public void testToGeometryFrom4D() { - final GeneralEnvelope e = new GeneralEnvelope(HardCodedCRS.GEOID_4D_MIXED_ORDER); + final var e = new GeneralEnvelope(HardCodedCRS.GEOID_4D_MIXED_ORDER); e.setRange(0, -20, 12); // Height e.setRange(1, 1000, 1007); // Time e.setRange(2, 2, 3); // Latitude diff --git a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequenceTest.java b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequenceTest.java new file mode 100644 index 0000000000..87199f5409 --- /dev/null +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/geometry/wrapper/jts/PackedCoordinateSequenceTest.java @@ -0,0 +1,115 @@ +/* + * 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.geometry.wrapper.jts; + +import java.util.function.DoubleFunction; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.CoordinateXY; +import org.locationtech.jts.geom.CoordinateXYM; +import org.locationtech.jts.geom.CoordinateXYZM; +import org.locationtech.jts.geom.CoordinateSequence; +import static java.lang.Double.NaN; + +// Test dependencies +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; +import org.apache.sis.test.TestCase; + + +/** + * Tests {@link PackedCoordinateSequence}. + * + * @author Martin Desruisseaux (Geomatys) + */ +public final class PackedCoordinateSequenceTest extends TestCase { + /** + * Creates a new test case. + */ + public PackedCoordinateSequenceTest() { + } + + /** + * Tests the creation from coordinates without <var>z</var> or <var>M</var> values. + * The absence of values should be detected even when using a specialized class such + * as {@link CoordinateXYZM}. + */ + @Test + public void testXY() { + createAndCompare(2, NaN, NaN, (x) -> new Coordinate (x, x+1, NaN)); + createAndCompare(2, NaN, NaN, (x) -> new CoordinateXY (x, x+1)); + createAndCompare(2, NaN, NaN, (x) -> new CoordinateXYM (x, x+1, NaN)); + createAndCompare(2, NaN, NaN, (x) -> new CoordinateXYZM(x, x+1, NaN, NaN)); + } + + /** + * Tests the creation from coordinates without <var>M</var> values. + */ + @Test + public void testXYZ() { + createAndCompare(3, 3, NaN, (x) -> new Coordinate (x, x+1, x+3)); + createAndCompare(3, 3, NaN, (x) -> new CoordinateXYZM(x, x+1, x+3, NaN)); + } + + /** + * Tests the creation from coordinates without <var>z</var> values. + */ + @Test + public void testXYM() { + createAndCompare(3, NaN, -5, (x) -> new CoordinateXYM (x, x+1, x-5)); + createAndCompare(3, NaN, -5, (x) -> new CoordinateXYZM(x, x+1, NaN, x-5)); + } + + /** + * Tests the creation from coordinates with all dimensions. + */ + @Test + public void testXYZM() { + createAndCompare(4, 7, -3, (x) -> new CoordinateXYZM(x, x+1, x+7, x-3)); + } + + /** + * Implementation of {@link #test2D()} creating points using the given function. + * The given function shall create point with <var>y</var> = <var>x</var>+1. + */ + private void createAndCompare(final int dim, final double z, final double m, + final DoubleFunction<Coordinate> creator) + { + final var points = new Coordinate[] { + creator.apply( 4), + creator.apply(-3), + creator.apply( 7) + }; + boolean doublePrecision = true; + do { + final CoordinateSequence cs = new PackedCoordinateSequenceFactory(doublePrecision).create(points); + assertEquals(dim, cs.getDimension()); + assertEquals( 3, cs.size()); + assertEquals( 4, cs.getX(0)); + assertEquals( 4+1, cs.getY(0)); + assertEquals( 4+z, cs.getZ(0)); + assertEquals( 4+m, cs.getM(0)); + assertEquals(-3, cs.getX(1)); + assertEquals(-3+1, cs.getY(1)); + assertEquals(-3+z, cs.getZ(1)); + assertEquals(-3+m, cs.getM(1)); + assertEquals( 7, cs.getX(2)); + assertEquals( 7+1, cs.getY(2)); + assertEquals( 7+z, cs.getZ(2)); + assertEquals( 7+m, cs.getM(2)); + } while ((doublePrecision = !doublePrecision) == false); + } +}
