[GitHub] [sedona] jiayuasu commented on a diff in pull request #948: [SEDONA-346] Add WorldToRaster APIs

["jiayuasu (via GitHub)" <gi...@apache.org>]

jiayuasu commented on code in PR #948:
URL: https://github.com/apache/sedona/pull/948#discussion_r1286595856


##########
common/src/main/java/org/apache/sedona/common/raster/RasterPredicates.java:
##########
@@ -21,15 +21,12 @@
 import org.geotools.coverage.grid.GridCoverage2D;
 import org.geotools.geometry.Envelope2D;
 import org.geotools.geometry.jts.JTS;
-import org.geotools.referencing.CRS;
-import org.geotools.referencing.crs.DefaultEngineeringCRS;
 import org.locationtech.jts.geom.Envelope;
 import org.locationtech.jts.geom.Geometry;
 import org.locationtech.jts.geom.GeometryFactory;
-import org.opengis.referencing.FactoryException;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
-import org.opengis.referencing.operation.MathTransform;
-import org.opengis.referencing.operation.TransformException;
+
+import static org.apache.sedona.common.utils.RasterUtils.convertCRSIfNeeded;

Review Comment:
   This import is not correct. Please fix this.



##########
common/src/main/java/org/apache/sedona/common/utils/RasterUtils.java:
##########
@@ -95,7 +100,7 @@ public static AffineTransform2D getAffineTransform(GridCoverage2D raster, PixelO
         return (AffineTransform2D) crsTransform;
     }
 
-    public static Point2D getCornerCoordinates(GridCoverage2D raster, int colX, int rowY) throws TransformException {
+    public static Point2D getWorldCornerCoordinates(GridCoverage2D raster, int colX, int rowY) throws TransformException {

Review Comment:
   The problem might not be related to this PR. However, I think only `getWorldCornerCoordinatesWithRangeCheck` should be kept. I understand some PostGIS flavors do not perform range check. Do we actually support the computation of out of boundary raster coordinate?



##########
common/src/main/java/org/apache/sedona/common/raster/RasterAccessors.java:
##########
@@ -75,12 +80,62 @@ public static double getScaleY(GridCoverage2D raster) {
         return RasterUtils.getGDALAffineTransform(raster).getScaleY();
     }
 
+    public static double getSkewX(GridCoverage2D raster) {
+        return RasterUtils.getGDALAffineTransform(raster).getShearX();
+    }
+
+    public static double getSkewY(GridCoverage2D raster) {
+        return RasterUtils.getGDALAffineTransform(raster).getShearY();
+    }
+
     public static double getWorldCoordX(GridCoverage2D raster, int colX, int rowY) throws TransformException {
-        return RasterUtils.getCornerCoordinates(raster, colX, rowY).getX();
+        return RasterUtils.getWorldCornerCoordinates(raster, colX, rowY).getX();
     }
 
     public static double getWorldCoordY(GridCoverage2D raster, int colX, int rowY) throws TransformException {
-        return RasterUtils.getCornerCoordinates(raster, colX, rowY).getY();
+        return RasterUtils.getWorldCornerCoordinates(raster, colX, rowY).getY();
+    }
+
+    public static Geometry getGridCoord(GridCoverage2D raster, double longitude, double latitude) throws TransformException {

Review Comment:
   Given that the CRS transformation follows the lat/lon order, the parameters here are wrong. To avoid unnecessary complication, please call them X/Y, instead of longitude and latitude.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org