Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Add CoordTransform::transform_bounds() #272

Merged
merged 5 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@

- <https://github.com/georust/gdal/pull/273>

- Add `gdal::srs::CoordTransform::transform_bounds` as wrapper for `OCTTransformBounds` for GDAL 3.4

- <https://github.com/georust/gdal/pull/272>

## 0.12

- Bump Rust edition to 2021
Expand Down
57 changes: 56 additions & 1 deletion src/spatial_ref/srs.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::utils::{_last_cpl_err, _last_null_pointer_err, _string};
use gdal_sys::{self, CPLErr, OGRCoordinateTransformationH, OGRErr, OGRSpatialReferenceH};
use libc::{c_char, c_int};
use libc::{c_char, c_double, c_int};
use std::ffi::{CStr, CString};
use std::ptr::{self, null_mut};
use std::str::FromStr;
Expand Down Expand Up @@ -33,6 +33,61 @@ impl CoordTransform {
})
}

/// Transform bounding box, densifying the edges to account for nonlinear
/// transformations.
///
/// # Arguments
/// * bounds - array of [axis0_min, axis1_min, axis0_max, axis1_min],
/// interpreted in the axis order of the source SpatialRef,
/// typically [xmin, ymin, xmax, ymax]
/// * densify_pts - number of points per edge (recommended: 21)
///
/// # Returns
/// Some([f64; 4]) with bounds in axis order of target SpatialRef
/// None if there is an error.
#[cfg(all(major_ge_3, minor_ge_4))]
pub fn transform_bounds(&self, bounds: &[f64; 4], densify_pts: i32) -> Result<([f64; 4])> {
let mut out_xmin: f64 = 0.;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have to move the use here to avoid the warning. Or you might be able to skip the casts, like in transform_coords, I'm not sure.

let mut out_ymin: f64 = 0.;
let mut out_xmax: f64 = 0.;
let mut out_ymax: f64 = 0.;

let ret_val = unsafe {
gdal_sys::OCTTransformBounds(
self.inner,
bounds[0] as c_double,
bounds[1] as c_double,
bounds[2] as c_double,
bounds[3] as c_double,
&mut out_xmin as *mut c_double,
&mut out_ymin as *mut c_double,
&mut out_xmax as *mut c_double,
&mut out_ymax as *mut c_double,
densify_pts as c_int,
) == 1
};

if ret_val {
Ok([out_xmin, out_ymin, out_xmax, out_ymax])
} else {
let err = _last_cpl_err(CPLErr::CE_Failure);
let msg = if let GdalError::CplError { msg, .. } = err {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A match might look nicer here (you can probably drop the trim), but it probably doesn't matter too much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was copy-pasted from transform_coords for consistency; ideally both would get updated to be the same if match is more appropriate. I can update just this one here and the other in a follow up PR, or both in a follow up PR.

I'd probably also want to invert the logic a little bit so that only the error case is in the conditional and have a bare Ok:

if !ret_val {...handle error and return... }

Ok([out_xmin, out_ymin, out_xmax, out_ymax])

But didn't do so in this PR out of consistency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can update just this one here and the other in a follow up PR, or both in a follow up PR.

Right, it's fine either way.

if msg.trim().is_empty() {
None
} else {
Some(msg)
}
} else {
return Err(err);
};
Err(GdalError::InvalidCoordinateRange {
from: self.from.clone(),
to: self.to.clone(),
msg,
})
}
}

/// Transform coordinates in place.
///
/// # Arguments
Expand Down
62 changes: 62 additions & 0 deletions src/spatial_ref/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,68 @@ fn comparison() {
assert!(spatial_ref5 == spatial_ref4);
}

#[cfg(all(major_ge_3, minor_ge_4))]
#[test]
fn transform_bounds() {
let bounds: [f64; 4] = [-180., -80., 180., 80.];
// bounds for y,x ordered SpatialRefs
let yx_bounds: [f64; 4] = [-80.0, -180.0, 80.0, 180.];

let spatial_ref1 = SpatialRef::from_definition("OGC:CRS84").unwrap();

// transforming between the same SpatialRef should return existing bounds
let mut transform = CoordTransform::new(&spatial_ref1, &spatial_ref1).unwrap();
let mut out_bounds = transform.transform_bounds(&bounds, 21).unwrap();
assert_almost_eq(out_bounds[0], bounds[0]);
assert_almost_eq(out_bounds[1], bounds[1]);
assert_almost_eq(out_bounds[2], bounds[2]);
assert_almost_eq(out_bounds[3], bounds[3]);

// EPSG:4326 is in y,x order by default; returned bounds are [ymin, xmin, ymax, xmax]
let mut spatial_ref2 = SpatialRef::from_epsg(4326).unwrap();
transform = CoordTransform::new(&spatial_ref1, &spatial_ref2).unwrap();
out_bounds = transform.transform_bounds(&bounds, 21).unwrap();
assert_almost_eq(out_bounds[0], yx_bounds[0]);
assert_almost_eq(out_bounds[1], yx_bounds[1]);
assert_almost_eq(out_bounds[2], yx_bounds[2]);
assert_almost_eq(out_bounds[3], yx_bounds[3]);

// if source SpatialRef is in y,x order and and target SpatialRef is in x,y order
// input bounds are interpreted as [ymin, xmin, ymax, xmax] and returns
// [xmin, ymin, xmax, ymax]
transform = CoordTransform::new(&spatial_ref2, &spatial_ref1).unwrap();
out_bounds = transform.transform_bounds(&yx_bounds, 21).unwrap();
assert_almost_eq(out_bounds[0], bounds[0]);
assert_almost_eq(out_bounds[1], bounds[1]);
assert_almost_eq(out_bounds[2], bounds[2]);
assert_almost_eq(out_bounds[3], bounds[3]);

// force EPSG:4326 into x,y order to match source SpatialRef
spatial_ref2
.set_axis_mapping_strategy(gdal_sys::OSRAxisMappingStrategy::OAMS_TRADITIONAL_GIS_ORDER);
transform = CoordTransform::new(&spatial_ref1, &spatial_ref2).unwrap();
out_bounds = transform.transform_bounds(&bounds, 21).unwrap();
assert_almost_eq(out_bounds[0], bounds[0]);
assert_almost_eq(out_bounds[1], bounds[1]);
assert_almost_eq(out_bounds[2], bounds[2]);
assert_almost_eq(out_bounds[3], bounds[3]);

spatial_ref2 = SpatialRef::from_epsg(3857).unwrap();
transform = CoordTransform::new(&spatial_ref1, &spatial_ref2).unwrap();
out_bounds = transform.transform_bounds(&bounds, 21).unwrap();

let expected_bounds: [f64; 4] = [
-20037508.342789244,
-15538711.096309224,
20037508.342789244,
15538711.09630923,
];
assert_almost_eq(out_bounds[0], expected_bounds[0]);
assert_almost_eq(out_bounds[1], expected_bounds[1]);
assert_almost_eq(out_bounds[2], expected_bounds[2]);
assert_almost_eq(out_bounds[3], expected_bounds[3]);
}

#[test]
fn transform_coordinates() {
let spatial_ref1 = SpatialRef::from_wkt("GEOGCS[\"WGS 84\",DATUM[\"WGS_1984\",SPHEROID[\"WGS 84\",6378137,298.257223563,AUTHORITY[\"EPSG\",7030]],TOWGS84[0,0,0,0,0,0,0],AUTHORITY[\"EPSG\",6326]],PRIMEM[\"Greenwich\",0,AUTHORITY[\"EPSG\",8901]],UNIT[\"DMSH\",0.0174532925199433,AUTHORITY[\"EPSG\",9108]],AXIS[\"Lat\",NORTH],AXIS[\"Long\",EAST],AUTHORITY[\"EPSG\",4326]]").unwrap();
Expand Down