Feature/negative offset fix

src/frame_buffer.rs

@@ -41,21 +41,28 @@ use cexr_type_aliases::*;
 /// Points to and describes in-memory image data for reading.
 pub struct FrameBuffer<'a> {
     handle: *mut CEXR_FrameBuffer,
+    origin: (i32, i32),
     dimensions: (u32, u32),
     _phantom_1: PhantomData<CEXR_FrameBuffer>,
     _phantom_2: PhantomData<&'a mut [u8]>,
 }
 
 impl<'a> FrameBuffer<'a> {
-    /// Creates an empty frame buffer with the given dimensions in pixels.
+    /// Creates an empty frame buffer with the given dimensions and zero offset in pixels.
     pub fn new(width: u32, height: u32) -> Self {
+    	Self::new_with_origin(0, 0, width, height)
+    }
+
+    /// Creates an empty frame buffer with the given dimensions in pixels.
+    pub fn new_with_origin(x: i32, y: i32, width: u32, height: u32) -> Self {
         assert!(
             width > 0 && height > 0,
             "FrameBuffers must be non-zero size in \
              both dimensions."
         );
         FrameBuffer {
             handle: unsafe { CEXR_FrameBuffer_new() },
+            origin: (x, y),
             dimensions: (width, height),
             _phantom_1: PhantomData,
             _phantom_2: PhantomData,
@@ -67,6 +74,23 @@ impl<'a> FrameBuffer<'a> {
         self.dimensions
     }
 
+    /// Return the origin of the frame buffer.
+    pub fn origin(&self) -> (i32, i32) {
+    	self.origin
+    }
+
+    /// Return the offset index of the data pixel at 0,0 with reference to the data pixel at window.min.x, window.min.y
+    pub fn origin_offset(&self) -> isize {
+        let width = self.dimensions.0;
+        let (x, y) = self.origin;
+        -(x as isize + y as isize * width as isize)
+    }
+
+    /// Return the offset byte of the data pixel at 0,0 with reference to the data pixel at window.min.x, window.min.y
+    fn origin_offset_byte<T: Sized>(&self) -> isize {
+        self.origin_offset() * mem::size_of::<T>() as isize
+    }
+
     /// Insert a single channel into the FrameBuffer.
     ///
     /// The channel will be given the name `name`.
@@ -84,11 +108,12 @@ impl<'a> FrameBuffer<'a> {
             );
         }
         let width = self.dimensions.0;
+        let origin_offset = self.origin_offset_byte::<T>();
         unsafe {
             self.insert_raw(
                 name,
                 T::pixel_type(),
-                data.as_ptr() as *const c_char,
+                (data.as_ptr() as *const c_char).offset(origin_offset),
                 (mem::size_of::<T>(), width as usize * mem::size_of::<T>()),
                 (1, 1),
                 0.0,
@@ -117,12 +142,13 @@ impl<'a> FrameBuffer<'a> {
             );
         }
         let width = self.dimensions.0;
+        let origin_offset = self.origin_offset_byte::<T>();
         for (name, (ty, offset)) in names.iter().zip(T::channels()) {
             unsafe {
                 self.insert_raw(
                     name,
                     ty,
-                    (data.as_ptr() as *const c_char).offset(offset as isize),
+                    (data.as_ptr() as *const c_char).offset(origin_offset + offset as isize),
                     (mem::size_of::<T>(), width as usize * mem::size_of::<T>()),
                     (1, 1),
                     0.0,
@@ -201,6 +227,13 @@ pub struct FrameBufferMut<'a> {
 }
 
 impl<'a> FrameBufferMut<'a> {
+    /// Creates an empty frame buffer with the given dimensions and zero offset in pixels.
+    pub fn new_with_origin(x: i32, y: i32, width: u32, height: u32) -> Self {
+        FrameBufferMut {
+            frame_buffer: FrameBuffer::new_with_origin(x, y, width, height),
+        }
+    }
+
     /// Creates an empty frame buffer with the given dimensions in pixels.
     pub fn new(width: u32, height: u32) -> Self {
         FrameBufferMut {
@@ -232,11 +265,12 @@ impl<'a> FrameBufferMut<'a> {
             );
         }
         let width = self.dimensions.0;
+        let origin_offset = self.origin_offset_byte::<T>();
         unsafe {
             self.insert_raw(
                 name,
                 T::pixel_type(),
-                data.as_mut_ptr() as *mut c_char,
+                (data.as_mut_ptr() as *mut c_char).offset(origin_offset),
                 (mem::size_of::<T>(), width as usize * mem::size_of::<T>()),
                 (1, 1),
                 fill,
@@ -271,12 +305,13 @@ impl<'a> FrameBufferMut<'a> {
             );
         }
         let width = self.dimensions.0;
+        let origin_offset = self.origin_offset_byte::<T>();
         for (&(name, fill), (ty, offset)) in names_and_fills.iter().zip(T::channels()) {
             unsafe {
                 self.insert_raw(
                     name,
                     ty,
-                    (data.as_mut_ptr() as *mut c_char).offset(offset as isize),
+                    (data.as_mut_ptr() as *mut c_char).offset(origin_offset + offset as isize),
                     (mem::size_of::<T>(), width as usize * mem::size_of::<T>()),
                     (1, 1),
                     fill,

src/header.rs

@@ -189,6 +189,12 @@ impl Header {
         self
     }
 
+    /// Convenience method for the dimensions of the data window.
+    pub fn data_origin(&self) -> (i32, i32) {
+        let window = self.data_window();
+        (window.min.x, window.min.y)
+    }
+
     /// Convenience method for the dimensions of the data window.
     pub fn data_dimensions(&self) -> (u32, u32) {
         let window = self.data_window();
@@ -280,6 +286,17 @@ impl Header {
         Ok(())
     }
 
+    /// Utility function to create a Box2i specifying its origin (bottom left) and size
+    pub fn box2i(x: i32, y: i32, width: u32, height: u32) -> Box2i {
+        Box2i {
+            min: CEXR_V2i { x, y },
+            max: CEXR_V2i {
+                x: x + width as i32 - 1,
+                y: y + height as i32 - 1,
+            },
+        }
+    }
+
     // Factored out shared code from the validate_framebuffer_* methods above.
     fn validate_channel(name: &str, h_chan: &Channel, fb_chan: &Channel) -> Result<()> {
         if fb_chan.pixel_type != h_chan.pixel_type {

src/input/mod.rs

@@ -183,6 +183,17 @@ impl<'a> InputFile<'a> {
             )));
         }
 
+        // Validation
+        if self.header().data_origin() != framebuffer.origin() {
+            return Err(Error::Generic(format!(
+                "framebuffer origin {},{} does not match image origin {},{}",
+                framebuffer.origin().0,
+                framebuffer.origin().1,
+                self.header().data_origin().0,
+                self.header().data_origin().1
+            )));
+        }
+
         self.header().validate_framebuffer_for_input(framebuffer)?;
 
         // Set up the framebuffer with the image

src/output/scanline_output_file.rs

@@ -141,15 +141,24 @@ impl<'a> ScanlineOutputFile<'a> {
 
         if self.header().data_dimensions() != framebuffer.dimensions() {
             return Err(Error::Generic(format!(
-                "framebuffer size {}x{} does not match \
-                 image dimensions {}x{}",
+                "framebuffer size {}x{} does not match image dimensions {}x{}",
                 framebuffer.dimensions().0,
                 framebuffer.dimensions().1,
                 self.header().data_dimensions().0,
                 self.header().data_dimensions().1
             )));
         }
 
+        if self.header().data_origin() != framebuffer.origin() {
+            return Err(Error::Generic(format!(
+                "framebuffer origin {}x{} does not match image origin {}x{}",
+                framebuffer.origin().0,
+                framebuffer.origin().1,
+                self.header().data_origin().0,
+                self.header().data_origin().1
+            )));
+        }
+
         self.header().validate_framebuffer_for_output(framebuffer)?;
 
         // Set up the framebuffer with the image

tests/non_zero_origin_window.rs

@@ -0,0 +1,111 @@
+extern crate half;
+extern crate openexr;
+extern crate openexr_sys;
+
+use half::f16;
+use openexr::{FrameBuffer, FrameBufferMut, Header, InputFile, ScanlineOutputFile};
+use std::fs::File;
+
+// OpenEXR file data.
+const NEGATIVE_OFFSET: &[u8] = include_bytes!("data/negative_window.exr");
+const POSITIVE_OFFSET: &[u8] = include_bytes!("data/positive_window.exr");
+
+fn load_and_test_with_offset_window_read_multiple_channels(data: &[u8]) {
+    let mut exr_file = InputFile::from_slice(data).unwrap();
+
+    let (width, height) = exr_file.header().data_dimensions();
+    let (x, y) = exr_file.header().data_origin();
+
+    let zero = f16::from_f32(0.0f32);
+
+    let mut pixel_data = vec![(zero, zero, zero); (width * height) as usize];
+
+    let read_with_offset = |exr_file: &mut InputFile, pixel_data: &mut [_], ox, oy| {
+        let mut fb = FrameBufferMut::new_with_origin(ox, oy, width, height);
+        println!("Loading from buffer as {},{} {}x{}", ox, oy, width, height);
+        fb.insert_channels(&[("R", 0.0), ("G", 0.0), ("B", 0.0)], pixel_data);
+        println!("Reading pixels from {},{},{}", "R", "G", "B");
+        (exr_file.read_pixels(&mut fb), fb.origin_offset())
+    };
+
+    // let's try a few mismatched origins
+    assert!(read_with_offset(&mut exr_file, &mut pixel_data, 0, 0)
+        .0
+        .is_err());
+    assert!(
+        read_with_offset(&mut exr_file, &mut pixel_data, x - 1, y - 1)
+            .0
+            .is_err()
+    );
+    assert!(read_with_offset(&mut exr_file, &mut pixel_data, -x, -y)
+        .0
+        .is_err());
+    // and then the real thing
+    let (read_result, origin_offset) = read_with_offset(&mut exr_file, &mut pixel_data, x, y);
+    assert!(read_result.is_ok());
+    // check the pixel value at coordinates (0,0) of the data window if 0,0 is within the frame buffer
+    if origin_offset >= 0 {
+        assert!(f32::abs(pixel_data[origin_offset as usize].0.to_f32() - 0.5f32) < 0.0001f32);
+    }
+
+    // we write the file back out with a different offset
+    {
+        let mut fb = FrameBuffer::new_with_origin(-x, -y, width, height);
+        println!("Loading buffer as {}x{}", width, height);
+        let mut file =
+            File::create("target/positive_window.exr").expect("Could not create output file");
+        let mut exr_file = ScanlineOutputFile::new(
+            &mut file,
+            &Header::new()
+                .set_data_window(Header::box2i(-x, -y, width, height))
+                .set_display_window(Header::box2i(0, 0, width - 16, height - 16))
+                .add_channel("R", openexr::PixelType::HALF)
+                .add_channel("G", openexr::PixelType::HALF)
+                .add_channel("B", openexr::PixelType::HALF),
+        )
+        .unwrap();
+
+        fb.insert_channels(&["R", "G", "B"], &pixel_data);
+        exr_file.write_pixels(&fb).unwrap();
+    }
+}
+
+fn load_and_test_with_offset_window_read_single_channel(data: &[u8]) {
+    let mut exr_file = InputFile::from_slice(data).unwrap();
+    let (width, height) = exr_file.header().data_dimensions();
+    let (x, y) = exr_file.header().data_origin();
+
+    let zero = f16::from_f32(0.0f32);
+
+    let mut pixel_data = vec![zero; (width * height) as usize];
+
+    let mut read_with_offset = |ox: i32, oy: i32| {
+        let mut fb = FrameBufferMut::new_with_origin(ox, oy, width, height);
+        println!(
+            "Loading from buffer as ({} {}) {}x{}",
+            ox, oy, width, height
+        );
+        fb.insert_channel("R", 0.0, &mut pixel_data);
+        println!("Reading pixels from {},{},{}", "R", "G", "B");
+        exr_file.read_pixels(&mut fb)
+    };
+    // let's try a few mismatched origins
+    assert!(read_with_offset(0, 0).is_err());
+    assert!(read_with_offset(x - 1, y - 1).is_err());
+    assert!(read_with_offset(-x, -y).is_err());
+    // and then the correct one
+    assert!(read_with_offset(x, y).is_ok());
+}
+
+#[test]
+fn window_read_multiple_channels() {
+    load_and_test_with_offset_window_read_multiple_channels(NEGATIVE_OFFSET);
+    load_and_test_with_offset_window_read_multiple_channels(POSITIVE_OFFSET);
+}
+
+// with one channel only as well
+#[test]
+fn negative_window_read_single_channel() {
+    load_and_test_with_offset_window_read_single_channel(NEGATIVE_OFFSET);
+    load_and_test_with_offset_window_read_single_channel(POSITIVE_OFFSET);
+}