This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 32c5356683565f3dafa4db617e5ce4ff6a85df76 Author: jsorel <[email protected]> AuthorDate: Fri Oct 16 15:30:51 2026 +0200 feat(Geometry): fix hashcode error in Vector class, add DataPoints.subset --- .../main/org/apache/sis/geometries/DataPoints.java | 43 +++++++++++ .../internal/shared/ArrayDataPoints.java | 35 +++++++++ .../main/org/apache/sis/maths/AbstractVector.java | 4 +- .../main/org/apache/sis/maths/Maths.java | 87 ++++++++++++++-------- .../org/apache/sis/geometries/DataPointsTest.java | 51 ++++++++++++- .../internal/shared/ArrayDataPointsTest.java | 15 ++++ .../org/apache/sis/maths/AbstractVectorTest.java | 6 ++ .../test/org/apache/sis/maths/MathsTest.java | 49 ++++++++++-- 8 files changed, 251 insertions(+), 39 deletions(-) diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/DataPoints.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/DataPoints.java index 43ee21e73f..f19652805d 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/DataPoints.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/DataPoints.java @@ -29,6 +29,7 @@ import org.apache.sis.maths.ReadOnly; import org.apache.sis.maths.SampleSystem; import org.apache.sis.maths.Vector; import org.apache.sis.maths.Vectors; +import org.apache.sis.util.ArraysExt; /** @@ -298,6 +299,48 @@ public interface DataPoints { return env; } + /** + * Returns a copy of the points at the given indices, with all their attributes. The point at index + * <var>i</var> of the returned {@code DataPoints} is a copy of the point at index {@code indices[i]} + * of this {@code DataPoints}. An index can be given more than once. The returned {@code DataPoints} + * is independent of this one: changing either of them does not change the other one. + * + * @param indices indices of the points to copy, in the order of the returned points. + * @return a new {@code DataPoints} of {@code indices.length} points with the attributes of this one. + * @throws IndexOutOfBoundsException if an index is negative or not smaller than {@link #size()}. + * + * @see #subset(int, int) + */ + default DataPoints subset(final int[] indices) { + final DataPointsType type = getType(); + final List<String> names = type.getAttributeNames(); + final DataPoints subset = GeometryFactory.DEFAULT.createDataPoints(type, indices.length); + final int size = size(); + for (int i = 0; i < indices.length; i++) { + final int index = Objects.checkIndex(indices[i], size); + for (final String name : names) { + subset.setAttribute(i, name, getAttribute(index, name)); + } + } + return subset; + } + + /** + * Returns a copy of the given range of points, with all their attributes. The returned {@code DataPoints} + * is independent of this one: changing either of them does not change the other one. + * + * @param start index of the first point to copy. + * @param length number of points to copy. + * @return a new {@code DataPoints} of {@code length} points with the attributes of this one. + * @throws IndexOutOfBoundsException if the range is not inside this {@code DataPoints}. + * + * @see #subset(int[]) + */ + default DataPoints subset(final int start, final int length) { + Objects.checkFromIndexSize(start, length, size()); + return subset(ArraysExt.range(start, start + length)); + } + /** * Returns a hash code value for the given {@code DataPoints}, computed from the attributes it declares * and from the values it holds for them. Implementations of this interface shall base their diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/internal/shared/ArrayDataPoints.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/internal/shared/ArrayDataPoints.java index de767cfe89..a3e1a2a570 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/internal/shared/ArrayDataPoints.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/internal/shared/ArrayDataPoints.java @@ -19,8 +19,10 @@ package org.apache.sis.geometries.internal.shared; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.apache.sis.geometries.DataPoints; import org.apache.sis.geometries.Point; @@ -163,6 +165,39 @@ public final class ArrayDataPoints implements DataPoints, DataPointsType { return NDArrays.computeRange(attributes.get(name)); } + /** + * Returns a copy of the points at the given indices. + * This implementation copies a {@linkplain NDArrays#subset(Array, int...) view} of each attribute array, + * without creating a vector for each point. + */ + @Override + public DataPoints subset(final int[] indices) { + final int size = size(); + for (final int index : indices) { + Objects.checkIndex(index, size); + } + final Map<String,Array> subset = new LinkedHashMap<>(); + attributes.forEach((name, array) -> subset.put(name, NDArrays.subset(array, indices).copy())); + return new ArrayDataPoints(subset); + } + + /** + * Returns a copy of the given range of points. + * This implementation copies the range of each attribute array at once. + */ + @Override + public DataPoints subset(final int start, final int length) { + Objects.checkFromIndexSize(start, length, size()); + final Map<String,Array> subset = new LinkedHashMap<>(); + for (final Map.Entry<String,Array> entry : attributes.entrySet()) { + final Array source = entry.getValue(); + final Array target = NDArrays.of(source.getSampleSystem(), source.getDataType(), length); + target.set(0, source, start, length); + subset.put(entry.getKey(), target); + } + return new ArrayDataPoints(subset); + } + @Override public int hashCode() { return DataPoints.hashCode(this); diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/AbstractVector.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/AbstractVector.java index ef6ec53783..ed2cc3a17b 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/AbstractVector.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/AbstractVector.java @@ -53,7 +53,9 @@ abstract class AbstractVector<T extends AbstractVector<T>> implements Vector<T> public int hashCode() { int hash = 31; for (int i = 0, n = getDimension(); i < n; i++) { - hash += 31 * hash + Double.hashCode(get(i)); + final double value = get(i); + //positive and negative zero are equal, so they shall have the same hash code. + hash += 31 * hash + Double.hashCode(value == 0 ? 0 : value); } return hash; } diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/Maths.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/Maths.java index fb90d5880b..41948a2609 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/Maths.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/maths/Maths.java @@ -111,6 +111,33 @@ public final class Maths { return res; } + /** + * Calculate normal of a polygon with Newell's method. Unlike the normal of a triangle made of three + * points of the polygon, this normal is reliable for any ring, including rings with collinear points + * and rings which are not exactly planar. The normal follows the right-hand rule: it points toward an + * observer who sees the ring counterclockwise. It is not normalized: for a planar ring, its length is + * twice the area of the polygon. + * + * @param positions positions of the ring, closed or not. Only the three first coordinates are used. + * @return polygon normal as (x,y,z) components, zero for a degenerate ring + * @throws IllegalArgumentException if the positions have less than 3 coordinates + */ + public static double[] calculatePolygonNormal(Array positions){ + final int dim = positions.getDimension(); + if (dim < 3) { + throw new IllegalArgumentException("Positions shall have at least 3 coordinates, found " + dim + "."); + } + final double[] c = positions.toArrayDouble(); + final double[] normal = new double[3]; + for (int a = 0; a < c.length; a += dim) { + final int b = (a + dim) % c.length; + normal[0] += (c[a+1] - c[b+1]) * (c[a+2] + c[b+2]); + normal[1] += (c[a+2] - c[b+2]) * (c[a] + c[b]); + normal[2] += (c[a] - c[b]) * (c[a+1] + c[b+1]); + } + return normal; + } + public static double[] subtract(float[] A, float[] B){ return new double[] { A[0] - (double) B[0], @@ -754,6 +781,34 @@ public final class Maths { return bary[1] >= 0.0 && bary[2] >= 0.0 && (bary[1] + bary[2]) <= 1.0; } + /** + * Test if a point is inside given triangle, its edges and corners included. Only X,Y ordinates are used. + * Robust variant : the side of the point compared to each edge is computed with + * {@link #lineSide(ReadOnly.Vector, ReadOnly.Vector, ReadOnly.Vector)}, without division. + * The triangle can be clockwise or counterclockwise. A degenerate triangle, whose three corners + * are on a same line, contains the points of that line between its corners. + * + * @param a first triangle point + * @param b second triangle point + * @param c third triangle point + * @param p point to test + * @return true if point is inside triangle + */ + public static boolean inTriangle(ReadOnly.Vector<?> a, ReadOnly.Vector<?> b, ReadOnly.Vector<?> c, ReadOnly.Vector<?> p) { + final double d1 = lineSide(a, b, p); + final double d2 = lineSide(b, c, p); + final double d3 = lineSide(c, a, p); + final boolean left = d1 > 0 || d2 > 0 || d3 > 0; + final boolean right = d1 < 0 || d2 < 0 || d3 < 0; + if (left || right) { + return !(left && right); + } + //the point is on the lines of the three edges, which happens only if the triangle is degenerate. + final double x = p.get(0), y = p.get(1); + return x >= Math.min(a.get(0), Math.min(b.get(0), c.get(0))) && x <= Math.max(a.get(0), Math.max(b.get(0), c.get(0))) + && y >= Math.min(a.get(1), Math.min(b.get(1), c.get(1))) && y <= Math.max(a.get(1), Math.max(b.get(1), c.get(1))); + } + /** * Calculate the barycentric value in triangle for given point. * @@ -1278,36 +1333,4 @@ public final class Maths { } } } - - /** - * Remove element at index. - * Move elements after index by -1. - * Last element is set to null - * - * @param values original array - * @param index removed element index - * @return removed object - */ - public static Object removeWithin(Object[] values, int index) { - final Object removedValue = values[index]; - System.arraycopy(values, index+1, values, index, values.length-index-1); - values[values.length-1] = null; //remove reference - return removedValue; - } - - /** - * Remove element at index. - * Move elements after index by -1. - * Last element is set to zero - * - * @param values original array - * @param index removed element index - * @return removed value - */ - public static int removeWithin(int[] values, int index) { - final int removedValue = values[index]; - System.arraycopy(values, index+1, values, index, values.length-index-1); - values[values.length-1] = 0; - return removedValue; - } } diff --git a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/DataPointsTest.java b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/DataPointsTest.java index 6e3b0e219f..f9b170071c 100644 --- a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/DataPointsTest.java +++ b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/DataPointsTest.java @@ -45,10 +45,59 @@ public class DataPointsTest { * The returned instance implements only the abstract methods, in order to test the default methods. */ private static DataPoints createSample() { + return GeometryTest.withInterpolations(createArraySample(), Map.of()); + } + + /** + * Creates the points of {@link #createSample()}, stored in the arrays of the default implementation. + */ + protected static DataPoints createArraySample() { final Map<String,Array> attributes = new LinkedHashMap<>(); attributes.put(DataPointsType.ATT_POSITION, NDArrays.of(GeometryTest.CRS_2D, new double[] {0,0, 10,5, 4,20})); attributes.put("temperature", NDArrays.of(1, new double[] {10, 30, 20})); - return GeometryTest.withInterpolations(GeometryFactory.DEFAULT.createDataPoints(attributes), Map.of()); + return GeometryFactory.DEFAULT.createDataPoints(attributes); + } + + /** + * Tests {@link DataPoints#subset(int[])} and {@link DataPoints#subset(int, int)}. + */ + @Test + public void testSubset() { + assertSubsets(createSample()); + } + + /** + * Verifies the copies of points made by the given {@code DataPoints}, + * which shall hold the points of {@link #createSample()}. + */ + protected static void assertSubsets(final DataPoints points) { + final DataPoints subset = points.subset(new int[] {2, 0, 2}); + assertEquals(3, subset.size()); + assertEquals(points.getCoordinateReferenceSystem(), subset.getCoordinateReferenceSystem()); + assertEquals(Set.copyOf(points.getType().getAttributeNames()), Set.copyOf(subset.getType().getAttributeNames())); + assertPositionEquals(4, 20, subset.getPosition(0)); + assertPositionEquals(0, 0, subset.getPosition(1)); + assertPositionEquals(4, 20, subset.getPosition(2)); + assertEquals(20, subset.getAttribute(0, "temperature").get(0)); + assertEquals(10, subset.getAttribute(1, "temperature").get(0)); + + final DataPoints range = points.subset(1, 2); + assertEquals(2, range.size()); + assertPositionEquals(10, 5, range.getPosition(0)); + assertPositionEquals( 4, 20, range.getPosition(1)); + assertEquals(30, range.getAttribute(0, "temperature").get(0)); + assertEquals(0, points.subset(3, 0).size()); + + //the copies are independent of the copied points. + final Vector<?> value = range.getAttribute(0, "temperature"); + value.set(0, 99); + range.setAttribute(0, "temperature", value); + assertEquals(30, points.getAttribute(1, "temperature").get(0)); + + assertThrows(IndexOutOfBoundsException.class, () -> points.subset(new int[] {3})); + assertThrows(IndexOutOfBoundsException.class, () -> points.subset(new int[] {-1})); + assertThrows(IndexOutOfBoundsException.class, () -> points.subset(2, 2)); + assertThrows(IndexOutOfBoundsException.class, () -> points.subset(-1, 1)); } /** diff --git a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/internal/shared/ArrayDataPointsTest.java b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/internal/shared/ArrayDataPointsTest.java index 5585a056ad..363d32f372 100644 --- a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/internal/shared/ArrayDataPointsTest.java +++ b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/geometries/internal/shared/ArrayDataPointsTest.java @@ -16,7 +16,11 @@ */ package org.apache.sis.geometries.internal.shared; +import org.apache.sis.geometries.DataPoints; + // Test dependencies +import static org.junit.jupiter.api.Assertions.*; +import org.junit.jupiter.api.Test; import org.apache.sis.geometries.DataPointsTest; @@ -26,4 +30,15 @@ import org.apache.sis.geometries.DataPointsTest; * @author Johann Sorel (Geomatys) */ public class ArrayDataPointsTest extends DataPointsTest { + /** + * Tests the copies of points made from the arrays, which do not use the default methods. + */ + @Test + public void testSubsetOfArrays() { + final DataPoints points = createArraySample(); + assertInstanceOf(ArrayDataPoints.class, points); + assertSubsets(points); + assertInstanceOf(ArrayDataPoints.class, points.subset(new int[] {1})); + assertInstanceOf(ArrayDataPoints.class, points.subset(0, 1)); + } } diff --git a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/AbstractVectorTest.java b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/AbstractVectorTest.java index 78bbd04e5e..c32d3223ba 100644 --- a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/AbstractVectorTest.java +++ b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/AbstractVectorTest.java @@ -244,6 +244,12 @@ public abstract class AbstractVectorTest { assertFalse(copy.equals(tuple)); tuple.set(0, Double.NaN); assertEquals(copy, tuple); + + //test signed zero equality, which shall give the same hash code + copy.set(0, 0.0); + tuple.set(0, -0.0); + assertEquals(copy, tuple); + assertEquals(copy.hashCode(), tuple.hashCode()); } } diff --git a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/MathsTest.java b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/MathsTest.java index ec2b77efe3..2d52192a4b 100644 --- a/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/MathsTest.java +++ b/incubator/src/org.apache.sis.geometry/test/org/apache/sis/maths/MathsTest.java @@ -121,6 +121,50 @@ public class MathsTest { } + /** + * Returns a vector of the given values. + */ + private static Vector<?> vector(final double... values) { + return Vectors.createDouble(values.length).set(values); + } + + /** + * Tests the robust {@link Maths#inTriangle(ReadOnly.Vector, ReadOnly.Vector, ReadOnly.Vector, ReadOnly.Vector)}, + * which includes the edges and corners of the triangle, whatever its orientation. + */ + @Test + public void testInTriangle() { + final Vector<?> a = vector(0, 0), b = vector(4, 0), c = vector(0, 4); + for (final Vector<?>[] triangle : new Vector<?>[][] {{a, b, c}, {a, c, b}}) { + assertTrue (Maths.inTriangle(triangle[0], triangle[1], triangle[2], vector(1, 1)), "Inside"); + assertTrue (Maths.inTriangle(triangle[0], triangle[1], triangle[2], vector(2, 0)), "On an edge"); + assertTrue (Maths.inTriangle(triangle[0], triangle[1], triangle[2], vector(4, 0)), "On a corner"); + assertFalse(Maths.inTriangle(triangle[0], triangle[1], triangle[2], vector(3, 3)), "Outside"); + assertFalse(Maths.inTriangle(triangle[0], triangle[1], triangle[2], vector(-1, 1)), "Outside"); + } + //a degenerate triangle contains the points of its line between its corners. + final Vector<?> d = vector(2, 2), e = vector(4, 4); + assertTrue (Maths.inTriangle(a, d, e, vector(1, 1))); + assertFalse(Maths.inTriangle(a, d, e, vector(5, 5))); + assertFalse(Maths.inTriangle(a, d, e, vector(1, 0))); + } + + /** + * Tests {@link Maths#calculatePolygonNormal(Array)}, whose length is twice the area of the polygon. + */ + @Test + public void testCalculatePolygonNormal() { + //a counterclockwise square in the (x,y) plane, closed or not. + assertArrayEquals(new double[] {0, 0, 8}, Maths.calculatePolygonNormal( + NDArrays.of(3, new double[] {0,0,0, 2,0,0, 2,2,0, 0,2,0, 0,0,0})), EPSILON); + assertArrayEquals(new double[] {0, 0, 8}, Maths.calculatePolygonNormal( + NDArrays.of(3, new double[] {0,0,0, 2,0,0, 2,2,0, 0,2,0})), EPSILON); + //a vertical square in the (x,z) plane, with collinear points. + assertArrayEquals(new double[] {0, -8, 0}, Maths.calculatePolygonNormal( + NDArrays.of(3, new double[] {0,1,0, 1,1,0, 2,1,0, 2,1,2, 0,1,2, 0,1,0})), EPSILON); + assertThrows(IllegalArgumentException.class, () -> Maths.calculatePolygonNormal(NDArrays.of(2, new double[] {0,0, 1,0, 0,1}))); + } + @Test public void testNormalizedByte() { assertEquals(-1.0, Maths.normalizedByte(Byte.MIN_VALUE), 0.0); @@ -230,11 +274,6 @@ public class MathsTest { assertEquals(0xFFFFFFFFFFFFFFFFL, Maths.toNormalizedULong(1.0)); } - @Test - public void testInTriangle() { - - } - /** * A known double precisions mathematical problem. */
