fix(raster): RS_Envelope returns axis-aligned bounding box for skewed rasters #594
Conversation
… rasters RS_Envelope was returning the convex hull instead of the AABB. Compute min/max X/Y across all four corners to match PostGIS ST_Envelope semantics. Also fix generate_test_rasters to give raster i=0 non-zero scales (i.max(1)), move build_noninvertible_raster helper to sedona-testing, and update cascading test expectations.
There was a problem hiding this comment.
Pull request overview
This PR aligns RS_Envelope semantics with PostGIS ST_Envelope by returning an axis-aligned bounding box (AABB) for skewed/rotated rasters, and updates test raster generation/utilities to avoid accidental non-invertible geotransforms in baseline test data.
Changes:
- Update
RS_Envelopeto compute an AABB from raster corner world-coordinates (instead of using the corner polygon/convex hull for skewed rasters). - Adjust
generate_test_rasterssoi=0has non-zero scales (invertible geotransform), and introduce a dedicatedbuild_noninvertible_raster()helper for inverse-transform error-path testing. - Update affected test expectations in raster coordinate, geotransform, and envelope tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rust/sedona-testing/src/rasters.rs | Makes baseline test rasters invertible for i=0 and adds a helper to intentionally produce a non-invertible geotransform. |
| rust/sedona-raster-functions/src/rs_rastercoordinate.rs | Switches non-invertible-geotransform tests to use the new explicit helper raster. |
| rust/sedona-raster-functions/src/rs_geotransform.rs | Updates expected ScaleX/ScaleY values after test raster scale change. |
| rust/sedona-raster-functions/src/rs_envelope.rs | Changes envelope computation to AABB and updates tests/docs to reflect PostGIS-aligned behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn build_noninvertible_raster() -> StructArray { | ||
| let mut builder = RasterBuilder::new(1); | ||
| let metadata = RasterMetadata { | ||
| width: 1, | ||
| height: 1, | ||
| upperleft_x: 0.0, | ||
| upperleft_y: 0.0, | ||
| scale_x: 0.0, | ||
| scale_y: 0.0, | ||
| skew_x: 0.0, | ||
| skew_y: 0.0, | ||
| }; | ||
| let crs = lnglat().unwrap().to_crs_string(); | ||
| builder | ||
| .start_raster(&metadata, Some(&crs)) | ||
| .expect("start raster"); | ||
| builder | ||
| .start_band(BandMetadata { | ||
| datatype: BandDataType::UInt8, | ||
| nodata_value: None, | ||
| storage_type: StorageType::InDb, | ||
| outdb_url: None, | ||
| outdb_band_id: None, | ||
| }) | ||
| .expect("start band"); | ||
| builder.band_data_writer().append_value([0u8]); | ||
| builder.finish_band().expect("finish band"); | ||
| builder.finish_raster().expect("finish raster"); | ||
| builder.finish().expect("finish") | ||
| } |
There was a problem hiding this comment.
build_noninvertible_raster() uses unwrap()/expect() throughout. Even in a testing utility crate, it’s typically preferable to return Result<StructArray> and use ? so failures report the real error context (and let the calling test decide to unwrap()). This also keeps the helper consistent with other builders like generate_test_rasters().
There was a problem hiding this comment.
All the other test data builders unwrap in place so I think this is OK.
2 participants
Summary
RS_Envelopeto return the axis-aligned bounding box (AABB) instead of the convex hull for skewed/rotated rasters, matching PostGISST_Envelopesemantics.generate_test_rastersto give rasteri=0non-zero scales viai.max(1), so it has an invertible geotransform.build_noninvertible_raster()helper tosedona-testingfor tests that need a raster with zero scales/skews.rs_envelope,rs_geotransform, andrs_rastercoordinate.