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

Made mapping between ResampleAlg and GDALRIOResampleAlg more direct. #309

Merged
merged 3 commits into from
Sep 18, 2022
Merged
Changes from 1 commit
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
56 changes: 34 additions & 22 deletions src/raster/rasterband.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::raster::{GDALDataType, GdalType};
use crate::utils::{_last_cpl_err, _last_null_pointer_err, _string};
use gdal_sys::{
self, CPLErr, GDALColorEntry, GDALColorInterp, GDALColorTableH, GDALComputeRasterMinMax,
GDALGetRasterStatistics, GDALMajorObjectH, GDALPaletteInterp, GDALRWFlag, GDALRasterBandH,
GDALRasterIOExtraArg,
GDALGetRasterStatistics, GDALMajorObjectH, GDALPaletteInterp, GDALRIOResampleAlg, GDALRWFlag,
GDALRasterBandH, GDALRasterIOExtraArg,
};
use libc::c_int;
use std::ffi::CString;
Expand All @@ -17,37 +17,49 @@ use ndarray::Array2;

use crate::errors::*;

/// Resampling algorithms, map GDAL defines
/// Resampling algorithms used throughout various GDAL raster I/O operations.
///
/// # Example
///
/// ```rust
/// use gdal::Dataset;
/// # fn main() -> gdal::errors::Result<()> {
/// use gdal::raster::ResampleAlg;
/// let ds = Dataset::open("fixtures/tinymarble.tif")?;
/// let band1 = ds.rasterband(1)?;
/// let stats = band1.get_statistics(true, false)?.unwrap();
/// // Down-sample a image using cubic-spline interpolation
/// let buf = band1.read_as::<f64>((0, 0), ds.raster_size(), (2, 2), Some(ResampleAlg::CubicSpline))?;
/// // In this particular image, resulting data should be close to the overall average.
/// assert!(buf.data.iter().all(|c| (c - stats.mean).abs() < stats.std_dev / 2.0));
/// # Ok(())
/// # }
/// ```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bonus documentation while I was in there :)

#[derive(Debug, Copy, Clone)]
#[repr(u32)]
Copy link
Contributor Author

@metasim metasim Sep 9, 2022

Choose a reason for hiding this comment

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

Not entirely sure about this value, or even if it should be specified.
GDALRIOResampleAlg::Type is libc::c_uint, which is u32 on *nix. The repr options maybe suggest #[repr(C)] should be used instead, and the right value will be selected based on the discriminant values? Advice appreciated.

Copy link
Member

@lnicola lnicola Sep 14, 2022

Choose a reason for hiding this comment

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

I don't think we need the #[repr], given the cast in to_gdal. Or does the compiler pick isize there?

EDIT: yeah, I think u32 is fine here. We could also keep the match in to_gdal.

pub enum ResampleAlg {
/// Nearest neighbour
NearestNeighbour,
NearestNeighbour = GDALRIOResampleAlg::GRIORA_NearestNeighbour,
/// Bilinear (2x2 kernel)
Bilinear,
Bilinear = GDALRIOResampleAlg::GRIORA_Bilinear,
/// Cubic Convolution Approximation (4x4 kernel)
Cubic,
Cubic = GDALRIOResampleAlg::GRIORA_Cubic,
/// Cubic B-Spline Approximation (4x4 kernel)
CubicSpline,
CubicSpline = GDALRIOResampleAlg::GRIORA_CubicSpline,
/// Lanczos windowed sinc interpolation (6x6 kernel)
Lanczos,
Lanczos = GDALRIOResampleAlg::GRIORA_Lanczos,
/// Average
Average,
Average = GDALRIOResampleAlg::GRIORA_Average,
/// Mode (selects the value which appears most often of all the sampled points)
Mode,
Mode = GDALRIOResampleAlg::GRIORA_Mode,
/// Gauss blurring
Gauss,
Gauss = GDALRIOResampleAlg::GRIORA_Gauss,
}

fn map_resample_alg(alg: &ResampleAlg) -> u32 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this was not private before, so there was no easy way to map values when calling into lower level gdal-sys functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, the return type should have been GDALRIOResampleAlg::Type.

match alg {
ResampleAlg::NearestNeighbour => 0,
ResampleAlg::Bilinear => 1,
ResampleAlg::Cubic => 2,
ResampleAlg::CubicSpline => 3,
ResampleAlg::Lanczos => 4,
ResampleAlg::Average => 5,
ResampleAlg::Mode => 6,
ResampleAlg::Gauss => 7,
impl ResampleAlg {
/// Convert Rust enum discriminant to value expected by [`GDALRasterIOExtraArg`].
pub fn to_gdal(&self) -> GDALRIOResampleAlg::Type {
*self as GDALRIOResampleAlg::Type
}
}

Expand Down Expand Up @@ -131,7 +143,7 @@ impl From<RasterIOExtraArg> for GDALRasterIOExtraArg {

GDALRasterIOExtraArg {
nVersion: n_version as c_int,
eResampleAlg: map_resample_alg(&e_resample_alg),
eResampleAlg: e_resample_alg.to_gdal(),
pfnProgress: pfn_progress,
pProgressData: p_progress_data,
bFloatingPointWindowValidity: b_floating_point_window_validity as c_int,
Expand Down