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 d9e8b4ffe750d4a23267f0a5ce50e56a5e5a93b3 Author: Martin Desruisseaux <[email protected]> AuthorDate: Fri Jun 20 14:18:07 2025 +0200 Review of the changes in the endorsed part of Apache SIS regarding geometries. Replaced the parameterization of `ensureWinding(…)` by method overloading for avoiding unsafe casts. --- .../org/apache/sis/geometry/wrapper/jts/JTS.java | 159 +++++++++++---------- .../src/org.apache.sis.util/main/module-info.java | 1 - .../org.apache.sis.geometry/main/module-info.java | 2 +- .../main/org/apache/sis/geometries/Geometries.java | 2 +- .../main/org/apache/sis/geometries/math/Tuple.java | 2 + .../sis/geometries/processor/ProcessorUtils.java | 4 +- .../simplify/greedyinsert/TINBuilder.java | 2 +- 7 files changed, 87 insertions(+), 85 deletions(-) diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/JTS.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/JTS.java index 2195a246c6..810e741785 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/JTS.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/geometry/wrapper/jts/JTS.java @@ -19,10 +19,15 @@ package org.apache.sis.geometry.wrapper.jts; import java.util.Map; import java.util.Objects; import java.awt.Shape; -import java.util.function.Predicate; +import org.locationtech.jts.algorithm.Orientation; +import org.locationtech.jts.geom.CoordinateSequence; +import org.locationtech.jts.geom.CoordinateSequences; import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; +import org.locationtech.jts.geom.LinearRing; +import org.locationtech.jts.geom.MultiPolygon; +import org.locationtech.jts.geom.Polygon; import org.opengis.metadata.Identifier; import org.opengis.util.FactoryException; import org.opengis.referencing.crs.CoordinateReferenceSystem; @@ -41,12 +46,7 @@ import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox; import org.apache.sis.geometry.GeneralEnvelope; import org.apache.sis.util.privy.Constants; import static org.apache.sis.geometry.wrapper.Geometries.LOGGER; -import org.locationtech.jts.algorithm.Orientation; -import org.locationtech.jts.geom.CoordinateSequence; -import org.locationtech.jts.geom.CoordinateSequences; -import org.locationtech.jts.geom.LinearRing; -import org.locationtech.jts.geom.MultiPolygon; -import org.locationtech.jts.geom.Polygon; +import org.locationtech.jts.geom.GeometryCollection; /** @@ -339,92 +339,93 @@ public final class JTS extends Static { public static Geometry fromAWT(final GeometryFactory factory, final Shape shape, final double flatness) { return ShapeConverter.create(factory, Objects.requireNonNull(shape), flatness); } - - + /** - * Create a geometry from given geometry. Will ensure that - * shells are in the requested winding and holes are in reverse-winding. - * Handles only Polygon, MultiPolygon and LinearRing Geometry type. - * Other geometry types are returned unchanged. + * Makes the given geometry clockwise or counterclockwise. + * This method ensures that shells are in the requested winding and holes are in reverse-winding. + * If the given geometry already has the desired orientation, then it is returned unchanged. + * + * <h4>Limitations</h4> + * The current implementation handles only the {@link GeometryCollection}, {@link MultiPolygon}, + * {@link Polygon} and {@link LinearRing} geometry types. Other geometry types are returned unchanged. * - * @param g The Geometry to make CW. - * @param clockwise true for exterior ring clockwise, false for counter-clockwise - * @return The "nice" Polygon. + * @param geometry the geometry to make clockwise or counterclockwise. + * @param clockwise {@code true} for making the exterior ring clockwise, or {@code false} for counterclockwise. + * @return the given geometry with the requested winding. May be the {@code geometry} returned directly. */ - public static <T extends Geometry> T ensureWinding(final T g, boolean clockwise) { - - Predicate<CoordinateSequence> evaluator = Orientation::isCCW; - if (clockwise) evaluator = evaluator.negate(); - - if (g instanceof MultiPolygon || g instanceof Polygon) { - final GeometryFactory gf = g.getFactory(); - boolean isMultiPolygon = false; - int nbPolygon = 1; - - if (g instanceof MultiPolygon) { - nbPolygon = g.getNumGeometries(); - isMultiPolygon = true; + public static Geometry ensureWinding(final Geometry geometry, final boolean clockwise) { + if (geometry instanceof MultiPolygon) { + final var collection = (MultiPolygon) geometry; + final var components = new Polygon[collection.getNumGeometries()]; + boolean changed = false; + for (int i=0; i<components.length; i++) { + final var c = (Polygon) geometry.getGeometryN(i); + changed |= (c != (components[i] = ensureWinding(c, clockwise))); } - final Polygon[] ps = new Polygon[nbPolygon]; - for (int i = 0; i < nbPolygon; i++) { - final Polygon p; - if (isMultiPolygon) { - p = (Polygon) g.getGeometryN(i); - } else { - p = (Polygon) g; - } - final LinearRing[] holes = new LinearRing[p.getNumInteriorRing()]; - LinearRing outer = p.getExteriorRing(); - if (!evaluator.test(outer.getCoordinateSequence())) { - outer = reverseRing(p.getExteriorRing()); - } - - for (int t = 0, tt = p.getNumInteriorRing(); t < tt; t++) { - holes[t] = p.getInteriorRingN(t); - if (evaluator.test(holes[t].getCoordinateSequence())) { - holes[t] = reverseRing(holes[t]); - } - } - ps[i] = gf.createPolygon(outer, holes); + if (changed) { + return geometry.getFactory().createMultiPolygon(components); } - - Geometry reversed; - if (isMultiPolygon) { - reversed = gf.createMultiPolygon(ps); - } else { - reversed = ps[0]; + } else if (geometry instanceof GeometryCollection) { + final var collection = (GeometryCollection) geometry; + final var components = new Geometry[collection.getNumGeometries()]; + boolean changed = false; + for (int i=0; i<components.length; i++) { + final var c = geometry.getGeometryN(i); + changed |= (c != (components[i] = ensureWinding(c, clockwise))); // Recursive method invocation. } - reversed.setSRID(g.getSRID()); - reversed.setUserData(g.getUserData()); - return (T) reversed; - - } else if (g instanceof LinearRing) { - LinearRing lr = (LinearRing) g; - if (!evaluator.test(lr.getCoordinateSequence())) { - lr = reverseRing(lr); + if (changed) { + return geometry.getFactory().createGeometryCollection(components); } - return (T) lr; + } else if (geometry instanceof Polygon) { + return ensureWinding((Polygon) geometry, clockwise); + } else if (geometry instanceof LinearRing) { + return ensureWinding((LinearRing) geometry, clockwise); + } + return geometry; + } - } else { - return g; + /** + * Makes the given polygon clockwise or counterclockwise. + * This method ensures that the shell is in the requested winding and holes are in reverse-winding. + * If the given polygon already has the desired orientation, then it is returned unchanged. + * + * @param geometry the polygon to make clockwise or counterclockwise. + * @param clockwise {@code true} for making the polygon clockwise, or {@code false} for counterclockwise. + * @return the given polygon with the requested winding. May be the {@code geometry} returned directly. + */ + public static Polygon ensureWinding(final Polygon geometry, final boolean clockwise) { + LinearRing outer = geometry.getExteriorRing(); + boolean same = (outer == (outer = ensureWinding(outer, clockwise))); + final var holes = new LinearRing[geometry.getNumInteriorRing()]; + for (int i=0; i<holes.length; i++) { + final LinearRing inner = geometry.getInteriorRingN(i); + same &= (inner == (holes[i] = ensureWinding(inner, !clockwise))); } + if (same) { + return geometry; + } + final Polygon reversed = geometry.getFactory().createPolygon(outer, holes); + copyMetadata(geometry, reversed); + return reversed; } - + /** - * Does what it says, reverses the order of the Coordinates in the ring. + * Makes the given ring clockwise or counterclockwise. + * If the given ring already has the desired orientation, then it is returned unchanged. * - * @param lr The ring to reverse. - * @return A new ring with the reversed Coordinates. + * @param geometry the ring to make clockwise or counterclockwise. + * @param clockwise {@code true} for making the ring clockwise, or {@code false} for counterclockwise. + * @return the given ring with the requested winding. May be the {@code geometry} returned directly. */ - private static LinearRing reverseRing(final LinearRing lr) { - final GeometryFactory gf = lr.getFactory(); - final CoordinateSequence cs = lr.getCoordinateSequence().copy(); + public static LinearRing ensureWinding(final LinearRing geometry, final boolean clockwise) { + CoordinateSequence cs = geometry.getCoordinateSequence(); + if (Orientation.isCCW(cs) != clockwise) { + return geometry; + } + cs = cs.copy(); CoordinateSequences.reverse(cs); - - final LinearRing reversed = gf.createLinearRing(cs); - reversed.setSRID(reversed.getSRID()); - reversed.setUserData(lr.getUserData()); + final LinearRing reversed = geometry.getFactory().createLinearRing(cs); + copyMetadata(geometry, reversed); return reversed; } - } diff --git a/endorsed/src/org.apache.sis.util/main/module-info.java b/endorsed/src/org.apache.sis.util/main/module-info.java index cf46ccb9ba..3b01093970 100644 --- a/endorsed/src/org.apache.sis.util/main/module-info.java +++ b/endorsed/src/org.apache.sis.util/main/module-info.java @@ -117,7 +117,6 @@ module org.apache.sis.util { org.apache.sis.cloud.aws, org.apache.sis.console, org.apache.sis.openoffice, - org.apache.sis.geometry, // In the "incubator" sub-project. org.apache.sis.gui, // In the "optional" sub-project. org.apache.sis.referencing.epsg, // In the "non-free" sub-project. org.apache.sis.referencing.database; // In the "non-free" sub-project. diff --git a/incubator/src/org.apache.sis.geometry/main/module-info.java b/incubator/src/org.apache.sis.geometry/main/module-info.java index a5a1420a43..2c58c63ab2 100644 --- a/incubator/src/org.apache.sis.geometry/main/module-info.java +++ b/incubator/src/org.apache.sis.geometry/main/module-info.java @@ -21,7 +21,7 @@ * @author Johann Sorel (Geomatys) */ module org.apache.sis.geometry { - requires esri.geometry.api; + requires esri.geometry.api; // TODO: remove (this is for tests). requires org.apache.sis.feature; requires org.apache.sis.util; requires transitive org.apache.sis.storage; diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/Geometries.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/Geometries.java index 92597ac8f8..59ef464615 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/Geometries.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/Geometries.java @@ -659,7 +659,7 @@ public final class Geometries extends Static { for (int i = 0, n = triangles.size(); i < n; i++) { org.locationtech.jts.geom.Polygon p = triangles.get(i); //counter clockwise to ensure up direction - p = JTS.ensureWinding(p,false); + p = JTS.ensureWinding(p, false); final CoordinateSequence ring = p.getExteriorRing().getCoordinateSequence(); int idx = i*3*dimension; switch (dimension) { diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/math/Tuple.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/math/Tuple.java index 7556701919..94824db4bf 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/math/Tuple.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/math/Tuple.java @@ -25,6 +25,8 @@ import org.opengis.referencing.crs.CoordinateReferenceSystem; * For interoperability with MathTransform operations a Tuple implements DirectPosition. * * @author Johann Sorel (Geomatys) + * + * @todo Remove the {@code extends DirectPosition} part. A tuple is not a direct position. */ public interface Tuple<T extends Tuple <T>> extends DirectPosition { diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/processor/ProcessorUtils.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/processor/ProcessorUtils.java index 2214342d74..96f200ae32 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/processor/ProcessorUtils.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/processor/ProcessorUtils.java @@ -82,9 +82,9 @@ public final class ProcessorUtils extends Static { /** - * @todo Duplicate of {@link org.geotoolkit.referencing.factory.ReferencingFactoryContainer}? + * @todo Duplicate of {@link org.apache.sis.referencing.privy.ReferencingFactoryContainer}? * - * @see <a href="https://issues.apache.org/jira/browse/SIS-162">https://issues.apache.org/jira/browse/SIS-162</a> + * @see <a href="https://issues.apache.org/jira/browse/SIS-162">SIS-162</a> */ private static CoordinateReferenceSystem getOrCreateSubCRS(CoordinateReferenceSystem crs, int lower, int upper) { if (crs == null) return crs; diff --git a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/simplify/greedyinsert/TINBuilder.java b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/simplify/greedyinsert/TINBuilder.java index f8614625bc..92a5d91ce9 100644 --- a/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/simplify/greedyinsert/TINBuilder.java +++ b/incubator/src/org.apache.sis.geometry/main/org/apache/sis/geometries/simplify/greedyinsert/TINBuilder.java @@ -77,7 +77,7 @@ public final class TINBuilder { * @param p2 third dem corner * @param p3 fourth dem corner * @param delta minimum distance to dem to include point - * @throws org.geotoolkit.process.ProcessException if an invalid geometry state occurs + * @throws OperationException if an invalid geometry state occurs */ public TINBuilder(Tuple p0, Tuple p1, Tuple p2, Tuple p3, double delta, BiFunction<Tuple,Triangle,Double> errorCalculator) throws OperationException { this.crs = p0.getCoordinateReferenceSystem();
