diff --git a/CHANGES.md b/CHANGES.md index 5414d2e..21962da 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,7 @@ # Unreleased +- Refactored options logic + # 0.31.0 - 2025-08-29 - Add Proj::as_wkt() function diff --git a/src/cstring_array.rs b/src/cstring_array.rs new file mode 100644 index 0000000..f96959a --- /dev/null +++ b/src/cstring_array.rs @@ -0,0 +1,101 @@ +use libc::c_char; +use std::ffi::{CString, NulError}; +use std::ptr; + +/// A null-terminated array of C strings for passing to PROJ functions. +/// +/// PROJ functions that accept options expect a null-terminated array of +/// null-terminated strings (`char* const*`), typically in `KEY=VALUE` format. +/// +/// Strings are converted to CStrings immediately when added via [`push`](Self::push), +/// and the null-terminated pointer array is maintained incrementally. +/// +/// # Example +/// ```ignore +/// let mut c_strings = CStringArray::new(); +/// c_strings.push("AUTHORITY=EPSG")?; +/// c_strings.push("ALLOW_BALLPARK=NO")?; +/// unsafe { proj_create_crs_to_crs_from_pj(ctx, src, tgt, area, c_strings.as_ptr()) }; +/// ``` +pub(crate) struct CStringArray { + /// Owns the CString data. + cstrings: Vec, + /// Null-terminated pointer array, maintained incrementally as strings are added. + ptrs: Vec<*const c_char>, +} + +impl CStringArray { + /// Creates a new empty `CStringArray`. + pub fn new() -> Self { + Self { + cstrings: Vec::new(), + ptrs: vec![ptr::null()], + } + } + + /// Adds a string to the array. + /// + /// Returns an error if the string contains an interior nul byte. + pub fn push(&mut self, s: impl Into) -> Result<(), NulError> { + debug_assert_eq!(self.ptrs.last(), Some(&ptr::null())); + debug_assert_eq!(self.ptrs.len(), self.cstrings.len() + 1); + + let cstring = CString::new(s.into())?; + // Insert before the null terminator + self.ptrs.insert(self.ptrs.len() - 1, cstring.as_ptr()); + self.cstrings.push(cstring); + Ok(()) + } + + /// Returns a pointer to a null-terminated array of C string pointers, + /// suitable for passing to PROJ C functions, or null if the list is empty. + pub fn as_ptr(&self) -> *const *const c_char { + debug_assert_eq!(self.ptrs.last(), Some(&ptr::null())); + debug_assert_eq!(self.ptrs.len(), self.cstrings.len() + 1); + if self.cstrings.is_empty() { + // return null ptr, rather than a ptr to an empty list + // + // Historically we were not consistent about this. It's likely that proj handles an + // empty list the same as NULL, but it seems better to consistently return NULL since + // it's explicitly documented. + ptr::null() + } else { + self.ptrs.as_ptr() + } + } +} + +impl Default for CStringArray { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_empty() { + let arr = CStringArray::new(); + let ptr = arr.as_ptr(); + assert!(ptr.is_null()); + } + + #[test] + fn test_single_string() { + let mut arr = CStringArray::new(); + arr.push("KEY=VALUE").unwrap(); + let ptr = arr.as_ptr(); + unsafe { + assert!(!(*ptr).is_null()); + assert!((*ptr.add(1)).is_null()); + } + } + + #[test] + fn test_nul_error() { + let mut arr = CStringArray::new(); + assert!(arr.push("invalid\0string").is_err()); + } +} diff --git a/src/lib.rs b/src/lib.rs index ef68880..7a3c9e8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -241,6 +241,7 @@ mod geo_types; #[macro_use] extern crate approx; +mod cstring_array; mod proj; mod transform; pub use transform::{Transform, TransformError}; diff --git a/src/proj.rs b/src/proj.rs index 1bdb182..e4d6a8f 100644 --- a/src/proj.rs +++ b/src/proj.rs @@ -14,13 +14,14 @@ use proj_sys::{ PJ_WKT_TYPE_PJ_WKT1_GDAL, PJ_WKT_TYPE_PJ_WKT2_2015, PJ_WKT_TYPE_PJ_WKT2_2015_SIMPLIFIED, PJ_WKT_TYPE_PJ_WKT2_2019, PJ_WKT_TYPE_PJ_WKT2_2019_SIMPLIFIED, PJ_XYZT, }; -use std::ptr; use std::{ convert, ffi, fmt::{self, Debug}, str, }; +use crate::cstring_array::CStringArray; + #[cfg(feature = "network")] use proj_sys::proj_context_set_enable_network; @@ -257,26 +258,13 @@ fn crs_to_crs_from_pj( let proj_area = unsafe { proj_area_create() }; area_set_bbox(proj_area, area); - // Convert options to C strings - let mut options_cstr: Vec = Vec::new(); - let mut options_ptrs: Vec<*const c_char> = Vec::new(); - - if let Some(opts) = options { - for opt in opts { - match ffi::CString::new(opt) { - Ok(c_str) => { - options_cstr.push(c_str); - } - Err(err) => return Err(ProjCreateError::ArgumentNulError(err)), - } + let mut proj_options = CStringArray::new(); + if let Some(options) = options { + for option in options { + proj_options + .push(option) + .map_err(ProjCreateError::ArgumentNulError)?; } - - options_ptrs = options_cstr.iter().map(|s| s.as_ptr()).collect(); - // Add null terminator - options_ptrs.push(ptr::null()); - } else { - // If no options, just use a null pointer - options_ptrs.push(ptr::null()); } let ptr = result_from_create(ctx, unsafe { @@ -285,7 +273,7 @@ fn crs_to_crs_from_pj( source_crs.c_proj, target_crs.c_proj, proj_area, - options_ptrs.as_ptr(), + proj_options.as_ptr(), ) }) .map_err(|e| ProjCreateError::ProjError(e.message(ctx)))?; @@ -1261,27 +1249,22 @@ impl Proj { indentation_width: Option, schema: Option<&str>, ) -> Result { - let mut opts = vec![]; + let mut proj_options = CStringArray::new(); if let Some(multiline) = multiline { if multiline { - opts.push(CString::new(String::from("MULTILINE=YES"))?) + proj_options.push("MULTILINE=YES")?; } else { - opts.push(CString::new(String::from("MULTILINE=NO"))?) + proj_options.push("MULTILINE=NO")?; } - }; + } if let Some(indentation_width) = indentation_width { - opts.push(CString::new(format!( - "INDENTATION_WIDTH={indentation_width}" - ))?) + proj_options.push(format!("INDENTATION_WIDTH={indentation_width}"))?; } if let Some(schema) = schema { - opts.push(CString::new(format!("SCHEMA={schema}"))?) + proj_options.push(format!("SCHEMA={schema}"))?; } - let mut opts_ptrs: Vec<_> = opts.iter().map(|cs| cs.as_ptr()).collect(); - // we always have to terminate with a null pointer, even if the opts are empty - opts_ptrs.push(ptr::null()); unsafe { - let out_ptr = proj_as_projjson(self.ctx, self.c_proj, opts_ptrs.as_ptr()); + let out_ptr = proj_as_projjson(self.ctx, self.c_proj, proj_options.as_ptr()); if out_ptr.is_null() { Err(ProjError::ExportToJson) } else { @@ -1295,76 +1278,54 @@ impl Proj { version: Option, options: Option, ) -> Result { - let options_str = if let Some(ref options) = options { - let mut opts = vec![]; + let mut proj_options = CStringArray::new(); + if let Some(options) = options { if let Some(multiline) = options.multiline { - opts.push(CString::new(format!( + proj_options.push(format!( "MULTILINE={}", if multiline { "YES" } else { "NO" } - ))?) - }; + ))?; + } if let Some(indentation_width) = options.indentation_width { - opts.push(CString::new(format!( - "INDENTATION_WIDTH={indentation_width}" - ))?) + proj_options.push(format!("INDENTATION_WIDTH={indentation_width}"))?; } if let Some(ref output_axis) = options.output_axis { - opts.push(CString::new(format!( + proj_options.push(format!( "OUTPUT_AXIS={}", match output_axis { WktOutputAxis::Auto => "AUTO", WktOutputAxis::Yes => "YES", WktOutputAxis::No => "NO", } - ))?); + ))?; } if let Some(strict) = options.strict { - opts.push(CString::new(format!( - "STRICT={}", - if strict { "YES" } else { "NO" } - ))?); + proj_options.push(format!("STRICT={}", if strict { "YES" } else { "NO" }))?; } if let Some(allow_ellipsoidal_height_as_vertical_crs) = options.allow_ellipsoidal_height_as_vertical_crs { - opts.push(CString::new(format!( + proj_options.push(format!( "ALLOW_ELLIPSOIDAL_HEIGHT_AS_VERTICAL_CRS={}", if allow_ellipsoidal_height_as_vertical_crs { "YES" } else { "NO" } - ))?); + ))?; } if let Some(allow_linunit_node) = options.allow_linunit_node { - opts.push(CString::new(format!( + proj_options.push(format!( "ALLOW_LINUNIT_NODE={}", if allow_linunit_node { "YES" } else { "NO" } - ))?); + ))?; } - - if opts.is_empty() { - None - } else { - Some(opts) - } - } else { - None - }; - - let opts_ptrs = options_str.as_ref().map(|o| { - // Each option is a CString (null-terminated string), but PROJ also expects - // the *array* of option pointers itself to be null-terminated (char* const*). - let mut ptrs: Vec<_> = o.iter().map(|cs| cs.as_ptr()).collect(); - // Add a trailing NULL pointer to terminate the list. - ptrs.push(ptr::null()); - ptrs - }); + } let wkt_type = match version.unwrap_or(WktVersion::Wkt2_2019) { WktVersion::Wkt2_2015 => PJ_WKT_TYPE_PJ_WKT2_2015, @@ -1376,16 +1337,7 @@ impl Proj { }; unsafe { - let wkt = proj_as_wkt( - self.ctx, - self.c_proj, - wkt_type, - opts_ptrs - .as_ref() - .map(|c| c.as_ptr()) - .unwrap_or(ptr::null()), - ); - + let wkt = proj_as_wkt(self.ctx, self.c_proj, wkt_type, proj_options.as_ptr()); Ok(_string(wkt)?) } } @@ -1524,8 +1476,9 @@ impl Drop for ProjBuilder { #[cfg(test)] mod test { use super::*; + use approx::{AbsDiffEq, RelativeEq}; - #[derive(Debug)] + #[derive(Debug, Clone, Copy, PartialEq)] struct MyPoint { x: f64, y: f64, @@ -1551,6 +1504,34 @@ mod test { } } + impl AbsDiffEq for MyPoint { + type Epsilon = f64; + + fn default_epsilon() -> Self::Epsilon { + f64::default_epsilon() + } + + fn abs_diff_eq(&self, other: &Self, epsilon: Self::Epsilon) -> bool { + self.x().abs_diff_eq(&other.x(), epsilon) && self.y().abs_diff_eq(&other.y(), epsilon) + } + } + + impl RelativeEq for MyPoint { + fn default_max_relative() -> Self::Epsilon { + f64::default_max_relative() + } + + fn relative_eq( + &self, + other: &Self, + epsilon: Self::Epsilon, + max_relative: Self::Epsilon, + ) -> bool { + self.x().relative_eq(&other.x(), epsilon, max_relative) + && self.y().relative_eq(&other.y(), epsilon, max_relative) + } + } + #[cfg(feature = "network")] #[test] fn test_network_enabled_conversion() { @@ -1628,6 +1609,35 @@ mod test { assert_relative_eq!(result.y(), 1141263.0111604782, epsilon = 1.0e-8); } + #[test] + fn test_create_crs_to_crs_from_pj_with_options() { + // WGS 84 / Equal Earth Americas + let to = Proj::new("EPSG:8858").unwrap(); + // WGS 84 / Equal Earth Asia-Pacific + let from = Proj::new("EPSG:8859").unwrap(); + + let input = MyPoint::new(0.0, 0.0); + + let transformer = from.create_crs_to_crs_from_pj(&to, None, None).unwrap(); + let result = transformer.convert(input).unwrap(); + assert_relative_eq!( + result, + MyPoint::new(-11495972.708144628, 0.0), + epsilon = 1.0e-8 + ); + + let options = vec!["FORCE_OVER=YES"]; + let transformer = from + .create_crs_to_crs_from_pj(&to, None, Some(options)) + .unwrap(); + let result = transformer.convert(input).unwrap(); + assert_relative_eq!( + result, + MyPoint::new(22991945.416289266, 0.0), + epsilon = 1.0e-8 + ); + } + #[cfg(feature = "network")] #[test] fn test_create_crs_to_crs_from_pj_using_builder_epoch() { @@ -1913,16 +1923,23 @@ mod test { let from = "EPSG:2230"; let to = "EPSG:26946"; let ft_to_m = Proj::new_known_crs(from, to, None).unwrap(); + + let default_output = ft_to_m.to_projjson(None, None, None).unwrap(); + assert!(default_output.contains("\n")); + // NOTE: Brittle! This will break if the default schema version gets bumped. + // Just bump the tests expectation here when that happens. + assert!(default_output.contains("https://proj.org/schemas/v0.7/projjson.schema.json")); + // Because libproj has been fussy about passing empty options strings we're testing both - let _ = ft_to_m + let options_output = ft_to_m .to_projjson( - Some(true), + Some(false), None, - Some("https://proj.org/schemas/v0.7/projjson.schema.json"), + Some("https://proj.org/schemas/v0.6/projjson.schema.json"), ) .unwrap(); - let _ = ft_to_m.to_projjson(None, None, None).unwrap(); - // TODO: do we want to compare one of the results to proj's output? + assert!(!options_output.contains("\n")); + assert!(options_output.contains("https://proj.org/schemas/v0.6/projjson.schema.json")); } #[test]