From bc71086a96169298ba7d82a373b20706738e65f6 Mon Sep 17 00:00:00 2001 From: Aurelia Molzer <5550310+197g@users.noreply.github.com> Date: Mon, 1 Sep 2025 00:10:29 +0200 Subject: [PATCH] Verify WhiteIsZero invert, add Gray(1) color type This was found through the fax4 decoding that is always bilevel. No longer silently ignore an unknown color type when a photometric interpretation requires an inversion. DICOM requires that WhiteIsZero and BlackIsZero are only used for samples-per-pixel=1 but we do not strictly enforce that early. Previously for the result Multiband color we just ignored the tag which is also not right. --- Cargo.toml | 2 +- src/decoder/image.rs | 9 ++++++--- src/decoder/mod.rs | 24 ++++++++++++++++++++---- tests/decode_images.rs | 6 ++++++ tests/images/imagemagick_group4.tiff | Bin 0 -> 1216 bytes 5 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 tests/images/imagemagick_group4.tiff diff --git a/Cargo.toml b/Cargo.toml index c61633b..f6c7bec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ half = { version = "2.4.1" } quick-error = "2.0.1" # Rename to avoid confusion with the feature name. -fax34 = { package = "fax", version = "0.2.4", optional = true } +fax34 = { package = "fax", version = "0.2.6", optional = true } flate2 = { version = "1.0.20", optional = true } weezl = { version = "0.1.10", optional = true } zstd = { version = "0.13", optional = true } diff --git a/src/decoder/image.rs b/src/decoder/image.rs index 3ce9b3e..a2673ce 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -344,6 +344,9 @@ impl Image { ), )), }, + // TODO: treatment of WhiteIsZero is not quite consistent with `invert_colors` that is + // later called when that interpretation is read. That function does not support + // Multiband as a color type and will error. It's unclear how to resolve that exactly. PhotometricInterpretation::BlackIsZero | PhotometricInterpretation::WhiteIsZero => { match self.samples { 1 => Ok(ColorType::Gray(self.bits_per_sample)), @@ -684,7 +687,7 @@ impl Image { ); } if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { - super::invert_colors(tile, color_type, self.sample_format); + super::invert_colors(tile, color_type, self.sample_format)?; } } else if chunk_row_bytes > data_row_bytes && self.predictor == Predictor::FloatingPoint { // The floating point predictor shuffles the padding bytes into the encoded output, so @@ -701,7 +704,7 @@ impl Image { _ => unreachable!(), } if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { - super::invert_colors(row, color_type, self.sample_format); + super::invert_colors(row, color_type, self.sample_format)?; } } } else { @@ -723,7 +726,7 @@ impl Image { predictor, ); if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { - super::invert_colors(row, color_type, self.sample_format); + super::invert_colors(row, color_type, self.sample_format)?; } } } diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 3e03a9d..dc4768a 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -475,11 +475,21 @@ fn fix_endianness_and_predict( } } -fn invert_colors(buf: &mut [u8], color_type: ColorType, sample_format: SampleFormat) { +fn invert_colors( + buf: &mut [u8], + color_type: ColorType, + sample_format: SampleFormat, +) -> TiffResult<()> { match (color_type, sample_format) { - (ColorType::Gray(8), SampleFormat::Uint) => { + (ColorType::Gray(n), SampleFormat::Uint) if 8 % n == 0 => { for x in buf { - *x = 0xff - *x; + // Equivalent to both of the following: + // + // *x = 0xff - *x + // *x = !*x + // + // since -x = !x+1 + *x = !*x; } } (ColorType::Gray(16), SampleFormat::Uint) => { @@ -512,8 +522,14 @@ fn invert_colors(buf: &mut [u8], color_type: ColorType, sample_format: SampleFor x.copy_from_slice(&(1.0 - v).to_ne_bytes()); } } - _ => {} + _ => { + return Err(TiffError::UnsupportedError( + TiffUnsupportedError::UnknownInterpretation, + )) + } } + + Ok(()) } /// Fix endianness. If `byte_order` matches the host, then conversion is a no-op. diff --git a/tests/decode_images.rs b/tests/decode_images.rs index 2ebe4fc..f6d7aa6 100644 --- a/tests/decode_images.rs +++ b/tests/decode_images.rs @@ -678,3 +678,9 @@ fn bytes_gray_f32() { fn test_fax4() { test_image_sum_u8("fax4.tiff", ColorType::Gray(1), 7802706); } + +#[test] +#[cfg(feature = "fax")] +fn test_fax4_white_is_min() { + test_image_sum_u8("imagemagick_group4.tiff", ColorType::Gray(1), 3742820); +} diff --git a/tests/images/imagemagick_group4.tiff b/tests/images/imagemagick_group4.tiff new file mode 100644 index 0000000000000000000000000000000000000000..a474528cb2acf6801edd97f3613e3a007357c788 GIT binary patch literal 1216 zcmYk6ZA?>F7{{+zYGnm^QJ_JwFH)J21z$)6CRo%dpt3DoBxTBQgjs5l7;Kkxw3}3D z9cy?QgMpbqxFoh*iVi4iDIFyQFhHPK5$bK#>;nbbF0t)hTFc$V@qzz`lizus|MQ%a zb52fX<`E#4007vg;+W^A*I5|$UO{;o_962B@a_boM@mm&+?0#X)E}r{93_26Ba$xu zI8&TU{40YxE8#Lm|B(E{lEj9$KU!qHIq8Cl%U2isH2IhN^@p9svw!Eri<;>(gE|sR z7f?Yut02i*2?skV$naTCYA-z4Eb+~~2{iDHKCfHJQz^|oaYi?`mW}j}Y-rd891&); z52l0lQaL#<^ea%b9*Wsju6;KmVX?`NjVNRxu}b%!dsDSSW}eqXE!>E(ThZ!+R)x_& zGJn|TP?`ODn3J{MSlwXjPpngh=yIrm2k6jzOm6*wNGE)gd zU&}&TzMU$lyIw1=oy+~5#edy);K_QK`AL}Yc0Tx#$J3x0`abR8ty2|k!Jf#`qI+iI zZ-KK9V`;yfqLDjB!{(~u?@SQr)MxwaUQ0fsZz>(b-t|}GhfJP*Hr@NB*yHzJm+eBa zWz6Q=7{bIT205LJ1Je_YB>AV)#jT9mpF5L_q5Ao#6P=8wDXGbI^eg9USB_KSsWT-d zF#(~aq{IwzXw^*d*^u%nV)>a2T75P5IO|~uDaESUV+b~osfInfspjX@%9X?_ZFYW6 zmwUIw)tDXB3f{k>77L2fVnhuBo4T^w3dzL17cPWv$mF>%&t-uG>ByubZn}r=W6MPI zQR8L*vWDwQW2R>?iyk?y(6BCimA zb-gACHVz^ScJB*~QRC|0m$U%qHI-SNHjmk>ge|^qN5i7mZ7Yg{4;fW#p9n?uHl9x{ z#0!4al;SrG<_%9xBM+f=C+qJ27n2XR{AyKp6# z1ar6`>@4s(dZ*+TRIRf(MwBZJ<6rE(BUQN+4WMGtDbQ`y+$N_X>M~U5hG6lu)OpnTAV=3Ug7!0ot@dPovI*ewopS#MFd!qC4ypv3?IVxa-$b+{ny$o|{(}Qo|e7Q<%R%_pQmem|3 zqFi8`68jAI2L=s2i7mMbQ=ygk`FeCh`-|v;lFs3ivB=o&A*obabtpNjYl1{MHcC2P zbF@5zkwKx25-*i!&?(0Z(YQhSmQGJ{buDoc9-XfW?8xhhFC}+O$%8f@>U-b}As7I5 z>;!fI0PsTqj$1s5&jYr2Gq-b#@oxkmZZVEP&^Axl4#I~%Bs?CzExvCXM{e