Skip to content

Commit

Permalink
PR feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
metasim committed Dec 18, 2023
1 parent c344b93 commit caf5aad
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 30 deletions.
29 changes: 15 additions & 14 deletions src/raster/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::raster::GdalType;
use ndarray::Array2;

/// A 2-D array backed by it's `size` (cols, rows) and a row-major `Vec<T>` and it's dimensions.
#[derive(Debug, Clone, PartialEq)]
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq)]
pub struct Buffer<T> {
pub size: (usize, usize),
pub data: Vec<T>,
Expand All @@ -13,11 +13,12 @@ pub struct Buffer<T> {
impl<T: GdalType> Buffer<T> {
/// Construct a new buffer from `size` (`(cols, rows)`) and `Vec<T>`.
///
/// # Notes
/// Assumes `size.0 * size.1 == data.len()`.
pub fn new(size: (usize, usize), data: Vec<T>) -> Buffer<T> {
debug_assert!(
size.0 * size.1 == data.len(),
/// # Panic
/// Will panic if `size.0 * size.1 != data.len()`.
pub fn new(size: (usize, usize), data: Vec<T>) -> Self {
assert_eq!(
size.0 * size.1,
data.len(),
"size {:?} does not match length {}",
size,
data.len()
Expand Down Expand Up @@ -51,14 +52,14 @@ impl<T: GdalType> TryFrom<Buffer<T>> for Array2<T> {
impl<T: GdalType + Copy> From<Array2<T>> for Buffer<T> {
fn from(value: Array2<T>) -> Self {
// Array2 shape is (rows, cols) and Buffer shape is (cols in x-axis, rows in y-axis)
let shape = value.shape().to_vec();
let data: Vec<T> = if value.is_standard_layout() {
value.into_raw_vec()
} else {
value.iter().copied().collect()
};

Buffer::new((shape[1], shape[0]), data)
let shape = value.shape();
let (rows, cols) = (shape[0], shape[1]);
let data = value
.as_standard_layout()
.iter()
.copied()
.collect::<Vec<T>>();

This comment has been minimized.

Copy link
@lnicola

lnicola Dec 18, 2023

Member

Oh, this isn't great, we can't use into_raw_vec because it's shared.

Buffer::new((cols, rows), data)
}
}

Expand Down
24 changes: 15 additions & 9 deletions src/raster/rasterband.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ impl<'a> RasterBand<'a> {
///
/// # Arguments
/// * `block_index` - the block index
/// * `block` - Data buffer to write to block.
///
/// # Notes
/// Blocks indexes start from 0 and are of form (x, y), where x grows in the horizontal direction.
Expand All @@ -566,6 +567,10 @@ impl<'a> RasterBand<'a> {
/// The last blocks in both directions can be smaller.
/// [`RasterBand::actual_block_size`] will report the correct dimensions of a block.
///
/// While drivers make sure that the content of the `block` buffer before and after the call
/// is equal, some drivers might temporarily modify it, e.g. to do byte swapping. Therefore
/// a `&mut` parameter is required.
///
/// # Errors
/// If the block index is not valid, GDAL will return an error.
///
Expand Down Expand Up @@ -601,14 +606,14 @@ impl<'a> RasterBand<'a> {
/// )?;
/// let mut band1 = dataset.rasterband(1)?;
/// let arr = Buffer::new((16, 16), (0..16*16).collect());
/// band1.write_block((0, 0), &arr.into())?;
/// band1.write_block((0, 0), &mut arr.into())?;
/// # Ok(())
/// # }
/// ```
pub fn write_block<T: Copy + GdalType>(
&mut self,
block_index: (usize, usize),
block: &Buffer<T>,
block: &mut Buffer<T>,
) -> Result<()> {
if T::gdal_ordinal() != self.band_type() as u32 {
return Err(GdalError::BadArgument(
Expand All @@ -621,11 +626,7 @@ impl<'a> RasterBand<'a> {
self.c_rasterband,
block_index.0 as c_int,
block_index.1 as c_int,
// This parameter is marked as `* mut c_void` because the C/C++ API for some reason
// doesn't mark it as `const void *`. From code inspection starting at the link
// below, it appears to be a read-only array.
// https://github.com/OSGeo/gdal/blob/b5d004fb9e3fb576b3ccf5f9740531b0bfa87ef4/gcore/gdalrasterband.cpp#L688
block.data.as_ptr() as *mut c_void,
block.data.as_mut_ptr() as *mut c_void,
)
};
if rv != CPLErr::CE_None {
Expand All @@ -641,11 +642,16 @@ impl<'a> RasterBand<'a> {
/// * `window_size` - the window size (GDAL will interpolate data if window_size != Buffer.size)
/// * `buffer` - the data to write into the window
///
/// # Notes
///
/// While drivers make sure that the content of the `block` buffer before and after the call
/// is equal, some drivers might temporarily modify it, e.g. to do byte swapping. Therefore
/// a `&mut` parameter is required.
pub fn write<T: GdalType + Copy>(
&mut self,
window: (isize, isize),
window_size: (usize, usize),
buffer: &Buffer<T>,
buffer: &mut Buffer<T>,
) -> Result<()> {
assert_eq!(buffer.data.len(), buffer.size.0 * buffer.size.1);
let rv = unsafe {
Expand All @@ -656,7 +662,7 @@ impl<'a> RasterBand<'a> {
window.1 as c_int,
window_size.0 as c_int,
window_size.1 as c_int,
buffer.data.as_ptr() as *mut c_void,
buffer.data.as_mut_ptr() as *mut c_void,
buffer.size.0 as c_int,
buffer.size.1 as c_int,
T::gdal_ordinal(),
Expand Down
16 changes: 9 additions & 7 deletions src/raster/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,15 @@ fn test_write_raster() {
let dataset = driver.create("", 20, 10, 1).unwrap();

// create a 2x1 raster
let raster = ByteBuffer {
let mut raster = ByteBuffer {
size: (2, 1),
data: vec![50u8, 20u8],
};

// epand it to fill the image (20x10)
// expand it to fill the image (20x10)
let mut rb = dataset.rasterband(1).unwrap();

let res = rb.write((0, 0), (20, 10), &raster);
let res = rb.write((0, 0), (20, 10), &mut raster);

assert!(res.is_ok());

Expand Down Expand Up @@ -427,12 +427,14 @@ fn test_write_block() {
let block_22 = Array2::from_shape_fn((16, 16), |(y, x)| y as u16 * 16 + x as u16 + 4000u16);

let mut band = dataset.rasterband(1).unwrap();
band.write_block((0, 0), &block_11.clone().into()).unwrap();
band.write_block((0, 1), &block_12.clone().into()).unwrap();
band.write_block((0, 0), &mut block_11.clone().into())
.unwrap();
band.write_block((0, 1), &mut block_12.clone().into())
.unwrap();
block_11.append(Axis(1), block_21.view()).unwrap();
band.write_block((1, 0), &block_21.into()).unwrap();
band.write_block((1, 0), &mut block_21.into()).unwrap();
block_12.append(Axis(1), block_22.view()).unwrap();
band.write_block((1, 1), &block_22.into()).unwrap();
band.write_block((1, 1), &mut block_22.into()).unwrap();
block_11.append(Axis(0), block_12.view()).unwrap();

let buf = band
Expand Down

0 comments on commit caf5aad

Please sign in to comment.