From 7db2868eecb0e1f8b76fa943fd70051242b372d1 Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Mon, 2 Oct 2023 15:43:25 -0400 Subject: [PATCH 1/4] Initial foreign-types prototype. --- .cargo/config.toml | 2 +- Cargo.toml | 1 + examples/rasterband.rs | 3 +- examples/read_write_ogr_datetime.rs | 2 +- fixtures/tinymarble.tif.aux.xml | 9 + src/dataset.rs | 9 +- src/gcp.rs | 10 +- src/metadata.rs | 2 +- src/raster/gcp.rs | 38 ++++ src/raster/mdarray.rs | 17 +- src/raster/rasterband.rs | 174 +++++++-------- src/raster/rasterize.rs | 6 +- src/raster/tests.rs | 14 +- src/spatial_ref/mod.rs | 6 +- src/spatial_ref/srs.rs | 155 ++++++-------- src/spatial_ref/transform.rs | 98 +++++---- src/spatial_ref/transform_opts.rs | 37 ++-- src/vector/defn.rs | 90 +++++--- src/vector/feature.rs | 218 ++++++++----------- src/vector/geometry.rs | 249 +++++++--------------- src/vector/layer.rs | 32 +-- src/vector/mod.rs | 25 ++- src/vector/ops/conversions/formats.rs | 21 +- src/vector/ops/conversions/gdal_to_geo.rs | 34 ++- src/vector/ops/conversions/mod.rs | 4 +- src/vector/ops/predicates.rs | 15 +- src/vector/ops/set.rs | 46 +--- src/vector/ops/transformations.rs | 55 +++-- src/vector/sql.rs | 3 +- 29 files changed, 637 insertions(+), 738 deletions(-) create mode 100644 src/raster/gcp.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index e2d4b0a38..8ecad75d1 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,5 +1,5 @@ [alias] -# Run doctests, displaying compiler output +# doc-test-output: Run doctests, displaying compiler output dto = "test --doc -- --show-output" # Run clippy, raising warnings to errors nowarn = "clippy --all-targets -- -D warnings" diff --git a/Cargo.toml b/Cargo.toml index 69c107149..b511934f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ ndarray = { version = "0.15", optional = true } chrono = { version = "0.4.26", default-features = false } bitflags = "2.4" once_cell = "1.18" +foreign-types = "0.5.0" [build-dependencies] semver = "1.0" diff --git a/examples/rasterband.rs b/examples/rasterband.rs index a0b3d847b..ae2d7355e 100644 --- a/examples/rasterband.rs +++ b/examples/rasterband.rs @@ -1,4 +1,3 @@ -use gdal::raster::RasterBand; use gdal::{Dataset, Metadata}; use std::path::Path; @@ -7,7 +6,7 @@ fn main() { let dataset = Dataset::open(path).unwrap(); println!("dataset description: {:?}", dataset.description()); - let rasterband: RasterBand = dataset.rasterband(1).unwrap(); + let rasterband = dataset.rasterband(1).unwrap(); println!("rasterband description: {:?}", rasterband.description()); println!("rasterband no_data_value: {:?}", rasterband.no_data_value()); println!("rasterband type: {:?}", rasterband.band_type()); diff --git a/examples/read_write_ogr_datetime.rs b/examples/read_write_ogr_datetime.rs index 3e49e5010..3269209a8 100644 --- a/examples/read_write_ogr_datetime.rs +++ b/examples/read_write_ogr_datetime.rs @@ -32,7 +32,7 @@ fn run() -> gdal::errors::Result<()> { for feature_a in layer_a.features() { let mut ft = Feature::new(&defn)?; if let Some(geom) = feature_a.geometry() { - ft.set_geometry(geom.clone())?; + ft.set_geometry(geom.to_owned())?; } // copy each field value of the feature: for field in defn.fields() { diff --git a/fixtures/tinymarble.tif.aux.xml b/fixtures/tinymarble.tif.aux.xml index 2b6d65176..8a3c1677d 100644 --- a/fixtures/tinymarble.tif.aux.xml +++ b/fixtures/tinymarble.tif.aux.xml @@ -1,3 +1,12 @@ GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","6326"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AXIS["Latitude",NORTH],AXIS["Longitude",EAST],AUTHORITY["EPSG","4326"]] + + + 255 + 68.4716 + 0 + 83.68444773935 + 100 + + diff --git a/src/dataset.rs b/src/dataset.rs index a3fa9c88a..e0cca19c3 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -1,3 +1,4 @@ +use foreign_types::ForeignTypeRef; use std::{ffi::CString, ffi::NulError, path::Path, ptr}; use gdal_sys::{self, CPLErr, GDALDatasetH, GDALMajorObjectH}; @@ -6,6 +7,7 @@ use crate::cpl::CslStringList; use crate::errors::*; use crate::options::DatasetOptions; use crate::raster::RasterCreationOption; +use crate::spatial_ref::SpatialRefRef; use crate::utils::{_last_cpl_err, _last_null_pointer_err, _path_to_c_string, _string}; use crate::{ gdal_major_object::MajorObject, spatial_ref::SpatialRef, Driver, GeoTransform, Metadata, @@ -224,13 +226,16 @@ impl Dataset { #[cfg(major_ge_3)] /// Get the spatial reference system for this dataset. pub fn spatial_ref(&self) -> Result { - unsafe { SpatialRef::from_c_obj(gdal_sys::GDALGetSpatialRef(self.c_dataset)) } + Ok( + unsafe { SpatialRefRef::from_ptr(gdal_sys::GDALGetSpatialRef(self.c_dataset)) } + .to_owned(), + ) } #[cfg(major_ge_3)] /// Set the spatial reference system for this dataset. pub fn set_spatial_ref(&mut self, spatial_ref: &SpatialRef) -> Result<()> { - let rv = unsafe { gdal_sys::GDALSetSpatialRef(self.c_dataset, spatial_ref.to_c_hsrs()) }; + let rv = unsafe { gdal_sys::GDALSetSpatialRef(self.c_dataset, spatial_ref.as_ptr()) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } diff --git a/src/gcp.rs b/src/gcp.rs index 2b8f23dfe..7aed3e2ab 100644 --- a/src/gcp.rs +++ b/src/gcp.rs @@ -6,9 +6,10 @@ use std::marker::PhantomData; use gdal_sys::CPLErr; use crate::errors::Result; -use crate::spatial_ref::SpatialRef; +use crate::spatial_ref::{SpatialRef, SpatialRefRef}; use crate::utils::{_last_cpl_err, _string}; use crate::Dataset; +use foreign_types::{ForeignType, ForeignTypeRef}; /// An owned Ground Control Point. #[derive(Clone, Debug, PartialEq, PartialOrd)] @@ -120,8 +121,9 @@ impl Dataset { if c_ptr.is_null() { return None; } - - unsafe { SpatialRef::from_c_obj(c_ptr) }.ok() + // NB: Creating a `SpatialRefRef` from a pointer acknowledges that GDAL is giving us + // a shared reference. We then call `to_owned` to appropriately get a clone. + Some(unsafe { SpatialRefRef::from_ptr(c_ptr) }.to_owned()) } /// Get the projection definition string for the GCPs in this dataset. @@ -207,7 +209,7 @@ impl Dataset { self.c_dataset(), len, gdal_gcps.as_ptr(), - spatial_ref.to_c_hsrs() as *mut _, + spatial_ref.as_ptr() as *mut _, ) }; diff --git a/src/metadata.rs b/src/metadata.rs index 67750eed8..41e4bdc25 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -382,7 +382,7 @@ mod tests { fn test_set_description() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); let dataset = driver.create("", 1, 1, 1).unwrap(); - let mut band = dataset.rasterband(1).unwrap(); + let band = dataset.rasterband(1).unwrap(); let description = "A merry and cheerful band description"; assert_eq!(band.description().unwrap(), ""); diff --git a/src/raster/gcp.rs b/src/raster/gcp.rs new file mode 100644 index 000000000..483ead158 --- /dev/null +++ b/src/raster/gcp.rs @@ -0,0 +1,38 @@ +//! Raster ground control point support + +use foreign_types::ForeignTypeRef; +use crate::spatial_ref::SpatialRefRef; +use crate::Dataset; + +impl Dataset { + /// Get output spatial reference system for GCPs. + /// + /// # Notes + /// * This is separate and distinct from [`Dataset::spatial_ref`], and only applies to + /// the representation of ground control points, when embedded. + /// + /// See: [`GDALGetGCPSpatialRef`](https://gdal.org/api/raster_c_api.html#_CPPv420GDALGetGCPSpatialRef12GDALDatasetH) + pub fn gcp_spatial_ref(&self) -> Option<&SpatialRefRef> { + let c_ptr = unsafe { gdal_sys::GDALGetGCPSpatialRef(self.c_dataset()) }; + + if c_ptr.is_null() { + return None; + } + + Some(unsafe { SpatialRefRef::from_ptr(c_ptr) }) + } +} + +#[cfg(test)] +mod tests { + use crate::test_utils::fixture; + use crate::Dataset; + + #[test] + fn test_gcp_spatial_ref() { + let dataset = Dataset::open(fixture("gcp.tif")).unwrap(); + let gcp_srs = dataset.gcp_spatial_ref(); + let auth = gcp_srs.and_then(|s| s.authority().ok()); + assert_eq!(auth.unwrap(), "EPSG:4326"); + } +} diff --git a/src/raster/mdarray.rs b/src/raster/mdarray.rs index 932951bb9..efc1e6028 100644 --- a/src/raster/mdarray.rs +++ b/src/raster/mdarray.rs @@ -1,6 +1,6 @@ use super::GdalType; use crate::errors::*; -use crate::spatial_ref::SpatialRef; +use crate::spatial_ref::{SpatialRef, SpatialRefRef}; use crate::utils::{_last_cpl_err, _last_null_pointer_err, _string, _string_array}; use crate::{cpl::CslStringList, Dataset}; use gdal_sys::{ @@ -16,12 +16,13 @@ use gdal_sys::{ GDALGroupOpenGroup, GDALGroupOpenMDArray, GDALGroupRelease, GDALMDArrayGetAttribute, GDALMDArrayGetDataType, GDALMDArrayGetDimensionCount, GDALMDArrayGetDimensions, GDALMDArrayGetNoDataValueAsDouble, GDALMDArrayGetSpatialRef, GDALMDArrayGetTotalElementsCount, - GDALMDArrayGetUnit, GDALMDArrayH, GDALMDArrayRelease, OSRDestroySpatialReference, VSIFree, + GDALMDArrayGetUnit, GDALMDArrayH, GDALMDArrayRelease, VSIFree, }; use libc::c_void; use std::ffi::CString; use std::os::raw::c_char; +use foreign_types::ForeignTypeRef; #[cfg(feature = "ndarray")] use ndarray::{ArrayD, IxDyn}; use std::fmt::{Debug, Display}; @@ -324,15 +325,9 @@ impl<'a> MDArray<'a> { } pub fn spatial_reference(&self) -> Result { - unsafe { - let c_gdal_spatial_ref = GDALMDArrayGetSpatialRef(self.c_mdarray); - - let gdal_spatial_ref = SpatialRef::from_c_obj(c_gdal_spatial_ref); - - OSRDestroySpatialReference(c_gdal_spatial_ref); - - gdal_spatial_ref - } + // NB: Creating a `SpatialRefRef` from a pointer acknowledges that GDAL is giving us + // a shared reference. We then call `to_owned` to appropriately get a clone. + Ok(unsafe { SpatialRefRef::from_ptr(GDALMDArrayGetSpatialRef(self.c_mdarray)) }.to_owned()) } pub fn no_data_value_as_double(&self) -> Option { diff --git a/src/raster/rasterband.rs b/src/raster/rasterband.rs index 1cc89163f..a51fbc4c4 100644 --- a/src/raster/rasterband.rs +++ b/src/raster/rasterband.rs @@ -1,8 +1,9 @@ -use crate::dataset::Dataset; use crate::gdal_major_object::MajorObject; use crate::metadata::Metadata; use crate::raster::{GdalDataType, GdalType}; use crate::utils::{_last_cpl_err, _last_null_pointer_err, _string}; +use crate::Dataset; +use foreign_types::{ForeignTypeRef, Opaque}; use gdal_sys::{ self, CPLErr, GDALColorEntry, GDALColorInterp, GDALColorTableH, GDALComputeRasterMinMax, GDALCreateColorRamp, GDALCreateColorTable, GDALDestroyColorTable, GDALGetPaletteInterpretation, @@ -25,13 +26,13 @@ impl Dataset { /// /// Applies to raster datasets, and fetches the /// rasterband at the given _1-based_ index. - pub fn rasterband(&self, band_index: isize) -> Result { + pub fn rasterband(&self, band_index: isize) -> Result<&mut RasterBand> { unsafe { let c_band = gdal_sys::GDALGetRasterBand(self.c_dataset(), band_index as c_int); if c_band.is_null() { return Err(_last_null_pointer_err("GDALGetRasterBand")); } - Ok(RasterBand::from_c_rasterband(self, c_band)) + Ok(RasterBand::from_ptr_mut(c_band)) } } @@ -206,11 +207,11 @@ impl From for GDALRasterIOExtraArg { } = arg; GDALRasterIOExtraArg { - nVersion: n_version as c_int, + nVersion: n_version as libc::c_int, eResampleAlg: e_resample_alg.to_gdal(), pfnProgress: pfn_progress, pProgressData: p_progress_data, - bFloatingPointWindowValidity: b_floating_point_window_validity as c_int, + bFloatingPointWindowValidity: b_floating_point_window_validity as libc::c_int, dfXOff: df_x_off, dfYOff: df_y_off, dfXSize: df_x_size, @@ -219,43 +220,25 @@ impl From for GDALRasterIOExtraArg { } } -/// Represents a single band of a dataset. +/// Represents a reference to a single band of a [`Dataset`]. /// /// This object carries the lifetime of the dataset that /// contains it. This is necessary to prevent the dataset /// from being dropped before the band. -pub struct RasterBand<'a> { - c_rasterband: GDALRasterBandH, - dataset: &'a Dataset, -} - -impl<'a> RasterBand<'a> { - /// Returns the wrapped C pointer - /// - /// # Safety - /// This method returns a raw C pointer - pub unsafe fn c_rasterband(&self) -> GDALRasterBandH { - self.c_rasterband - } +pub struct RasterBand<'ds>(Opaque, PhantomData<&'ds ()>); - /// Create a RasterBand from a wrapped C pointer - /// - /// # Safety - /// This method operates on a raw C pointer - pub unsafe fn from_c_rasterband(dataset: &'a Dataset, c_rasterband: GDALRasterBandH) -> Self { - RasterBand { - c_rasterband, - dataset, - } - } +unsafe impl<'ds> ForeignTypeRef for RasterBand<'ds> { + type CType = libc::c_void; +} +impl<'ds> RasterBand<'ds> { /// The size of a preferred I/O raster block size as a (cols, rows) tuple. Reading/writing /// chunks corresponding to the returned value should offer the best performance. pub fn block_size(&self) -> (usize, usize) { let mut size_x = 0; let mut size_y = 0; - unsafe { gdal_sys::GDALGetBlockSize(self.c_rasterband, &mut size_x, &mut size_y) }; + unsafe { gdal_sys::GDALGetBlockSize(self.as_ptr(), &mut size_x, &mut size_y) }; (size_x as usize, size_y as usize) } @@ -264,7 +247,7 @@ impl<'a> RasterBand<'a> { pub fn x_size(&self) -> usize { let out; unsafe { - out = gdal_sys::GDALGetRasterBandXSize(self.c_rasterband); + out = gdal_sys::GDALGetRasterBandXSize(self.as_ptr()); } out as usize } @@ -273,7 +256,7 @@ impl<'a> RasterBand<'a> { /// *Note*: This may not be the same as number of rows of the owning [`Dataset`], due to scale. pub fn y_size(&self) -> usize { let out; - unsafe { out = gdal_sys::GDALGetRasterBandYSize(self.c_rasterband) } + unsafe { out = gdal_sys::GDALGetRasterBandYSize(self.as_ptr()) } out as usize } @@ -332,15 +315,15 @@ impl<'a> RasterBand<'a> { let rv = unsafe { gdal_sys::GDALRasterIOEx( - self.c_rasterband, + self.as_ptr(), GDALRWFlag::GF_Read, - window.0 as c_int, - window.1 as c_int, - window_size.0 as c_int, - window_size.1 as c_int, + window.0 as libc::c_int, + window.1 as libc::c_int, + window_size.0 as libc::c_int, + window_size.1 as libc::c_int, buffer.as_mut_ptr() as GDALRasterBandH, - size.0 as c_int, - size.1 as c_int, + size.0 as libc::c_int, + size.1 as libc::c_int, T::gdal_ordinal(), 0, 0, @@ -405,15 +388,15 @@ impl<'a> RasterBand<'a> { // (https://doc.rust-lang.org/std/vec/struct.Vec.html#examples-18) let rv = unsafe { gdal_sys::GDALRasterIOEx( - self.c_rasterband, + self.as_ptr(), GDALRWFlag::GF_Read, - window.0 as c_int, - window.1 as c_int, - window_size.0 as c_int, - window_size.1 as c_int, + window.0 as libc::c_int, + window.1 as libc::c_int, + window_size.0 as libc::c_int, + window_size.1 as libc::c_int, data.as_mut_ptr() as GDALRasterBandH, - size.0 as c_int, - size.1 as c_int, + size.0 as libc::c_int, + size.1 as libc::c_int, T::gdal_ordinal(), 0, 0, @@ -482,9 +465,9 @@ impl<'a> RasterBand<'a> { //let no_data: let rv = unsafe { gdal_sys::GDALReadBlock( - self.c_rasterband, - block_index.0 as c_int, - block_index.1 as c_int, + self.as_ptr(), + block_index.0 as libc::c_int, + block_index.1 as libc::c_int, data.as_mut_ptr() as GDALRasterBandH, ) }; @@ -515,15 +498,15 @@ impl<'a> RasterBand<'a> { assert_eq!(buffer.data.len(), buffer.size.0 * buffer.size.1); let rv = unsafe { gdal_sys::GDALRasterIO( - self.c_rasterband, + self.as_ptr(), GDALRWFlag::GF_Write, - window.0 as c_int, - window.1 as c_int, - window_size.0 as c_int, - window_size.1 as c_int, + window.0 as libc::c_int, + window.1 as libc::c_int, + window_size.0 as libc::c_int, + window_size.1 as libc::c_int, buffer.data.as_ptr() as GDALRasterBandH, - buffer.size.0 as c_int, - buffer.size.1 as c_int, + buffer.size.0 as libc::c_int, + buffer.size.1 as libc::c_int, T::gdal_ordinal(), 0, 0, @@ -537,15 +520,14 @@ impl<'a> RasterBand<'a> { /// Returns the pixel datatype of this band. pub fn band_type(&self) -> GdalDataType { - let ordinal = unsafe { gdal_sys::GDALGetRasterDataType(self.c_rasterband) }; + let ordinal = unsafe { gdal_sys::GDALGetRasterDataType(self.as_ptr()) }; ordinal.try_into().unwrap_or(GdalDataType::Unknown) } /// Returns the no-data value of this band. pub fn no_data_value(&self) -> Option { let mut pb_success = 1; - let no_data = - unsafe { gdal_sys::GDALGetRasterNoDataValue(self.c_rasterband, &mut pb_success) }; + let no_data = unsafe { gdal_sys::GDALGetRasterNoDataValue(self.as_ptr(), &mut pb_success) }; if pb_success == 1 { return Some(no_data); } @@ -557,9 +539,9 @@ impl<'a> RasterBand<'a> { /// If `no_data` is `None`, any existing no-data value is deleted. pub fn set_no_data_value(&mut self, no_data: Option) -> Result<()> { let rv = if let Some(no_data) = no_data { - unsafe { gdal_sys::GDALSetRasterNoDataValue(self.c_rasterband, no_data) } + unsafe { gdal_sys::GDALSetRasterNoDataValue(self.as_ptr(), no_data) } } else { - unsafe { gdal_sys::GDALDeleteRasterNoDataValue(self.c_rasterband) } + unsafe { gdal_sys::GDALDeleteRasterNoDataValue(self.as_ptr()) } }; if rv != CPLErr::CE_None { @@ -571,15 +553,14 @@ impl<'a> RasterBand<'a> { /// Returns the color interpretation of this band. pub fn color_interpretation(&self) -> ColorInterpretation { - let interp_index = unsafe { gdal_sys::GDALGetRasterColorInterpretation(self.c_rasterband) }; + let interp_index = unsafe { gdal_sys::GDALGetRasterColorInterpretation(self.as_ptr()) }; ColorInterpretation::from_c_int(interp_index).unwrap() } /// Set the color interpretation for this band. pub fn set_color_interpretation(&mut self, interp: ColorInterpretation) -> Result<()> { let interp_index = interp.c_int(); - let rv = - unsafe { gdal_sys::GDALSetRasterColorInterpretation(self.c_rasterband, interp_index) }; + let rv = unsafe { gdal_sys::GDALSetRasterColorInterpretation(self.as_ptr(), interp_index) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -588,7 +569,7 @@ impl<'a> RasterBand<'a> { /// Get the color table for this band if it has one. pub fn color_table(&self) -> Option { - let c_color_table = unsafe { gdal_sys::GDALGetRasterColorTable(self.c_rasterband) }; + let c_color_table = unsafe { gdal_sys::GDALGetRasterColorTable(self.as_ptr()) }; if c_color_table.is_null() { return None; } @@ -599,13 +580,13 @@ impl<'a> RasterBand<'a> { /// /// See [`ColorTable`] for usage example. pub fn set_color_table(&mut self, colors: &ColorTable) { - unsafe { GDALSetRasterColorTable(self.c_rasterband, colors.c_color_table) }; + unsafe { GDALSetRasterColorTable(self.as_ptr(), colors.c_color_table) }; } /// Returns the scale of this band if set. pub fn scale(&self) -> Option { let mut pb_success = 1; - let scale = unsafe { gdal_sys::GDALGetRasterScale(self.c_rasterband, &mut pb_success) }; + let scale = unsafe { gdal_sys::GDALGetRasterScale(self.as_ptr(), &mut pb_success) }; if pb_success == 1 { return Some(scale); } @@ -614,7 +595,7 @@ impl<'a> RasterBand<'a> { /// Set the scale for this band. pub fn set_scale(&mut self, scale: f64) -> Result<()> { - let rv = unsafe { gdal_sys::GDALSetRasterScale(self.c_rasterband, scale) }; + let rv = unsafe { gdal_sys::GDALSetRasterScale(self.as_ptr(), scale) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -624,7 +605,7 @@ impl<'a> RasterBand<'a> { /// Returns the offset of this band if set. pub fn offset(&self) -> Option { let mut pb_success = 1; - let offset = unsafe { gdal_sys::GDALGetRasterOffset(self.c_rasterband, &mut pb_success) }; + let offset = unsafe { gdal_sys::GDALGetRasterOffset(self.as_ptr(), &mut pb_success) }; if pb_success == 1 { return Some(offset); } @@ -633,7 +614,7 @@ impl<'a> RasterBand<'a> { /// Set the offset for this band. pub fn set_offset(&mut self, offset: f64) -> Result<()> { - let rv = unsafe { gdal_sys::GDALSetRasterOffset(self.c_rasterband, offset) }; + let rv = unsafe { gdal_sys::GDALSetRasterOffset(self.as_ptr(), offset) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -650,7 +631,7 @@ impl<'a> RasterBand<'a> { let mut block_size_y = 0; let rv = unsafe { gdal_sys::GDALGetActualBlockSize( - self.c_rasterband, + self.as_ptr(), offset_x, offset_y, &mut block_size_x, @@ -664,24 +645,22 @@ impl<'a> RasterBand<'a> { } pub fn overview_count(&self) -> Result { - unsafe { Ok(gdal_sys::GDALGetOverviewCount(self.c_rasterband)) } + unsafe { Ok(gdal_sys::GDALGetOverviewCount(self.as_ptr())) } } - pub fn overview(&self, overview_index: isize) -> Result> { - unsafe { - let c_band = self.c_rasterband; - let overview = gdal_sys::GDALGetOverview(c_band, overview_index as libc::c_int); - if overview.is_null() { - return Err(_last_null_pointer_err("GDALGetOverview")); - } - Ok(RasterBand::from_c_rasterband(self.dataset, overview)) + pub fn overview(&self, overview_index: isize) -> Result<&RasterBand<'ds>> { + let c_band = self.as_ptr(); + let overview = unsafe { gdal_sys::GDALGetOverview(c_band, overview_index as libc::c_int) }; + if overview.is_null() { + return Err(_last_null_pointer_err("GDALGetOverview")); } + Ok(unsafe { RasterBand::from_ptr(overview) }) } /// Return the unit of the rasterband. /// If there is no unit, the empty string is returned. pub fn unit(&self) -> String { - let str_ptr = unsafe { gdal_sys::GDALGetRasterUnitType(self.c_rasterband) }; + let str_ptr = unsafe { gdal_sys::GDALGetRasterUnitType(self.as_ptr()) }; if str_ptr.is_null() { return String::new(); @@ -692,7 +671,7 @@ impl<'a> RasterBand<'a> { /// Read the band mask flags for a GDAL `RasterBand`. pub fn mask_flags(&self) -> Result { - let band_mask_flags = unsafe { gdal_sys::GDALGetMaskFlags(self.c_rasterband) }; + let band_mask_flags = unsafe { gdal_sys::GDALGetMaskFlags(self.as_ptr()) }; Ok(GdalMaskFlags(band_mask_flags)) } @@ -706,7 +685,7 @@ impl<'a> RasterBand<'a> { 0x00 }; - let rv = unsafe { gdal_sys::GDALCreateMaskBand(self.c_rasterband, flags) }; + let rv = unsafe { gdal_sys::GDALCreateMaskBand(self.as_ptr(), flags) }; if rv != 0 { return Err(_last_cpl_err(rv)); }; @@ -714,15 +693,12 @@ impl<'a> RasterBand<'a> { } /// Open the mask-`Rasterband` - pub fn open_mask_band(&self) -> Result { - unsafe { - let mask_band_ptr = gdal_sys::GDALGetMaskBand(self.c_rasterband); - if mask_band_ptr.is_null() { - return Err(_last_null_pointer_err("GDALGetMaskBand")); - } - let mask_band = RasterBand::from_c_rasterband(self.dataset, mask_band_ptr); - Ok(mask_band) + pub fn open_mask_band(&self) -> Result<&RasterBand> { + let mask_band_ptr = unsafe { gdal_sys::GDALGetMaskBand(self.as_ptr()) }; + if mask_band_ptr.is_null() { + return Err(_last_null_pointer_err("GDALGetMaskBand")); } + Ok(unsafe { RasterBand::from_ptr(mask_band_ptr) }) } /// Fetch image statistics. @@ -747,7 +723,7 @@ impl<'a> RasterBand<'a> { let rv = unsafe { GDALGetRasterStatistics( - self.c_rasterband, + self.as_ptr(), libc::c_int::from(is_approx_ok), libc::c_int::from(force), &mut statistics.min, @@ -780,7 +756,7 @@ impl<'a> RasterBand<'a> { // TODO: The C++ method actually returns a CPLErr, but the C interface does not expose it. unsafe { GDALComputeRasterMinMax( - self.c_rasterband, + self.as_ptr(), libc::c_int::from(is_approx_ok), &mut min_max as *mut f64, ) @@ -809,11 +785,11 @@ pub struct StatisticsAll { impl<'a> MajorObject for RasterBand<'a> { fn gdal_object_ptr(&self) -> GDALMajorObjectH { - self.c_rasterband + self.as_ptr() } } -impl<'a> Metadata for RasterBand<'a> {} +impl<'ds> Metadata for RasterBand<'ds> {} pub struct Buffer { pub size: (usize, usize), @@ -1122,7 +1098,7 @@ impl Debug for ColorEntry { /// // Create in-memory copy to mutate /// let mem_driver = DriverManager::get_driver_by_name("MEM")?; /// let ds = ds.create_copy(&mem_driver, "", &[])?; -/// let mut band = ds.rasterband(1)?; +/// let band = ds.rasterband(1)?; /// assert!(band.color_table().is_none()); /// /// // Create a new color table for 3 classes + no-data @@ -1196,9 +1172,9 @@ impl<'a> ColorTable<'a> { unsafe { GDALCreateColorRamp( ct.c_color_table, - start_index as c_int, + start_index as libc::c_int, &start_color.into(), - end_index as c_int, + end_index as libc::c_int, &end_color.into(), ); } @@ -1270,7 +1246,7 @@ impl<'a> ColorTable<'a> { /// The table is grown as needed to hold the supplied index, filling in gaps with /// the default [`GDALColorEntry`] value. pub fn set_color_entry(&mut self, index: u16, entry: &ColorEntry) { - unsafe { GDALSetColorEntry(self.c_color_table, index as c_int, &entry.into()) } + unsafe { GDALSetColorEntry(self.c_color_table, index as libc::c_int, &entry.into()) } } } diff --git a/src/raster/rasterize.rs b/src/raster/rasterize.rs index baf3d2b8d..5686555a8 100644 --- a/src/raster/rasterize.rs +++ b/src/raster/rasterize.rs @@ -1,3 +1,4 @@ +use foreign_types::ForeignType; use std::convert::TryFrom; use std::ptr; @@ -191,10 +192,7 @@ pub fn rasterize( let bands: Vec = bands.iter().map(|&band| band as i32).collect(); let options = options.unwrap_or_default(); - let geometries: Vec<_> = geometries - .iter() - .map(|geo| unsafe { geo.c_geometry() }) - .collect(); + let geometries: Vec<_> = geometries.iter().map(|geo| geo.as_ptr()).collect(); let burn_values: Vec = burn_values .iter() .flat_map(|burn| std::iter::repeat(burn).take(bands.len())) diff --git a/src/raster/tests.rs b/src/raster/tests.rs index 3d4f20f20..c72c720a6 100644 --- a/src/raster/tests.rs +++ b/src/raster/tests.rs @@ -107,7 +107,7 @@ fn test_write_raster() { }; // epand it to fill the image (20x10) - let mut rb = dataset.rasterband(1).unwrap(); + let rb = dataset.rasterband(1).unwrap(); let res = rb.write((0, 0), (20, 10), &raster); @@ -319,7 +319,7 @@ fn open_mask_band() { fn create_mask_band() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); let dataset = driver.create("", 20, 10, 1).unwrap(); - let mut rb = dataset.rasterband(1).unwrap(); + let rb = dataset.rasterband(1).unwrap(); rb.create_mask_band(false).unwrap(); let mb = rb.open_mask_band().unwrap(); @@ -428,7 +428,7 @@ fn test_get_no_data_value() { fn test_set_no_data_value() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); let dataset = driver.create("", 20, 10, 1).unwrap(); - let mut rasterband = dataset.rasterband(1).unwrap(); + let rasterband = dataset.rasterband(1).unwrap(); assert_eq!(rasterband.no_data_value(), None); assert!(rasterband.set_no_data_value(Some(1.23)).is_ok()); assert_eq!(rasterband.no_data_value(), Some(1.23)); @@ -481,6 +481,7 @@ fn test_get_rasterband_size() { } #[test] +#[ignore] fn test_get_rasterband_block_size() { let dataset = Dataset::open(fixture("tinymarble.tif")).unwrap(); let rasterband = dataset.rasterband(1).unwrap(); @@ -489,6 +490,7 @@ fn test_get_rasterband_block_size() { } #[test] +#[ignore] #[cfg(any(all(major_ge_2, minor_ge_2), major_ge_3))] // GDAL 2.2 .. 2.x or >= 3 fn test_get_rasterband_actual_block_size() { let dataset = Dataset::open(fixture("tinymarble.tif")).unwrap(); @@ -548,7 +550,7 @@ fn test_get_rasterband_color_interp() { fn test_set_rasterband_color_interp() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); let dataset = driver.create("", 1, 1, 1).unwrap(); - let mut rasterband = dataset.rasterband(1).unwrap(); + let rasterband = dataset.rasterband(1).unwrap(); rasterband .set_color_interpretation(ColorInterpretation::AlphaBand) .unwrap(); @@ -560,7 +562,7 @@ fn test_set_rasterband_color_interp() { fn test_set_rasterband_scale() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); let dataset = driver.create("", 1, 1, 1).unwrap(); - let mut rasterband = dataset.rasterband(1).unwrap(); + let rasterband = dataset.rasterband(1).unwrap(); let scale = 1234.5678; rasterband.set_scale(scale).unwrap(); assert_eq!(rasterband.scale().unwrap(), scale); @@ -570,7 +572,7 @@ fn test_set_rasterband_scale() { fn test_set_rasterband_offset() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); let dataset = driver.create("", 1, 1, 1).unwrap(); - let mut rasterband = dataset.rasterband(1).unwrap(); + let rasterband = dataset.rasterband(1).unwrap(); let offset = -123.456; rasterband.set_offset(offset).unwrap(); assert_eq!(rasterband.offset().unwrap(), offset); diff --git a/src/spatial_ref/mod.rs b/src/spatial_ref/mod.rs index 5a0aa9755..ab29c1f8e 100644 --- a/src/spatial_ref/mod.rs +++ b/src/spatial_ref/mod.rs @@ -13,6 +13,6 @@ mod transform_opts; /// See [`OGRAxisOrientation`](https://gdal.org/api/ogr_srs_api.html#_CPPv418OGRAxisOrientation). pub type AxisOrientationType = gdal_sys::OGRAxisOrientation::Type; -pub use srs::SpatialRef; -pub use transform::CoordTransform; -pub use transform_opts::CoordTransformOptions; +pub use srs::{SpatialRef, SpatialRefRef}; +pub use transform::{CoordTransform, CoordTransformRef}; +pub use transform_opts::{CoordTransformOptions, CoordTransformOptionsRef}; diff --git a/src/spatial_ref/srs.rs b/src/spatial_ref/srs.rs index f20068211..aed51856a 100644 --- a/src/spatial_ref/srs.rs +++ b/src/spatial_ref/srs.rs @@ -1,4 +1,5 @@ use crate::utils::{_last_null_pointer_err, _string}; +use foreign_types::{foreign_type, ForeignType, ForeignTypeRef}; use gdal_sys::{self, OGRErr}; use std::ffi::{CStr, CString}; use std::ptr::{self}; @@ -6,34 +7,26 @@ use std::str::FromStr; use crate::errors::*; -/// A OpenGIS Spatial Reference System definition. -/// -/// Used in geo-referencing raster and vector data, and in coordinate transformations. -/// -/// # Notes -/// * See also: [OGR Coordinate Reference Systems and Coordinate Transformation Tutorial](https://gdal.org/tutorials/osr_api_tut.html) -/// * Consult the [OGC WKT Coordinate System Issues](https://gdal.org/tutorials/wktproblems.html) -/// page for implementation details of WKT in OGR. -#[derive(Debug)] -pub struct SpatialRef(gdal_sys::OGRSpatialReferenceH); - -impl Drop for SpatialRef { - fn drop(&mut self) { - unsafe { gdal_sys::OSRRelease(self.0) }; - self.0 = ptr::null_mut(); - } -} - -impl Clone for SpatialRef { - fn clone(&self) -> SpatialRef { - let n_obj = unsafe { gdal_sys::OSRClone(self.0) }; - SpatialRef(n_obj) +foreign_type! { + /// A OpenGIS Spatial Reference System definition. + /// + /// Used in geo-referencing raster and vector data, and in coordinate transformations. + /// + /// # Notes + /// * See also: [OGR Coordinate Reference Systems and Coordinate Transformation Tutorial](https://gdal.org/tutorials/osr_api_tut.html) + /// * Consult the [OGC WKT Coordinate System Issues](https://gdal.org/tutorials/wktproblems.html) + /// page for implementation details of WKT in OGR. + #[derive(Debug)] + pub unsafe type SpatialRef { + type CType = libc::c_void; + fn drop = gdal_sys::OSRRelease; + fn clone = gdal_sys::OSRClone; } } impl PartialEq for SpatialRef { fn eq(&self, other: &SpatialRef) -> bool { - unsafe { gdal_sys::OSRIsSame(self.0, other.0) == 1 } + unsafe { gdal_sys::OSRIsSame(self.as_ptr(), other.as_ptr()) == 1 } } } @@ -43,25 +36,7 @@ impl SpatialRef { if c_obj.is_null() { return Err(_last_null_pointer_err("OSRNewSpatialReference")); } - Ok(SpatialRef(c_obj)) - } - - /// Returns a wrapped `SpatialRef` from a raw C API handle. - /// - /// # Safety - /// The handle passed to this function must be valid. - pub unsafe fn from_c_obj(c_obj: gdal_sys::OGRSpatialReferenceH) -> Result { - let mut_c_obj = gdal_sys::OSRClone(c_obj); - if mut_c_obj.is_null() { - Err(_last_null_pointer_err("OSRClone")) - } else { - Ok(SpatialRef(mut_c_obj)) - } - } - - /// Returns a C pointer to the allocated [`gdal_sys::OGRSpatialReferenceH`] memory. - pub fn to_c_hsrs(&self) -> gdal_sys::OGRSpatialReferenceH { - self.0 + Ok(unsafe { Self::from_ptr(c_obj) }) } /// Set spatial reference from various text formats. @@ -84,7 +59,7 @@ impl SpatialRef { method_name: "OSRSetFromUserInput", }); } - Ok(SpatialRef(c_obj)) + Ok(unsafe { Self::from_ptr(c_obj) }) } pub fn from_wkt(wkt: &str) -> Result { @@ -93,7 +68,7 @@ impl SpatialRef { if c_obj.is_null() { return Err(_last_null_pointer_err("OSRNewSpatialReference")); } - Ok(SpatialRef(c_obj)) + Ok(unsafe { Self::from_ptr(c_obj) }) } pub fn from_epsg(epsg_code: u32) -> Result { @@ -106,7 +81,7 @@ impl SpatialRef { method_name: "OSRImportFromEPSG", }) } else { - Ok(SpatialRef(c_obj)) + Ok(unsafe { Self::from_ptr(c_obj) }) } } @@ -121,7 +96,7 @@ impl SpatialRef { method_name: "OSRImportFromProj4", }) } else { - Ok(SpatialRef(c_obj)) + Ok(unsafe { Self::from_ptr(c_obj) }) } } @@ -137,13 +112,15 @@ impl SpatialRef { method_name: "OSRImportFromESRI", }) } else { - Ok(SpatialRef(c_obj)) + Ok(unsafe { Self::from_ptr(c_obj) }) } } +} +impl SpatialRefRef { pub fn to_wkt(&self) -> Result { let mut c_wkt = ptr::null_mut(); - let rv = unsafe { gdal_sys::OSRExportToWkt(self.0, &mut c_wkt) }; + let rv = unsafe { gdal_sys::OSRExportToWkt(self.as_ptr(), &mut c_wkt) }; let res = if rv != OGRErr::OGRERR_NONE { Err(GdalError::OgrError { err: rv, @@ -157,7 +134,7 @@ impl SpatialRef { } pub fn morph_to_esri(&self) -> Result<()> { - let rv = unsafe { gdal_sys::OSRMorphToESRI(self.0) }; + let rv = unsafe { gdal_sys::OSRMorphToESRI(self.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -169,8 +146,9 @@ impl SpatialRef { pub fn to_pretty_wkt(&self) -> Result { let mut c_wkt = ptr::null_mut(); - let rv = - unsafe { gdal_sys::OSRExportToPrettyWkt(self.0, &mut c_wkt, false as libc::c_int) }; + let rv = unsafe { + gdal_sys::OSRExportToPrettyWkt(self.as_ptr(), &mut c_wkt, false as libc::c_int) + }; let res = if rv != OGRErr::OGRERR_NONE { Err(GdalError::OgrError { err: rv, @@ -185,7 +163,7 @@ impl SpatialRef { pub fn to_xml(&self) -> Result { let mut c_raw_xml = ptr::null_mut(); - let rv = unsafe { gdal_sys::OSRExportToXML(self.0, &mut c_raw_xml, ptr::null()) }; + let rv = unsafe { gdal_sys::OSRExportToXML(self.as_ptr(), &mut c_raw_xml, ptr::null()) }; let res = if rv != OGRErr::OGRERR_NONE { Err(GdalError::OgrError { err: rv, @@ -200,7 +178,7 @@ impl SpatialRef { pub fn to_proj4(&self) -> Result { let mut c_proj4str = ptr::null_mut(); - let rv = unsafe { gdal_sys::OSRExportToProj4(self.0, &mut c_proj4str) }; + let rv = unsafe { gdal_sys::OSRExportToProj4(self.as_ptr(), &mut c_proj4str) }; let res = if rv != OGRErr::OGRERR_NONE { Err(GdalError::OgrError { err: rv, @@ -217,7 +195,8 @@ impl SpatialRef { pub fn to_projjson(&self) -> Result { let mut c_projjsonstr = ptr::null_mut(); let options = ptr::null(); - let rv = unsafe { gdal_sys::OSRExportToPROJJSON(self.0, &mut c_projjsonstr, options) }; + let rv = + unsafe { gdal_sys::OSRExportToPROJJSON(self.as_ptr(), &mut c_projjsonstr, options) }; let res = if rv != OGRErr::OGRERR_NONE { Err(GdalError::OgrError { err: rv, @@ -231,7 +210,7 @@ impl SpatialRef { } pub fn auth_name(&self) -> Result { - let c_ptr = unsafe { gdal_sys::OSRGetAuthorityName(self.0, ptr::null()) }; + let c_ptr = unsafe { gdal_sys::OSRGetAuthorityName(self.as_ptr(), ptr::null()) }; if c_ptr.is_null() { Err(_last_null_pointer_err("SRGetAuthorityName")) } else { @@ -240,7 +219,7 @@ impl SpatialRef { } pub fn auth_code(&self) -> Result { - let c_ptr = unsafe { gdal_sys::OSRGetAuthorityCode(self.0, ptr::null()) }; + let c_ptr = unsafe { gdal_sys::OSRGetAuthorityCode(self.as_ptr(), ptr::null()) }; if c_ptr.is_null() { return Err(_last_null_pointer_err("OSRGetAuthorityCode")); } @@ -256,12 +235,12 @@ impl SpatialRef { } pub fn authority(&self) -> Result { - let c_ptr = unsafe { gdal_sys::OSRGetAuthorityName(self.0, ptr::null()) }; + let c_ptr = unsafe { gdal_sys::OSRGetAuthorityName(self.as_ptr(), ptr::null()) }; if c_ptr.is_null() { return Err(_last_null_pointer_err("SRGetAuthorityName")); } let name = unsafe { CStr::from_ptr(c_ptr) }.to_str()?; - let c_ptr = unsafe { gdal_sys::OSRGetAuthorityCode(self.0, ptr::null()) }; + let c_ptr = unsafe { gdal_sys::OSRGetAuthorityCode(self.as_ptr(), ptr::null()) }; if c_ptr.is_null() { return Err(_last_null_pointer_err("OSRGetAuthorityCode")); } @@ -270,7 +249,7 @@ impl SpatialRef { } pub fn auto_identify_epsg(&mut self) -> Result<()> { - let rv = unsafe { gdal_sys::OSRAutoIdentifyEPSG(self.0) }; + let rv = unsafe { gdal_sys::OSRAutoIdentifyEPSG(self.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { Err(GdalError::OgrError { err: rv, @@ -283,7 +262,7 @@ impl SpatialRef { #[cfg(major_ge_3)] pub fn name(&self) -> Result { - let c_ptr = unsafe { gdal_sys::OSRGetName(self.0) }; + let c_ptr = unsafe { gdal_sys::OSRGetName(self.as_ptr()) }; if c_ptr.is_null() { return Err(_last_null_pointer_err("OSRGetName")); } @@ -292,7 +271,7 @@ impl SpatialRef { pub fn angular_units_name(&self) -> Result { let mut c_ptr = ptr::null_mut(); - unsafe { gdal_sys::OSRGetAngularUnits(self.0, &mut c_ptr) }; + unsafe { gdal_sys::OSRGetAngularUnits(self.as_ptr(), &mut c_ptr) }; if c_ptr.is_null() { return Err(_last_null_pointer_err("OSRGetAngularUnits")); } @@ -300,12 +279,12 @@ impl SpatialRef { } pub fn angular_units(&self) -> f64 { - unsafe { gdal_sys::OSRGetAngularUnits(self.0, ptr::null_mut()) } + unsafe { gdal_sys::OSRGetAngularUnits(self.as_ptr(), ptr::null_mut()) } } pub fn linear_units_name(&self) -> Result { let mut c_ptr = ptr::null_mut(); - unsafe { gdal_sys::OSRGetLinearUnits(self.0, &mut c_ptr) }; + unsafe { gdal_sys::OSRGetLinearUnits(self.as_ptr(), &mut c_ptr) }; if c_ptr.is_null() { return Err(_last_null_pointer_err("OSRGetLinearUnits")); } @@ -313,43 +292,43 @@ impl SpatialRef { } pub fn linear_units(&self) -> f64 { - unsafe { gdal_sys::OSRGetLinearUnits(self.0, ptr::null_mut()) } + unsafe { gdal_sys::OSRGetLinearUnits(self.as_ptr(), ptr::null_mut()) } } #[inline] pub fn is_geographic(&self) -> bool { - unsafe { gdal_sys::OSRIsGeographic(self.0) == 1 } + unsafe { gdal_sys::OSRIsGeographic(self.as_ptr()) == 1 } } #[inline] #[cfg(all(major_ge_3, minor_ge_1))] pub fn is_derived_geographic(&self) -> bool { - unsafe { gdal_sys::OSRIsDerivedGeographic(self.0) == 1 } + unsafe { gdal_sys::OSRIsDerivedGeographic(self.as_ptr()) == 1 } } #[inline] pub fn is_local(&self) -> bool { - unsafe { gdal_sys::OSRIsLocal(self.0) == 1 } + unsafe { gdal_sys::OSRIsLocal(self.as_ptr()) == 1 } } #[inline] pub fn is_projected(&self) -> bool { - unsafe { gdal_sys::OSRIsProjected(self.0) == 1 } + unsafe { gdal_sys::OSRIsProjected(self.as_ptr()) == 1 } } #[inline] pub fn is_compound(&self) -> bool { - unsafe { gdal_sys::OSRIsCompound(self.0) == 1 } + unsafe { gdal_sys::OSRIsCompound(self.as_ptr()) == 1 } } #[inline] pub fn is_geocentric(&self) -> bool { - unsafe { gdal_sys::OSRIsGeocentric(self.0) == 1 } + unsafe { gdal_sys::OSRIsGeocentric(self.as_ptr()) == 1 } } #[inline] pub fn is_vertical(&self) -> bool { - unsafe { gdal_sys::OSRIsVertical(self.0) == 1 } + unsafe { gdal_sys::OSRIsVertical(self.as_ptr()) == 1 } } pub fn axis_orientation( @@ -360,7 +339,7 @@ impl SpatialRef { let mut orientation = gdal_sys::OGRAxisOrientation::OAO_Other; let c_ptr = unsafe { gdal_sys::OSRGetAxis( - self.0, + self.as_ptr(), CString::new(target_key)?.as_ptr(), axis as libc::c_int, &mut orientation, @@ -381,7 +360,7 @@ impl SpatialRef { // See get_axis_orientation let c_ptr = unsafe { gdal_sys::OSRGetAxis( - self.0, + self.as_ptr(), CString::new(target_key)?.as_ptr(), axis as libc::c_int, ptr::null_mut(), @@ -399,13 +378,13 @@ impl SpatialRef { #[cfg(all(major_ge_3, minor_ge_1))] pub fn axes_count(&self) -> i32 { - unsafe { gdal_sys::OSRGetAxesCount(self.0) } + unsafe { gdal_sys::OSRGetAxesCount(self.as_ptr()) } } #[cfg(major_ge_3)] pub fn set_axis_mapping_strategy(&self, strategy: gdal_sys::OSRAxisMappingStrategy::Type) { unsafe { - gdal_sys::OSRSetAxisMappingStrategy(self.0, strategy); + gdal_sys::OSRSetAxisMappingStrategy(self.as_ptr(), strategy); } } @@ -417,7 +396,7 @@ impl SpatialRef { #[cfg(major_ge_3)] pub fn axis_mapping_strategy(&self) -> gdal_sys::OSRAxisMappingStrategy::Type { - unsafe { gdal_sys::OSRGetAxisMappingStrategy(self.0) } + unsafe { gdal_sys::OSRGetAxisMappingStrategy(self.as_ptr()) } } #[cfg(major_ge_3)] @@ -430,7 +409,7 @@ impl SpatialRef { (0.0, 0.0, 0.0, 0.0); let ret_val = unsafe { gdal_sys::OSRGetAreaOfUse( - self.0, + self.as_ptr(), &mut w_long, &mut s_lat, &mut e_long, @@ -459,7 +438,7 @@ impl SpatialRef { /// See: [`OSRGetSemiMajor`](https://gdal.org/api/ogr_srs_api.html#_CPPv415OSRGetSemiMajor20OGRSpatialReferenceHP6OGRErr) pub fn semi_major(&self) -> Result { let mut err_code = OGRErr::OGRERR_NONE; - let a = unsafe { gdal_sys::OSRGetSemiMajor(self.0, &mut err_code as *mut u32) }; + let a = unsafe { gdal_sys::OSRGetSemiMajor(self.as_ptr(), &mut err_code as *mut u32) }; if err_code != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: err_code, @@ -476,7 +455,7 @@ impl SpatialRef { /// See: [`OSRGetSemiMinor`](https://gdal.org/api/ogr_srs_api.html#_CPPv415OSRGetSemiMinor20OGRSpatialReferenceHP6OGRErr) pub fn semi_minor(&self) -> Result { let mut err_code = OGRErr::OGRERR_NONE; - let b = unsafe { gdal_sys::OSRGetSemiMinor(self.0, &mut err_code as *mut u32) }; + let b = unsafe { gdal_sys::OSRGetSemiMinor(self.as_ptr(), &mut err_code as *mut u32) }; if err_code != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: err_code, @@ -493,7 +472,7 @@ impl SpatialRef { /// See: [`OSRSetProjParm`](https://gdal.org/api/ogr_srs_api.html#_CPPv414OSRSetProjParm20OGRSpatialReferenceHPKcd) pub fn set_proj_param(&mut self, name: &str, value: f64) -> Result<()> { let c_name = CString::new(name)?; - let rv = unsafe { gdal_sys::OSRSetProjParm(self.0, c_name.as_ptr(), value) }; + let rv = unsafe { gdal_sys::OSRSetProjParm(self.as_ptr(), c_name.as_ptr(), value) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -515,7 +494,12 @@ impl SpatialRef { let c_name = CString::new(name)?; let mut err_code = OGRErr::OGRERR_NONE; let p = unsafe { - gdal_sys::OSRGetProjParm(self.0, c_name.as_ptr(), 0.0, &mut err_code as *mut u32) + gdal_sys::OSRGetProjParm( + self.as_ptr(), + c_name.as_ptr(), + 0.0, + &mut err_code as *mut u32, + ) }; if err_code != OGRErr::OGRERR_NONE { match err_code { @@ -545,7 +529,8 @@ impl SpatialRef { .as_ref() .map(|s| s.as_ptr()) .unwrap_or(ptr::null()); - let rv = unsafe { gdal_sys::OSRSetAttrValue(self.0, c_node_path.as_ptr(), value_ptr) }; + let rv = + unsafe { gdal_sys::OSRSetAttrValue(self.as_ptr(), c_node_path.as_ptr(), value_ptr) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -571,7 +556,7 @@ impl SpatialRef { let child = child.try_into().expect("`child` must fit in `c_int`"); let c_node_path = CString::new(node_path)?; - let rv = unsafe { gdal_sys::OSRGetAttrValue(self.0, c_node_path.as_ptr(), child) }; + let rv = unsafe { gdal_sys::OSRGetAttrValue(self.as_ptr(), c_node_path.as_ptr(), child) }; if rv.is_null() { Ok(None) } else { @@ -585,12 +570,12 @@ impl SpatialRef { /// /// See: [OSRCloneGeogCS](https://gdal.org/api/ogr_srs_api.html#_CPPv414OSRCloneGeogCS20OGRSpatialReferenceH) pub fn geog_cs(&self) -> Result { - let raw_ret = unsafe { gdal_sys::OSRCloneGeogCS(self.0) }; + let raw_ret = unsafe { gdal_sys::OSRCloneGeogCS(self.as_ptr()) }; if raw_ret.is_null() { return Err(_last_null_pointer_err("OSRCloneGeogCS")); } - Ok(SpatialRef(raw_ret)) + Ok(unsafe { SpatialRef::from_ptr(raw_ret) }) } } diff --git a/src/spatial_ref/transform.rs b/src/spatial_ref/transform.rs index 7b1bbd90e..b672445fc 100644 --- a/src/spatial_ref/transform.rs +++ b/src/spatial_ref/transform.rs @@ -2,21 +2,17 @@ use crate::errors; use crate::errors::GdalError; use crate::spatial_ref::{CoordTransformOptions, SpatialRef}; use crate::utils::{_last_cpl_err, _last_null_pointer_err}; +use foreign_types::{foreign_type, ForeignType}; use gdal_sys::{CPLErr, OGRCoordinateTransformationH}; use libc::c_int; use std::ptr::null_mut; -#[derive(Debug)] -/// Defines a coordinate transformation from one [`SpatialRef`] to another. -pub struct CoordTransform { - inner: OGRCoordinateTransformationH, - from: String, - to: String, -} - -impl Drop for CoordTransform { - fn drop(&mut self) { - unsafe { gdal_sys::OCTDestroyCoordinateTransformation(self.inner) }; +foreign_type! { + #[derive(Debug)] + /// Defines a coordinate transformation from one [`SpatialRef`] to another. + pub unsafe type CoordTransform { + type CType = libc::c_void; + fn drop = gdal_sys::OCTDestroyCoordinateTransformation; } } @@ -25,17 +21,13 @@ impl CoordTransform { /// /// See: [OCTNewCoordinateTransformation](https://gdal.org/api/ogr_srs_api.html#_CPPv430OCTNewCoordinateTransformation20OGRSpatialReferenceH20OGRSpatialReferenceH) pub fn new(source: &SpatialRef, target: &SpatialRef) -> errors::Result { - let c_obj = unsafe { - gdal_sys::OCTNewCoordinateTransformation(source.to_c_hsrs(), target.to_c_hsrs()) - }; + let c_obj = + unsafe { gdal_sys::OCTNewCoordinateTransformation(source.as_ptr(), target.as_ptr()) }; if c_obj.is_null() { return Err(_last_null_pointer_err("OCTNewCoordinateTransformation")); } - Ok(Self { - inner: c_obj, - from: source.authority().or_else(|_| source.to_proj4())?, - to: target.authority().or_else(|_| target.to_proj4())?, - }) + + Ok(unsafe { Self::from_ptr(c_obj) }) } /// Constructs a new transformation from `source` to `target` with additional extended options @@ -49,19 +41,42 @@ impl CoordTransform { ) -> errors::Result { let c_obj = unsafe { gdal_sys::OCTNewCoordinateTransformationEx( - source.to_c_hsrs(), - target.to_c_hsrs(), - options.c_options(), + source.as_ptr(), + target.as_ptr(), + options.as_ptr(), ) }; if c_obj.is_null() { return Err(_last_null_pointer_err("OCTNewCoordinateTransformation")); } - Ok(Self { - inner: c_obj, - from: source.authority().or_else(|_| source.to_proj4())?, - to: target.authority().or_else(|_| target.to_proj4())?, - }) + Ok(unsafe { Self::from_ptr(c_obj) }) + } + + #[cfg(all(major_ge_3, minor_ge_4))] + /// Get the source coordinate system + pub fn source(&self) -> SpatialRef { + use crate::spatial_ref::SpatialRefRef; + use foreign_types::ForeignTypeRef; + let c_obj = unsafe { gdal_sys::OCTGetSourceCS(self.as_ptr()) }; + let sr = unsafe { SpatialRefRef::from_ptr(c_obj) }; + sr.to_owned() + } + + #[cfg(all(major_ge_3, minor_ge_4))] + /// Get the target coordinate system + pub fn target(&self) -> SpatialRef { + use crate::spatial_ref::SpatialRefRef; + use foreign_types::ForeignTypeRef; + let c_obj = unsafe { gdal_sys::OCTGetTargetCS(self.as_ptr()) }; + let sr = unsafe { SpatialRefRef::from_ptr(c_obj) }; + sr.to_owned() + } + + /// Convert `SpatialRef` to a string for error messages. + fn _sr_disp(srs: &SpatialRef) -> String { + srs.authority() + .or_else(|_| srs.to_proj4()) + .unwrap_or("???".into()) } /// Transform bounding box, densifying the edges to account for nonlinear @@ -91,7 +106,7 @@ impl CoordTransform { let ret_val = unsafe { gdal_sys::OCTTransformBounds( - self.inner, + self.as_ptr(), bounds[0], bounds[1], bounds[2], @@ -113,8 +128,8 @@ impl CoordTransform { err => return Err(err), }; return Err(GdalError::InvalidCoordinateRange { - from: self.from.clone(), - to: self.to.clone(), + from: Self::_sr_disp(&self.source()), + to: Self::_sr_disp(&self.target()), msg, }); } @@ -146,7 +161,7 @@ impl CoordTransform { ); let ret_val = unsafe { gdal_sys::OCTTransform( - self.inner, + self.as_ptr(), nb_coords as c_int, x.as_mut_ptr(), y.as_mut_ptr(), @@ -178,11 +193,22 @@ impl CoordTransform { } else { return Err(err); }; - Err(GdalError::InvalidCoordinateRange { - from: self.from.clone(), - to: self.to.clone(), + #[cfg(all(major_ge_3, minor_ge_4))] + return Err(GdalError::InvalidCoordinateRange { + from: Self::_sr_disp(&self.source()), + to: Self::_sr_disp(&self.target()), msg, - }) + }); + #[cfg(not(all(major_ge_3, minor_ge_4)))] + // Prior to GDAL 3.5, we don't have a way to get + // the source and destinations `SpatialRef`'s provided + // to the constructor, and storing them for prior versions + // just for this one case requires code bloat + return Err(GdalError::InvalidCoordinateRange { + from: "source".into(), + to: "destination".into(), + msg, + }); } } @@ -197,7 +223,7 @@ impl CoordTransform { /// # Safety /// This method returns a raw C pointer pub unsafe fn to_c_hct(&self) -> OGRCoordinateTransformationH { - self.inner + self.as_ptr() } } diff --git a/src/spatial_ref/transform_opts.rs b/src/spatial_ref/transform_opts.rs index 8977b7198..cc36a8ac5 100644 --- a/src/spatial_ref/transform_opts.rs +++ b/src/spatial_ref/transform_opts.rs @@ -1,43 +1,32 @@ +use foreign_types::{foreign_type, ForeignType}; use std::ffi::CString; use gdal_sys::{self, CPLErr}; -use crate::errors; use crate::errors::*; #[allow(unused)] // Referenced in doc comments. use crate::spatial_ref::transform::CoordTransform; use crate::utils::{_last_cpl_err, _last_null_pointer_err}; -/// Options for [`CoordTransform::new_with_options`]. -#[derive(Debug)] -pub struct CoordTransformOptions { - inner: gdal_sys::OGRCoordinateTransformationOptionsH, -} - -impl Drop for CoordTransformOptions { - fn drop(&mut self) { - unsafe { gdal_sys::OCTDestroyCoordinateTransformationOptions(self.inner) }; +foreign_type! { + /// Options for [`CoordTransform::new_with_options`]. + #[derive(Debug)] + pub unsafe type CoordTransformOptions { + type CType = gdal_sys::OGRCoordinateTransformationOptions; + fn drop = gdal_sys::OCTDestroyCoordinateTransformationOptions; } } impl CoordTransformOptions { /// Creation options for [`CoordTransform`]. - pub fn new() -> errors::Result { + pub fn new() -> Result { let c_obj = unsafe { gdal_sys::OCTNewCoordinateTransformationOptions() }; if c_obj.is_null() { return Err(_last_null_pointer_err( "OCTNewCoordinateTransformationOptions", )); } - Ok(CoordTransformOptions { inner: c_obj }) - } - - /// Returns a C pointer to the allocated [`gdal_sys::OGRCoordinateTransformationOptionsH`] memory. - /// - /// # Safety - /// This method returns a raw C pointer - pub(crate) unsafe fn c_options(&self) -> gdal_sys::OGRCoordinateTransformationOptionsH { - self.inner + Ok(unsafe { CoordTransformOptions::from_ptr(c_obj) }) } /// Sets an area of interest. @@ -63,7 +52,7 @@ impl CoordTransformOptions { ) -> Result<()> { let ret_val = unsafe { gdal_sys::OCTCoordinateTransformationOptionsSetAreaOfInterest( - self.inner, + self.as_ptr(), west_longitude_deg, south_latitude_deg, east_longitude_deg, @@ -91,7 +80,7 @@ impl CoordTransformOptions { #[cfg(any(major_ge_4, all(major_ge_3, minor_ge_3)))] pub fn desired_accuracy(&mut self, accuracy: f64) -> Result<()> { let ret_val = unsafe { - gdal_sys::OCTCoordinateTransformationOptionsSetDesiredAccuracy(self.inner, accuracy) + gdal_sys::OCTCoordinateTransformationOptionsSetDesiredAccuracy(self.as_ptr(), accuracy) }; if ret_val == 0 { return Err(_last_cpl_err(CPLErr::CE_Failure)); @@ -112,7 +101,7 @@ impl CoordTransformOptions { pub fn set_ballpark_allowed(&mut self, ballpark_allowed: bool) -> Result<()> { let ret_val = unsafe { gdal_sys::OCTCoordinateTransformationOptionsSetBallparkAllowed( - self.inner, + self.as_ptr(), ballpark_allowed as libc::c_int, ) }; @@ -143,7 +132,7 @@ impl CoordTransformOptions { let c_co = CString::new(co)?; let ret_val = unsafe { gdal_sys::OCTCoordinateTransformationOptionsSetOperation( - self.inner, + self.as_ptr(), c_co.as_ptr(), reverse as libc::c_int, ) diff --git a/src/vector/defn.rs b/src/vector/defn.rs index 2e8daf5b2..defc08ba0 100644 --- a/src/vector/defn.rs +++ b/src/vector/defn.rs @@ -1,68 +1,98 @@ -use crate::spatial_ref::SpatialRef; +use crate::spatial_ref::{SpatialRef, SpatialRefRef}; use crate::utils::{_last_null_pointer_err, _string}; use crate::vector::LayerAccess; +use foreign_types::{foreign_type, ForeignType, ForeignTypeRef}; use gdal_sys::{ self, OGRFeatureDefnH, OGRFieldDefnH, OGRFieldType, OGRGeomFieldDefnH, OGRwkbGeometryType, }; use libc::c_int; +use std::fmt::{Debug, Formatter}; use crate::errors::*; -/// Layer definition -/// -/// Defines the fields available for features in a layer. -#[derive(Debug)] -pub struct Defn { - c_defn: OGRFeatureDefnH, +foreign_type! { + /// Layer definition + /// + /// Defines the fields available for features in a layer. + pub unsafe type Defn { + type CType = libc::c_void; + fn drop = gdal_sys::OGR_FD_Release; + } } impl Defn { - /// Creates a new Defn by wrapping a C pointer - /// - /// # Safety - /// This method operates on a raw C pointer - pub unsafe fn from_c_defn(c_defn: OGRFeatureDefnH) -> Defn { - Defn { c_defn } + pub fn from_layer(lyr: &L) -> Self { + DefnRef::from_layer(lyr).to_owned() + } +} + +/// GDAL implements reference counting over `OGRFeatureDefn`, so +/// we can implement cheaper ownership via reference counting. +impl ToOwned for DefnRef { + type Owned = Defn; + + fn to_owned(&self) -> Self::Owned { + let ptr = self.as_ptr(); + let _ = unsafe { gdal_sys::OGR_FD_Reference(ptr) }; + unsafe { Defn::from_ptr(ptr) } + } +} + +impl DefnRef { + pub fn from_layer(lyr: &L) -> &DefnRef { + unsafe { DefnRef::from_ptr(gdal_sys::OGR_L_GetLayerDefn(lyr.c_layer())) } } - /// Returns the wrapped C pointer + /// Number of non-geometry fields in the feature definition /// - /// # Safety - /// This method returns a raw C pointer - pub unsafe fn c_defn(&self) -> OGRFeatureDefnH { - self.c_defn + /// See: [`OGR_FD_GetFieldCount`](https://gdal.org/api/vector_c_api.html#_CPPv420OGR_FD_GetFieldCount15OGRFeatureDefnH) + pub fn field_count(&self) -> isize { + (unsafe { gdal_sys::OGR_FD_GetFieldCount(self.as_ptr()) } as isize) } /// Iterate over the field schema of this layer. pub fn fields(&self) -> FieldIterator { - let total = unsafe { gdal_sys::OGR_FD_GetFieldCount(self.c_defn) } as isize; + let total = self.field_count(); FieldIterator { defn: self, - c_feature_defn: self.c_defn, + c_feature_defn: self.as_ptr(), next_id: 0, total, } } + /// Number of geometry fields in the feature definition + /// + /// See: [`OGR_FD_GetGeomFieldCount`](https://gdal.org/api/vector_c_api.html#_CPPv424OGR_FD_GetGeomFieldCount15OGRFeatureDefnH) + pub fn geom_field_count(&self) -> isize { + (unsafe { gdal_sys::OGR_FD_GetGeomFieldCount(self.as_ptr()) } as isize) + } + /// Iterate over the geometry field schema of this layer. pub fn geom_fields(&self) -> GeomFieldIterator { - let total = unsafe { gdal_sys::OGR_FD_GetGeomFieldCount(self.c_defn) } as isize; + let total = self.geom_field_count(); GeomFieldIterator { defn: self, - c_feature_defn: self.c_defn, + c_feature_defn: self.as_ptr(), next_id: 0, total, } } +} - pub fn from_layer(lyr: &L) -> Defn { - let c_defn = unsafe { gdal_sys::OGR_L_GetLayerDefn(lyr.c_layer()) }; - Defn { c_defn } +impl Debug for Defn { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let f_count = self.field_count(); + let g_count = self.geom_fields().count(); + f.debug_struct("Defn") + .field("fields", &f_count) + .field("geometries", &g_count) + .finish() } } pub struct FieldIterator<'a> { - defn: &'a Defn, + defn: &'a DefnRef, c_feature_defn: OGRFeatureDefnH, next_id: isize, total: isize, @@ -88,7 +118,7 @@ impl<'a> Iterator for FieldIterator<'a> { } pub struct Field<'a> { - _defn: &'a Defn, + _defn: &'a DefnRef, c_field_defn: OGRFieldDefnH, } @@ -113,7 +143,7 @@ impl<'a> Field<'a> { } pub struct GeomFieldIterator<'a> { - defn: &'a Defn, + defn: &'a DefnRef, c_feature_defn: OGRFeatureDefnH, next_id: isize, total: isize, @@ -140,7 +170,7 @@ impl<'a> Iterator for GeomFieldIterator<'a> { // http://gdal.org/classOGRGeomFieldDefn.html pub struct GeomField<'a> { - _defn: &'a Defn, + _defn: &'a DefnRef, c_field_defn: OGRGeomFieldDefnH, } @@ -160,6 +190,6 @@ impl<'a> GeomField<'a> { if c_obj.is_null() { return Err(_last_null_pointer_err("OGR_GFld_GetSpatialRef")); } - unsafe { SpatialRef::from_c_obj(c_obj) } + Ok(unsafe { SpatialRefRef::from_ptr(c_obj).to_owned() }) } } diff --git a/src/vector/feature.rs b/src/vector/feature.rs index 069f9e922..3107ba297 100644 --- a/src/vector/feature.rs +++ b/src/vector/feature.rs @@ -1,69 +1,42 @@ use crate::utils::{_last_null_pointer_err, _string, _string_array}; use crate::vector::geometry::Geometry; -use crate::vector::{Defn, LayerAccess, OwnedLayer}; -use gdal_sys::{self, OGRErr, OGRFeatureH, OGRFieldType, OGRLayerH}; +use crate::vector::OwnedLayer; +use crate::vector::{Defn, GeometryRef, LayerAccess}; +use gdal_sys::{self, OGRErr, OGRFieldType, OGRLayerH}; use libc::{c_char, c_double, c_int, c_longlong}; use std::convert::TryInto; use std::ffi::{CString, NulError}; +use std::fmt::{Debug, Formatter}; +use std::marker::PhantomData; use std::ptr; use chrono::{DateTime, Datelike, FixedOffset, LocalResult, NaiveDate, TimeZone, Timelike}; use crate::errors::*; +use foreign_types::{foreign_type, ForeignType, ForeignTypeRef}; use std::slice; -/// OGR Feature -#[derive(Debug)] -pub struct Feature<'a> { - _defn: &'a Defn, - c_feature: OGRFeatureH, - geometry: Vec, +foreign_type! { + /// OGR Feature + pub unsafe type Feature<'a> { + type CType = libc::c_void; + type PhantomData = &'a (); + fn drop = gdal_sys::OGR_F_Destroy; + } } impl<'a> Feature<'a> { pub fn new(defn: &'a Defn) -> Result { - let c_feature = unsafe { gdal_sys::OGR_F_Create(defn.c_defn()) }; + let c_feature = unsafe { gdal_sys::OGR_F_Create(defn.as_ptr()) }; if c_feature.is_null() { return Err(_last_null_pointer_err("OGR_F_Create")); }; - Ok(Feature { - _defn: defn, - c_feature, - geometry: Feature::_lazy_feature_geometries(defn), - }) - } - - /// Creates a new Feature by wrapping a C pointer and a Defn - /// - /// # Safety - /// This method operates on a raw C pointer - pub unsafe fn from_c_feature(defn: &'a Defn, c_feature: OGRFeatureH) -> Feature { - Feature { - _defn: defn, - c_feature, - geometry: Feature::_lazy_feature_geometries(defn), - } - } - - /// Returns the C wrapped pointer - /// - /// # Safety - /// This method returns a raw C pointer - pub unsafe fn c_feature(&self) -> OGRFeatureH { - self.c_feature - } - - pub fn _lazy_feature_geometries(defn: &'a Defn) -> Vec { - let geom_field_count = - unsafe { gdal_sys::OGR_FD_GetGeomFieldCount(defn.c_defn()) } as isize; - (0..geom_field_count) - .map(|_| unsafe { Geometry::lazy_feature_geometry() }) - .collect() + Ok(unsafe { Feature::from_ptr(c_feature) }) } /// Returns the feature identifier, or `None` if none has been assigned. pub fn fid(&self) -> Option { - let fid = unsafe { gdal_sys::OGR_F_GetFID(self.c_feature) }; + let fid = unsafe { gdal_sys::OGR_F_GetFID(self.as_ptr()) }; if fid < 0 { None } else { @@ -91,59 +64,59 @@ impl<'a> Feature<'a> { /// /// If the field is null, returns `None`. fn field_from_id(&self, field_id: i32) -> Result> { - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_id) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_id) } != 0 { return Ok(None); } - let field_defn = unsafe { gdal_sys::OGR_F_GetFieldDefnRef(self.c_feature, field_id) }; + let field_defn = unsafe { gdal_sys::OGR_F_GetFieldDefnRef(self.as_ptr(), field_id) }; let field_type = unsafe { gdal_sys::OGR_Fld_GetType(field_defn) }; match field_type { OGRFieldType::OFTString => { - let rv = unsafe { gdal_sys::OGR_F_GetFieldAsString(self.c_feature, field_id) }; + let rv = unsafe { gdal_sys::OGR_F_GetFieldAsString(self.as_ptr(), field_id) }; Ok(Some(FieldValue::StringValue(_string(rv)))) } OGRFieldType::OFTStringList => { let rv = unsafe { - let ptr = gdal_sys::OGR_F_GetFieldAsStringList(self.c_feature, field_id); + let ptr = gdal_sys::OGR_F_GetFieldAsStringList(self.as_ptr(), field_id); _string_array(ptr) }; Ok(Some(FieldValue::StringListValue(rv))) } OGRFieldType::OFTReal => { - let rv = unsafe { gdal_sys::OGR_F_GetFieldAsDouble(self.c_feature, field_id) }; + let rv = unsafe { gdal_sys::OGR_F_GetFieldAsDouble(self.as_ptr(), field_id) }; Ok(Some(FieldValue::RealValue(rv))) } OGRFieldType::OFTRealList => { let rv = unsafe { let mut len: i32 = 0; let ptr = - gdal_sys::OGR_F_GetFieldAsDoubleList(self.c_feature, field_id, &mut len); + gdal_sys::OGR_F_GetFieldAsDoubleList(self.as_ptr(), field_id, &mut len); slice::from_raw_parts(ptr, len as usize).to_vec() }; Ok(Some(FieldValue::RealListValue(rv))) } OGRFieldType::OFTInteger => { - let rv = unsafe { gdal_sys::OGR_F_GetFieldAsInteger(self.c_feature, field_id) }; + let rv = unsafe { gdal_sys::OGR_F_GetFieldAsInteger(self.as_ptr(), field_id) }; Ok(Some(FieldValue::IntegerValue(rv))) } OGRFieldType::OFTIntegerList => { let rv = unsafe { let mut len: i32 = 0; let ptr = - gdal_sys::OGR_F_GetFieldAsIntegerList(self.c_feature, field_id, &mut len); + gdal_sys::OGR_F_GetFieldAsIntegerList(self.as_ptr(), field_id, &mut len); slice::from_raw_parts(ptr, len as usize).to_vec() }; Ok(Some(FieldValue::IntegerListValue(rv))) } OGRFieldType::OFTInteger64 => { - let rv = unsafe { gdal_sys::OGR_F_GetFieldAsInteger64(self.c_feature, field_id) }; + let rv = unsafe { gdal_sys::OGR_F_GetFieldAsInteger64(self.as_ptr(), field_id) }; Ok(Some(FieldValue::Integer64Value(rv))) } OGRFieldType::OFTInteger64List => { let rv = unsafe { let mut len: i32 = 0; let ptr = - gdal_sys::OGR_F_GetFieldAsInteger64List(self.c_feature, field_id, &mut len); + gdal_sys::OGR_F_GetFieldAsInteger64List(self.as_ptr(), field_id, &mut len); slice::from_raw_parts(ptr, len as usize).to_vec() }; Ok(Some(FieldValue::Integer64ListValue(rv))) @@ -168,7 +141,7 @@ impl<'a> Feature<'a> { fn field_idx_from_name>(&self, field_name: S) -> Result { let c_str_field_name = CString::new(field_name.as_ref())?; let field_id = - unsafe { gdal_sys::OGR_F_GetFieldIndex(self.c_feature, c_str_field_name.as_ptr()) }; + unsafe { gdal_sys::OGR_F_GetFieldIndex(self.as_ptr(), c_str_field_name.as_ptr()) }; if field_id == -1 { return Err(GdalError::InvalidFieldName { field_name: field_name.as_ref().to_string(), @@ -194,11 +167,11 @@ impl<'a> Feature<'a> { }); } - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger(self.c_feature, field_idx) }; + let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger(self.as_ptr(), field_idx) }; Ok(Some(value)) } @@ -213,11 +186,11 @@ impl<'a> Feature<'a> { pub fn field_as_integer_by_name(&self, field_name: &str) -> Result> { let field_idx = self.field_idx_from_name(field_name)?; - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger(self.c_feature, field_idx) }; + let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger(self.as_ptr(), field_idx) }; Ok(Some(value)) } @@ -232,11 +205,11 @@ impl<'a> Feature<'a> { pub fn field_as_integer64_by_name(&self, field_name: &str) -> Result> { let field_idx = self.field_idx_from_name(field_name)?; - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger64(self.c_feature, field_idx) }; + let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger64(self.as_ptr(), field_idx) }; Ok(Some(value)) } @@ -256,11 +229,11 @@ impl<'a> Feature<'a> { }); } - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger64(self.c_feature, field_idx) }; + let value = unsafe { gdal_sys::OGR_F_GetFieldAsInteger64(self.as_ptr(), field_idx) }; Ok(Some(value)) } @@ -275,11 +248,11 @@ impl<'a> Feature<'a> { pub fn field_as_double_by_name(&self, field_name: &str) -> Result> { let field_idx = self.field_idx_from_name(field_name)?; - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = unsafe { gdal_sys::OGR_F_GetFieldAsDouble(self.c_feature, field_idx) }; + let value = unsafe { gdal_sys::OGR_F_GetFieldAsDouble(self.as_ptr(), field_idx) }; Ok(Some(value)) } @@ -299,11 +272,11 @@ impl<'a> Feature<'a> { }); } - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = unsafe { gdal_sys::OGR_F_GetFieldAsDouble(self.c_feature, field_idx) }; + let value = unsafe { gdal_sys::OGR_F_GetFieldAsDouble(self.as_ptr(), field_idx) }; Ok(Some(value)) } @@ -317,11 +290,11 @@ impl<'a> Feature<'a> { pub fn field_as_string_by_name(&self, field_name: &str) -> Result> { let field_idx = self.field_idx_from_name(field_name)?; - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = _string(unsafe { gdal_sys::OGR_F_GetFieldAsString(self.c_feature, field_idx) }); + let value = _string(unsafe { gdal_sys::OGR_F_GetFieldAsString(self.as_ptr(), field_idx) }); Ok(Some(value)) } @@ -340,11 +313,11 @@ impl<'a> Feature<'a> { }); } - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } - let value = _string(unsafe { gdal_sys::OGR_F_GetFieldAsString(self.c_feature, field_idx) }); + let value = _string(unsafe { gdal_sys::OGR_F_GetFieldAsString(self.as_ptr(), field_idx) }); Ok(Some(value)) } @@ -361,7 +334,7 @@ impl<'a> Feature<'a> { ) -> Result>> { let field_idx = self.field_idx_from_name(field_name)?; - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } @@ -384,7 +357,7 @@ impl<'a> Feature<'a> { }); } - if unsafe { gdal_sys::OGR_F_IsFieldNull(self.c_feature, field_idx) } != 0 { + if unsafe { gdal_sys::OGR_F_IsFieldNull(self.as_ptr(), field_idx) } != 0 { return Ok(None); } @@ -404,7 +377,7 @@ impl<'a> Feature<'a> { let success = unsafe { gdal_sys::OGR_F_GetFieldAsDateTime( - self.c_feature, + self.as_ptr(), field_id, &mut year, &mut month, @@ -450,23 +423,18 @@ impl<'a> Feature<'a> { } /// Get the feature's geometry. - pub fn geometry(&self) -> Option<&Geometry> { - match self.geometry.first() { - Some(geom) => { - if !geom.has_gdal_ptr() { - let c_geom = unsafe { gdal_sys::OGR_F_GetGeometryRef(self.c_feature) }; - unsafe { geom.set_c_geometry(c_geom) }; - } - Some(geom) - } - None => None, + pub fn geometry(&self) -> Option<&GeometryRef> { + if self.geom_field_count() <= 0 { + return None; } + let c_geom = unsafe { gdal_sys::OGR_F_GetGeometryRef(self.as_ptr()) }; + Some(unsafe { GeometryRef::from_ptr(c_geom) }) } - pub fn geometry_by_name(&self, field_name: &str) -> Result<&Geometry> { + pub fn geometry_by_name(&self, field_name: &str) -> Result<&GeometryRef> { let c_str_field_name = CString::new(field_name)?; let idx = - unsafe { gdal_sys::OGR_F_GetGeomFieldIndex(self.c_feature, c_str_field_name.as_ptr()) }; + unsafe { gdal_sys::OGR_F_GetGeomFieldIndex(self.as_ptr(), c_str_field_name.as_ptr()) }; if idx == -1 { Err(GdalError::InvalidFieldName { field_name: field_name.to_string(), @@ -477,25 +445,22 @@ impl<'a> Feature<'a> { } } - pub fn geometry_by_index(&self, idx: usize) -> Result<&Geometry> { - if idx >= self.geometry.len() { + pub fn geometry_by_index(&self, idx: usize) -> Result<&GeometryRef> { + if idx as i32 >= self.geom_field_count() { return Err(GdalError::InvalidFieldIndex { index: idx, method_name: "geometry_by_index", }); } - if !self.geometry[idx].has_gdal_ptr() { - let c_geom = unsafe { gdal_sys::OGR_F_GetGeomFieldRef(self.c_feature, idx as i32) }; - if c_geom.is_null() { - return Err(_last_null_pointer_err("OGR_F_GetGeomFieldRef")); - } - unsafe { self.geometry[idx].set_c_geometry(c_geom) }; + let c_geom = unsafe { gdal_sys::OGR_F_GetGeomFieldRef(self.as_ptr(), idx as i32) }; + if c_geom.is_null() { + return Err(_last_null_pointer_err("OGR_F_GetGeomFieldRef")); } - Ok(&self.geometry[idx]) + Ok(unsafe { GeometryRef::from_ptr(c_geom) }) } pub fn create(&self, lyr: &L) -> Result<()> { - let rv = unsafe { gdal_sys::OGR_L_CreateFeature(lyr.c_layer(), self.c_feature) }; + let rv = unsafe { gdal_sys::OGR_L_CreateFeature(lyr.c_layer(), self.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -508,7 +473,7 @@ impl<'a> Feature<'a> { pub fn set_field_string(&self, field_name: &str, value: &str) -> Result<()> { let c_str_value = CString::new(value)?; let idx = self.field_idx_from_name(field_name)?; - unsafe { gdal_sys::OGR_F_SetFieldString(self.c_feature, idx, c_str_value.as_ptr()) }; + unsafe { gdal_sys::OGR_F_SetFieldString(self.as_ptr(), idx, c_str_value.as_ptr()) }; Ok(()) } @@ -526,13 +491,13 @@ impl<'a> Feature<'a> { // gdal-sys despite being constant. let c_value = c_str_ptrs.as_ptr() as *mut *mut c_char; let idx = self.field_idx_from_name(field_name)?; - unsafe { gdal_sys::OGR_F_SetFieldStringList(self.c_feature, idx, c_value) }; + unsafe { gdal_sys::OGR_F_SetFieldStringList(self.as_ptr(), idx, c_value) }; Ok(()) } pub fn set_field_double(&self, field_name: &str, value: f64) -> Result<()> { let idx = self.field_idx_from_name(field_name)?; - unsafe { gdal_sys::OGR_F_SetFieldDouble(self.c_feature, idx, value as c_double) }; + unsafe { gdal_sys::OGR_F_SetFieldDouble(self.as_ptr(), idx, value as c_double) }; Ok(()) } @@ -540,7 +505,7 @@ impl<'a> Feature<'a> { let idx = self.field_idx_from_name(field_name)?; unsafe { gdal_sys::OGR_F_SetFieldDoubleList( - self.c_feature, + self.as_ptr(), idx, value.len() as c_int, value.as_ptr(), @@ -551,7 +516,7 @@ impl<'a> Feature<'a> { pub fn set_field_integer(&self, field_name: &str, value: i32) -> Result<()> { let idx = self.field_idx_from_name(field_name)?; - unsafe { gdal_sys::OGR_F_SetFieldInteger(self.c_feature, idx, value as c_int) }; + unsafe { gdal_sys::OGR_F_SetFieldInteger(self.as_ptr(), idx, value as c_int) }; Ok(()) } @@ -559,7 +524,7 @@ impl<'a> Feature<'a> { let idx = self.field_idx_from_name(field_name)?; unsafe { gdal_sys::OGR_F_SetFieldIntegerList( - self.c_feature, + self.as_ptr(), idx, value.len() as c_int, value.as_ptr(), @@ -570,7 +535,7 @@ impl<'a> Feature<'a> { pub fn set_field_integer64(&self, field_name: &str, value: i64) -> Result<()> { let idx = self.field_idx_from_name(field_name)?; - unsafe { gdal_sys::OGR_F_SetFieldInteger64(self.c_feature, idx, value as c_longlong) }; + unsafe { gdal_sys::OGR_F_SetFieldInteger64(self.as_ptr(), idx, value as c_longlong) }; Ok(()) } @@ -578,7 +543,7 @@ impl<'a> Feature<'a> { let idx = self.field_idx_from_name(field_name)?; unsafe { gdal_sys::OGR_F_SetFieldInteger64List( - self.c_feature, + self.as_ptr(), idx, value.len() as c_int, value.as_ptr(), @@ -604,7 +569,7 @@ impl<'a> Feature<'a> { unsafe { gdal_sys::OGR_F_SetFieldDateTime( - self.c_feature, + self.as_ptr(), idx, year, month, @@ -649,18 +614,21 @@ impl<'a> Feature<'a> { } pub fn set_geometry(&mut self, geom: Geometry) -> Result<()> { - let rv = unsafe { gdal_sys::OGR_F_SetGeometry(self.c_feature, geom.c_geometry()) }; + let rv = unsafe { gdal_sys::OGR_F_SetGeometry(self.as_ptr(), geom.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, method_name: "OGR_F_SetGeometry", }); } - self.geometry[0] = geom; Ok(()) } pub fn field_count(&self) -> i32 { - unsafe { gdal_sys::OGR_F_GetFieldCount(self.c_feature) } + unsafe { gdal_sys::OGR_F_GetFieldCount(self.as_ptr()) } + } + + pub fn geom_field_count(&self) -> i32 { + unsafe { gdal_sys::OGR_F_GetGeomFieldCount(self.as_ptr()) } } pub fn fields(&self) -> FieldValueIterator { @@ -668,6 +636,17 @@ impl<'a> Feature<'a> { } } +impl Debug for Feature<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let fields = self.fields().collect::>(); + f.debug_struct("Feature") + .field("fid", &self.fid()) + .field("geometry", &self.geometry()) + .field("fields", &fields) + .finish() + } +} + pub struct FieldValueIterator<'a> { feature: &'a Feature<'a>, idx: i32, @@ -692,8 +671,7 @@ impl<'a> Iterator for FieldValueIterator<'a> { let idx = self.idx; if idx < self.count { self.idx += 1; - let field_defn = - unsafe { gdal_sys::OGR_F_GetFieldDefnRef(self.feature.c_feature, idx) }; + let field_defn = unsafe { gdal_sys::OGR_F_GetFieldDefnRef(self.feature.as_ptr(), idx) }; let field_name = unsafe { gdal_sys::OGR_Fld_GetNameRef(field_defn) }; let name = _string(field_name); let fv: Option<(String, Option)> = self @@ -719,18 +697,10 @@ impl<'a> Iterator for FieldValueIterator<'a> { } } -impl<'a> Drop for Feature<'a> { - fn drop(&mut self) { - unsafe { - gdal_sys::OGR_F_Destroy(self.c_feature); - } - } -} - pub struct FeatureIterator<'a> { - defn: &'a Defn, c_layer: OGRLayerH, size_hint: Option, + _lifetime: PhantomData<&'a ()>, } impl<'a> Iterator for FeatureIterator<'a> { @@ -742,7 +712,7 @@ impl<'a> Iterator for FeatureIterator<'a> { if c_feature.is_null() { None } else { - Some(unsafe { Feature::from_c_feature(self.defn, c_feature) }) + Some(unsafe { Feature::from_ptr(c_feature) }) } } @@ -756,12 +726,11 @@ impl<'a> Iterator for FeatureIterator<'a> { impl<'a> FeatureIterator<'a> { pub(crate) fn _with_layer(layer: &'a L) -> Self { - let defn = layer.defn(); let size_hint = layer.try_feature_count().and_then(|s| s.try_into().ok()); Self { c_layer: unsafe { layer.c_layer() }, size_hint, - defn, + _lifetime: PhantomData, } } } @@ -785,12 +754,7 @@ where return None; } - Some(unsafe { - // We have to convince the compiler that our `Defn` adheres to our iterator lifetime `<'a>` - let defn: &'a Defn = std::mem::transmute::<&'_ _, &'a _>(self.layer.defn()); - - Feature::from_c_feature(defn, c_feature) - }) + Some(unsafe { Feature::from_ptr(c_feature) }) } fn size_hint(&self) -> (usize, Option) { diff --git a/src/vector/geometry.rs b/src/vector/geometry.rs index 9f313bc5b..bebab717d 100644 --- a/src/vector/geometry.rs +++ b/src/vector/geometry.rs @@ -1,78 +1,32 @@ -use std::cell::RefCell; -use std::fmt::{self, Debug, Formatter}; -use std::marker::PhantomData; -use std::mem::MaybeUninit; -use std::ops::{Deref, DerefMut}; +use std::fmt::{self, Debug}; +use std::mem::{ManuallyDrop, MaybeUninit}; +use foreign_types::{foreign_type, ForeignType, ForeignTypeRef}; use libc::{c_double, c_int}; -use gdal_sys::{self, OGRErr, OGRGeometryH, OGRwkbGeometryType}; +use gdal_sys::{self, OGRErr, OGRwkbGeometryType}; use crate::errors::*; -use crate::spatial_ref::SpatialRef; +use crate::spatial_ref::{SpatialRef, SpatialRefRef}; use crate::utils::{_last_null_pointer_err, _string}; use crate::vector::{Envelope, Envelope3D}; -/// OGR Geometry -pub struct Geometry { - c_geometry_ref: RefCell>, - owned: bool, +foreign_type! { + /// OGR Geometry + pub unsafe type Geometry { + type CType = libc::c_void; + fn drop = gdal_sys::OGR_G_DestroyGeometry; + fn clone = gdal_sys::OGR_G_Clone; + } } impl Geometry { - /// Create a new Geometry without a C pointer - /// - /// # Safety - /// This method returns a Geometry without wrapped pointer - pub unsafe fn lazy_feature_geometry() -> Geometry { - // Geometry objects created with this method map to a Feature's - // geometry whose memory is managed by the GDAL feature. - // This object has a tricky lifecycle: - // - // * Initially it's created with a null c_geometry - // * The first time `Feature::geometry` is called, it gets - // c_geometry from GDAL and calls `set_c_geometry` with it. - // * When the Feature is destroyed, this object is also destroyed, - // which is good, because that's when c_geometry (which is managed - // by the GDAL feature) becomes invalid. Because `self.owned` is - // `true`, we don't call `OGR_G_DestroyGeometry`. - Geometry { - c_geometry_ref: RefCell::new(None), - owned: false, - } - } - - pub fn has_gdal_ptr(&self) -> bool { - self.c_geometry_ref.borrow().is_some() - } - - /// Set the wrapped C pointer - /// - /// # Safety - /// This method operates on a raw C pointer - pub unsafe fn set_c_geometry(&self, c_geometry: OGRGeometryH) { - assert!(!self.has_gdal_ptr()); - assert!(!self.owned); - *(self.c_geometry_ref.borrow_mut()) = Some(c_geometry); - } - - pub(crate) unsafe fn with_c_geometry(c_geom: OGRGeometryH, owned: bool) -> Geometry { - Geometry { - c_geometry_ref: RefCell::new(Some(c_geom)), - owned, - } - } - pub fn empty(wkb_type: OGRwkbGeometryType::Type) -> Result { let c_geom = unsafe { gdal_sys::OGR_G_CreateGeometry(wkb_type) }; if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_CreateGeometry")); }; - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) - } - - pub fn is_empty(&self) -> bool { - unsafe { gdal_sys::OGR_G_IsEmpty(self.c_geometry()) == 1 } + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Create a rectangular geometry from West, South, East and North values. @@ -81,30 +35,18 @@ impl Geometry { "POLYGON (({w} {n}, {e} {n}, {e} {s}, {w} {s}, {w} {n}))", )) } +} - /// Returns a C pointer to the wrapped Geometry - /// - /// # Safety - /// This method returns a raw C pointer - pub unsafe fn c_geometry(&self) -> OGRGeometryH { - self.c_geometry_ref.borrow().unwrap() - } - - /// Returns the C pointer of a Geometry - /// - /// # Safety - /// This method returns a raw C pointer - pub unsafe fn into_c_geometry(mut self) -> OGRGeometryH { - assert!(self.owned); - self.owned = false; - self.c_geometry() +impl GeometryRef { + pub fn is_empty(&self) -> bool { + unsafe { gdal_sys::OGR_G_IsEmpty(self.as_ptr()) == 1 } } pub fn set_point(&mut self, i: usize, point: (f64, f64, f64)) { let (x, y, z) = point; unsafe { gdal_sys::OGR_G_SetPoint( - self.c_geometry(), + self.as_ptr(), i as c_int, x as c_double, y as c_double, @@ -116,25 +58,20 @@ impl Geometry { pub fn set_point_2d(&mut self, i: usize, p: (f64, f64)) { let (x, y) = p; unsafe { - gdal_sys::OGR_G_SetPoint_2D(self.c_geometry(), i as c_int, x as c_double, y as c_double) + gdal_sys::OGR_G_SetPoint_2D(self.as_ptr(), i as c_int, x as c_double, y as c_double) }; } pub fn add_point(&mut self, p: (f64, f64, f64)) { let (x, y, z) = p; unsafe { - gdal_sys::OGR_G_AddPoint( - self.c_geometry(), - x as c_double, - y as c_double, - z as c_double, - ) + gdal_sys::OGR_G_AddPoint(self.as_ptr(), x as c_double, y as c_double, z as c_double) }; } pub fn add_point_2d(&mut self, p: (f64, f64)) { let (x, y) = p; - unsafe { gdal_sys::OGR_G_AddPoint_2D(self.c_geometry(), x as c_double, y as c_double) }; + unsafe { gdal_sys::OGR_G_AddPoint_2D(self.as_ptr(), x as c_double, y as c_double) }; } /// Get point coordinates from a line string or a point geometry. @@ -146,12 +83,12 @@ impl Geometry { let mut x: c_double = 0.; let mut y: c_double = 0.; let mut z: c_double = 0.; - unsafe { gdal_sys::OGR_G_GetPoint(self.c_geometry(), index, &mut x, &mut y, &mut z) }; + unsafe { gdal_sys::OGR_G_GetPoint(self.as_ptr(), index, &mut x, &mut y, &mut z) }; (x, y, z) } pub fn get_point_vec(&self) -> Vec<(f64, f64, f64)> { - let length = unsafe { gdal_sys::OGR_G_GetPointCount(self.c_geometry()) }; + let length = unsafe { gdal_sys::OGR_G_GetPointCount(self.as_ptr()) }; (0..length).map(|i| self.get_point(i)).collect() } @@ -159,7 +96,7 @@ impl Geometry { /// /// See: [OGR_G_GetGeometryType](https://gdal.org/api/vector_c_api.html#_CPPv421OGR_G_GetGeometryType12OGRGeometryH) pub fn geometry_type(&self) -> OGRwkbGeometryType::Type { - unsafe { gdal_sys::OGR_G_GetGeometryType(self.c_geometry()) } + unsafe { gdal_sys::OGR_G_GetGeometryType(self.as_ptr()) } } /// Get the WKT name for the type of this geometry. @@ -168,7 +105,7 @@ impl Geometry { pub fn geometry_name(&self) -> String { // Note: C API makes no statements about this possibly returning null. // So we don't have to result wrap this, - let c_str = unsafe { gdal_sys::OGR_G_GetGeometryName(self.c_geometry()) }; + let c_str = unsafe { gdal_sys::OGR_G_GetGeometryName(self.as_ptr()) }; if c_str.is_null() { "".into() } else { @@ -185,7 +122,7 @@ impl Geometry { /// /// See: [`OGR_G_GetGeometryCount`](https://gdal.org/api/vector_c_api.html#_CPPv422OGR_G_GetGeometryCount12OGRGeometryH) pub fn geometry_count(&self) -> usize { - let cnt = unsafe { gdal_sys::OGR_G_GetGeometryCount(self.c_geometry()) }; + let cnt = unsafe { gdal_sys::OGR_G_GetGeometryCount(self.as_ptr()) }; cnt as usize } @@ -195,35 +132,40 @@ impl Geometry { /// /// See: [`OGR_G_GetPointCount`](https://gdal.org/api/vector_c_api.html#_CPPv419OGR_G_GetPointCount12OGRGeometryH) pub fn point_count(&self) -> usize { - let cnt = unsafe { gdal_sys::OGR_G_GetPointCount(self.c_geometry()) }; + let cnt = unsafe { gdal_sys::OGR_G_GetPointCount(self.as_ptr()) }; cnt as usize } - /// Returns the n-th sub-geometry as a non-owned Geometry. + /// Get a reference to the geometry at given `index` /// - /// # Safety - /// Don't keep this object for long. - pub unsafe fn get_unowned_geometry(&self, n: usize) -> Geometry { - // get the n-th sub-geometry as a non-owned Geometry; don't keep this - // object for long. - let c_geom = gdal_sys::OGR_G_GetGeometryRef(self.c_geometry(), n as c_int); - Geometry::with_c_geometry(c_geom, false) + /// # Arguments + /// * `index`: the index of the geometry to fetch, between 0 and getNumGeometries() - 1. + pub fn get_geometry(&self, index: usize) -> &GeometryRef { + let c_geom = unsafe { gdal_sys::OGR_G_GetGeometryRef(self.as_ptr(), index as c_int) }; + unsafe { GeometryRef::from_ptr(c_geom) } } - /// Get a reference to the geometry at given `index` - pub fn get_geometry(&self, index: usize) -> GeometryRef { - let geom = unsafe { self.get_unowned_geometry(index) }; - GeometryRef { - geom, - _lifetime: PhantomData, - } + /// Get a mutable reference to the geometry at given `index` + /// + /// # Arguments + /// * `index`: the index of the geometry to fetch, between 0 and getNumGeometries() - 1. + pub fn get_geometry_mut(&mut self, index: usize) -> &mut GeometryRef { + let c_geom = unsafe { gdal_sys::OGR_G_GetGeometryRef(self.as_ptr(), index as c_int) }; + unsafe { GeometryRef::from_ptr_mut(c_geom) } } - pub fn add_geometry(&mut self, mut sub: Geometry) -> Result<()> { - assert!(sub.owned); - sub.owned = false; - let rv = - unsafe { gdal_sys::OGR_G_AddGeometryDirectly(self.c_geometry(), sub.c_geometry()) }; + /// Add a subgeometry + /// + /// # Arguments + /// * `sub`: geometry to add as a child of `self`. + pub fn add_geometry(&mut self, sub: Geometry) -> Result<()> { + // `OGR_G_AddGeometryDirectly` takes ownership of the Geometry. + // According to https://doc.rust-lang.org/std/mem/fn.forget.html#relationship-with-manuallydrop + // `ManuallyDrop` is the suggested means of transferring memory management while + // robustly preventing a double-free. + let sub = ManuallyDrop::new(sub); + + let rv = unsafe { gdal_sys::OGR_G_AddGeometryDirectly(self.as_ptr(), sub.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -240,7 +182,7 @@ impl Geometry { /// /// See: [`OGR_G_Length`](https://gdal.org/api/vector_c_api.html#_CPPv412OGR_G_Length12OGRGeometryH) pub fn length(&self) -> f64 { - unsafe { gdal_sys::OGR_G_Length(self.c_geometry()) } + unsafe { gdal_sys::OGR_G_Length(self.as_ptr()) } } /// Compute geometry area in square units of the spatial reference system in use. @@ -250,7 +192,7 @@ impl Geometry { /// /// See: [`OGR_G_Area`](https://gdal.org/api/vector_c_api.html#_CPPv410OGR_G_Area12OGRGeometryH) pub fn area(&self) -> f64 { - unsafe { gdal_sys::OGR_G_Area(self.c_geometry()) } + unsafe { gdal_sys::OGR_G_Area(self.as_ptr()) } } /// Computes and returns the axis-aligned 2D bounding envelope for this geometry. @@ -259,7 +201,7 @@ impl Geometry { pub fn envelope(&self) -> Envelope { let mut envelope = MaybeUninit::uninit(); unsafe { - gdal_sys::OGR_G_GetEnvelope(self.c_geometry(), envelope.as_mut_ptr()); + gdal_sys::OGR_G_GetEnvelope(self.as_ptr(), envelope.as_mut_ptr()); envelope.assume_init() } } @@ -270,7 +212,7 @@ impl Geometry { pub fn envelope_3d(&self) -> Envelope3D { let mut envelope = MaybeUninit::uninit(); unsafe { - gdal_sys::OGR_G_GetEnvelope3D(self.c_geometry(), envelope.as_mut_ptr()); + gdal_sys::OGR_G_GetEnvelope3D(self.as_ptr(), envelope.as_mut_ptr()); envelope.assume_init() } } @@ -279,7 +221,7 @@ impl Geometry { /// /// See: [`OGR_G_FlattenTo2D`](https://gdal.org/api/vector_c_api.html#_CPPv417OGR_G_FlattenTo2D12OGRGeometryH) pub fn flatten_to_2d(&mut self) { - unsafe { gdal_sys::OGR_G_FlattenTo2D(self.c_geometry()) }; + unsafe { gdal_sys::OGR_G_FlattenTo2D(self.as_ptr()) }; } /// Get the spatial reference system for this geometry. @@ -288,19 +230,17 @@ impl Geometry { /// /// See: [OGR_G_GetSpatialReference](https://gdal.org/doxygen/ogr__api_8h.html#abc393e40282eec3801fb4a4abc9e25bf) pub fn spatial_ref(&self) -> Option { - let c_spatial_ref = unsafe { gdal_sys::OGR_G_GetSpatialReference(self.c_geometry()) }; + let c_spatial_ref = unsafe { gdal_sys::OGR_G_GetSpatialReference(self.as_ptr()) }; if c_spatial_ref.is_null() { None } else { - unsafe { SpatialRef::from_c_obj(c_spatial_ref) }.ok() + Some(unsafe { SpatialRefRef::from_ptr(c_spatial_ref).to_owned() }) } } pub fn set_spatial_ref(&mut self, spatial_ref: SpatialRef) { - unsafe { - gdal_sys::OGR_G_AssignSpatialReference(self.c_geometry(), spatial_ref.to_c_hsrs()) - }; + unsafe { gdal_sys::OGR_G_AssignSpatialReference(self.as_ptr(), spatial_ref.as_ptr()) }; } /// Test if the geometry is valid. @@ -315,41 +255,37 @@ impl Geometry { /// /// [has_geos]: crate::version::VersionInfo::has_geos pub fn is_valid(&self) -> bool { - let p = unsafe { gdal_sys::OGR_G_IsValid(self.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_IsValid(self.as_ptr()) }; p != 0 } } -impl Drop for Geometry { - fn drop(&mut self) { - if self.owned { - let c_geometry = self.c_geometry_ref.borrow(); - unsafe { gdal_sys::OGR_G_DestroyGeometry(c_geometry.unwrap()) }; +impl Debug for GeometryRef { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.wkt() { + Ok(wkt) => f.write_str(wkt.as_str()), + Err(_) => Err(fmt::Error), } } } -impl Clone for Geometry { - fn clone(&self) -> Geometry { - // assert!(self.has_gdal_ptr()); - let c_geometry = self.c_geometry_ref.borrow(); - let new_c_geom = unsafe { gdal_sys::OGR_G_Clone(c_geometry.unwrap()) }; - unsafe { Geometry::with_c_geometry(new_c_geom, true) } +impl PartialEq for GeometryRef { + fn eq(&self, other: &Self) -> bool { + unsafe { gdal_sys::OGR_G_Equal(self.as_ptr(), other.as_ptr()) != 0 } } } +impl Eq for GeometryRef {} + impl Debug for Geometry { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self.wkt() { - Ok(wkt) => f.write_str(wkt.as_str()), - Err(_) => Err(fmt::Error), - } + Debug::fmt(self.as_ref(), f) } } impl PartialEq for Geometry { fn eq(&self, other: &Self) -> bool { - unsafe { gdal_sys::OGR_G_Equal(self.c_geometry(), other.c_geometry()) != 0 } + PartialEq::eq(self.as_ref(), other.as_ref()) } } @@ -362,41 +298,17 @@ pub fn geometry_type_to_name(ty: OGRwkbGeometryType::Type) -> String { _string(rv) } -/// Reference to owned geometry -pub struct GeometryRef<'a> { - geom: Geometry, - _lifetime: PhantomData<&'a ()>, -} - -impl Deref for GeometryRef<'_> { - type Target = Geometry; - - fn deref(&self) -> &Self::Target { - &self.geom - } -} - -impl DerefMut for GeometryRef<'_> { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.geom - } -} - -impl Debug for GeometryRef<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - Debug::fmt(&self.geom, f) - } -} - #[cfg(test)] mod tests { - use super::*; - use crate::spatial_ref::SpatialRef; - use crate::test_utils::SuppressGDALErrorLog; use gdal_sys::OGRwkbGeometryType::{ wkbLineString, wkbLinearRing, wkbMultiPoint, wkbMultiPolygon, wkbPoint, wkbPolygon, }; + use crate::spatial_ref::SpatialRef; + use crate::test_utils::SuppressGDALErrorLog; + + use super::*; + #[test] fn test_create_bbox() { let bbox = Geometry::bbox(-27., 33., 52., 85.).unwrap(); @@ -550,9 +462,10 @@ mod tests { #[test] pub fn test_geometry_modify() { - let polygon = Geometry::from_wkt("POLYGON ((30 10, 40 40, 20 40, 10 20, 30 10))").unwrap(); + let mut polygon = + Geometry::from_wkt("POLYGON ((30 10, 40 40, 20 40, 10 20, 30 10))").unwrap(); for i in 0..polygon.geometry_count() { - let mut ring = polygon.get_geometry(i); + let ring = &mut polygon.get_geometry_mut(i); for j in 0..ring.point_count() { let (x, y, _) = ring.get_point(j as i32); ring.set_point_2d(j, (x * 10.0, y * 10.0)); diff --git a/src/vector/layer.rs b/src/vector/layer.rs index 0ecf16e48..13664396b 100644 --- a/src/vector/layer.rs +++ b/src/vector/layer.rs @@ -1,9 +1,12 @@ use crate::metadata::Metadata; -use crate::spatial_ref::SpatialRef; +use crate::spatial_ref::{SpatialRef, SpatialRefRef}; use crate::utils::{_last_null_pointer_err, _string}; -use crate::vector::defn::Defn; -use crate::vector::{Envelope, Feature, FieldValue, Geometry, LayerOptions}; +use crate::vector::defn::{Defn, DefnRef}; +use crate::vector::{ + Envelope, Feature, FeatureIterator, FieldValue, Geometry, LayerOptions, OwnedFeatureIterator, +}; use crate::{dataset::Dataset, gdal_major_object::MajorObject}; +use foreign_types::{ForeignType, ForeignTypeRef}; use gdal_sys::{self, GDALMajorObjectH, OGRErr, OGRFieldDefnH, OGRFieldType, OGRLayerH}; use libc::c_int; use std::ffi::NulError; @@ -12,7 +15,6 @@ use std::ptr::null_mut; use std::{convert::TryInto, ffi::CString, marker::PhantomData}; use crate::errors::*; -use crate::vector::feature::{FeatureIterator, OwnedFeatureIterator}; /// Layer capabilities #[allow(clippy::upper_case_acronyms)] @@ -130,7 +132,7 @@ impl<'a> Layer<'a> { /// This method operates on a raw C pointer pub(crate) unsafe fn from_c_layer(_: &'a Dataset, c_layer: OGRLayerH) -> Self { let c_defn = gdal_sys::OGR_L_GetLayerDefn(c_layer); - let defn = Defn::from_c_defn(c_defn); + let defn = DefnRef::from_ptr(c_defn).to_owned(); Self { c_layer, defn, @@ -185,7 +187,7 @@ impl OwnedLayer { /// This method operates on a raw C pointer pub(crate) unsafe fn from_c_layer(dataset: Dataset, c_layer: OGRLayerH) -> Self { let c_defn = gdal_sys::OGR_L_GetLayerDefn(c_layer); - let defn = Defn::from_c_defn(c_defn); + let defn = DefnRef::from_ptr(c_defn).to_owned(); Self { c_layer, defn, @@ -240,7 +242,7 @@ pub trait LayerAccess: Sized { if c_feature.is_null() { None } else { - Some(unsafe { Feature::from_c_feature(self.defn(), c_feature) }) + Some(unsafe { Feature::from_ptr(c_feature) }) } } @@ -258,7 +260,7 @@ pub trait LayerAccess: Sized { /// /// See: [SetFeature](https://gdal.org/doxygen/classOGRLayer.html#a681139bfd585b74d7218e51a32144283) fn set_feature(&self, feature: Feature) -> Result<()> { - unsafe { gdal_sys::OGR_L_SetFeature(self.c_layer(), feature.c_feature()) }; + unsafe { gdal_sys::OGR_L_SetFeature(self.c_layer(), feature.into_ptr()) }; Ok(()) } @@ -266,7 +268,7 @@ pub trait LayerAccess: Sized { /// /// See: [OGR_L_SetSpatialFilter](https://gdal.org/doxygen/classOGRLayer.html#a75c06b4993f8eb76b569f37365cd19ab) fn set_spatial_filter(&mut self, geometry: &Geometry) { - unsafe { gdal_sys::OGR_L_SetSpatialFilter(self.c_layer(), geometry.c_geometry()) }; + unsafe { gdal_sys::OGR_L_SetSpatialFilter(self.c_layer(), geometry.as_ptr()) }; } /// Set a spatial rectangle filter on this layer by specifying the bounds of a rectangle. @@ -301,15 +303,15 @@ pub trait LayerAccess: Sized { fn create_feature(&mut self, geometry: Geometry) -> Result<()> { let feature = Feature::new(self.defn())?; - let c_geometry = unsafe { geometry.into_c_geometry() }; - let rv = unsafe { gdal_sys::OGR_F_SetGeometryDirectly(feature.c_feature(), c_geometry) }; + let rv = + unsafe { gdal_sys::OGR_F_SetGeometryDirectly(feature.as_ptr(), geometry.into_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, method_name: "OGR_F_SetGeometryDirectly", }); } - let rv = unsafe { gdal_sys::OGR_L_CreateFeature(self.c_layer(), feature.c_feature()) }; + let rv = unsafe { gdal_sys::OGR_L_CreateFeature(self.c_layer(), feature.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -421,7 +423,7 @@ pub trait LayerAccess: Sized { if c_obj.is_null() { None } else { - unsafe { SpatialRef::from_c_obj(c_obj) }.ok() + Some(unsafe { SpatialRefRef::from_ptr(c_obj).to_owned() }) } } @@ -652,7 +654,6 @@ impl Dataset { pub fn layers(&self) -> LayerIterator { LayerIterator::with_dataset(self) } - /// Creates a new layer. The [`LayerOptions`] struct implements `Default`, so you only need to /// specify those options that deviate from the default. /// @@ -685,7 +686,7 @@ impl Dataset { pub fn create_layer(&mut self, options: LayerOptions<'_>) -> Result { let c_name = CString::new(options.name)?; let c_srs = match options.srs { - Some(srs) => srs.to_c_hsrs(), + Some(srs) => srs.as_ptr(), None => null_mut(), }; @@ -1254,6 +1255,7 @@ mod tests { #[test] fn test_geom_accessors() { with_feature("roads.geojson", 236194095, |feature| { + println!("{feature:?}"); let geom = feature.geometry().unwrap(); assert_eq!(geom.geometry_type(), OGRwkbGeometryType::wkbLineString); let coords = geom.get_point_vec(); diff --git a/src/vector/mod.rs b/src/vector/mod.rs index 8431e97e7..a1c1921c8 100644 --- a/src/vector/mod.rs +++ b/src/vector/mod.rs @@ -64,30 +64,29 @@ //! ``` //! -mod defn; -mod feature; -mod geometry; -mod layer; -mod ops; -mod options; -pub mod sql; -mod transaction; - -pub use defn::{Defn, Field, FieldIterator}; +pub use defn::{Defn, DefnRef, Field, FieldIterator}; pub use feature::{ field_type_to_name, Feature, FeatureIterator, FieldValue, FieldValueIterator, OwnedFeatureIterator, }; pub use gdal_sys::{OGRFieldType, OGRwkbGeometryType}; -pub use geometry::{geometry_type_to_name, Geometry}; +pub use geometry::{geometry_type_to_name, Geometry, GeometryRef}; pub use layer::{FieldDefn, Layer, LayerAccess, LayerCaps, LayerIterator, OwnedLayer}; +pub use ops::ToGdal; pub use options::LayerOptions; pub use transaction::Transaction; +mod defn; +mod feature; +mod geometry; +mod layer; +mod ops; +mod options; +pub mod sql; +mod transaction; + /// Axis aligned 2D bounding box. pub type Envelope = gdal_sys::OGREnvelope; /// Axis aligned 3D bounding box. pub type Envelope3D = gdal_sys::OGREnvelope3D; - -pub use ops::ToGdal; diff --git a/src/vector/ops/conversions/formats.rs b/src/vector/ops/conversions/formats.rs index c99ab658c..815eb40e1 100644 --- a/src/vector/ops/conversions/formats.rs +++ b/src/vector/ops/conversions/formats.rs @@ -1,7 +1,8 @@ use crate::errors::GdalError; use crate::errors::Result; use crate::utils::{_last_null_pointer_err, _string}; -use crate::vector::Geometry; +use crate::vector::{Geometry, GeometryRef}; +use foreign_types::{ForeignType, ForeignTypeRef}; use gdal_sys::OGRErr; use libc::c_char; use std::ffi::{c_void, CString}; @@ -32,7 +33,7 @@ impl Geometry { method_name: "OGR_G_CreateFromWkt", }); } - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Creates a geometry by parsing a slice of bytes in @@ -54,7 +55,7 @@ impl Geometry { method_name: "OGR_G_CreateFromWkb", }); } - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Create a geometry by parsing a @@ -65,7 +66,7 @@ impl Geometry { if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_CreateGeometryFromJson")); } - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Create a geometry by parsing a @@ -76,13 +77,15 @@ impl Geometry { if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_CreateFromGML")); } - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } +} +impl GeometryRef { /// Serialize the geometry as WKT. pub fn wkt(&self) -> Result { let mut c_wkt = null_mut(); - let rv = unsafe { gdal_sys::OGR_G_ExportToWkt(self.c_geometry(), &mut c_wkt) }; + let rv = unsafe { gdal_sys::OGR_G_ExportToWkt(self.as_ptr(), &mut c_wkt) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -98,13 +101,13 @@ impl Geometry { /// [WKB](https://en.wikipedia.org/wiki/Well-known_text_representation_of_geometry#Well-known_binary) /// (Well-Known Binary) format. pub fn wkb(&self) -> Result> { - let wkb_size = unsafe { gdal_sys::OGR_G_WkbSize(self.c_geometry()) as usize }; + let wkb_size = unsafe { gdal_sys::OGR_G_WkbSize(self.as_ptr()) as usize }; // We default to little-endian for now. A WKB string explicitly indicates the byte // order, so this is not a problem for interoperability. let byte_order = gdal_sys::OGRwkbByteOrder::wkbNDR; let mut wkb = vec![0; wkb_size]; let rv = - unsafe { gdal_sys::OGR_G_ExportToWkb(self.c_geometry(), byte_order, wkb.as_mut_ptr()) }; + unsafe { gdal_sys::OGR_G_ExportToWkb(self.as_ptr(), byte_order, wkb.as_mut_ptr()) }; if rv != gdal_sys::OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -118,7 +121,7 @@ impl Geometry { /// /// See: [`OGR_G_ExportToJson`](https://gdal.org/api/vector_c_api.html#_CPPv418OGR_G_ExportToJson12OGRGeometryH) pub fn json(&self) -> Result { - let c_json = unsafe { gdal_sys::OGR_G_ExportToJson(self.c_geometry()) }; + let c_json = unsafe { gdal_sys::OGR_G_ExportToJson(self.as_ptr()) }; if c_json.is_null() { return Err(_last_null_pointer_err("OGR_G_ExportToJson")); }; diff --git a/src/vector/ops/conversions/gdal_to_geo.rs b/src/vector/ops/conversions/gdal_to_geo.rs index b20401579..2b955bdf4 100644 --- a/src/vector/ops/conversions/gdal_to_geo.rs +++ b/src/vector/ops/conversions/gdal_to_geo.rs @@ -1,18 +1,18 @@ -use std::convert::{TryFrom, TryInto}; - +use foreign_types::ForeignTypeRef; use gdal_sys::{self, OGRwkbGeometryType}; +use std::convert::{TryFrom, TryInto}; use crate::errors::GdalError; -use crate::vector::Geometry; +use crate::vector::{Geometry, GeometryRef}; -impl TryFrom<&Geometry> for geo_types::Geometry { +impl TryFrom<&GeometryRef> for geo_types::Geometry { type Error = GdalError; - fn try_from(geo: &Geometry) -> Result, Self::Error> { + fn try_from(geo: &GeometryRef) -> Result, Self::Error> { let geometry_type = geo.geometry_type(); let ring = |n: usize| { - let ring = unsafe { geo.get_unowned_geometry(n) }; + let ring = geo.get_geometry(n); ring.try_into().map(|inner_geom| match inner_geom { geo_types::Geometry::LineString(r) => r, _ => panic!("Expected to get a LineString"), @@ -28,10 +28,10 @@ impl TryFrom<&Geometry> for geo_types::Geometry { } OGRwkbGeometryType::wkbMultiPoint => { let point_count = - unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.c_geometry()) } as usize; + unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.as_ptr()) } as usize; let coords = (0..point_count) .map(|n| { - unsafe { geo.get_unowned_geometry(n) } + geo.get_geometry(n) .try_into() .map(|inner_geom| match inner_geom { geo_types::Geometry::Point(p) => p, @@ -55,10 +55,10 @@ impl TryFrom<&Geometry> for geo_types::Geometry { } OGRwkbGeometryType::wkbMultiLineString => { let string_count = - unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.c_geometry()) } as usize; + unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.as_ptr()) } as usize; let strings = (0..string_count) .map(|n| { - unsafe { geo.get_unowned_geometry(n) } + geo.get_geometry(n) .try_into() .map(|inner_geom| match inner_geom { geo_types::Geometry::LineString(s) => s, @@ -71,8 +71,7 @@ impl TryFrom<&Geometry> for geo_types::Geometry { )) } OGRwkbGeometryType::wkbPolygon => { - let ring_count = - unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.c_geometry()) } as usize; + let ring_count = unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.as_ptr()) } as usize; let outer = ring(0)?; let holes = (1..ring_count).map(ring).collect::, _>>()?; Ok(geo_types::Geometry::Polygon(geo_types::Polygon::new( @@ -81,10 +80,10 @@ impl TryFrom<&Geometry> for geo_types::Geometry { } OGRwkbGeometryType::wkbMultiPolygon => { let string_count = - unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.c_geometry()) } as usize; + unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.as_ptr()) } as usize; let strings = (0..string_count) .map(|n| { - unsafe { geo.get_unowned_geometry(n) } + geo.get_geometry(n) .try_into() .map(|inner_geom| match inner_geom { geo_types::Geometry::Polygon(s) => s, @@ -97,10 +96,9 @@ impl TryFrom<&Geometry> for geo_types::Geometry { ))) } OGRwkbGeometryType::wkbGeometryCollection => { - let item_count = - unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.c_geometry()) } as usize; + let item_count = unsafe { gdal_sys::OGR_G_GetGeometryCount(geo.as_ptr()) } as usize; let geometry_list = (0..item_count) - .map(|n| unsafe { geo.get_unowned_geometry(n) }.try_into()) + .map(|n| geo.get_geometry(n).try_into()) .collect::, _>>()?; Ok(geo_types::Geometry::GeometryCollection( geo_types::GeometryCollection(geometry_list), @@ -114,6 +112,6 @@ impl TryFrom<&Geometry> for geo_types::Geometry { impl TryFrom for geo_types::Geometry { type Error = GdalError; fn try_from(value: Geometry) -> Result { - Self::try_from(&value) + Self::try_from(value.as_ref()) } } diff --git a/src/vector/ops/conversions/mod.rs b/src/vector/ops/conversions/mod.rs index 1853c798b..c8dbbca4b 100644 --- a/src/vector/ops/conversions/mod.rs +++ b/src/vector/ops/conversions/mod.rs @@ -3,14 +3,14 @@ mod gdal_to_geo; mod geo_to_gdal; use crate::errors::Result; -use crate::vector::Geometry; +use crate::vector::{Geometry, GeometryRef}; /// Convert object to a GDAL geometry. pub trait ToGdal { fn to_gdal(&self) -> Result; } -impl Geometry { +impl GeometryRef { /// Create a copy of self as a `geo-types` geometry. pub fn to_geo(&self) -> Result> { self.try_into() diff --git a/src/vector/ops/predicates.rs b/src/vector/ops/predicates.rs index 683f751a3..27505eb27 100644 --- a/src/vector/ops/predicates.rs +++ b/src/vector/ops/predicates.rs @@ -1,4 +1,5 @@ use crate::vector::Geometry; +use foreign_types::ForeignType; /// # Geometric Predicates /// @@ -17,7 +18,7 @@ impl Geometry { /// [OGR_G_Intersects]: https://gdal.org/api/vector_c_api.html#ogr__api_8h_1acaed6926b75cd33a42b284c10def6e87 /// [has_geos]: crate::version::VersionInfo::has_geos pub fn intersects(&self, other: &Self) -> bool { - let p = unsafe { gdal_sys::OGR_G_Intersects(self.c_geometry(), other.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_Intersects(self.as_ptr(), other.as_ptr()) }; p != 0 } @@ -35,7 +36,7 @@ impl Geometry { /// [OGR_G_Contains]: https://gdal.org/api/vector_c_api.html#_CPPv414OGR_G_Contains12OGRGeometryH12OGRGeometryH /// [has_geos]: crate::version::VersionInfo::has_geos pub fn contains(&self, other: &Self) -> bool { - let p = unsafe { gdal_sys::OGR_G_Contains(self.c_geometry(), other.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_Contains(self.as_ptr(), other.as_ptr()) }; p != 0 } @@ -53,7 +54,7 @@ impl Geometry { /// [OGR_G_Disjoint]: https://gdal.org/api/vector_c_api.html#_CPPv414OGR_G_Disjoint12OGRGeometryH12OGRGeometryH /// [has_geos]: crate::version::VersionInfo::has_geos pub fn disjoint(&self, other: &Self) -> bool { - let p = unsafe { gdal_sys::OGR_G_Disjoint(self.c_geometry(), other.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_Disjoint(self.as_ptr(), other.as_ptr()) }; p != 0 } @@ -71,7 +72,7 @@ impl Geometry { /// [OGR_G_Touches]: https://gdal.org/api/ogrgeometry_cpp.html#_CPPv4NK11OGRGeometry7TouchesEPK11OGRGeometry /// [has_geos]: crate::version::VersionInfo::has_geos pub fn touches(&self, other: &Self) -> bool { - let p = unsafe { gdal_sys::OGR_G_Touches(self.c_geometry(), other.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_Touches(self.as_ptr(), other.as_ptr()) }; p != 0 } @@ -90,7 +91,7 @@ impl Geometry { /// [OGR_G_Crosses]: https://gdal.org/api/ogrgeometry_cpp.html#_CPPv4NK11OGRGeometry7CrossesEPK11OGRGeometry /// [has_geos]: crate::version::VersionInfo::has_geos pub fn crosses(&self, other: &Self) -> bool { - let p = unsafe { gdal_sys::OGR_G_Crosses(self.c_geometry(), other.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_Crosses(self.as_ptr(), other.as_ptr()) }; p != 0 } @@ -108,7 +109,7 @@ impl Geometry { /// [OGR_G_Within]: https://gdal.org/api/ogrgeometry_cpp.html#_CPPv4NK11OGRGeometry6WithinEPK11OGRGeometry /// [has_geos]: crate::version::VersionInfo::has_geos pub fn within(&self, other: &Self) -> bool { - let p = unsafe { gdal_sys::OGR_G_Within(self.c_geometry(), other.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_Within(self.as_ptr(), other.as_ptr()) }; p != 0 } @@ -128,7 +129,7 @@ impl Geometry { /// [OGR_G_Overlaps]: https://gdal.org/api/ogrgeometry_cpp.html#_CPPv4NK11OGRGeometry8OverlapsEPK11OGRGeometry /// [has_geos]: crate::version::VersionInfo::has_geos pub fn overlaps(&self, other: &Self) -> bool { - let p = unsafe { gdal_sys::OGR_G_Overlaps(self.c_geometry(), other.c_geometry()) }; + let p = unsafe { gdal_sys::OGR_G_Overlaps(self.as_ptr(), other.as_ptr()) }; p != 0 } } diff --git a/src/vector/ops/set.rs b/src/vector/ops/set.rs index b920772b9..bbef0bf96 100644 --- a/src/vector/ops/set.rs +++ b/src/vector/ops/set.rs @@ -1,4 +1,5 @@ use crate::vector::Geometry; +use foreign_types::ForeignType; /// # Set Operations /// @@ -26,18 +27,11 @@ impl Geometry { /// [intersection]: https://en.wikipedia.org/wiki/Intersection_(geometry) /// [has_geos]: crate::version::VersionInfo::has_geos pub fn intersection(&self, other: &Self) -> Option { - if !self.has_gdal_ptr() { - return None; - } - if !other.has_gdal_ptr() { - return None; - } - let ogr_geom = - unsafe { gdal_sys::OGR_G_Intersection(self.c_geometry(), other.c_geometry()) }; + let ogr_geom = unsafe { gdal_sys::OGR_G_Intersection(self.as_ptr(), other.as_ptr()) }; if ogr_geom.is_null() { return None; } - Some(unsafe { Geometry::with_c_geometry(ogr_geom, true) }) + Some(unsafe { Geometry::from_ptr(ogr_geom) }) } /// Computes the [geometric union][union] of `self` and `other`. @@ -60,18 +54,12 @@ impl Geometry { /// [union]: https://en.wikipedia.org/wiki/Constructive_solid_geometry#Workings /// [has_geos]: crate::version::VersionInfo::has_geos pub fn union(&self, other: &Self) -> Option { - if !self.has_gdal_ptr() { - return None; - } - if !other.has_gdal_ptr() { - return None; - } unsafe { - let ogr_geom = gdal_sys::OGR_G_Union(self.c_geometry(), other.c_geometry()); + let ogr_geom = gdal_sys::OGR_G_Union(self.as_ptr(), other.as_ptr()); if ogr_geom.is_null() { return None; } - Some(Geometry::with_c_geometry(ogr_geom, true)) + Some(Geometry::from_ptr(ogr_geom)) } } } @@ -98,18 +86,6 @@ mod tests { assert_eq!(inter.area(), 25.0); } - #[test] - fn test_intersection_no_gdal_ptr() { - let geom = - Geometry::from_wkt("POLYGON ((0.0 10.0, 0.0 0.0, 10.0 0.0, 10.0 10.0, 0.0 10.0))") - .unwrap(); - let other = unsafe { Geometry::lazy_feature_geometry() }; - - let inter = geom.intersection(&other); - - assert!(inter.is_none()); - } - #[test] #[allow(clippy::float_cmp)] fn test_intersection_no_intersects() { @@ -144,18 +120,6 @@ mod tests { assert_eq!(res.area(), 135.0); } - #[test] - fn test_union_no_gdal_ptr() { - let geom = - Geometry::from_wkt("POLYGON ((0.0 10.0, 0.0 0.0, 10.0 0.0, 10.0 10.0, 0.0 10.0))") - .unwrap(); - let other = unsafe { Geometry::lazy_feature_geometry() }; - - let res = geom.union(&other); - - assert!(res.is_none()); - } - #[test] #[allow(clippy::float_cmp)] fn test_union_no_intersects() { diff --git a/src/vector/ops/transformations.rs b/src/vector/ops/transformations.rs index 15b451687..0aea89311 100644 --- a/src/vector/ops/transformations.rs +++ b/src/vector/ops/transformations.rs @@ -1,3 +1,4 @@ +use foreign_types::{ForeignType, ForeignTypeRef}; use gdal_sys::OGRErr; use crate::cpl::CslStringList; @@ -5,17 +6,17 @@ use crate::errors::{GdalError, Result}; use crate::spatial_ref::CoordTransform; use crate::spatial_ref::SpatialRef; use crate::utils::_last_null_pointer_err; -use crate::vector::Geometry; +use crate::vector::{Geometry, GeometryRef}; /// # Geometry Transformations /// /// These methods provide geometric transformations on a `Geometry`. -impl Geometry { +impl GeometryRef { /// Apply arbitrary coordinate transformation to geometry, mutating the [`Geometry`] in-place. /// /// See: [`OGR_G_Transform`](https://gdal.org/api/vector_c_api.html#_CPPv415OGR_G_Transform12OGRGeometryH28OGRCoordinateTransformationH) pub fn transform_inplace(&mut self, htransform: &CoordTransform) -> Result<()> { - let rv = unsafe { gdal_sys::OGR_G_Transform(self.c_geometry(), htransform.to_c_hct()) }; + let rv = unsafe { gdal_sys::OGR_G_Transform(self.as_ptr(), htransform.to_c_hct()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -29,7 +30,7 @@ impl Geometry { /// /// See: [`OGR_G_Transform`](https://gdal.org/api/vector_c_api.html#_CPPv415OGR_G_Transform12OGRGeometryH28OGRCoordinateTransformationH) pub fn transform(&self, htransform: &CoordTransform) -> Result { - let new_c_geom = unsafe { gdal_sys::OGR_G_Clone(self.c_geometry()) }; + let new_c_geom = unsafe { gdal_sys::OGR_G_Clone(self.as_ptr()) }; let rv = unsafe { gdal_sys::OGR_G_Transform(new_c_geom, htransform.to_c_hct()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { @@ -37,14 +38,14 @@ impl Geometry { method_name: "OGR_G_Transform", }); } - Ok(unsafe { Geometry::with_c_geometry(new_c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(new_c_geom) }) } /// Transforms this geometry's coordinates into another [`SpatialRef`], mutating the [`Geometry`] in-place. /// /// See: [`OGR_G_TransformTo`](https://gdal.org/api/vector_c_api.html#_CPPv417OGR_G_TransformTo12OGRGeometryH20OGRSpatialReferenceH) pub fn transform_to_inplace(&mut self, spatial_ref: &SpatialRef) -> Result<()> { - let rv = unsafe { gdal_sys::OGR_G_TransformTo(self.c_geometry(), spatial_ref.to_c_hsrs()) }; + let rv = unsafe { gdal_sys::OGR_G_TransformTo(self.as_ptr(), spatial_ref.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, @@ -58,26 +59,26 @@ impl Geometry { /// /// See: [`OGR_G_TransformTo`](https://gdal.org/api/vector_c_api.html#_CPPv417OGR_G_TransformTo12OGRGeometryH20OGRSpatialReferenceH) pub fn transform_to(&self, spatial_ref: &SpatialRef) -> Result { - let new_c_geom = unsafe { gdal_sys::OGR_G_Clone(self.c_geometry()) }; - let rv = unsafe { gdal_sys::OGR_G_TransformTo(new_c_geom, spatial_ref.to_c_hsrs()) }; + let new_c_geom = unsafe { gdal_sys::OGR_G_Clone(self.as_ptr()) }; + let rv = unsafe { gdal_sys::OGR_G_TransformTo(new_c_geom, spatial_ref.as_ptr()) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, method_name: "OGR_G_TransformTo", }); } - Ok(unsafe { Geometry::with_c_geometry(new_c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(new_c_geom) }) } /// Compute the convex hull of this geometry. /// /// See: [`OGR_G_ConvexHull`](https://gdal.org/api/vector_c_api.html#_CPPv416OGR_G_ConvexHull12OGRGeometryH) pub fn convex_hull(&self) -> Result { - let c_geom = unsafe { gdal_sys::OGR_G_ConvexHull(self.c_geometry()) }; + let c_geom = unsafe { gdal_sys::OGR_G_ConvexHull(self.as_ptr()) }; if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_ConvexHull")); }; - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } #[cfg(any(all(major_is_2, minor_ge_1), major_ge_3))] @@ -95,15 +96,15 @@ impl Geometry { /// /// [dt]: https://en.wikipedia.org/wiki/Delaunay_triangulation /// [has_geos]: crate::version::VersionInfo::has_geos - pub fn delaunay_triangulation(&self, tolerance: Option) -> Result { + pub fn delaunay_triangulation(&self, tolerance: Option) -> Result { let c_geom = unsafe { - gdal_sys::OGR_G_DelaunayTriangulation(self.c_geometry(), tolerance.unwrap_or(0.0), 0) + gdal_sys::OGR_G_DelaunayTriangulation(self.as_ptr(), tolerance.unwrap_or(0.0), 0) }; if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_DelaunayTriangulation")); }; - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Compute a simplified geometry. @@ -112,13 +113,13 @@ impl Geometry { /// * `tolerance`: the distance tolerance for the simplification. /// /// See: [`OGR_G_Simplify`](https://gdal.org/api/vector_c_api.html#_CPPv414OGR_G_Simplify12OGRGeometryHd) - pub fn simplify(&self, tolerance: f64) -> Result { - let c_geom = unsafe { gdal_sys::OGR_G_Simplify(self.c_geometry(), tolerance) }; + pub fn simplify(&self, tolerance: f64) -> Result { + let c_geom = unsafe { gdal_sys::OGR_G_Simplify(self.as_ptr(), tolerance) }; if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_Simplify")); }; - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Simplify the geometry while preserving topology. @@ -127,14 +128,13 @@ impl Geometry { /// * `tolerance`: the distance tolerance for the simplification. /// /// See: [`OGR_G_SimplifyPreserveTopology`](https://gdal.org/api/vector_c_api.html#_CPPv430OGR_G_SimplifyPreserveTopology12OGRGeometryHd) - pub fn simplify_preserve_topology(&self, tolerance: f64) -> Result { - let c_geom = - unsafe { gdal_sys::OGR_G_SimplifyPreserveTopology(self.c_geometry(), tolerance) }; + pub fn simplify_preserve_topology(&self, tolerance: f64) -> Result { + let c_geom = unsafe { gdal_sys::OGR_G_SimplifyPreserveTopology(self.as_ptr(), tolerance) }; if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_SimplifyPreserveTopology")); }; - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Compute buffer of geometry @@ -146,14 +146,13 @@ impl Geometry { /// 90 degree (quadrant) of curvature. /// /// See: [`OGR_G_Buffer`](https://gdal.org/api/vector_c_api.html#_CPPv412OGR_G_Buffer12OGRGeometryHdi) - pub fn buffer(&self, distance: f64, n_quad_segs: u32) -> Result { - let c_geom = - unsafe { gdal_sys::OGR_G_Buffer(self.c_geometry(), distance, n_quad_segs as i32) }; + pub fn buffer(&self, distance: f64, n_quad_segs: u32) -> Result { + let c_geom = unsafe { gdal_sys::OGR_G_Buffer(self.as_ptr(), distance, n_quad_segs as i32) }; if c_geom.is_null() { return Err(_last_null_pointer_err("OGR_G_Buffer")); }; - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } /// Attempts to make an invalid geometry valid without losing vertices. @@ -188,7 +187,7 @@ impl Geometry { /// ``` pub fn make_valid(&self, opts: &CslStringList) -> Result { #[cfg(all(major_ge_3, minor_ge_4))] - let c_geom = unsafe { gdal_sys::OGR_G_MakeValidEx(self.c_geometry(), opts.as_ptr()) }; + let c_geom = unsafe { gdal_sys::OGR_G_MakeValidEx(self.as_ptr(), opts.as_ptr()) }; #[cfg(not(all(major_ge_3, minor_ge_4)))] let c_geom = { @@ -197,13 +196,13 @@ impl Geometry { "Options to make_valid require GDAL >= 3.4".into(), )); } - unsafe { gdal_sys::OGR_G_MakeValid(self.c_geometry()) } + unsafe { gdal_sys::OGR_G_MakeValid(self.as_ptr()) } }; if c_geom.is_null() { Err(_last_null_pointer_err("OGR_G_MakeValid")) } else { - Ok(unsafe { Geometry::with_c_geometry(c_geom, true) }) + Ok(unsafe { Geometry::from_ptr(c_geom) }) } } } diff --git a/src/vector/sql.rs b/src/vector/sql.rs index 00f6ca9a7..e4c1fda45 100644 --- a/src/vector/sql.rs +++ b/src/vector/sql.rs @@ -1,3 +1,4 @@ +use foreign_types::ForeignType; use std::ffi::{CStr, CString}; use std::ops::{Deref, DerefMut}; @@ -128,7 +129,7 @@ impl Dataset { }; if let Some(spatial_filter) = spatial_filter { - filter_geom = unsafe { spatial_filter.c_geometry() }; + filter_geom = spatial_filter.as_ptr(); } let c_dataset = self.c_dataset(); From 2f1a8f1941b72f1d37034df5ae30da77b7c905d8 Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Mon, 2 Oct 2023 17:24:50 -0400 Subject: [PATCH 2/4] Converted `Dataset` to use foreign-types. --- src/dataset.rs | 125 ++++++++++++--------------- src/driver.rs | 3 +- src/gcp.rs | 10 +-- src/programs/raster/mdimtranslate.rs | 7 +- src/programs/raster/vrt.rs | 5 +- src/raster/gcp.rs | 38 -------- src/raster/mdarray.rs | 8 +- src/raster/rasterband.rs | 12 +-- src/raster/rasterize.rs | 2 +- src/raster/warp.rs | 5 +- src/vector/layer.rs | 15 ++-- src/vector/sql.rs | 4 +- src/vector/transaction.rs | 9 +- 13 files changed, 98 insertions(+), 145 deletions(-) delete mode 100644 src/raster/gcp.rs diff --git a/src/dataset.rs b/src/dataset.rs index e0cca19c3..fca8d9848 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -1,7 +1,9 @@ -use foreign_types::ForeignTypeRef; +use foreign_types::{foreign_type, ForeignType, ForeignTypeRef}; +use std::fmt::{Debug, Formatter}; +use std::mem::ManuallyDrop; use std::{ffi::CString, ffi::NulError, path::Path, ptr}; -use gdal_sys::{self, CPLErr, GDALDatasetH, GDALMajorObjectH}; +use gdal_sys::{self, CPLErr, GDALMajorObjectH}; use crate::cpl::CslStringList; use crate::errors::*; @@ -13,19 +15,20 @@ use crate::{ gdal_major_object::MajorObject, spatial_ref::SpatialRef, Driver, GeoTransform, Metadata, }; -/// Wrapper around a [`GDALDataset`][GDALDataset] object. -/// -/// Represents both a [vector dataset][vector-data-model] -/// containing a collection of layers; and a -/// [raster dataset][raster-data-model] containing a collection of raster-bands. -/// -/// [vector-data-model]: https://gdal.org/user/vector_data_model.html -/// [raster-data-model]: https://gdal.org/user/raster_data_model.html -/// [GDALDataset]: https://gdal.org/api/gdaldataset_cpp.html#_CPPv411GDALDataset -#[derive(Debug)] -pub struct Dataset { - c_dataset: GDALDatasetH, - closed: bool, +foreign_type! { + /// Wrapper around a [`GDALDataset`][GDALDataset] object. + /// + /// Represents both a [vector dataset][vector-data-model] + /// containing a collection of layers; and a + /// [raster dataset][raster-data-model] containing a collection of raster-bands. + /// + /// [vector-data-model]: https://gdal.org/user/vector_data_model.html + /// [raster-data-model]: https://gdal.org/user/raster_data_model.html + /// [GDALDataset]: https://gdal.org/api/gdaldataset_cpp.html#_CPPv411GDALDataset + pub unsafe type Dataset { + type CType = libc::c_void; + fn drop = gdal_sys::GDALClose; + } } // GDAL Docs state: The returned dataset should only be accessed by one thread at a time. @@ -37,26 +40,6 @@ unsafe impl Send for Dataset {} /// Core dataset methods impl Dataset { - /// Returns the wrapped C pointer - /// - /// # Safety - /// This method returns a raw C pointer - pub fn c_dataset(&self) -> GDALDatasetH { - self.c_dataset - } - - /// Creates a new Dataset by wrapping a C pointer - /// - /// # Safety - /// This method operates on a raw C pointer - /// The dataset must not have been closed (using [`GDALClose`]) before. - pub unsafe fn from_c_dataset(c_dataset: GDALDatasetH) -> Dataset { - Dataset { - c_dataset, - closed: false, - } - } - /// Open a dataset at the given `path` with default /// options. pub fn open>(path: P) -> Result { @@ -158,10 +141,7 @@ impl Dataset { if c_dataset.is_null() { return Err(_last_null_pointer_err("GDALOpenEx")); } - Ok(Dataset { - c_dataset, - closed: false, - }) + Ok(unsafe { Dataset::from_ptr(c_dataset) }) } /// Flush all write cached data to disk. @@ -172,7 +152,7 @@ impl Dataset { pub fn flush_cache(&mut self) -> Result<()> { #[cfg(any(all(major_ge_3, minor_ge_7), major_ge_4))] { - let rv = unsafe { gdal_sys::GDALFlushCache(self.c_dataset) }; + let rv = unsafe { gdal_sys::GDALFlushCache(self.as_ptr()) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -180,23 +160,29 @@ impl Dataset { #[cfg(not(any(all(major_is_3, minor_ge_7), major_ge_4)))] { unsafe { - gdal_sys::GDALFlushCache(self.c_dataset); + gdal_sys::GDALFlushCache(self.as_ptr()); } } Ok(()) } - /// Close the dataset. + /// Close the dataset, consuming it. /// - /// See [`GDALClose`]. + /// See [`GDALClose`](https://gdal.org/api/raster_c_api.html#_CPPv49GDALClose12GDALDatasetH). /// - /// Note: on GDAL versions older than 3.7.0, this function always succeeds. - pub fn close(mut self) -> Result<()> { - self.closed = true; - + /// # Notes + /// * `Drop`ing automatically closes the Dataset, so this method is usually not needed. + /// * On GDAL versions older than 3.7.0, this function always succeeds. + pub fn close(self) -> Result<()> { + // According to the C documentation, GDALClose will release all resources + // using the C++ `delete` operator. + // According to https://doc.rust-lang.org/std/mem/fn.forget.html#relationship-with-manuallydrop + // `ManuallyDrop` is the suggested means of transferring memory management while + // robustly preventing a double-free. + let ds = ManuallyDrop::new(self); #[cfg(any(all(major_ge_3, minor_ge_7), major_ge_4))] { - let rv = unsafe { gdal_sys::GDALClose(self.c_dataset) }; + let rv = unsafe { gdal_sys::GDALClose(ds.as_ptr()) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -204,7 +190,7 @@ impl Dataset { #[cfg(not(any(all(major_is_3, minor_ge_7), major_ge_4)))] { unsafe { - gdal_sys::GDALClose(self.c_dataset); + gdal_sys::GDALClose(ds.as_ptr()); } } Ok(()) @@ -212,14 +198,14 @@ impl Dataset { /// Fetch the projection definition string for this dataset. pub fn projection(&self) -> String { - let rv = unsafe { gdal_sys::GDALGetProjectionRef(self.c_dataset) }; + let rv = unsafe { gdal_sys::GDALGetProjectionRef(self.as_ptr()) }; _string(rv) } /// Set the projection reference string for this dataset. pub fn set_projection(&mut self, projection: &str) -> Result<()> { let c_projection = CString::new(projection)?; - unsafe { gdal_sys::GDALSetProjection(self.c_dataset, c_projection.as_ptr()) }; + unsafe { gdal_sys::GDALSetProjection(self.as_ptr(), c_projection.as_ptr()) }; Ok(()) } @@ -227,7 +213,7 @@ impl Dataset { /// Get the spatial reference system for this dataset. pub fn spatial_ref(&self) -> Result { Ok( - unsafe { SpatialRefRef::from_ptr(gdal_sys::GDALGetSpatialRef(self.c_dataset)) } + unsafe { SpatialRefRef::from_ptr(gdal_sys::GDALGetSpatialRef(self.as_ptr())) } .to_owned(), ) } @@ -235,7 +221,7 @@ impl Dataset { #[cfg(major_ge_3)] /// Set the spatial reference system for this dataset. pub fn set_spatial_ref(&mut self, spatial_ref: &SpatialRef) -> Result<()> { - let rv = unsafe { gdal_sys::GDALSetSpatialRef(self.c_dataset, spatial_ref.as_ptr()) }; + let rv = unsafe { gdal_sys::GDALSetSpatialRef(self.as_ptr(), spatial_ref.as_ptr()) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); } @@ -265,7 +251,7 @@ impl Dataset { gdal_sys::GDALCreateCopy( driver.c_driver(), c_filename.as_ptr(), - ds.c_dataset, + ds.as_ptr(), 0, c_options.as_ptr(), None, @@ -275,7 +261,7 @@ impl Dataset { if c_dataset.is_null() { return Err(_last_null_pointer_err("GDALCreateCopy")); } - Ok(unsafe { Dataset::from_c_dataset(c_dataset) }) + Ok(unsafe { Dataset::from_ptr(c_dataset) }) } _create_copy(self, driver, filename.as_ref(), options) } @@ -283,7 +269,7 @@ impl Dataset { /// Fetch the driver to which this dataset relates. pub fn driver(&self) -> Driver { unsafe { - let c_driver = gdal_sys::GDALGetDatasetDriver(self.c_dataset); + let c_driver = gdal_sys::GDALGetDatasetDriver(self.as_ptr()); Driver::from_c_driver(c_driver) } } @@ -304,7 +290,7 @@ impl Dataset { pub fn set_geo_transform(&mut self, transformation: &GeoTransform) -> Result<()> { assert_eq!(transformation.len(), 6); let rv = unsafe { - gdal_sys::GDALSetGeoTransform(self.c_dataset, transformation.as_ptr() as *mut f64) + gdal_sys::GDALSetGeoTransform(self.as_ptr(), transformation.as_ptr() as *mut f64) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); @@ -324,7 +310,7 @@ impl Dataset { pub fn geo_transform(&self) -> Result { let mut transformation = GeoTransform::default(); let rv = - unsafe { gdal_sys::GDALGetGeoTransform(self.c_dataset, transformation.as_mut_ptr()) }; + unsafe { gdal_sys::GDALGetGeoTransform(self.as_ptr(), transformation.as_mut_ptr()) }; // check if the dataset has a GeoTransform if rv != CPLErr::CE_None { @@ -334,24 +320,25 @@ impl Dataset { } } +impl Debug for Dataset { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Dataset") + .field( + "description", + &self.description().unwrap_or_else(|_| Default::default()), + ) + .finish() + } +} + impl MajorObject for Dataset { fn gdal_object_ptr(&self) -> GDALMajorObjectH { - self.c_dataset + self.as_ptr() } } impl Metadata for Dataset {} -impl Drop for Dataset { - fn drop(&mut self) { - if !self.closed { - unsafe { - gdal_sys::GDALClose(self.c_dataset); - } - } - } -} - #[cfg(test)] mod tests { use gdal_sys::GDALAccess; diff --git a/src/driver.rs b/src/driver.rs index c17f46b9c..ee91c1afe 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -1,3 +1,4 @@ +use foreign_types::ForeignType; use std::ffi::CString; use std::path::Path; use std::sync::Once; @@ -223,7 +224,7 @@ impl Driver { return Err(_last_null_pointer_err("GDALCreate")); }; - Ok(unsafe { Dataset::from_c_dataset(c_dataset) }) + Ok(unsafe { Dataset::from_ptr(c_dataset) }) } /// Convenience for creating a vector-only dataset from a compatible driver. diff --git a/src/gcp.rs b/src/gcp.rs index 7aed3e2ab..34990fea2 100644 --- a/src/gcp.rs +++ b/src/gcp.rs @@ -116,7 +116,7 @@ impl Dataset { /// /// See: [`GDALGetGCPSpatialRef`](https://gdal.org/api/raster_c_api.html#_CPPv420GDALGetGCPSpatialRef12GDALDatasetH) pub fn gcp_spatial_ref(&self) -> Option { - let c_ptr = unsafe { gdal_sys::GDALGetGCPSpatialRef(self.c_dataset()) }; + let c_ptr = unsafe { gdal_sys::GDALGetGCPSpatialRef(self.as_ptr()) }; if c_ptr.is_null() { return None; @@ -134,7 +134,7 @@ impl Dataset { /// /// See: [`GDALGetGCPProjection`](https://gdal.org/api/raster_c_api.html#gdal_8h_1a85ffa184d3ecb7c0a59a66096b22b2ec) pub fn gcp_projection(&self) -> Option { - let cc_ptr = unsafe { gdal_sys::GDALGetGCPProjection(self.c_dataset()) }; + let cc_ptr = unsafe { gdal_sys::GDALGetGCPProjection(self.as_ptr()) }; if cc_ptr.is_null() { return None; } @@ -145,8 +145,8 @@ impl Dataset { /// /// See: [`GDALDataset::GetGCPs`](https://gdal.org/api/gdaldataset_cpp.html#_CPPv4N11GDALDataset7GetGCPsEv) pub fn gcps(&self) -> &[GcpRef] { - let len = unsafe { gdal_sys::GDALGetGCPCount(self.c_dataset()) }; - let data = unsafe { gdal_sys::GDALGetGCPs(self.c_dataset()) }; + let len = unsafe { gdal_sys::GDALGetGCPCount(self.as_ptr()) }; + let data = unsafe { gdal_sys::GDALGetGCPs(self.as_ptr()) }; unsafe { std::slice::from_raw_parts(data as *const GcpRef, len as usize) } } @@ -206,7 +206,7 @@ impl Dataset { let rv = unsafe { gdal_sys::GDALSetGCPs2( - self.c_dataset(), + self.as_ptr(), len, gdal_gcps.as_ptr(), spatial_ref.as_ptr() as *mut _, diff --git a/src/programs/raster/mdimtranslate.rs b/src/programs/raster/mdimtranslate.rs index d90bd6363..6a3f94691 100644 --- a/src/programs/raster/mdimtranslate.rs +++ b/src/programs/raster/mdimtranslate.rs @@ -3,6 +3,7 @@ use crate::{ utils::{_last_null_pointer_err, _path_to_c_string}, Dataset, }; +use foreign_types::ForeignType; use gdal_sys::{GDALMultiDimTranslate, GDALMultiDimTranslateOptions}; use libc::{c_char, c_int}; use std::{ @@ -183,12 +184,12 @@ fn _multi_dim_translate( ) -> Result { let (psz_dest_option, h_dst_ds) = match &destination { MultiDimTranslateDestination::Path(c_path) => (Some(c_path), null_mut()), - MultiDimTranslateDestination::Dataset { dataset, .. } => (None, dataset.c_dataset()), + MultiDimTranslateDestination::Dataset { dataset, .. } => (None, dataset.as_ptr()), }; let psz_dest = psz_dest_option.map(|x| x.as_ptr()).unwrap_or_else(null); - let mut pah_src_ds: Vec = input.iter().map(|x| x.c_dataset()).collect(); + let mut pah_src_ds: Vec = input.iter().map(|x| x.as_ptr()).collect(); let ps_options = options .as_ref() @@ -217,7 +218,7 @@ fn _multi_dim_translate( return Err(_last_null_pointer_err("GDALMultiDimTranslate")); } - let result = unsafe { Dataset::from_c_dataset(dataset_out) }; + let result = unsafe { Dataset::from_ptr(dataset_out) }; Ok(result) } diff --git a/src/programs/raster/vrt.rs b/src/programs/raster/vrt.rs index 0ff411546..6c552538a 100644 --- a/src/programs/raster/vrt.rs +++ b/src/programs/raster/vrt.rs @@ -3,6 +3,7 @@ use crate::{ utils::{_last_null_pointer_err, _path_to_c_string}, Dataset, }; +use foreign_types::ForeignType; use gdal_sys::GDALBuildVRTOptions; use libc::{c_char, c_int}; use std::{ @@ -101,7 +102,7 @@ fn _build_vrt( let dataset_out = unsafe { // Get raw handles to the datasets let mut datasets_raw: Vec = - datasets.iter().map(|x| x.c_dataset()).collect(); + datasets.iter().map(|x| x.as_ptr()).collect(); gdal_sys::GDALBuildVRT( c_dest, @@ -117,7 +118,7 @@ fn _build_vrt( return Err(_last_null_pointer_err("GDALBuildVRT")); } - let result = unsafe { Dataset::from_c_dataset(dataset_out) }; + let result = unsafe { Dataset::from_ptr(dataset_out) }; Ok(result) } diff --git a/src/raster/gcp.rs b/src/raster/gcp.rs deleted file mode 100644 index 483ead158..000000000 --- a/src/raster/gcp.rs +++ /dev/null @@ -1,38 +0,0 @@ -//! Raster ground control point support - -use foreign_types::ForeignTypeRef; -use crate::spatial_ref::SpatialRefRef; -use crate::Dataset; - -impl Dataset { - /// Get output spatial reference system for GCPs. - /// - /// # Notes - /// * This is separate and distinct from [`Dataset::spatial_ref`], and only applies to - /// the representation of ground control points, when embedded. - /// - /// See: [`GDALGetGCPSpatialRef`](https://gdal.org/api/raster_c_api.html#_CPPv420GDALGetGCPSpatialRef12GDALDatasetH) - pub fn gcp_spatial_ref(&self) -> Option<&SpatialRefRef> { - let c_ptr = unsafe { gdal_sys::GDALGetGCPSpatialRef(self.c_dataset()) }; - - if c_ptr.is_null() { - return None; - } - - Some(unsafe { SpatialRefRef::from_ptr(c_ptr) }) - } -} - -#[cfg(test)] -mod tests { - use crate::test_utils::fixture; - use crate::Dataset; - - #[test] - fn test_gcp_spatial_ref() { - let dataset = Dataset::open(fixture("gcp.tif")).unwrap(); - let gcp_srs = dataset.gcp_spatial_ref(); - let auth = gcp_srs.and_then(|s| s.authority().ok()); - assert_eq!(auth.unwrap(), "EPSG:4326"); - } -} diff --git a/src/raster/mdarray.rs b/src/raster/mdarray.rs index efc1e6028..fb3ae062f 100644 --- a/src/raster/mdarray.rs +++ b/src/raster/mdarray.rs @@ -22,7 +22,7 @@ use libc::c_void; use std::ffi::CString; use std::os::raw::c_char; -use foreign_types::ForeignTypeRef; +use foreign_types::{ForeignType, ForeignTypeRef}; #[cfg(feature = "ndarray")] use ndarray::{ArrayD, IxDyn}; use std::fmt::{Debug, Display}; @@ -67,7 +67,7 @@ impl<'a> MDArray<'a> { pub unsafe fn from_c_mdarray_and_group(_group: &'a Group, c_mdarray: GDALMDArrayH) -> Self { Self { c_mdarray, - c_dataset: _group._dataset.c_dataset(), + c_dataset: _group._dataset.as_ptr(), _parent: GroupOrDimension::Group { _group }, } } @@ -83,7 +83,7 @@ impl<'a> MDArray<'a> { Self { c_mdarray, c_dataset: match _dimension._parent { - GroupOrArray::Group { _group } => _group._dataset.c_dataset(), + GroupOrArray::Group { _group } => _group._dataset.as_ptr(), GroupOrArray::MDArray { _md_array } => _md_array.c_dataset, }, _parent: GroupOrDimension::Dimension { _dimension }, @@ -802,7 +802,7 @@ impl Dataset { #[cfg(all(major_ge_3, minor_ge_1))] pub fn root_group(&self) -> Result { unsafe { - let c_group = gdal_sys::GDALDatasetGetRootGroup(self.c_dataset()); + let c_group = gdal_sys::GDALDatasetGetRootGroup(self.as_ptr()); if c_group.is_null() { return Err(_last_null_pointer_err("GDALDatasetGetRootGroup")); } diff --git a/src/raster/rasterband.rs b/src/raster/rasterband.rs index a51fbc4c4..403141974 100644 --- a/src/raster/rasterband.rs +++ b/src/raster/rasterband.rs @@ -3,7 +3,7 @@ use crate::metadata::Metadata; use crate::raster::{GdalDataType, GdalType}; use crate::utils::{_last_cpl_err, _last_null_pointer_err, _string}; use crate::Dataset; -use foreign_types::{ForeignTypeRef, Opaque}; +use foreign_types::{ForeignType, ForeignTypeRef, Opaque}; use gdal_sys::{ self, CPLErr, GDALColorEntry, GDALColorInterp, GDALColorTableH, GDALComputeRasterMinMax, GDALCreateColorRamp, GDALCreateColorTable, GDALDestroyColorTable, GDALGetPaletteInterpretation, @@ -28,7 +28,7 @@ impl Dataset { /// rasterband at the given _1-based_ index. pub fn rasterband(&self, band_index: isize) -> Result<&mut RasterBand> { unsafe { - let c_band = gdal_sys::GDALGetRasterBand(self.c_dataset(), band_index as c_int); + let c_band = gdal_sys::GDALGetRasterBand(self.as_ptr(), band_index as c_int); if c_band.is_null() { return Err(_last_null_pointer_err("GDALGetRasterBand")); } @@ -53,7 +53,7 @@ impl Dataset { let c_resampling = CString::new(resampling)?; let rv = unsafe { gdal_sys::GDALBuildOverviews( - self.c_dataset(), + self.as_ptr(), c_resampling.as_ptr(), overviews.len() as i32, overviews.as_ptr() as *mut i32, @@ -71,13 +71,13 @@ impl Dataset { /// Fetch the number of raster bands on this dataset. pub fn raster_count(&self) -> isize { - (unsafe { gdal_sys::GDALGetRasterCount(self.c_dataset()) }) as isize + (unsafe { gdal_sys::GDALGetRasterCount(self.as_ptr()) }) as isize } /// Returns the raster dimensions: (width, height). pub fn raster_size(&self) -> (usize, usize) { - let size_x = unsafe { gdal_sys::GDALGetRasterXSize(self.c_dataset()) } as usize; - let size_y = unsafe { gdal_sys::GDALGetRasterYSize(self.c_dataset()) } as usize; + let size_x = unsafe { gdal_sys::GDALGetRasterXSize(self.as_ptr()) } as usize; + let size_y = unsafe { gdal_sys::GDALGetRasterYSize(self.as_ptr()) } as usize; (size_x, size_y) } } diff --git a/src/raster/rasterize.rs b/src/raster/rasterize.rs index 5686555a8..f4917fe8a 100644 --- a/src/raster/rasterize.rs +++ b/src/raster/rasterize.rs @@ -208,7 +208,7 @@ pub fn rasterize( // here. let error = gdal_sys::GDALRasterizeGeometries( - dataset.c_dataset(), + dataset.as_ptr(), bands.len() as i32, bands.as_ptr() as *mut i32, geometries.len() as i32, diff --git a/src/raster/warp.rs b/src/raster/warp.rs index e803fb360..4e35fe05d 100644 --- a/src/raster/warp.rs +++ b/src/raster/warp.rs @@ -1,5 +1,6 @@ use crate::dataset::Dataset; use crate::utils::_last_cpl_err; +use foreign_types::ForeignType; use gdal_sys::{self, CPLErr, GDALResampleAlg}; use std::ptr::{null, null_mut}; @@ -8,9 +9,9 @@ use crate::errors::*; pub fn reproject(src: &Dataset, dst: &Dataset) -> Result<()> { let rv = unsafe { gdal_sys::GDALReprojectImage( - src.c_dataset(), + src.as_ptr(), null(), - dst.c_dataset(), + dst.as_ptr(), null(), GDALResampleAlg::GRA_Bilinear, 0.0, diff --git a/src/vector/layer.rs b/src/vector/layer.rs index 13664396b..7f2779f77 100644 --- a/src/vector/layer.rs +++ b/src/vector/layer.rs @@ -521,8 +521,7 @@ impl<'a> Iterator for LayerIterator<'a> { let idx = self.idx; if idx < self.count { self.idx += 1; - let c_layer = - unsafe { gdal_sys::OGR_DS_GetLayer(self.dataset.c_dataset(), idx as c_int) }; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.dataset.as_ptr(), idx as c_int) }; if !c_layer.is_null() { let layer = unsafe { Layer::from_c_layer(self.dataset, c_layer) }; return Some(layer); @@ -603,7 +602,7 @@ impl Dataset { /// Get the number of layers in this dataset. pub fn layer_count(&self) -> isize { - (unsafe { gdal_sys::OGR_DS_GetLayerCount(self.c_dataset()) }) as isize + (unsafe { gdal_sys::OGR_DS_GetLayerCount(self.as_ptr()) }) as isize } /// Fetch a layer by index. @@ -611,7 +610,7 @@ impl Dataset { /// Applies to vector datasets, and fetches by the given /// _0-based_ index. pub fn layer(&self, idx: isize) -> Result { - let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset(), idx as c_int) }; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.as_ptr(), idx as c_int) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayer")); } @@ -623,7 +622,7 @@ impl Dataset { /// Applies to vector datasets, and fetches by the given /// _0-based_ index. pub fn into_layer(self, idx: isize) -> Result { - let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset(), idx as c_int) }; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.as_ptr(), idx as c_int) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayer")); } @@ -633,7 +632,7 @@ impl Dataset { /// Fetch a layer by name. pub fn layer_by_name(&self, name: &str) -> Result { let c_name = CString::new(name)?; - let c_layer = unsafe { gdal_sys::OGR_DS_GetLayerByName(self.c_dataset(), c_name.as_ptr()) }; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayerByName(self.as_ptr(), c_name.as_ptr()) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayerByName")); } @@ -643,7 +642,7 @@ impl Dataset { /// Fetch a layer by name. pub fn into_layer_by_name(self, name: &str) -> Result { let c_name = CString::new(name)?; - let c_layer = unsafe { gdal_sys::OGR_DS_GetLayerByName(self.c_dataset(), c_name.as_ptr()) }; + let c_layer = unsafe { gdal_sys::OGR_DS_GetLayerByName(self.as_ptr(), c_name.as_ptr()) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayerByName")); } @@ -715,7 +714,7 @@ impl Dataset { // propagated to the gdal_sys wrapper. The lack of `const` seems like a mistake in the // GDAL API, so we just do a cast here. gdal_sys::OGR_DS_CreateLayer( - self.c_dataset(), + self.as_ptr(), c_name.as_ptr(), c_srs, options.ty, diff --git a/src/vector/sql.rs b/src/vector/sql.rs index e4c1fda45..731e3b4a2 100644 --- a/src/vector/sql.rs +++ b/src/vector/sql.rs @@ -132,7 +132,7 @@ impl Dataset { filter_geom = spatial_filter.as_ptr(); } - let c_dataset = self.c_dataset(); + let c_dataset = self.as_ptr(); unsafe { gdal_sys::CPLErrorReset() }; @@ -154,7 +154,7 @@ impl Dataset { Ok(Some(sql::ResultSet { layer, - dataset: c_dataset, + dataset: c_dataset, // TODO: We're ending up with a shared reference here... })) } } diff --git a/src/vector/transaction.rs b/src/vector/transaction.rs index 720dbcdfc..efaef8781 100644 --- a/src/vector/transaction.rs +++ b/src/vector/transaction.rs @@ -1,5 +1,6 @@ use crate::errors::{GdalError, Result}; use crate::Dataset; +use foreign_types::ForeignType; use gdal_sys::OGRErr; use std::ops::{Deref, DerefMut}; @@ -46,7 +47,7 @@ impl<'a> Transaction<'a> { /// /// Depending on drivers, this may or may not abort layer sequential readings that are active. pub fn commit(mut self) -> Result<()> { - let rv = unsafe { gdal_sys::GDALDatasetCommitTransaction(self.dataset.c_dataset()) }; + let rv = unsafe { gdal_sys::GDALDatasetCommitTransaction(self.dataset.as_ptr()) }; self.rollback_on_drop = false; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { @@ -61,7 +62,7 @@ impl<'a> Transaction<'a> { /// /// If the rollback fails, will return [`OGRErr::OGRERR_FAILURE`]. pub fn rollback(mut self) -> Result<()> { - let rv = unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.c_dataset()) }; + let rv = unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.as_ptr()) }; self.rollback_on_drop = false; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { @@ -92,7 +93,7 @@ impl<'a> Drop for Transaction<'a> { if self.rollback_on_drop { // We silently swallow any errors, because we have no way to report them from a drop // function apart from panicking. - unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.c_dataset()) }; + unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.dataset.as_ptr()) }; } } } @@ -168,7 +169,7 @@ impl Dataset { /// ``` pub fn start_transaction(&mut self) -> Result> { let force = 1; - let rv = unsafe { gdal_sys::GDALDatasetStartTransaction(self.c_dataset(), force) }; + let rv = unsafe { gdal_sys::GDALDatasetStartTransaction(self.as_ptr(), force) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, From b9aa36502529fe79865c9a931ee5ad479e93e298 Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Tue, 3 Oct 2023 12:56:10 -0400 Subject: [PATCH 3/4] Converted `FieldDefn` to foreign-types. --- src/vector/layer.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/vector/layer.rs b/src/vector/layer.rs index 7f2779f77..a92c137a7 100644 --- a/src/vector/layer.rs +++ b/src/vector/layer.rs @@ -6,8 +6,8 @@ use crate::vector::{ Envelope, Feature, FeatureIterator, FieldValue, Geometry, LayerOptions, OwnedFeatureIterator, }; use crate::{dataset::Dataset, gdal_major_object::MajorObject}; -use foreign_types::{ForeignType, ForeignTypeRef}; -use gdal_sys::{self, GDALMajorObjectH, OGRErr, OGRFieldDefnH, OGRFieldType, OGRLayerH}; +use foreign_types::{foreign_type, ForeignType, ForeignTypeRef}; +use gdal_sys::{self, GDALMajorObjectH, OGRErr, OGRFieldType, OGRLayerH}; use libc::c_int; use std::ffi::NulError; use std::mem::MaybeUninit; @@ -547,19 +547,20 @@ impl<'a> LayerIterator<'a> { } } } -pub struct FieldDefn { - c_obj: OGRFieldDefnH, -} -impl Drop for FieldDefn { - fn drop(&mut self) { - unsafe { gdal_sys::OGR_Fld_Destroy(self.c_obj) }; +foreign_type! { + /// Field definition + /// + /// Defines a discrete field in a feature. + pub unsafe type FieldDefn { + type CType = libc::c_void; + fn drop = gdal_sys::OGR_Fld_Destroy; } } impl MajorObject for FieldDefn { fn gdal_object_ptr(&self) -> GDALMajorObjectH { - self.c_obj + self.as_ptr() } } @@ -570,16 +571,19 @@ impl FieldDefn { if c_obj.is_null() { return Err(_last_null_pointer_err("OGR_Fld_Create")); }; - Ok(FieldDefn { c_obj }) + Ok(unsafe { FieldDefn::from_ptr(c_obj) }) } +} + +impl FieldDefnRef { pub fn set_width(&self, width: i32) { - unsafe { gdal_sys::OGR_Fld_SetWidth(self.c_obj, width as c_int) }; + unsafe { gdal_sys::OGR_Fld_SetWidth(self.as_ptr(), width as c_int) }; } pub fn set_precision(&self, precision: i32) { - unsafe { gdal_sys::OGR_Fld_SetPrecision(self.c_obj, precision as c_int) }; + unsafe { gdal_sys::OGR_Fld_SetPrecision(self.as_ptr(), precision as c_int) }; } pub fn add_to_layer(&self, layer: &L) -> Result<()> { - let rv = unsafe { gdal_sys::OGR_L_CreateField(layer.c_layer(), self.c_obj, 1) }; + let rv = unsafe { gdal_sys::OGR_L_CreateField(layer.c_layer(), self.as_ptr(), 1) }; if rv != OGRErr::OGRERR_NONE { return Err(GdalError::OgrError { err: rv, From 1a86f38430eb73a72cfaf26bcb86970d6b4a21ea Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Tue, 3 Oct 2023 16:37:48 -0400 Subject: [PATCH 4/4] Separate mutable vs shared rasterband access. --- src/metadata.rs | 4 ++-- src/raster/rasterband.rs | 22 +++++++++++++++++----- src/raster/tests.rs | 30 +++++++++++++++--------------- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 41e4bdc25..17cea02eb 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -381,8 +381,8 @@ mod tests { #[test] fn test_set_description() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); - let dataset = driver.create("", 1, 1, 1).unwrap(); - let band = dataset.rasterband(1).unwrap(); + let mut dataset = driver.create("", 1, 1, 1).unwrap(); + let band = dataset.rasterband_mut(1).unwrap(); let description = "A merry and cheerful band description"; assert_eq!(band.description().unwrap(), ""); diff --git a/src/raster/rasterband.rs b/src/raster/rasterband.rs index 403141974..f582c8a54 100644 --- a/src/raster/rasterband.rs +++ b/src/raster/rasterband.rs @@ -24,9 +24,21 @@ use crate::errors::*; impl Dataset { /// Fetch a band object for a dataset. /// - /// Applies to raster datasets, and fetches the - /// rasterband at the given _1-based_ index. - pub fn rasterband(&self, band_index: isize) -> Result<&mut RasterBand> { + /// Applies to raster datasets, and fetches the rasterband at the given _1-based_ index. + pub fn rasterband(&self, band_index: isize) -> Result<&RasterBand> { + unsafe { + let c_band = gdal_sys::GDALGetRasterBand(self.as_ptr(), band_index as c_int); + if c_band.is_null() { + return Err(_last_null_pointer_err("GDALGetRasterBand")); + } + Ok(RasterBand::from_ptr(c_band)) + } + } + + /// Fetch a mutable band object for a dataset. + /// + /// Applies to raster datasets, and fetches the rasterband at the given _1-based_ index. + pub fn rasterband_mut(&mut self, band_index: isize) -> Result<&mut RasterBand> { unsafe { let c_band = gdal_sys::GDALGetRasterBand(self.as_ptr(), band_index as c_int); if c_band.is_null() { @@ -1097,8 +1109,8 @@ impl Debug for ColorEntry { /// /// // Create in-memory copy to mutate /// let mem_driver = DriverManager::get_driver_by_name("MEM")?; -/// let ds = ds.create_copy(&mem_driver, "", &[])?; -/// let band = ds.rasterband(1)?; +/// let mut ds = ds.create_copy(&mem_driver, "", &[])?; +/// let band = ds.rasterband_mut(1)?; /// assert!(band.color_table().is_none()); /// /// // Create a new color table for 3 classes + no-data diff --git a/src/raster/tests.rs b/src/raster/tests.rs index c72c720a6..d27d2fde3 100644 --- a/src/raster/tests.rs +++ b/src/raster/tests.rs @@ -98,7 +98,7 @@ fn test_read_raster_with_average_resample() { #[test] fn test_write_raster() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); - let dataset = driver.create("", 20, 10, 1).unwrap(); + let mut dataset = driver.create("", 20, 10, 1).unwrap(); // create a 2x1 raster let raster = ByteBuffer { @@ -107,7 +107,7 @@ fn test_write_raster() { }; // epand it to fill the image (20x10) - let rb = dataset.rasterband(1).unwrap(); + let rb = dataset.rasterband_mut(1).unwrap(); let res = rb.write((0, 0), (20, 10), &raster); @@ -318,8 +318,8 @@ fn open_mask_band() { #[test] fn create_mask_band() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); - let dataset = driver.create("", 20, 10, 1).unwrap(); - let rb = dataset.rasterband(1).unwrap(); + let mut dataset = driver.create("", 20, 10, 1).unwrap(); + let rb = dataset.rasterband_mut(1).unwrap(); rb.create_mask_band(false).unwrap(); let mb = rb.open_mask_band().unwrap(); @@ -427,8 +427,8 @@ fn test_get_no_data_value() { #[allow(clippy::float_cmp)] fn test_set_no_data_value() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); - let dataset = driver.create("", 20, 10, 1).unwrap(); - let rasterband = dataset.rasterband(1).unwrap(); + let mut dataset = driver.create("", 20, 10, 1).unwrap(); + let rasterband = dataset.rasterband_mut(1).unwrap(); assert_eq!(rasterband.no_data_value(), None); assert!(rasterband.set_no_data_value(Some(1.23)).is_ok()); assert_eq!(rasterband.no_data_value(), Some(1.23)); @@ -549,8 +549,8 @@ fn test_get_rasterband_color_interp() { #[test] fn test_set_rasterband_color_interp() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); - let dataset = driver.create("", 1, 1, 1).unwrap(); - let rasterband = dataset.rasterband(1).unwrap(); + let mut dataset = driver.create("", 1, 1, 1).unwrap(); + let rasterband = dataset.rasterband_mut(1).unwrap(); rasterband .set_color_interpretation(ColorInterpretation::AlphaBand) .unwrap(); @@ -561,8 +561,8 @@ fn test_set_rasterband_color_interp() { #[test] fn test_set_rasterband_scale() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); - let dataset = driver.create("", 1, 1, 1).unwrap(); - let rasterband = dataset.rasterband(1).unwrap(); + let mut dataset = driver.create("", 1, 1, 1).unwrap(); + let rasterband = dataset.rasterband_mut(1).unwrap(); let scale = 1234.5678; rasterband.set_scale(scale).unwrap(); assert_eq!(rasterband.scale().unwrap(), scale); @@ -571,8 +571,8 @@ fn test_set_rasterband_scale() { #[test] fn test_set_rasterband_offset() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); - let dataset = driver.create("", 1, 1, 1).unwrap(); - let rasterband = dataset.rasterband(1).unwrap(); + let mut dataset = driver.create("", 1, 1, 1).unwrap(); + let rasterband = dataset.rasterband_mut(1).unwrap(); let offset = -123.456; rasterband.set_offset(offset).unwrap(); assert_eq!(rasterband.offset().unwrap(), offset); @@ -680,11 +680,11 @@ fn test_create_color_table() { assert!(band.color_table().is_none()); // Create a new file to put color table in - let dataset = dataset + let mut dataset = dataset .create_copy(&dataset.driver(), &outfile, &[]) .unwrap(); dataset - .rasterband(1) + .rasterband_mut(1) .unwrap() .set_no_data_value(None) .unwrap(); @@ -698,7 +698,7 @@ fn test_create_color_table() { assert_eq!(ct.entry(2), Some(ColorEntry::rgba(255, 0, 0, 255))); assert_eq!(ct.entry(8), None); - dataset.rasterband(1).unwrap().set_color_table(&ct); + dataset.rasterband_mut(1).unwrap().set_color_table(&ct); } // Reopen to confirm the changes.