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

Add raster creation options #193

Merged
merged 21 commits into from
Jun 22, 2021
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
21 changes: 21 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,27 @@

## Unreleased

- Add support for raster dataset creation options. A new struct (`RasterCreationOption`) and function (`driver.create_with_band_type_with_options()`) are now available for this.

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

```rust
let driver = Driver::get("GTiff").unwrap();
let options = &[
RasterCreationOption {
key: "COMPRESS",
value: "LZW",
},
RasterCreationOption {
key: "TILED",
value: "YES",
},
];
let mut dataset = driver
.create_with_band_type_with_options::<u8>("testing.tif", 2048, 2048, 1, options)
.unwrap();
```

- **Breaking**: Add support to select a resampling algorithm when reading a raster

- <https://github.com/georust/gdal/pull/141>
Expand Down
37 changes: 34 additions & 3 deletions src/driver.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::dataset::Dataset;
use crate::gdal_major_object::MajorObject;
use crate::metadata::Metadata;
use crate::raster::GdalType;
use crate::raster::{GdalType, RasterCreationOption};
use crate::utils::{_last_null_pointer_err, _string};
use gdal_sys::{self, GDALDriverH, GDALMajorObjectH};
use libc::c_int;
use libc::{c_char, c_int};
use std::ffi::CString;
use std::ptr::null_mut;
use std::sync::Once;
Expand All @@ -26,6 +26,16 @@ pub struct Driver {
c_driver: GDALDriverH,
}

type CSLConstList = *mut *mut c_char;

extern "C" {
fn CSLSetNameValue(
papszOptions: CSLConstList,
pszName: *const libc::c_char,
pszValue: *const libc::c_char,
) -> *mut *mut libc::c_char;
}

impl Driver {
pub fn get(name: &str) -> Result<Driver> {
_register_drivers();
Expand Down Expand Up @@ -80,6 +90,27 @@ impl Driver {
size_y: isize,
bands: isize,
) -> Result<Dataset> {
let options = [];
self.create_with_band_type_with_options::<T>(filename, size_x, size_y, bands, &options)
}

pub fn create_with_band_type_with_options<T: GdalType>(
&self,
filename: &str,
size_x: isize,
size_y: isize,
bands: isize,
options: &[RasterCreationOption],
) -> Result<Dataset> {
let mut options_c = null_mut();
for option in options {
let psz_name = CString::new(option.key)?;
let psz_value = CString::new(option.value)?;
unsafe {
options_c = CSLSetNameValue(options_c, psz_name.as_ptr(), psz_value.as_ptr());
}
}

let c_filename = CString::new(filename)?;
let c_dataset = unsafe {
gdal_sys::GDALCreate(
Expand All @@ -89,7 +120,7 @@ impl Driver {
size_y as c_int,
bands as c_int,
T::gdal_type(),
null_mut(),
options_c as *mut *mut i8,
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be a *mut * mut char?

Copy link
Member

@lnicola lnicola Jun 18, 2021

Choose a reason for hiding this comment

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

You mean c_char?

Seen as char** from C and const char* const* from C++

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, options_c as *mut *mut i8 is correct. I'm not sure how it ended up being null_mut() again, sorry.

Copy link
Member

Choose a reason for hiding this comment

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

It's not null_mut(), it's options_c as *mut *mut i8, jdroenner is asking if it should be options_c as *mut *mut c_char.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, sorry! my bad. Blame Friday night.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gdal_sys::GDALCreate expects *mut *mut i8.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's defined as char** in the GDAL headers. We might be able to improve the bindings, but I think we shouldn't block merging this.

Copy link
Contributor Author

@geohardtke geohardtke Jun 21, 2021

Choose a reason for hiding this comment

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

Any thoughts @jdroenner? Do the bindings need to be updated before merging this? I think I don't have enough knowledge to do that.

Copy link
Member

Choose a reason for hiding this comment

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

no its fine. i just assumed it to be a char in the bindings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool! Thanks!

)
};
if c_dataset.is_null() {
Expand Down
6 changes: 6 additions & 0 deletions src/raster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,11 @@ pub use rasterband::{Buffer, ByteBuffer, ColorInterpretation, RasterBand};
pub use types::{GDALDataType, GdalType};
pub use warp::reproject;

#[derive(Debug)]
pub struct RasterCreationOption<'a> {
pub key: &'a str,
pub value: &'a str,
}

#[cfg(test)]
mod tests;
49 changes: 48 additions & 1 deletion src/raster/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::dataset::Dataset;
use crate::metadata::Metadata;
use crate::raster::rasterband::ResampleAlg;
use crate::raster::{ByteBuffer, ColorInterpretation};
use crate::raster::{ByteBuffer, ColorInterpretation, RasterCreationOption};
use crate::Driver;
use gdal_sys::GDALDataType;
use std::path::Path;
Expand Down Expand Up @@ -239,6 +239,53 @@ fn test_create_with_band_type() {
assert_eq!(rb.band_type(), GDALDataType::GDT_Float32)
}

#[test]
fn test_create_with_band_type_with_options() {
let driver = Driver::get("GTiff").unwrap();
let options = [
RasterCreationOption {
key: "TILED",
value: "YES",
},
RasterCreationOption {
key: "BLOCKXSIZE",
value: "128",
},
RasterCreationOption {
key: "BLOCKYSIZE",
value: "64",
},
RasterCreationOption {
key: "COMPRESS",
value: "LZW",
},
RasterCreationOption {
key: "INTERLEAVE",
value: "BAND",
},
];
lnicola marked this conversation as resolved.
Show resolved Hide resolved

let tmp_filename = "/tmp/test.tif";
{
let dataset = driver
.create_with_band_type_with_options::<u8>(tmp_filename, 256, 256, 1, &options)
.unwrap();
let rasterband = dataset.rasterband(1).unwrap();
let block_size = rasterband.block_size();
assert_eq!(block_size, (128, 64));
}

let dataset = Dataset::open(Path::new(tmp_filename)).unwrap();
let key = "INTERLEAVE";
let domain = "IMAGE_STRUCTURE";
let meta = dataset.metadata_item(key, domain);
assert_eq!(meta.as_deref(), Some("BAND"));
let key = "COMPRESSION";
let domain = "IMAGE_STRUCTURE";
let meta = dataset.metadata_item(key, domain);
assert_eq!(meta.as_deref(), Some("LZW"));
}

#[test]
fn test_create_copy() {
let driver = Driver::get("MEM").unwrap();
Expand Down