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();

Reply via email to