paleolimbot commented on code in PR #695: URL: https://github.com/apache/sedona-db/pull/695#discussion_r2936083758
########## c/sedona-gdal/src/geo_transform.rs: ########## @@ -0,0 +1,152 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/geo_transform.rs>. +//! Original code is licensed under MIT. +//! +//! GeoTransform type and extension trait. +//! +//! The [`apply`](GeoTransformEx::apply) and [`invert`](GeoTransformEx::invert) +//! methods are pure-Rust reimplementations of GDAL's `GDALApplyGeoTransform` +//! and `GDALInvGeoTransform` (from `alg/gdaltransformer.cpp`). No FFI call or +//! thread-local state is needed. + +use crate::errors; +use crate::errors::GdalError; + +/// An affine geo-transform: six coefficients mapping pixel/line to projection coordinates. +/// +/// - `[0]`: x-coordinate of the upper-left corner of the upper-left pixel. +/// - `[1]`: W-E pixel resolution (pixel width). +/// - `[2]`: row rotation (typically zero). +/// - `[3]`: y-coordinate of the upper-left corner of the upper-left pixel. +/// - `[4]`: column rotation (typically zero). +/// - `[5]`: N-S pixel resolution (pixel height, negative for North-up). +pub type GeoTransform = [f64; 6]; + +/// Extension methods on [`GeoTransform`]. +pub trait GeoTransformEx { + /// Apply the geo-transform to a pixel/line coordinate, returning (geo_x, geo_y). + fn apply(&self, x: f64, y: f64) -> (f64, f64); + + /// Invert this geo-transform, returning the inverse coefficients for + /// computing (geo_x, geo_y) -> (x, y) transformations. + fn invert(&self) -> errors::Result<GeoTransform>; +} + +impl GeoTransformEx for GeoTransform { + /// Pure-Rust equivalent of GDAL's `GDALApplyGeoTransform`. + fn apply(&self, x: f64, y: f64) -> (f64, f64) { + let geo_x = self[0] + x * self[1] + y * self[2]; + let geo_y = self[3] + x * self[4] + y * self[5]; + (geo_x, geo_y) + } Review Comment: I think this is the third way in our code base that we have this (once in sedona-raster, once using a separate rust crate, and here). It may be worth just using the rust dependency rate instead of maintaining tests for this. ########## c/sedona-gdal/src/spatial_ref.rs: ########## @@ -0,0 +1,172 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/spatial_ref/srs.rs>. +//! Original code is licensed under MIT. + +use std::ffi::{CStr, CString}; +use std::ptr; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::OGRERR_NONE; +use crate::gdal_dyn_bindgen::*; + +/// An OGR spatial reference system. +pub struct SpatialRef { + api: &'static GdalApi, + c_srs: OGRSpatialReferenceH, +} + +// SAFETY: `SpatialRef` has unique ownership of its GDAL handle and only moves that +// ownership between threads. The handle is released exactly once on drop, and this +// wrapper does not provide shared concurrent access, so `Send` is sound while `Sync` +// remains intentionally unimplemented. +unsafe impl Send for SpatialRef {} + +impl Drop for SpatialRef { + fn drop(&mut self) { + if !self.c_srs.is_null() { + unsafe { call_gdal_api!(self.api, OSRRelease, self.c_srs) }; + } + } +} + +impl SpatialRef { + /// Create a new SpatialRef from a WKT string. + pub fn from_wkt(api: &'static GdalApi, wkt: &str) -> Result<Self> { + let c_wkt = CString::new(wkt)?; + let c_srs = unsafe { call_gdal_api!(api, OSRNewSpatialReference, c_wkt.as_ptr()) }; Review Comment: Does this also handle other types of input or should we expose the version that does (maybe called SetFromUserInput?). Do we have to set the axis mapping strategy to avoid flipped lon/lat definitions for EPSG:4326? ########## c/sedona-gdal/src/vsi.rs: ########## @@ -0,0 +1,209 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/vsi.rs>. +//! Original code is licensed under MIT. +//! +//! GDAL Virtual File System (VSI) wrappers. + +use std::ffi::CString; + +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; + +/// Creates a new VSI in-memory file from a given buffer. +/// +/// The data is copied into GDAL-allocated memory (via `VSIMalloc`) so that +/// GDAL can safely free it with `VSIFree` when ownership is taken, without +/// crossing allocator boundaries back into Rust. +pub fn create_mem_file(api: &'static GdalApi, file_name: &str, data: &[u8]) -> Result<()> { + let c_file_name = CString::new(file_name)?; + let len = data.len(); + let len_i64 = i64::try_from(len)?; + + let gdal_buf = if len == 0 { + std::ptr::null_mut() + } else { + // Allocate via GDAL's allocator so GDAL can safely free it. + let gdal_buf = unsafe { call_gdal_api!(api, VSIMalloc, len) } as *mut u8; + if gdal_buf.is_null() { + return Err(GdalError::NullPointer { + method_name: "VSIMalloc", + msg: format!("failed to allocate {len} bytes"), + }); + } + + // Copy data into GDAL-allocated buffer. + unsafe { + std::ptr::copy_nonoverlapping(data.as_ptr(), gdal_buf, len); + } + gdal_buf + }; + + let handle = unsafe { + call_gdal_api!( + api, + VSIFileFromMemBuffer, + c_file_name.as_ptr(), + gdal_buf, + len_i64, + 1 // bTakeOwnership = true — GDAL will VSIFree gdal_buf + ) + }; + + if handle.is_null() { + // GDAL did not take ownership, so we must free. + if !gdal_buf.is_null() { + unsafe { call_gdal_api!(api, VSIFree, gdal_buf as *mut std::ffi::c_void) }; + } + return Err(GdalError::NullPointer { + method_name: "VSIFileFromMemBuffer", + msg: String::new(), + }); + } + + unsafe { + call_gdal_api!(api, VSIFCloseL, handle); + } + + Ok(()) +} + +/// Unlink (delete) a VSI in-memory file. +pub fn unlink_mem_file(api: &'static GdalApi, file_name: &str) -> Result<()> { + let c_file_name = CString::new(file_name)?; + + let rv = unsafe { call_gdal_api!(api, VSIUnlink, c_file_name.as_ptr()) }; + + if rv != 0 { + return Err(GdalError::UnlinkMemFile { + file_name: file_name.to_string(), + }); + } + + Ok(()) +} + +/// Copies the bytes of the VSI in-memory file, taking ownership and freeing the GDAL memory. +pub fn get_vsi_mem_file_bytes_owned(api: &'static GdalApi, file_name: &str) -> Result<Vec<u8>> { Review Comment: Maybe this could return a proxy to avoid the copy? ```rust struct VSIBuffer { data: mut* u8, length: usize } impl Drop for VSIBuffer { ... } impl AsRef<[u8]> for VSIBuffer { ... } ``` We could even attach this to BinaryView without copying I believe if we're using this for output somehow. -- 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]
