Copilot commented on code in PR #699: URL: https://github.com/apache/sedona-db/pull/699#discussion_r2955220247
########## c/sedona-gdal/src/dataset.rs: ########## @@ -0,0 +1,462 @@ +// 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/dataset.rs>. +//! Original code is licensed under MIT. + +use std::ffi::{CStr, CString}; +use std::ptr; + +use crate::cpl::CslStringList; +use crate::driver::Driver; +use crate::errors::Result; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::*; +use crate::raster::rasterband::RasterBand; +use crate::raster::types::{DatasetOptions, GdalDataType as RustGdalDataType}; +use crate::spatial_ref::SpatialRef; +use crate::vector::layer::Layer; + +/// A GDAL dataset. +pub struct Dataset { + api: &'static GdalApi, + c_dataset: GDALDatasetH, + owned: bool, +} + +unsafe impl Send for Dataset {} + +impl Drop for Dataset { + fn drop(&mut self) { + if self.owned && !self.c_dataset.is_null() { + unsafe { call_gdal_api!(self.api, GDALClose, self.c_dataset) }; + } + } +} + +impl Dataset { + /// Open a dataset with extended options. + pub fn open_ex( + api: &'static GdalApi, + path: &str, + open_flags: GDALOpenFlags, + allowed_drivers: Option<&[&str]>, + open_options: Option<&[&str]>, + sibling_files: Option<&[&str]>, + ) -> Result<Self> { + let c_path = CString::new(path)?; + + // Build CslStringLists from Option<&[&str]>. + // None → null pointer (use GDAL default). + // Some(&[]) → pointer to [null] (explicitly empty list). + let drivers_csl = allowed_drivers + .map(|v| CslStringList::try_from_iter(v.iter().copied())) + .transpose()?; + let options_csl = open_options + .map(|v| CslStringList::try_from_iter(v.iter().copied())) + .transpose()?; + let siblings_csl = sibling_files + .map(|v| CslStringList::try_from_iter(v.iter().copied())) + .transpose()?; + + let c_dataset = unsafe { + call_gdal_api!( + api, + GDALOpenEx, + c_path.as_ptr(), + open_flags, + drivers_csl + .as_ref() + .map_or(ptr::null(), |csl| csl.as_ptr() as *const *const _), + options_csl + .as_ref() + .map_or(ptr::null(), |csl| csl.as_ptr() as *const *const _), + siblings_csl + .as_ref() + .map_or(ptr::null(), |csl| csl.as_ptr() as *const *const _) + ) + }; + + if c_dataset.is_null() { + return Err(api.last_cpl_err(CE_Failure as u32)); + } + + Ok(Self { + api, + c_dataset, + owned: true, + }) + } + + /// Create a new owned Dataset from a C handle. + pub(crate) fn new_owned(api: &'static GdalApi, c_dataset: GDALDatasetH) -> Self { + Self { + api, + c_dataset, + owned: true, + } + } + + /// Wrap an existing C dataset handle (non-owning). + /// + /// # Safety + /// + /// The caller must ensure the handle is valid and outlives this `Dataset`. + pub unsafe fn from_c_dataset(api: &'static GdalApi, c_dataset: GDALDatasetH) -> Self { + Self { + api, + c_dataset, + owned: false, + } + } + + /// Return the raw C dataset handle. + pub fn c_dataset(&self) -> GDALDatasetH { + self.c_dataset + } + + /// Return raster size as (x_size, y_size). + pub fn raster_size(&self) -> (usize, usize) { + let x = unsafe { call_gdal_api!(self.api, GDALGetRasterXSize, self.c_dataset) }; + let y = unsafe { call_gdal_api!(self.api, GDALGetRasterYSize, self.c_dataset) }; + (x as usize, y as usize) + } + + /// Return the number of raster bands. + pub fn raster_count(&self) -> usize { + unsafe { call_gdal_api!(self.api, GDALGetRasterCount, self.c_dataset) as usize } + } + + /// Get a raster band (1-indexed). + pub fn rasterband(&self, band_index: usize) -> Result<RasterBand<'_>> { + let band_index_i32 = i32::try_from(band_index)?; + let c_band = + unsafe { call_gdal_api!(self.api, GDALGetRasterBand, self.c_dataset, band_index_i32) }; + if c_band.is_null() { + return Err(self.api.last_null_pointer_err("GDALGetRasterBand")); + } + Ok(RasterBand::new(self.api, c_band, self)) + } + + /// Get the geo-transform. + pub fn geo_transform(&self) -> Result<[f64; 6]> { + let mut gt = [0.0f64; 6]; + let rv = unsafe { + call_gdal_api!( + self.api, + GDALGetGeoTransform, + self.c_dataset, + gt.as_mut_ptr() + ) + }; + if rv != CE_None { + return Err(self.api.last_cpl_err(rv as u32)); + } + Ok(gt) + } + + /// Set the geo-transform. + pub fn set_geo_transform(&self, gt: &[f64; 6]) -> Result<()> { + let rv = unsafe { + call_gdal_api!( + self.api, + GDALSetGeoTransform, + self.c_dataset, + gt.as_ptr() as *mut f64 Review Comment: `set_geo_transform` casts an immutable `&[f64; 6]` to `*mut f64` to satisfy the FFI signature. If GDAL writes to this buffer (the parameter type is mutable in the binding), this is undefined behavior because the data is borrowed immutably. Prefer taking `&mut [f64; 6]` or copying into a local mutable array and passing `as_mut_ptr()`. ########## c/sedona-gdal/src/dataset.rs: ########## @@ -0,0 +1,462 @@ +// 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/dataset.rs>. +//! Original code is licensed under MIT. + +use std::ffi::{CStr, CString}; +use std::ptr; + +use crate::cpl::CslStringList; +use crate::driver::Driver; +use crate::errors::Result; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::*; +use crate::raster::rasterband::RasterBand; +use crate::raster::types::{DatasetOptions, GdalDataType as RustGdalDataType}; +use crate::spatial_ref::SpatialRef; +use crate::vector::layer::Layer; + +/// A GDAL dataset. +pub struct Dataset { + api: &'static GdalApi, + c_dataset: GDALDatasetH, + owned: bool, +} + Review Comment: This `unsafe impl Send for Dataset` has no accompanying safety rationale, while other wrappers (e.g., `SpatialRef`, `Geometry`, `VSIBuffer`) document why `Send` is sound and why `Sync` is not implemented. Adding a short SAFETY comment here would help reviewers/users understand the thread-safety assumptions around `GDALDatasetH` and the `owned` flag. ########## c/sedona-gdal/src/vector/layer.rs: ########## @@ -0,0 +1,118 @@ +// 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/vector/layer.rs>. +//! Original code is licensed under MIT. + +use std::marker::PhantomData; + +use crate::dataset::Dataset; +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::*; +use crate::vector::feature::{Feature, FieldDefn}; + +/// An OGR layer (borrowed from a Dataset). +pub struct Layer<'a> { + api: &'static GdalApi, + c_layer: OGRLayerH, + _dataset: PhantomData<&'a Dataset>, +} + +impl<'a> Layer<'a> { + pub(crate) fn new(api: &'static GdalApi, c_layer: OGRLayerH, _dataset: &'a Dataset) -> Self { + Self { + api, + c_layer, + _dataset: PhantomData, + } + } + + /// Return the raw C layer handle. + pub fn c_layer(&self) -> OGRLayerH { + self.c_layer + } + + /// Reset reading to the first feature. + pub fn reset_reading(&self) { + unsafe { call_gdal_api!(self.api, OGR_L_ResetReading, self.c_layer) }; + } + + /// Get the next feature (returns None when exhausted). + pub fn next_feature(&self) -> Option<Feature<'_>> { + let c_feature = unsafe { call_gdal_api!(self.api, OGR_L_GetNextFeature, self.c_layer) }; + if c_feature.is_null() { + None + } else { + Some(Feature::new(self.api, c_feature)) + } + } + + /// Create a field on this layer. + pub fn create_field(&self, field_defn: &FieldDefn) -> Result<()> { + let rv = unsafe { + call_gdal_api!( + self.api, + OGR_L_CreateField, + self.c_layer, + field_defn.c_field_defn(), + 1 // bApproxOK + ) + }; + if rv != OGRERR_NONE { + return Err(GdalError::OgrError { + err: rv, + method_name: "OGR_L_CreateField", + }); + } + Ok(()) + } + + /// Get the number of features in this layer. + /// + /// If `force` is true, the count will be computed even if it is expensive. + pub fn feature_count(&self, force: bool) -> i64 { + unsafe { + call_gdal_api!( + self.api, + OGR_L_GetFeatureCount, + self.c_layer, + if force { 1 } else { 0 } + ) + } + } + + /// Iterate over all features. + pub fn features(&self) -> FeatureIterator<'_> { + self.reset_reading(); + FeatureIterator { layer: self } + } +} + +/// Iterator over features in a layer. +pub struct FeatureIterator<'a> { + layer: &'a Layer<'a>, +} + +impl<'a> Iterator for FeatureIterator<'a> { + type Item = Feature<'a>; Review Comment: `FeatureIterator` ties the borrow lifetime of `&Layer` to the dataset lifetime parameter (`layer: &'a Layer<'a>`). This is overly restrictive and is likely to fail to compile (or prevent valid uses) because the iterator borrow (`&self`) can be shorter than the dataset borrow that produced `Layer<'dataset>`. Consider giving the iterator two lifetimes (e.g., one for the iterator borrow and one for the dataset) or redesigning so `Feature`/iterator don't require matching lifetimes. ########## c/sedona-gdal/src/raster/rasterband.rs: ########## @@ -0,0 +1,368 @@ +// 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/raster/rasterband.rs>. +//! Original code is licensed under MIT. + +use std::marker::PhantomData; + +use crate::dataset::Dataset; +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::raster::types::{Buffer, GdalType, ResampleAlg}; +use crate::{gdal_dyn_bindgen::*, raster::types::GdalDataType}; + +/// A raster band of a dataset. +pub struct RasterBand<'a> { + api: &'static GdalApi, + c_rasterband: GDALRasterBandH, + _dataset: PhantomData<&'a Dataset>, +} + +impl<'a> RasterBand<'a> { + pub(crate) fn new( + api: &'static GdalApi, + c_rasterband: GDALRasterBandH, + _dataset: &'a Dataset, + ) -> Self { + Self { + api, + c_rasterband, + _dataset: PhantomData, + } + } + + /// Return the raw C raster band handle. + pub fn c_rasterband(&self) -> GDALRasterBandH { + self.c_rasterband + } + + /// Read a region of the band as a typed buffer. + /// + /// If `e_resample_alg` is `None`, nearest-neighbour resampling is used. + pub fn read_as<T: GdalType + Copy>( + &self, + window: (isize, isize), + window_size: (usize, usize), + size: (usize, usize), + e_resample_alg: Option<ResampleAlg>, + ) -> Result<Buffer<T>> { + let len = size.0 * size.1; + // Safety: all GdalType implementations are numeric primitives (u8, i8, u16, ..., f64), + // for which zeroed memory is a valid bit pattern. + let mut data: Vec<T> = vec![unsafe { std::mem::zeroed() }; len]; + + let resample_alg = e_resample_alg.unwrap_or(ResampleAlg::NearestNeighbour); + let mut extra_arg = GDALRasterIOExtraArg { + eResampleAlg: resample_alg.to_gdal(), + ..GDALRasterIOExtraArg::default() + }; + + let rv = unsafe { + call_gdal_api!( + self.api, + GDALRasterIOEx, + self.c_rasterband, + GF_Read, + i32::try_from(window.0)?, + i32::try_from(window.1)?, + i32::try_from(window_size.0)?, + i32::try_from(window_size.1)?, + data.as_mut_ptr() as *mut std::ffi::c_void, + i32::try_from(size.0)?, + i32::try_from(size.1)?, + T::gdal_ordinal(), + 0, // nPixelSpace (auto) + 0, // nLineSpace (auto) + &mut extra_arg + ) + }; + if rv != CE_None { + return Err(self.api.last_cpl_err(rv as u32)); + } + + Ok(Buffer::new(size, data)) + } + + /// Write a buffer to this raster band. + pub fn write<T: GdalType + Copy>( + &self, + window: (isize, isize), + window_size: (usize, usize), + buffer: &mut Buffer<T>, + ) -> Result<()> { + let expected_len = buffer.shape.0 * buffer.shape.1; + if buffer.data.len() != expected_len { + return Err(GdalError::BufferSizeMismatch( + buffer.data.len(), + buffer.shape, + )); + } + let rv = unsafe { + call_gdal_api!( + self.api, + GDALRasterIO, + self.c_rasterband, + GF_Write, + i32::try_from(window.0)?, + i32::try_from(window.1)?, + i32::try_from(window_size.0)?, + i32::try_from(window_size.1)?, + buffer.data.as_mut_ptr() as *mut std::ffi::c_void, + i32::try_from(buffer.shape.0)?, + i32::try_from(buffer.shape.1)?, + T::gdal_ordinal(), + 0, // nPixelSpace (auto) + 0 // nLineSpace (auto) + ) + }; + if rv != CE_None { + return Err(self.api.last_cpl_err(rv as u32)); + } + Ok(()) + } + + /// Get the data type of this band. + pub fn band_type(&self) -> GdalDataType { + GdalDataType::from_c(self.c_band_type()).unwrap_or(GdalDataType::Unknown) + } + + /// Get the GDAL data type of this band. + pub fn c_band_type(&self) -> GDALDataType { + unsafe { call_gdal_api!(self.api, GDALGetRasterDataType, self.c_rasterband) } + } + + /// Get band size as (x_size, y_size). + pub fn size(&self) -> (usize, usize) { + let x = unsafe { call_gdal_api!(self.api, GDALGetRasterBandXSize, self.c_rasterband) }; + let y = unsafe { call_gdal_api!(self.api, GDALGetRasterBandYSize, self.c_rasterband) }; + (x as usize, y as usize) + } + + /// Get the block size as (x_size, y_size). + pub fn block_size(&self) -> (usize, usize) { + let mut x: i32 = 0; + let mut y: i32 = 0; + unsafe { + call_gdal_api!( + self.api, + GDALGetBlockSize, + self.c_rasterband, + &mut x, + &mut y + ) + }; + (x as usize, y as usize) + } + + /// Get the no-data value. Returns `Some(value)` if set, `None` otherwise. + pub fn no_data_value(&self) -> Option<f64> { + let mut success: i32 = 0; + let value = unsafe { + call_gdal_api!( + self.api, + GDALGetRasterNoDataValue, + self.c_rasterband, + &mut success + ) + }; + if success != 0 { + Some(value) + } else { + None + } + } + + /// Set or clear the no-data value. + pub fn set_no_data_value(&self, value: Option<f64>) -> Result<()> { + let rv = if let Some(val) = value { + unsafe { call_gdal_api!(self.api, GDALSetRasterNoDataValue, self.c_rasterband, val) } + } else { + unsafe { call_gdal_api!(self.api, GDALDeleteRasterNoDataValue, self.c_rasterband) } + }; + if rv != CE_None { + return Err(self.api.last_cpl_err(rv as u32)); + } + Ok(()) + } + + /// Set or clear the no-data value as u64. + pub fn set_no_data_value_u64(&self, value: Option<u64>) -> Result<()> { + let rv = if let Some(val) = value { + unsafe { + call_gdal_api!( + self.api, + GDALSetRasterNoDataValueAsUInt64, + self.c_rasterband, + val + ) + } + } else { + unsafe { call_gdal_api!(self.api, GDALDeleteRasterNoDataValue, self.c_rasterband) } + }; + if rv != CE_None { + return Err(self.api.last_cpl_err(rv as u32)); + } + Ok(()) + } + + /// Set or clear the no-data value as i64. + pub fn set_no_data_value_i64(&self, value: Option<i64>) -> Result<()> { + let rv = if let Some(val) = value { + unsafe { + call_gdal_api!( + self.api, + GDALSetRasterNoDataValueAsInt64, + self.c_rasterband, + val + ) + } + } else { + unsafe { call_gdal_api!(self.api, GDALDeleteRasterNoDataValue, self.c_rasterband) } + }; + if rv != CE_None { + return Err(self.api.last_cpl_err(rv as u32)); + } + Ok(()) + } + + /// Get the GDAL API reference. + pub fn api(&self) -> &'static GdalApi { + self.api + } +} + +/// Compute the actual block size (clamped to raster extent) for a given block index. +pub fn actual_block_size( + band: &RasterBand<'_>, + block_index: (usize, usize), +) -> Result<(usize, usize)> { + let (block_x, block_y) = band.block_size(); + let (raster_x, raster_y) = band.size(); + let x_off = block_index.0 * block_x; + let y_off = block_index.1 * block_y; + if x_off >= raster_x || y_off >= raster_y { + return Err(GdalError::BadArgument(format!( + "block index ({}, {}) is out of bounds for raster size ({}, {})", + block_index.0, block_index.1, raster_x, raster_y + ))); + } + let actual_x = if x_off + block_x > raster_x { + raster_x - x_off + } else { + block_x + }; + let actual_y = if y_off + block_y > raster_y { + raster_y - y_off + } else { + block_y + }; + Ok((actual_x, actual_y)) +} + +#[cfg(all(test, feature = "gdal-sys"))] +mod tests { + use crate::dataset::Dataset; + use crate::driver::DriverManager; + use crate::gdal_dyn_bindgen::*; + use crate::global::with_global_gdal_api; + use crate::raster::types::ResampleAlg; + + fn fixture(name: &str) -> String { + sedona_testing::data::test_raster(name).unwrap() + } + + #[test] + fn test_read_raster() { + with_global_gdal_api(|api| { + let path = fixture("tinymarble.tif"); + let dataset = Dataset::open_ex(api, &path, GDAL_OF_READONLY, None, None, None).unwrap(); + let rb = dataset.rasterband(1).unwrap(); + let rv = rb.read_as::<u8>((20, 30), (2, 3), (2, 3), None).unwrap(); + assert_eq!(rv.shape, (2, 3)); + assert_eq!(rv.data(), [7, 7, 7, 10, 8, 12]); + }) + .unwrap(); + } + + #[test] + fn test_read_raster_with_default_resample() { + with_global_gdal_api(|api| { + let path = fixture("tinymarble.tif"); + let dataset = Dataset::open_ex(api, &path, GDAL_OF_READONLY, None, None, None).unwrap(); + let rb = dataset.rasterband(1).unwrap(); + let rv = rb.read_as::<u8>((20, 30), (4, 4), (2, 2), None).unwrap(); + assert_eq!(rv.shape, (2, 2)); + // Default is NearestNeighbour; exact values are GDAL-version-dependent + // when downsampling from 4x4 to 2x2. Just verify shape and non-emptiness. + assert_eq!(rv.data().len(), 4); + }) + .unwrap(); + } + + #[test] + fn test_read_raster_with_average_resample() { + with_global_gdal_api(|api| { + let path = fixture("tinymarble.tif"); + let dataset = Dataset::open_ex(api, &path, GDAL_OF_READONLY, None, None, None).unwrap(); + let rb = dataset.rasterband(1).unwrap(); + let rv = rb + .read_as::<u8>((20, 30), (4, 4), (2, 2), Some(ResampleAlg::Average)) + .unwrap(); + assert_eq!(rv.shape, (2, 2)); + // Average resampling; exact values are GDAL-version-dependent. + // Verify shape and that results differ from the non-resampled full read. + assert_eq!(rv.data().len(), 4); Review Comment: The comment says the average-resample case should "Verify shape and that results differ", but the test currently only checks the buffer length (same assertion as the previous test). Either adjust the comment to match what is asserted, or add an assertion that actually distinguishes the resampling behavior (in a way that’s robust across GDAL versions). ########## c/sedona-gdal/src/vector/layer.rs: ########## @@ -0,0 +1,118 @@ +// 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/vector/layer.rs>. +//! Original code is licensed under MIT. + +use std::marker::PhantomData; + +use crate::dataset::Dataset; +use crate::errors::{GdalError, Result}; +use crate::gdal_api::{call_gdal_api, GdalApi}; +use crate::gdal_dyn_bindgen::*; +use crate::vector::feature::{Feature, FieldDefn}; + +/// An OGR layer (borrowed from a Dataset). +pub struct Layer<'a> { + api: &'static GdalApi, + c_layer: OGRLayerH, + _dataset: PhantomData<&'a Dataset>, +} + +impl<'a> Layer<'a> { + pub(crate) fn new(api: &'static GdalApi, c_layer: OGRLayerH, _dataset: &'a Dataset) -> Self { + Self { + api, + c_layer, + _dataset: PhantomData, + } + } + + /// Return the raw C layer handle. + pub fn c_layer(&self) -> OGRLayerH { + self.c_layer + } + + /// Reset reading to the first feature. + pub fn reset_reading(&self) { + unsafe { call_gdal_api!(self.api, OGR_L_ResetReading, self.c_layer) }; + } + + /// Get the next feature (returns None when exhausted). + pub fn next_feature(&self) -> Option<Feature<'_>> { + let c_feature = unsafe { call_gdal_api!(self.api, OGR_L_GetNextFeature, self.c_layer) }; + if c_feature.is_null() { + None + } else { + Some(Feature::new(self.api, c_feature)) + } + } + + /// Create a field on this layer. + pub fn create_field(&self, field_defn: &FieldDefn) -> Result<()> { + let rv = unsafe { + call_gdal_api!( + self.api, + OGR_L_CreateField, + self.c_layer, + field_defn.c_field_defn(), + 1 // bApproxOK + ) + }; + if rv != OGRERR_NONE { + return Err(GdalError::OgrError { + err: rv, + method_name: "OGR_L_CreateField", + }); + } + Ok(()) + } + + /// Get the number of features in this layer. + /// + /// If `force` is true, the count will be computed even if it is expensive. + pub fn feature_count(&self, force: bool) -> i64 { + unsafe { + call_gdal_api!( + self.api, + OGR_L_GetFeatureCount, + self.c_layer, + if force { 1 } else { 0 } + ) + } + } + + /// Iterate over all features. + pub fn features(&self) -> FeatureIterator<'_> { + self.reset_reading(); + FeatureIterator { layer: self } + } +} Review Comment: New vector layer/feature wrappers are introduced here but there are no tests exercising basic iteration (`reset_reading`/`next_feature`/`features`) against an in-memory vector dataset. Adding a minimal round-trip test would help catch FFI/lifetime issues early, similar to the existing tests in `vector/geometry.rs`. -- 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]
