diff --git a/Cargo.lock b/Cargo.lock index 486159e5..d698e7c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,7 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +version = 3 + [[package]] name = "adler" version = "1.0.2" @@ -131,9 +133,9 @@ dependencies = [ [[package]] name = "deflate" -version = "0.9.1" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f95bf05dffba6e6cce8dfbb30def788154949ccd9aed761b472119c21e01c70" +checksum = "c86f7e25f518f4b81808a2cf1c50996a61f5c2eb394b2393bd87f2a4780a432f" dependencies = [ "adler32", ] @@ -146,11 +148,11 @@ checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457" [[package]] name = "exr" -version = "1.3.0" +version = "1.4.0" dependencies = [ "bencher", "bit_field", - "deflate 0.9.1", + "deflate 1.0.0", "flume", "half", "image", @@ -165,15 +167,15 @@ dependencies = [ [[package]] name = "flume" -version = "0.10.5" +version = "0.10.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa9d66b91e902db43baefd8e40c8678ce29db2cf1d88ebd715174368d5fe70a9" +checksum = "24c3fd473b3a903a62609e413ed7538f99e10b665ecb502b5e481a95283f8ab4" dependencies = [ "futures-core", "futures-sink", "nanorand", "pin-project", - "spinning_top", + "spin", ] [[package]] @@ -213,9 +215,9 @@ dependencies = [ [[package]] name = "half" -version = "1.7.1" +version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62aca2aba2d62b4a7f5b33f3712cb1b0692779a56fb510499d5c0aa594daeaf3" +checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7" [[package]] name = "hermit-abi" @@ -338,9 +340,9 @@ dependencies = [ [[package]] name = "nanorand" -version = "0.5.2" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac1378b66f7c93a1c0f8464a19bf47df8795083842e5090f4b7305973d5a22d0" +checksum = "729eb334247daa1803e0a094d0a5c55711b85571179f5ec6e53eccfdf7008958" dependencies = [ "getrandom", ] @@ -454,9 +456,9 @@ dependencies = [ [[package]] name = "rand" -version = "0.8.3" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ef9e7e66b4468674bfcb0c81af8b7fa0bb154fa9f28eb840da5c447baeb8d7e" +checksum = "2e7573632e6454cf6b99d7aac4ccca54be06da05aca2ef7423d22d27d4d4bcd8" dependencies = [ "libc", "rand_chacha", @@ -540,15 +542,15 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "smallvec" -version = "1.6.1" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe0f37c9e8f3c5a4a66ad655a93c74daac4ad00c441533bf5c6e7990bb42604e" +checksum = "1ecab6c735a6bb4139c0caafd0cc3635748bbb3acf4550e8138122099251f309" [[package]] -name = "spinning_top" -version = "0.2.4" +name = "spin" +version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75adad84ee84b521fb2cca2d4fd0f1dab1d8d026bda3c5bea4ca63b5f9f9293c" +checksum = "511254be0c5bcf062b019a6c89c01a664aa359ded62f78aa72c6fc137c0590e5" dependencies = [ "lock_api", ] diff --git a/Cargo.toml b/Cargo.toml index 8b01f706..d9be36de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ description = "Read and write OpenEXR files without any unsafe code" keywords = ["exr", "openexr", "file", "binary", "io"] categories = ["encoding", "filesystem", "graphics", "multimedia"] -version = "1.3.0" +version = "1.4.0" edition = "2018" authors = ["johannesvollmer "] @@ -26,21 +26,21 @@ plugin = false proc-macro = false [dependencies] -lebe = "0.5.1" # generic binary serialization -half = "1.7.1" # 16 bit float pixel data type -bit_field = "0.10.1" # exr file version bit flags -deflate = "0.9.1" # DEFLATE compression -inflate = "0.4.5" # DEFLATE decompression -smallvec = "1.6.1" # make cache-friendly allocations TODO profile if smallvec is really an improvement! -threadpool = "1.8.1" # threading for parallel compression TODO make this an optional feature? -flume = "0.10.5" # crossbeam, but less unsafe code TODO make this an optional feature? +lebe = "^0.5.1" # generic binary serialization +half = "^1.8.2" # 16 bit float pixel data type +bit_field = "^0.10.1" # exr file version bit flags +deflate = "^1.0.0" # DEFLATE compression +inflate = "^0.4.5" # DEFLATE decompression +smallvec = "^1.7.0" # make cache-friendly allocations TODO profile if smallvec is really an improvement! +threadpool = "^1.8.1" # threading for parallel compression TODO make this an optional feature? +flume = "^0.10.9" # crossbeam, but less unsafe code TODO make this an optional feature? [dev-dependencies] image = { version = "0.23.14", features = ["png"] } # used to convert one exr to some pngs bencher = "0.1.5" walkdir = "2.3.2" # automatically test things for all files in a directory -rand = "0.8.3" # used for fuzz testing +rand = "0.8.4" # used for fuzz testing rayon = "1.5.1" # run tests for many files in parallel diff --git a/GUIDE.md b/GUIDE.md index 0182f9cb..4f3cfaa2 100644 --- a/GUIDE.md +++ b/GUIDE.md @@ -522,7 +522,7 @@ The samples can currently only be `FlatSamples` or `Levels`, and in ### Samples Currently, only flat samples are supported. These do not contain deep data. Construct flat samples directly using `FlatSamples::F16(samples_vec)`, `FlatSamples::F32(samples_vec)`, or `FlatSamples::U32(samples_vec)`. -The vector contains all samples of the layer, row by row (bottom up), from left to right. +The vector contains all samples of the layer, row by row (from top to bottom), from left to right. ### Levels Optionally include Mip Maps or Rip Maps. diff --git a/README.md b/README.md index d2e0474c..8b54c5c9 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ __Big-endian code is not yet fully implemented. Help wanted.__ Add this to your `Cargo.toml`: ```toml [dependencies] -exr = "1.3.0" +exr = "1.4.0" # also, optionally add this to your crate for smaller binary size # and better runtime performance diff --git a/examples/4_specific_read.rs b/examples/4_specific_read.rs index 1a5a5856..9c2492f3 100644 --- a/examples/4_specific_read.rs +++ b/examples/4_specific_read.rs @@ -41,7 +41,7 @@ fn main() { let (alpha, luma, luma_right) = layer.channel_data.pixels.first().unwrap().first().unwrap(); println!( - "bottom left color of layer `{}`: (a, y, yr) = {:?}", + "top left color of layer `{}`: (a, y, yr) = {:?}", layer.attributes.layer_name.clone().unwrap_or_default(), (alpha.to_f32(), luma, luma_right) ) diff --git a/examples/README.md b/examples/README.md index 3f1b4d3b..02a29f5d 100644 --- a/examples/README.md +++ b/examples/README.md @@ -3,6 +3,7 @@ These are examples that demonstrate how to use `exrs`. The examples for any specific `exrs` version can be found on the `docs.rs` page: +- [docs.rs/crate/exr/1.4.0/source/examples/](https://docs.rs/crate/exr/1.4.0/source/examples/) - [docs.rs/crate/exr/1.3.0/source/examples/](https://docs.rs/crate/exr/1.3.0/source/examples/) - [docs.rs/crate/exr/1.2.0/source/examples/](https://docs.rs/crate/exr/1.2.0/source/examples/) - [docs.rs/crate/exr/1.1.0/source/examples/](https://docs.rs/crate/exr/1.1.0/source/examples/) diff --git a/releasing.md b/releasing.md index 199fcf44..582c81b7 100644 --- a/releasing.md +++ b/releasing.md @@ -18,6 +18,7 @@ Yanking shouldn't be the default. 1. ensure `#![warn(missing_docs)]` in `lib.rs` 1. Example in README.md should be up-to-date 1. GUIDE.md should be up-to-date +1. Update Dependencies while you're at it? ## Tasks 1. Bump version in diff --git a/src/block/mod.rs b/src/block/mod.rs index f3ad4d11..1d20aa89 100644 --- a/src/block/mod.rs +++ b/src/block/mod.rs @@ -36,10 +36,11 @@ pub struct BlockIndex { /// Index of the layer. pub layer: usize, - /// Index of the bottom left pixel from the block within the data window. + /// Index of the top left pixel from the block within the data window. pub pixel_position: Vec2, - /// Number of pixels in this block. Stays the same across all resolution levels. + /// Number of pixels in this block, extending to the right and downwards. + /// Stays the same across all resolution levels. pub pixel_size: Vec2, /// Index of the mip or rip level in the image. diff --git a/src/image/crop.rs b/src/image/crop.rs index 7ba3def2..63aadbf3 100644 --- a/src/image/crop.rs +++ b/src/image/crop.rs @@ -21,7 +21,8 @@ pub trait InspectSample: GetBounds { /// The type of pixel in this pixel grid. type Sample; - /// Index is not in world coordinates. Position `(0,0)` always represents the bottom left pixel. + /// Index is not in world coordinates, but within the data window. + /// Position `(0,0)` always represents the top left pixel. fn inspect_sample(&self, local_index: Vec2) -> Self::Sample; } @@ -292,50 +293,53 @@ impl ApplyCroppedView for Layer>> { /// Return the smallest bounding rectangle including all pixels that satisfy the predicate. /// Worst case: Fully transparent image, visits each pixel once. -/// Best case: Fully opaque image, visits four pixels. +/// Best case: Fully opaque image, visits two pixels. /// Returns `None` if the image is fully transparent. /// Returns `[(0,0), size]` if the image is fully opaque. -fn try_find_smaller_bounds(current_bounds: IntegerBounds, pixel_at: impl Fn(Vec2) -> bool) -> Option { +/// Designed to be cache-friendly linear search. Optimized for row-major image vectors. +pub fn try_find_smaller_bounds(current_bounds: IntegerBounds, pixel_at: impl Fn(Vec2) -> bool) -> Option { assert_ne!(current_bounds.size.area(), 0, "cannot find smaller bounds of an image with zero width or height"); let Vec2(width, height) = current_bounds.size; // scans top to bottom (left to right) - let first_top_left_pixel = (0 .. height).rev() + let first_top_left_pixel = (0 .. height) .flat_map(|y| (0 .. width).map(move |x| Vec2(x,y))) - .find(|&position| pixel_at(position)); - - let first_top_left_pixel = { - if let Some(xy) = first_top_left_pixel { xy } - else { return None } - }; + .find(|&position| pixel_at(position))?; // return none if no pixel should be kept // scans bottom to top (right to left) - let first_bottom_right_pixel = (0 .. first_top_left_pixel.y()) // excluding the top line - .flat_map(|y| (first_top_left_pixel.x() + 1 .. width).rev().map(move |x| Vec2(x, y))) // excluding some left pixel - .find(|&position| pixel_at(position)) - .unwrap_or(first_top_left_pixel); // did not inspect but we know top has a pixel + let first_bottom_right_pixel = (first_top_left_pixel.y() + 1 .. height) // excluding the top line + .flat_map(|y| (0 .. width).map(move |x| Vec2(x, y))) // x search cannot start at first_top.x, because this must catch all bottom pixels + .rev().find(|&position| pixel_at(position)) + .unwrap_or(first_top_left_pixel); // did not find any at bottom, but we know top has some pixel - // now we know exactly how much we can throw away top and bottom + // now we know exactly how much we can throw away top and bottom, + // but we don't know exactly about left or right let top = first_top_left_pixel.y(); let bottom = first_bottom_right_pixel.y(); - // but we only now some random left and right bounds which we need to refine - let mut min_left_x = first_top_left_pixel.x(); - let mut max_right_x = first_bottom_right_pixel.x(); + // we only now some arbitrary left and right bounds which we need to refine. + // because the actual image contents might be wider than the corner points. + // we know that we do not need to look in the center between min x and max x, + // as these must be included in any case. + let mut min_left_x = first_top_left_pixel.x().min(first_bottom_right_pixel.x()); + let mut max_right_x = first_bottom_right_pixel.x().max(first_top_left_pixel.x()); // requires for loop, because bounds change while searching - for y in (bottom ..= top).rev() { + for y in top ..= bottom { + // escape the loop if there is nothing left to crop if min_left_x == 0 && max_right_x == width - 1 { break; } - // search from right bound to image center for existing pixels + // search from right image edge towards image center, until known max x, for existing pixels, + // possibly including some pixels that would have been cropped otherwise if max_right_x != width - 1 { max_right_x = (max_right_x + 1 .. width).rev() // excluding current max .find(|&x| pixel_at(Vec2(x, y))) .unwrap_or(max_right_x); } - // search from left bound to image center for existing pixels + // search from left image edge towards image center, until known min x, for existing pixels, + // possibly including some pixels that would have been cropped otherwise if min_left_x != 0 { min_left_x = (0 .. min_left_x) // excluding current min .find(|&x| pixel_at(Vec2(x, y))) @@ -344,8 +348,8 @@ fn try_find_smaller_bounds(current_bounds: IntegerBounds, pixel_at: impl Fn(Vec2 } // TODO add 1px margin to avoid interpolation issues? - let local_start = Vec2(min_left_x, bottom); - let local_end = Vec2(max_right_x + 1, top + 1); + let local_start = Vec2(min_left_x, top); + let local_end = Vec2(max_right_x + 1, bottom + 1); Some(IntegerBounds::new( current_bounds.position + local_start.to_i32(), local_end - local_start @@ -400,20 +404,23 @@ mod test { } } - fn assert_found_smaller_bounds(offset: Vec2, uncropped_lines: Vec>, cropped_lines: Vec>) { + fn assert_found_smaller_bounds(offset: Vec2, uncropped_lines: Vec>, expected_cropped_lines: Vec>) { let old_bounds = find_bounds(offset, &uncropped_lines); let found_bounds = try_find_smaller_bounds( - old_bounds, |position| uncropped_lines[position.y()][position.x()] != 0 + old_bounds, + |position| uncropped_lines[position.y()][position.x()] != 0 ).unwrap(); - // convert bounds to relative index - let found_bounds = found_bounds.with_origin(-offset); - for (y, uncropped_line) in uncropped_lines[found_bounds.position.y() as usize .. found_bounds.end().y() as usize].iter().enumerate() { - for (x, &value) in uncropped_line[found_bounds.position.x() as usize .. found_bounds.end().x() as usize].iter().enumerate() { - assert_eq!(value, cropped_lines[y][x], "find crop bounds test case failed") - } - } + let found_bounds = found_bounds.with_origin(-offset); // make indices local + + let cropped_lines: Vec> = + uncropped_lines[found_bounds.position.y() as usize .. found_bounds.end().y() as usize] + .iter().map(|uncropped_line|{ + uncropped_line[found_bounds.position.x() as usize .. found_bounds.end().x() as usize].to_vec() + }).collect(); + + assert_eq!(cropped_lines, expected_cropped_lines); } assert_found_smaller_bounds( @@ -485,6 +492,56 @@ mod test { ] ); + assert_found_smaller_bounds( + Vec2(3,3), + + vec![ + vec![ 0, 0, 0, 0 ], + vec![ 1, 1, 2, 1 ], + vec![ 1, 3, 1, 1 ], + vec![ 1, 1, 1, 1 ], + vec![ 0, 0, 0, 0 ], + ], + + vec![ + vec![ 1, 1, 2, 1 ], + vec![ 1, 3, 1, 1 ], + vec![ 1, 1, 1, 1 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(3,3), + + vec![ + vec![ 0, 1, 1, 2, 1, 0 ], + vec![ 0, 0, 3, 1, 0, 0 ], + vec![ 0, 1, 1, 1, 1, 0 ], + ], + + vec![ + vec![ 1, 1, 2, 1 ], + vec![ 0, 3, 1, 0 ], + vec![ 1, 1, 1, 1 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(3,3), + + vec![ + vec![ 0, 0, 1, 2, 0, 0 ], + vec![ 0, 1, 3, 1, 1, 0 ], + vec![ 0, 0, 1, 1, 0, 0 ], + ], + + vec![ + vec![ 0, 1, 2, 0 ], + vec![ 1, 3, 1, 1 ], + vec![ 0, 1, 1, 0 ], + ] + ); + assert_found_smaller_bounds( Vec2(1,3), @@ -547,6 +604,146 @@ mod test { ] ); + assert_found_smaller_bounds( + Vec2(1000,-300), + + vec![ + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 1, 1, 0, 0 ], + vec![ 0, 1, 1, 1, 1, 1, 0 ], + vec![ 0, 0, 1, 1, 1, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + ], + + vec![ + vec![ 0, 1, 1, 1, 0 ], + vec![ 1, 1, 1, 1, 1 ], + vec![ 0, 1, 1, 1, 0 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(-10,-300), + + vec![ + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 0, 1, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 0, 1, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + ], + + vec![ + vec![ 1, 0, 1 ], + vec![ 0, 0, 0 ], + vec![ 1, 0, 1 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(-10,-300), + + vec![ + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 0, 1, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + ], + + vec![ + vec![ 1, 0, 1 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(-10,-300), + + vec![ + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 1, 0, 0, 0 ], + vec![ 0, 0, 0, 2, 0, 0, 0 ], + vec![ 0, 0, 3, 3, 3, 0, 0 ], + vec![ 0, 0, 0, 4, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + ], + + vec![ + vec![ 0, 1, 0 ], + vec![ 0, 2, 0 ], + vec![ 3, 3, 3 ], + vec![ 0, 4, 0 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(-10,-300), + + vec![ + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 1, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + ], + + vec![ + vec![ 0, 0, 1 ], + vec![ 0, 0, 0 ], + vec![ 0, 0, 0 ], + vec![ 1, 0, 0 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(-10,-300), + + vec![ + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 1, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + ], + + vec![ + vec![ 1, 0, 0, 0 ], + vec![ 0, 0, 0, 0 ], + vec![ 0, 0, 0, 1 ], + ] + ); + + assert_found_smaller_bounds( + Vec2(-10,-300), + + vec![ + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + vec![ 0, 0, 1, 0, 0, 0, 0 ], + vec![ 0, 0, 0, 0, 0, 0, 0 ], + ], + + vec![ + vec![ 1 ], + vec![ 0 ], + vec![ 0 ], + vec![ 1 ], + ] + ); + assert_found_smaller_bounds( Vec2(-1,-3), diff --git a/src/meta/attribute.rs b/src/meta/attribute.rs index f4620e8d..21ff4b4e 100644 --- a/src/meta/attribute.rs +++ b/src/meta/attribute.rs @@ -194,14 +194,17 @@ pub type Matrix4x4 = [f32; 4*4]; pub type Matrix3x3 = [f32; 3*3]; /// A rectangular section anywhere in 2D integer space. +/// Valid from minimum coordinate (including) `-1,073,741,822` +/// to maximum coordinate (including) `1,073,741,822`, the value of (`i32::MAX/2 -1`). #[derive(Clone, Copy, Debug, Eq, PartialEq, Default, Hash)] pub struct IntegerBounds { - /// The bottom left corner of this rectangle. + /// The top left corner of this rectangle. /// The `Box2I32` includes this pixel if the size is not zero. pub position: Vec2, /// How many pixels to include in this `Box2I32`. + /// Extends to the right and downwards. /// Does not include the actual boundary, just like `Vec::len()`. pub size: Vec2, } @@ -210,10 +213,10 @@ pub struct IntegerBounds { #[derive(Clone, Copy, Debug, PartialEq)] pub struct FloatRect { - /// The bottom left corner location of the rectangle (inclusive) + /// The top left corner location of the rectangle (inclusive) pub min: Vec2, - /// The top right corner location of the rectangle (inclusive) + /// The bottom right corner location of the rectangle (inclusive) pub max: Vec2 } @@ -595,6 +598,11 @@ impl Text { Ok(()) } + /// The underlying bytes that represent this text. + pub fn bytes(&self) -> &[u8] { + self.bytes.as_slice() + } + /// Iterate over the individual chars in this text, similar to `String::chars()`. /// Does not do any heap-allocation but borrows from this instance instead. pub fn chars(&self) -> impl '_ + Iterator { @@ -721,6 +729,12 @@ impl ChannelList { Some((previous_position, channel)) }) } + + /// Return the index of the channel with the exact name, case sensitive, or none. + /// Potentially uses less than linear time. + pub fn find_index_of_channel(&self, exact_name: &Text) -> Option { + self.list.binary_search_by_key(&exact_name.bytes(), |chan| chan.name.bytes()).ok() + } } impl BlockType { @@ -794,23 +808,30 @@ impl IntegerBounds { } /// Validate this instance. - pub fn validate(&self, max: Option>) -> UnitResult { - if let Some(max) = max { - if self.size.width() > max.width() || self.size.height() > max.height() { + pub fn validate(&self, max_size: Option>) -> UnitResult { + if let Some(max_size) = max_size { + if self.size.width() > max_size.width() || self.size.height() > max_size.height() { return Err(Error::invalid("window attribute dimension value")); } } - let max_int = i32::MAX as i64 / 2; // cannot go bigger than that ever + let min_i64 = Vec2(self.position.x() as i64, self.position.y() as i64); - let self_max = Vec2( + let max_i64 = Vec2( self.position.x() as i64 + self.size.width() as i64, self.position.y() as i64 + self.size.height() as i64, ); - if self_max.x() >= max_int || self_max.y() >= max_int - || self.position.x() as i64 <= -max_int - || self.position.y() as i64 <= -max_int + Self::validate_min_max_u64(min_i64, max_i64) + } + + fn validate_min_max_u64(min: Vec2, max: Vec2) -> UnitResult { + let max_box_size_as_i64 = (i32::MAX / 2) as i64; // as defined in the original c++ library + + if max.x() >= max_box_size_as_i64 + || max.y() >= max_box_size_as_i64 + || min.x() <= -max_box_size_as_i64 + || min.y() <= -max_box_size_as_i64 { return Err(Error::invalid("window size exceeding integer maximum")); } @@ -843,8 +864,16 @@ impl IntegerBounds { let y_max = i32::read(read)?; let min = Vec2(x_min.min(x_max), y_min.min(y_max)); - let max = Vec2(x_min.max(x_max), y_min.max(y_max)); // these are inclusive! - let size = Vec2(max.x() + 1 - min.x(), max.y() + 1 - min.y()); // which is why we add 1 + let max = Vec2(x_min.max(x_max), y_min.max(y_max)); + + // prevent addition overflow + Self::validate_min_max_u64( + Vec2(min.x() as i64, min.y() as i64), + Vec2(max.x() as i64, max.y() as i64), + )?; + + // add one to max because the max inclusive, but the size is not + let size = Vec2(max.x() + 1 - min.x(), max.y() + 1 - min.y()); let size = size.to_usize("box coordinates")?; Ok(IntegerBounds { position: min, size }) @@ -2011,6 +2040,27 @@ mod test { max: Vec2(68623.0, 3.12425926538), }), ), + ( + Text::from("rabbit area int"), + AttributeValue::IntegerBounds(IntegerBounds { + position: Vec2(23, 345), + size: Vec2(68623, 3), + }), + ), + ( + Text::from("rabbit area int"), + AttributeValue::IntegerBounds(IntegerBounds { + position: Vec2(-(i32::MAX / 2 - 1), -(i32::MAX / 2 - 1)), + size: Vec2(i32::MAX as usize - 2, i32::MAX as usize - 2), + }), + ), + ( + Text::from("rabbit area int 2"), + AttributeValue::IntegerBounds(IntegerBounds { + position: Vec2(0, 0), + size: Vec2(i32::MAX as usize / 2 - 1, i32::MAX as usize / 2 - 1), + }), + ), ( Text::from("tests are difficult"), AttributeValue::TextVector(vec![ diff --git a/src/meta/header.rs b/src/meta/header.rs index aaa3c5ca..e10489b9 100644 --- a/src/meta/header.rs +++ b/src/meta/header.rs @@ -114,9 +114,9 @@ pub struct LayerAttributes { // As this is an attribute value, it is not restricted in length, may even be empty pub layer_name: Option, - /// The bottom left corner of the rectangle that positions this layer + /// The top left corner of the rectangle that positions this layer /// within the global infinite 2D space of the whole file. - /// Equivalent to the position of the `DataWindow`. + /// This represents the position of the `DataWindow`. pub layer_position: Vec2, /// Part of the perspective projection. Default should be `(0, 0)`. diff --git a/tests/images/fuzzed/invalid_integer_bounds.exr b/tests/images/fuzzed/invalid_integer_bounds.exr new file mode 100644 index 00000000..5f370f1c Binary files /dev/null and b/tests/images/fuzzed/invalid_integer_bounds.exr differ