Copilot commented on code in PR #698: URL: https://github.com/apache/sedona-db/pull/698#discussion_r2998524012
########## c/sedona-gdal/src/vrt.rs: ########## @@ -0,0 +1,326 @@ +// 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. + +//! GDAL VRT (Virtual Raster) API wrappers. + +use std::ffi::CString; +use std::ops::{Deref, DerefMut}; +use std::ptr::null_mut; + +use crate::cpl::CslStringList; +use crate::dataset::Dataset; +use crate::errors::Result; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::raster::rasterband::RasterBand; +use crate::{gdal_dyn_bindgen::*, raster::types::GdalDataType}; + +/// Special value indicating that nodata is not set for a VRT source. +/// Matches `VRT_NODATA_UNSET` from GDAL's `gdal_vrt.h`. +pub const NODATA_UNSET: f64 = -1234.56; + +/// A VRT (Virtual Raster) dataset. +pub struct VrtDataset { + dataset: Dataset, +} + +unsafe impl Send for VrtDataset {} + +impl VrtDataset { + /// Create an empty VRT dataset with the given raster size. + pub fn create(api: &'static GdalApi, x_size: usize, y_size: usize) -> Result<Self> { + let x: i32 = x_size.try_into()?; + let y: i32 = y_size.try_into()?; + let c_dataset = unsafe { call_gdal_api!(api, VRTCreate, x, y) }; + + if c_dataset.is_null() { + return Err(api.last_null_pointer_err("VRTCreate")); + } + + Ok(VrtDataset { + dataset: Dataset::new(api, c_dataset), + }) + } + + /// Return the underlying `Dataset`, transferring ownership. + pub fn as_dataset(self) -> Dataset { + let VrtDataset { dataset } = self; + dataset + } + + /// Add a band to this VRT dataset. + /// Return the 1-based index of the new band. + pub fn add_band(&mut self, data_type: GdalDataType, options: Option<&[&str]>) -> Result<usize> { + let csl = CslStringList::try_from_iter(options.unwrap_or(&[]).iter().copied())?; + + // Preserve null semantics: pass null when no options given. + let opts_ptr = if csl.is_empty() { + null_mut() + } else { + csl.as_ptr() + }; + + let rv = unsafe { + call_gdal_api!( + self.dataset.api(), + GDALAddBand, + self.dataset.c_dataset(), + data_type.to_c(), + opts_ptr + ) + }; + + if rv != CE_None { + return Err(self.dataset.api().last_cpl_err(rv as u32)); + } + + Ok(self.raster_count()) + } + + /// Fetch a VRT band by 1-indexed band number. + pub fn rasterband(&self, band_index: usize) -> Result<VrtRasterBand<'_>> { + let band = self.dataset.rasterband(band_index)?; + Ok(VrtRasterBand { band }) + } +} + +impl Deref for VrtDataset { + type Target = Dataset; + + fn deref(&self) -> &Self::Target { + &self.dataset + } +} + +impl DerefMut for VrtDataset { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.dataset + } +} + +impl AsRef<Dataset> for VrtDataset { + fn as_ref(&self) -> &Dataset { + &self.dataset + } +} + +/// A raster band within a VRT dataset. +pub struct VrtRasterBand<'a> { + band: RasterBand<'a>, +} + +impl<'a> VrtRasterBand<'a> { + /// Return the raw GDAL raster band handle. + pub fn c_rasterband(&self) -> GDALRasterBandH { + self.band.c_rasterband() + } + + /// Add a simple source to this VRT band. + /// Map a source window to a destination window, with optional resampling and nodata. + pub fn add_simple_source( + &self, + source_band: &RasterBand<'a>, + src_window: (i32, i32, i32, i32), + dst_window: (i32, i32, i32, i32), + resampling: Option<&str>, + nodata: Option<f64>, + ) -> Result<()> { Review Comment: `add_simple_source` unnecessarily ties `source_band` to the same lifetime as the VRT band (`&RasterBand<'a>`). This is more restrictive than needed and can make it hard to use a source band from a different dataset/borrow scope. Consider introducing an independent lifetime parameter for `source_band` (or accepting `&RasterBand<'_>`) so the source dataset borrow doesn’t have to outlive the VRT dataset borrow. ########## c/sedona-gdal/src/vrt.rs: ########## @@ -0,0 +1,326 @@ +// 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. + +//! GDAL VRT (Virtual Raster) API wrappers. + +use std::ffi::CString; +use std::ops::{Deref, DerefMut}; +use std::ptr::null_mut; + +use crate::cpl::CslStringList; +use crate::dataset::Dataset; +use crate::errors::Result; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::raster::rasterband::RasterBand; +use crate::{gdal_dyn_bindgen::*, raster::types::GdalDataType}; + +/// Special value indicating that nodata is not set for a VRT source. +/// Matches `VRT_NODATA_UNSET` from GDAL's `gdal_vrt.h`. +pub const NODATA_UNSET: f64 = -1234.56; + +/// A VRT (Virtual Raster) dataset. +pub struct VrtDataset { + dataset: Dataset, +} + +unsafe impl Send for VrtDataset {} + Review Comment: `VrtDataset` should already be `Send` automatically because it only contains `Dataset` (which has its own `unsafe impl Send`). The explicit `unsafe impl Send for VrtDataset {}` is redundant and also breaks the local convention of adding a SAFETY justification comment for unsafe auto-trait impls. Consider removing this `unsafe impl` entirely (preferred) or adding a SAFETY comment explaining why it’s sound. ```suggestion ``` ########## c/sedona-gdal/src/raster/rasterize_affine.rs: ########## @@ -0,0 +1,362 @@ +// 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. + +//! Fast affine-transformer rasterize wrapper. +//! +//! GDALRasterizeGeometries() will internally call GDALCreateGenImgProjTransformer2() +//! if pfnTransformer is NULL, even in the common case where only a GeoTransform-based +//! affine conversion from georeferenced coords to pixel/line is needed. +//! +//! This module supplies a minimal GDALTransformerFunc that applies the dataset +//! GeoTransform (and its inverse), avoiding expensive transformer creation. + +use std::ffi::{c_int, c_void}; +use std::ptr; + +use crate::dataset::Dataset; +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::CE_None; +use crate::geo_transform::{GeoTransform, GeoTransformEx}; +use crate::vector::geometry::Geometry; + +#[repr(C)] +struct AffineTransformArg { + gt: GeoTransform, + inv_gt: GeoTransform, +} + +unsafe extern "C" fn affine_transformer( + p_transformer_arg: *mut c_void, + b_dst_to_src: c_int, + n_point_count: c_int, + x: *mut f64, + y: *mut f64, + _z: *mut f64, + pan_success: *mut c_int, +) -> c_int { + if p_transformer_arg.is_null() || x.is_null() || y.is_null() || pan_success.is_null() { + return 0; + } + if n_point_count < 0 { + return 0; + } + + // Treat transformer arg as immutable. + let arg = &*(p_transformer_arg as *const AffineTransformArg); + let affine = if b_dst_to_src == 0 { + &arg.inv_gt + } else { + &arg.gt + }; + + let n = n_point_count as usize; + for i in 0..n { + // SAFETY: x/y/pan_success are assumed to point to arrays of length n_point_count. + let xin = unsafe { *x.add(i) }; + let yin = unsafe { *y.add(i) }; + let (xout, yout) = affine.apply(xin, yin); + unsafe { + *x.add(i) = xout; + *y.add(i) = yout; + *pan_success.add(i) = 1; + } + } + + 1 +} + +/// Rasterize geometries using the dataset geotransform as the transformer. +/// Geometry coordinates must already be in the dataset georeferenced coordinate space. +pub fn rasterize_affine( + api: &'static GdalApi, + dataset: &Dataset, + bands: &[usize], + geometries: &[Geometry], + burn_values: &[f64], + all_touched: bool, +) -> Result<()> { + if bands.is_empty() { + return Err(GdalError::BadArgument( + "`bands` must not be empty".to_string(), + )); + } + if burn_values.len() != geometries.len() { + return Err(GdalError::BadArgument(format!( + "Burn values length ({}) must match geometries length ({})", + burn_values.len(), + geometries.len() + ))); + } + + let raster_count = dataset.raster_count(); + for band in bands { + let is_good = *band > 0 && *band <= raster_count; + if !is_good { + return Err(GdalError::BadArgument(format!( + "Band index {} is out of bounds", + *band + ))); + } + } + + let bands_i32: Vec<c_int> = bands.iter().map(|&band| band as c_int).collect(); + + let c_options = if all_touched { + [c"ALL_TOUCHED=TRUE".as_ptr(), ptr::null_mut()] + } else { + [c"ALL_TOUCHED=FALSE".as_ptr(), ptr::null_mut()] + }; Review Comment: `bands_i32` is built with `band as c_int`, which can silently truncate on platforms where `usize` is wider than `c_int`. Since the function already validates band bounds, it would be safer to convert each band with `try_into()` (and return `BadArgument` on overflow) rather than using `as` casts. ########## c/sedona-gdal/src/vrt.rs: ########## @@ -0,0 +1,326 @@ +// 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. + +//! GDAL VRT (Virtual Raster) API wrappers. + +use std::ffi::CString; +use std::ops::{Deref, DerefMut}; +use std::ptr::null_mut; + +use crate::cpl::CslStringList; +use crate::dataset::Dataset; +use crate::errors::Result; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::raster::rasterband::RasterBand; +use crate::{gdal_dyn_bindgen::*, raster::types::GdalDataType}; + +/// Special value indicating that nodata is not set for a VRT source. +/// Matches `VRT_NODATA_UNSET` from GDAL's `gdal_vrt.h`. +pub const NODATA_UNSET: f64 = -1234.56; + +/// A VRT (Virtual Raster) dataset. +pub struct VrtDataset { + dataset: Dataset, +} + +unsafe impl Send for VrtDataset {} + +impl VrtDataset { + /// Create an empty VRT dataset with the given raster size. + pub fn create(api: &'static GdalApi, x_size: usize, y_size: usize) -> Result<Self> { + let x: i32 = x_size.try_into()?; + let y: i32 = y_size.try_into()?; + let c_dataset = unsafe { call_gdal_api!(api, VRTCreate, x, y) }; + + if c_dataset.is_null() { + return Err(api.last_null_pointer_err("VRTCreate")); + } + + Ok(VrtDataset { + dataset: Dataset::new(api, c_dataset), + }) + } + + /// Return the underlying `Dataset`, transferring ownership. + pub fn as_dataset(self) -> Dataset { + let VrtDataset { dataset } = self; + dataset + } + + /// Add a band to this VRT dataset. + /// Return the 1-based index of the new band. + pub fn add_band(&mut self, data_type: GdalDataType, options: Option<&[&str]>) -> Result<usize> { + let csl = CslStringList::try_from_iter(options.unwrap_or(&[]).iter().copied())?; + + // Preserve null semantics: pass null when no options given. + let opts_ptr = if csl.is_empty() { + null_mut() + } else { + csl.as_ptr() + }; + + let rv = unsafe { + call_gdal_api!( + self.dataset.api(), + GDALAddBand, + self.dataset.c_dataset(), + data_type.to_c(), + opts_ptr + ) + }; + + if rv != CE_None { + return Err(self.dataset.api().last_cpl_err(rv as u32)); + } + + Ok(self.raster_count()) + } + + /// Fetch a VRT band by 1-indexed band number. + pub fn rasterband(&self, band_index: usize) -> Result<VrtRasterBand<'_>> { + let band = self.dataset.rasterband(band_index)?; + Ok(VrtRasterBand { band }) + } +} + +impl Deref for VrtDataset { + type Target = Dataset; + + fn deref(&self) -> &Self::Target { + &self.dataset + } +} + +impl DerefMut for VrtDataset { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.dataset + } +} + +impl AsRef<Dataset> for VrtDataset { + fn as_ref(&self) -> &Dataset { + &self.dataset + } +} + +/// A raster band within a VRT dataset. +pub struct VrtRasterBand<'a> { + band: RasterBand<'a>, +} + +impl<'a> VrtRasterBand<'a> { + /// Return the raw GDAL raster band handle. + pub fn c_rasterband(&self) -> GDALRasterBandH { + self.band.c_rasterband() + } + + /// Add a simple source to this VRT band. + /// Map a source window to a destination window, with optional resampling and nodata. + pub fn add_simple_source( + &self, + source_band: &RasterBand<'a>, + src_window: (i32, i32, i32, i32), + dst_window: (i32, i32, i32, i32), + resampling: Option<&str>, + nodata: Option<f64>, + ) -> Result<()> { + let c_resampling = resampling.map(CString::new).transpose()?; + + let resampling_ptr = c_resampling + .as_ref() + .map(|s| s.as_ptr()) + .unwrap_or(null_mut()); + Review Comment: `resampling_ptr` is passed to GDAL as a `*const c_char`, but the fallback uses `null_mut()`. Using `std::ptr::null()` (or importing `null`) keeps the pointer const-correct and avoids casting a mutable null pointer to const. ########## c/sedona-gdal/src/raster/rasterize_affine.rs: ########## @@ -0,0 +1,362 @@ +// 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. + +//! Fast affine-transformer rasterize wrapper. +//! +//! GDALRasterizeGeometries() will internally call GDALCreateGenImgProjTransformer2() +//! if pfnTransformer is NULL, even in the common case where only a GeoTransform-based +//! affine conversion from georeferenced coords to pixel/line is needed. +//! +//! This module supplies a minimal GDALTransformerFunc that applies the dataset +//! GeoTransform (and its inverse), avoiding expensive transformer creation. + +use std::ffi::{c_int, c_void}; +use std::ptr; + +use crate::dataset::Dataset; +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::CE_None; +use crate::geo_transform::{GeoTransform, GeoTransformEx}; +use crate::vector::geometry::Geometry; + +#[repr(C)] +struct AffineTransformArg { + gt: GeoTransform, + inv_gt: GeoTransform, +} + +unsafe extern "C" fn affine_transformer( + p_transformer_arg: *mut c_void, + b_dst_to_src: c_int, + n_point_count: c_int, + x: *mut f64, + y: *mut f64, + _z: *mut f64, + pan_success: *mut c_int, +) -> c_int { + if p_transformer_arg.is_null() || x.is_null() || y.is_null() || pan_success.is_null() { + return 0; + } + if n_point_count < 0 { + return 0; + } + + // Treat transformer arg as immutable. + let arg = &*(p_transformer_arg as *const AffineTransformArg); + let affine = if b_dst_to_src == 0 { + &arg.inv_gt + } else { + &arg.gt + }; + + let n = n_point_count as usize; + for i in 0..n { + // SAFETY: x/y/pan_success are assumed to point to arrays of length n_point_count. + let xin = unsafe { *x.add(i) }; + let yin = unsafe { *y.add(i) }; + let (xout, yout) = affine.apply(xin, yin); + unsafe { + *x.add(i) = xout; + *y.add(i) = yout; + *pan_success.add(i) = 1; + } + } + + 1 +} + +/// Rasterize geometries using the dataset geotransform as the transformer. +/// Geometry coordinates must already be in the dataset georeferenced coordinate space. +pub fn rasterize_affine( + api: &'static GdalApi, + dataset: &Dataset, + bands: &[usize], + geometries: &[Geometry], + burn_values: &[f64], + all_touched: bool, +) -> Result<()> { + if bands.is_empty() { + return Err(GdalError::BadArgument( + "`bands` must not be empty".to_string(), + )); + } + if burn_values.len() != geometries.len() { + return Err(GdalError::BadArgument(format!( + "Burn values length ({}) must match geometries length ({})", + burn_values.len(), + geometries.len() + ))); + } + + let raster_count = dataset.raster_count(); + for band in bands { + let is_good = *band > 0 && *band <= raster_count; + if !is_good { + return Err(GdalError::BadArgument(format!( + "Band index {} is out of bounds", + *band + ))); + } + } + + let bands_i32: Vec<c_int> = bands.iter().map(|&band| band as c_int).collect(); + + let c_options = if all_touched { + [c"ALL_TOUCHED=TRUE".as_ptr(), ptr::null_mut()] + } else { + [c"ALL_TOUCHED=FALSE".as_ptr(), ptr::null_mut()] + }; + + let geometries_c: Vec<_> = geometries.iter().map(|geo| geo.c_geometry()).collect(); + let burn_values_expanded: Vec<f64> = burn_values + .iter() + .flat_map(|burn| std::iter::repeat_n(burn, bands_i32.len())) + .copied() + .collect(); + + let gt = dataset.geo_transform().map_err(|_e| { + GdalError::BadArgument( + "Missing geotransform: only geotransform-based affine rasterize is supported" + .to_string(), + ) + })?; + let inv_gt = gt.invert().map_err(|_e| { + GdalError::BadArgument( + "Non-invertible geotransform: only geotransform-based affine rasterize is supported" + .to_string(), + ) + })?; + let mut arg = AffineTransformArg { gt, inv_gt }; + + unsafe { + let error = call_gdal_api!( + api, + GDALRasterizeGeometries, + dataset.c_dataset(), + bands_i32.len() as c_int, + bands_i32.as_ptr(), + geometries_c.len() as c_int, + geometries_c.as_ptr(), Review Comment: Passing `affine_transformer` into the binding’s `pfnTransformer: *mut c_void` relies on casting a Rust function pointer to a data pointer. That representation is not guaranteed to be portable on all targets. Please add an explicit SAFETY comment at the cast explaining the ABI assumption, and consider updating the generated binding to use GDAL’s actual transformer function-pointer type so this doesn’t depend on `fn`-ptr ↔ `void*` casts. ```suggestion geometries_c.as_ptr(), // SAFETY: The GDAL C API expects `pfnTransformer` to be a pointer to a // `GDALTransformerFunc`-compatible C function. `affine_transformer` is // defined with a compatible `extern "C"` ABI, and on supported targets // function pointers are represented in a way that allows them to be // passed through a `void *` field in this binding, as done by GDAL. // We rely on that platform ABI equivalence here when casting the Rust // function pointer to `*mut c_void`. ``` ########## c/sedona-gdal/src/raster/rasterize_affine.rs: ########## @@ -0,0 +1,362 @@ +// 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. + +//! Fast affine-transformer rasterize wrapper. +//! +//! GDALRasterizeGeometries() will internally call GDALCreateGenImgProjTransformer2() +//! if pfnTransformer is NULL, even in the common case where only a GeoTransform-based +//! affine conversion from georeferenced coords to pixel/line is needed. +//! +//! This module supplies a minimal GDALTransformerFunc that applies the dataset +//! GeoTransform (and its inverse), avoiding expensive transformer creation. + +use std::ffi::{c_int, c_void}; +use std::ptr; + +use crate::dataset::Dataset; +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::CE_None; +use crate::geo_transform::{GeoTransform, GeoTransformEx}; +use crate::vector::geometry::Geometry; + +#[repr(C)] +struct AffineTransformArg { + gt: GeoTransform, + inv_gt: GeoTransform, +} + +unsafe extern "C" fn affine_transformer( + p_transformer_arg: *mut c_void, + b_dst_to_src: c_int, + n_point_count: c_int, + x: *mut f64, + y: *mut f64, + _z: *mut f64, + pan_success: *mut c_int, +) -> c_int { + if p_transformer_arg.is_null() || x.is_null() || y.is_null() || pan_success.is_null() { + return 0; + } + if n_point_count < 0 { + return 0; + } + + // Treat transformer arg as immutable. + let arg = &*(p_transformer_arg as *const AffineTransformArg); + let affine = if b_dst_to_src == 0 { + &arg.inv_gt + } else { + &arg.gt + }; + + let n = n_point_count as usize; + for i in 0..n { + // SAFETY: x/y/pan_success are assumed to point to arrays of length n_point_count. + let xin = unsafe { *x.add(i) }; + let yin = unsafe { *y.add(i) }; + let (xout, yout) = affine.apply(xin, yin); + unsafe { + *x.add(i) = xout; + *y.add(i) = yout; + *pan_success.add(i) = 1; + } + } + + 1 +} + +/// Rasterize geometries using the dataset geotransform as the transformer. +/// Geometry coordinates must already be in the dataset georeferenced coordinate space. +pub fn rasterize_affine( + api: &'static GdalApi, + dataset: &Dataset, + bands: &[usize], + geometries: &[Geometry], + burn_values: &[f64], + all_touched: bool, +) -> Result<()> { + if bands.is_empty() { + return Err(GdalError::BadArgument( + "`bands` must not be empty".to_string(), + )); + } + if burn_values.len() != geometries.len() { + return Err(GdalError::BadArgument(format!( + "Burn values length ({}) must match geometries length ({})", + burn_values.len(), + geometries.len() + ))); + } + + let raster_count = dataset.raster_count(); + for band in bands { + let is_good = *band > 0 && *band <= raster_count; + if !is_good { + return Err(GdalError::BadArgument(format!( + "Band index {} is out of bounds", + *band + ))); + } + } + + let bands_i32: Vec<c_int> = bands.iter().map(|&band| band as c_int).collect(); + + let c_options = if all_touched { + [c"ALL_TOUCHED=TRUE".as_ptr(), ptr::null_mut()] + } else { + [c"ALL_TOUCHED=FALSE".as_ptr(), ptr::null_mut()] + }; + + let geometries_c: Vec<_> = geometries.iter().map(|geo| geo.c_geometry()).collect(); + let burn_values_expanded: Vec<f64> = burn_values + .iter() + .flat_map(|burn| std::iter::repeat_n(burn, bands_i32.len())) + .copied() + .collect(); + + let gt = dataset.geo_transform().map_err(|_e| { + GdalError::BadArgument( + "Missing geotransform: only geotransform-based affine rasterize is supported" + .to_string(), + ) + })?; + let inv_gt = gt.invert().map_err(|_e| { + GdalError::BadArgument( + "Non-invertible geotransform: only geotransform-based affine rasterize is supported" + .to_string(), + ) + })?; + let mut arg = AffineTransformArg { gt, inv_gt }; + + unsafe { + let error = call_gdal_api!( + api, + GDALRasterizeGeometries, + dataset.c_dataset(), + bands_i32.len() as c_int, + bands_i32.as_ptr(), + geometries_c.len() as c_int, + geometries_c.as_ptr(), + (affine_transformer as *const ()).cast::<c_void>() as *mut c_void, + (&mut arg as *mut AffineTransformArg).cast::<c_void>(), + burn_values_expanded.as_ptr(), + c_options.as_ptr() as *mut *mut i8, + ptr::null_mut(), Review Comment: `c_options.as_ptr() as *mut *mut i8` hard-codes `i8` instead of `c_char`, which is incorrect on targets where `c_char` is `u8` (and also forces an extra cast). Prefer building a `CslStringList` like `rasterize.rs` does, or at least use `*mut *mut c_char` throughout so the FFI signature matches GDAL on all platforms. -- 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]
