Skip to content

Conversation

@Kontinuation
Copy link
Member

Did you read the Contributor Guide?

Is this PR related to a ticket?

  • Yes, and the PR name follows the format [SEDONA-XXX] my subject.

What changes were proposed in this PR?

The world coordinates were silently casted from double to float when getting world coordinates from grid coordinates. This affects the precision of RS_RasterToWorldCoord*, RS_PixelAsPolygon, etc.

How was this patch tested?

Updated the expected results of tests to more precise coordinates.

Did this PR include necessary documentation updates?

  • No, this PR does not affect any public API so no need to change the documentation.

Comment on lines +469 to +473
Point2D.Double gridCoord = new DirectPosition2D(colX - 1, rowY - 1);
return raster
.getGridGeometry()
.getGridToCRS2D(PixelOrientation.UPPER_LEFT)
.transform(new GridCoordinates2D(colX - 1, rowY - 1), null);
.transform(gridCoord, null);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GridCoordinates2D is not a subclass of Point2D.Double, so the result of transform will be Point2D.Float: https://github.com/openjdk/jdk/blob/jdk-17%2B35/src/java.desktop/share/classes/java/awt/geom/AffineTransform.java#L2899-L2906. We explicitly use Point2D.Double here to avoid loss of precision.

@Kontinuation Kontinuation marked this pull request as ready for review March 17, 2025 14:38
@Kontinuation Kontinuation requested a review from jiayuasu as a code owner March 17, 2025 14:38
@jiayuasu jiayuasu added the bug label Mar 17, 2025
@jiayuasu jiayuasu added this to the sedona-1.8.0 milestone Mar 17, 2025
@jiayuasu jiayuasu merged commit 8314e0a into apache:master Mar 17, 2025
40 checks passed
@jiayuasu jiayuasu modified the milestones: sedona-1.8.0, 1.7.2 May 30, 2025
jiayuasu pushed a commit that referenced this pull request May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants