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

Support for accessing the SpatialRef of GCPs. #397

Merged
merged 2 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -81,6 +81,10 @@

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

- Added support for getting the `SpatialRef` of embedded ground control points (GCPs) via `Dataset::gcp_spatial_ref`.

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

## 0.14

- Added new content to `README.md` and the root docs.
Expand Down
Binary file added fixtures/gcp.tif
Binary file not shown.
1 change: 1 addition & 0 deletions src/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ bitflags! {
///
/// [`GDALOpenEx`]: https://gdal.org/doxygen/gdal_8h.html#a9cb8585d0b3c16726b08e25bcc94274a
#[derive(Debug)]
#[allow(clippy::assign_op_pattern)]
pub struct GdalOpenFlags: c_uint {
/// Open in read-only mode (default).
const GDAL_OF_READONLY = 0x00;
Expand Down
37 changes: 37 additions & 0 deletions src/raster/gcp.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//! Raster ground control point support

use crate::spatial_ref::SpatialRef;
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<SpatialRef> {
let c_ptr = unsafe { gdal_sys::GDALGetGCPSpatialRef(self.c_dataset()) };

if c_ptr.is_null() {
return None;
}

unsafe { SpatialRef::from_c_obj(c_ptr) }.ok()
Copy link
Member

@lnicola lnicola May 17, 2023

Choose a reason for hiding this comment

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

I wonder if this is leaking the OGRSpatialReferenceH (check out the implementation of from_c_obj). No, it doesn't.

Does foreign_types really solve issues like this? I only skimmed your changes, but it would be great if it did.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lnicola It solves it in the sense that we can more accurately reflect the owned vs borrowed semantics described in the GDAL docs, in a principled manner. After digging into the Layer and MDArray bindings, I'm pretty sure we're either leaking resources or dangerously borrowing them (no proof yet, just suspicion). Using foreign_types would also replace GeometryRef and OwnedLayer.

Copy link
Member

Choose a reason for hiding this comment

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

That would be awesome, I'll try to look into it.

}
}

#[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");
}
}
1 change: 1 addition & 0 deletions src/raster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
//! ...
//! ```

mod gcp;
#[cfg(all(major_ge_3, minor_ge_1))]
mod mdarray;
mod rasterband;
Expand Down
Loading