Copilot commented on code in PR #594:
URL: https://github.com/apache/sedona-db/pull/594#discussion_r2788708173
##########
rust/sedona-testing/src/rasters.rs:
##########
@@ -149,6 +149,39 @@ pub fn generate_tiled_rasters(
Ok(raster_builder.finish()?)
}
+/// Builds a 1x1 single-band raster with a non-invertible geotransform (zero
scales and skews).
+/// Useful for testing error handling of inverse affine transforms.
+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")
+}
Review 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()`.
##########
rust/sedona-raster-functions/src/rs_envelope.rs:
##########
@@ -94,25 +94,37 @@ impl SedonaScalarKernel for RsEnvelope {
}
}
-/// Create WKB for a polygon for the raster
+/// Create WKB for the axis-aligned bounding box (envelope) of the raster.
+///
+/// This computes the four corners of the raster in world coordinates, then
+/// derives the min/max X and Y to produce an axis-aligned bounding box.
+/// For skewed/rotated rasters, this differs from the convex hull.
fn create_envelope_wkb(raster: &dyn RasterRef, out: &mut impl std::io::Write)
-> Result<()> {
- // Compute the four corners of the raster in world coordinates.
- // Due to skew/rotation in the affine transformation, each corner must be
- // computed individually.
-
let width = raster.metadata().width() as i64;
let height = raster.metadata().height() as i64;
- // Compute the four corners in pixel coordinates:
- // Upper-left (0, 0), Upper-right (width, 0), Lower-right (width, height),
Lower-left (0, height)
+ // Compute the four corners in world coordinates
let (ulx, uly) = to_world_coordinate(raster, 0, 0);
let (urx, ury) = to_world_coordinate(raster, width, 0);
let (lrx, lry) = to_world_coordinate(raster, width, height);
let (llx, lly) = to_world_coordinate(raster, 0, height);
+ // Compute the axis-aligned bounding box
+ let min_x = ulx.min(urx).min(lrx).min(llx);
+ let max_x = ulx.max(urx).max(lrx).max(llx);
+ let min_y = uly.min(ury).min(lry).min(lly);
+ let max_y = uly.max(ury).max(lry).max(lly);
+
write_wkb_polygon(
out,
- [(ulx, uly), (urx, ury), (lrx, lry), (llx, lly), (ulx,
uly)].into_iter(),
+ [
+ (min_x, max_y),
+ (max_x, max_y),
+ (max_x, min_y),
+ (min_x, min_y),
+ (min_x, max_y),
Review Comment:
The envelope ring is emitted in a top-left-first, clockwise order. If the
goal is to match PostGIS `ST_Envelope` semantics closely (and common OGC
conventions), consider emitting the rectangle in a canonical ordering (often
starting at `(min_x, min_y)` and proceeding counter-clockwise). This reduces
the chance of surprising WKB/WKT byte-for-byte differences for consumers that
compare encodings rather than geometry equality.
```suggestion
(min_x, min_y),
(max_x, min_y),
(max_x, max_y),
(min_x, max_y),
(min_x, min_y),
```
--
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]
