Kontinuation commented on code in PR #2275:
URL: https://github.com/apache/sedona/pull/2275#discussion_r2275232723


##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -67,4 +68,174 @@ public static Geography geogCollFromText(String wkt, int 
srid) throws ParseExcep
     }
     return geogFromWKT(wkt, srid);
   }
+
+  public static Geometry geogToGeometry(Geography geography, GeometryFactory 
geometryFactory)
+      throws ParseException {
+    return geogToGeometry(geography, geometryFactory, geography.getSRID());
+  }
+
+  public static Geometry geogToGeometry(
+      Geography geography, GeometryFactory geometryFactory, int srid) {
+    if (geography == null) return null;

Review Comment:
   The `srid` is not used in `geogToGeometry(Geography geography, 
GeometryFactory geometryFactory, int srid)`.
   
   A better approach is to define these 2 functions:
   
   ```
   Geometry geogToGeometry(Geography geography, GeometryFactory 
geometryFactory); #1
   Geometry geogToGeometry(Geography geography, int srid); #2
   ```
   
   `GeometryFactory` object has an SRID property, the geometry objects created 
by that factory will automatically have that SRID. The variant #1 will 
implicitly use the SRID of the `geometryFactory`.
   
   Variant #2 creates a new `GeometryFactory` object with specified `srid`, and 
calls #1 to create geometry objects with specified SRID.



##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -67,4 +68,174 @@ public static Geography geogCollFromText(String wkt, int 
srid) throws ParseExcep
     }
     return geogFromWKT(wkt, srid);
   }
+
+  public static Geometry geogToGeometry(Geography geography, GeometryFactory 
geometryFactory)
+      throws ParseException {
+    return geogToGeometry(geography, geometryFactory, geography.getSRID());
+  }
+
+  public static Geometry geogToGeometry(
+      Geography geography, GeometryFactory geometryFactory, int srid) {
+    if (geography == null) return null;
+
+    Geography.GeographyKind kind = 
Geography.GeographyKind.fromKind(geography.getKind());
+    switch (kind) {
+      case SINGLEPOINT:
+      case POINT:
+        return pointToGeom(geography, geometryFactory);
+      case SINGLEPOLYLINE:
+      case POLYLINE:
+        return polylineToGeom(geography, geometryFactory);
+      case POLYGON:
+      case MULTIPOLYGON:
+        return polygonToGeom(geography, geometryFactory);
+      case GEOGRAPHY_COLLECTION:
+        return collectionToGeom(geography, geometryFactory);
+      default:
+        return null;
+    }
+  }
+
+  // POINT/SINGLEPOINT
+  private static Geometry pointToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof SinglePointGeography) {
+      S2Point p = ((SinglePointGeography) g).getPoints().get(0);
+      S2LatLng ll = S2LatLng.fromPoint(p);
+      return gf.createPoint(new Coordinate(ll.lngDegrees(), ll.latDegrees()));
+    } else if (g instanceof PointGeography) {
+      List<S2Point> pts = ((PointGeography) g).getPoints();
+      Coordinate[] cs = new Coordinate[pts.size()];
+      for (int i = 0; i < pts.size(); i++) {
+        S2LatLng ll = S2LatLng.fromPoint(pts.get(i));
+        cs[i] = new Coordinate(ll.lngDegrees(), ll.latDegrees());
+      }
+      return gf.createMultiPointFromCoords(cs);
+    }
+    return null;
+  }
+
+  // POLYLINE/SINGLEPOLYLINE
+  private static Geometry polylineToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof PolylineGeography) {
+      List<S2Polyline> lines = ((PolylineGeography) g).getPolylines();
+      LineString[] lss = new LineString[lines.size()];
+      for (int i = 0; i < lines.size(); i++) {
+        S2Polyline pl = lines.get(i);
+        int n = pl.numVertices();
+        Coordinate[] cs = new Coordinate[n];
+        for (int k = 0; k < n; k++) {
+          S2LatLng ll = S2LatLng.fromPoint(pl.vertex(k));
+          cs[k] = new Coordinate(ll.lngDegrees(), ll.latDegrees());
+        }
+        lss[i] = gf.createLineString(cs);
+      }
+      return lss.length == 1 ? lss[0] : gf.createMultiLineString(lss);
+    }
+    return null;
+  }
+
+  // POLYGON / MULTIPOLYGON
+  private static Geometry polygonToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof PolygonGeography) {
+      S2Polygon s2p = ((PolygonGeography) g).polygon;
+      return s2LoopsToJts(s2p.getLoops(), gf);
+    } else if (g instanceof MultiPolygonGeography) {
+      List<Geography> parts = ((MultiPolygonGeography) g).getFeatures();
+      Polygon[] polys = new Polygon[parts.size()];
+      for (int i = 0; i < parts.size(); i++) {
+        polys[i] = (Polygon) s2LoopsToJts(((PolygonGeography) 
parts.get(i)).polygon.getLoops(), gf);
+      }
+      return polys.length == 1 ? polys[0] : gf.createMultiPolygon(polys);

Review Comment:
   I think it is OK to always return multi polygon to preserve its original 
geography type.



##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -67,4 +68,174 @@ public static Geography geogCollFromText(String wkt, int 
srid) throws ParseExcep
     }
     return geogFromWKT(wkt, srid);
   }
+
+  public static Geometry geogToGeometry(Geography geography, GeometryFactory 
geometryFactory)
+      throws ParseException {
+    return geogToGeometry(geography, geometryFactory, geography.getSRID());
+  }
+
+  public static Geometry geogToGeometry(
+      Geography geography, GeometryFactory geometryFactory, int srid) {
+    if (geography == null) return null;
+
+    Geography.GeographyKind kind = 
Geography.GeographyKind.fromKind(geography.getKind());
+    switch (kind) {
+      case SINGLEPOINT:
+      case POINT:
+        return pointToGeom(geography, geometryFactory);
+      case SINGLEPOLYLINE:
+      case POLYLINE:
+        return polylineToGeom(geography, geometryFactory);
+      case POLYGON:
+      case MULTIPOLYGON:
+        return polygonToGeom(geography, geometryFactory);
+      case GEOGRAPHY_COLLECTION:
+        return collectionToGeom(geography, geometryFactory);
+      default:
+        return null;
+    }
+  }
+
+  // POINT/SINGLEPOINT
+  private static Geometry pointToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof SinglePointGeography) {
+      S2Point p = ((SinglePointGeography) g).getPoints().get(0);
+      S2LatLng ll = S2LatLng.fromPoint(p);
+      return gf.createPoint(new Coordinate(ll.lngDegrees(), ll.latDegrees()));
+    } else if (g instanceof PointGeography) {
+      List<S2Point> pts = ((PointGeography) g).getPoints();
+      Coordinate[] cs = new Coordinate[pts.size()];
+      for (int i = 0; i < pts.size(); i++) {
+        S2LatLng ll = S2LatLng.fromPoint(pts.get(i));
+        cs[i] = new Coordinate(ll.lngDegrees(), ll.latDegrees());
+      }
+      return gf.createMultiPointFromCoords(cs);
+    }
+    return null;
+  }
+
+  // POLYLINE/SINGLEPOLYLINE
+  private static Geometry polylineToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof PolylineGeography) {
+      List<S2Polyline> lines = ((PolylineGeography) g).getPolylines();
+      LineString[] lss = new LineString[lines.size()];
+      for (int i = 0; i < lines.size(); i++) {
+        S2Polyline pl = lines.get(i);
+        int n = pl.numVertices();
+        Coordinate[] cs = new Coordinate[n];
+        for (int k = 0; k < n; k++) {
+          S2LatLng ll = S2LatLng.fromPoint(pl.vertex(k));
+          cs[k] = new Coordinate(ll.lngDegrees(), ll.latDegrees());
+        }
+        lss[i] = gf.createLineString(cs);
+      }
+      return lss.length == 1 ? lss[0] : gf.createMultiLineString(lss);

Review Comment:
   Should be determine the type of converted geometry by inspecting the type of 
`g` (`PolylineGeography` or `SinglePolylineGeography`)? It is easy for us to 
preserve the original type here.



##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -67,4 +68,174 @@ public static Geography geogCollFromText(String wkt, int 
srid) throws ParseExcep
     }
     return geogFromWKT(wkt, srid);
   }
+
+  public static Geometry geogToGeometry(Geography geography, GeometryFactory 
geometryFactory)
+      throws ParseException {
+    return geogToGeometry(geography, geometryFactory, geography.getSRID());
+  }
+
+  public static Geometry geogToGeometry(
+      Geography geography, GeometryFactory geometryFactory, int srid) {
+    if (geography == null) return null;
+
+    Geography.GeographyKind kind = 
Geography.GeographyKind.fromKind(geography.getKind());
+    switch (kind) {
+      case SINGLEPOINT:
+      case POINT:
+        return pointToGeom(geography, geometryFactory);
+      case SINGLEPOLYLINE:
+      case POLYLINE:
+        return polylineToGeom(geography, geometryFactory);
+      case POLYGON:
+      case MULTIPOLYGON:
+        return polygonToGeom(geography, geometryFactory);
+      case GEOGRAPHY_COLLECTION:
+        return collectionToGeom(geography, geometryFactory);
+      default:
+        return null;
+    }
+  }
+
+  // POINT/SINGLEPOINT
+  private static Geometry pointToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof SinglePointGeography) {
+      S2Point p = ((SinglePointGeography) g).getPoints().get(0);
+      S2LatLng ll = S2LatLng.fromPoint(p);
+      return gf.createPoint(new Coordinate(ll.lngDegrees(), ll.latDegrees()));
+    } else if (g instanceof PointGeography) {
+      List<S2Point> pts = ((PointGeography) g).getPoints();
+      Coordinate[] cs = new Coordinate[pts.size()];
+      for (int i = 0; i < pts.size(); i++) {
+        S2LatLng ll = S2LatLng.fromPoint(pts.get(i));
+        cs[i] = new Coordinate(ll.lngDegrees(), ll.latDegrees());
+      }
+      return gf.createMultiPointFromCoords(cs);
+    }
+    return null;
+  }
+
+  // POLYLINE/SINGLEPOLYLINE
+  private static Geometry polylineToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof PolylineGeography) {
+      List<S2Polyline> lines = ((PolylineGeography) g).getPolylines();
+      LineString[] lss = new LineString[lines.size()];
+      for (int i = 0; i < lines.size(); i++) {
+        S2Polyline pl = lines.get(i);
+        int n = pl.numVertices();
+        Coordinate[] cs = new Coordinate[n];
+        for (int k = 0; k < n; k++) {
+          S2LatLng ll = S2LatLng.fromPoint(pl.vertex(k));
+          cs[k] = new Coordinate(ll.lngDegrees(), ll.latDegrees());
+        }
+        lss[i] = gf.createLineString(cs);
+      }
+      return lss.length == 1 ? lss[0] : gf.createMultiLineString(lss);
+    }
+    return null;
+  }
+
+  // POLYGON / MULTIPOLYGON
+  private static Geometry polygonToGeom(Geography g, GeometryFactory gf) {
+    if (g instanceof PolygonGeography) {
+      S2Polygon s2p = ((PolygonGeography) g).polygon;
+      return s2LoopsToJts(s2p.getLoops(), gf);
+    } else if (g instanceof MultiPolygonGeography) {
+      List<Geography> parts = ((MultiPolygonGeography) g).getFeatures();
+      Polygon[] polys = new Polygon[parts.size()];
+      for (int i = 0; i < parts.size(); i++) {
+        polys[i] = (Polygon) s2LoopsToJts(((PolygonGeography) 
parts.get(i)).polygon.getLoops(), gf);
+      }
+      return polys.length == 1 ? polys[0] : gf.createMultiPolygon(polys);
+    }
+    return null;
+  }
+
+  private static Geometry s2LoopsToJts(List<S2Loop> loops, GeometryFactory gf) 
{
+    if (loops == null || loops.isEmpty()) return gf.createPolygon();
+
+    List<LinearRing> shells = new ArrayList<>();
+    List<List<LinearRing>> holesPerShell = new ArrayList<>();
+
+    // Stack of current ancestor shells: each frame = {shellIndex, depth}
+    //    depth 0: Shell A
+    //          depth 1: Hole H1  (a lake in A)
+    //          depth 2: Shell S2 (an island in that lake A)
+    //                depth 3: Hole H3 (a pond on that island)
+    //    depth 0: Shell B     (disjoint area)
+    //          depth 1: Hole H2   (a lake in B)
+    Deque<int[]> shellStack = new ArrayDeque<>();

Review Comment:
   It is a stack by looking at the naming of this variable and the actual usage 
of it, maybe ArrayList is a better container for this?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to