Document Decoder::open behavior

CHANGES.md

@@ -11,12 +11,6 @@ Changes:
   directory state if they fail. This includes the failure to interpret an IFD
   as a supported image in the case of `*_image` variants.
 
-Compatibility:
-- Changed how tiled TIFF files are detected. The presence of a TileWidth OR
-  TileHeight/TileLength tag now marks an image as tiled. Data is no retrieved
-  from either StripOffsets or TileOffsets with the latter taking precedence.
-  This is closer to the behavior of libtiff.
-
 # Version 0.11.3
 
 Additions:

src/decoder/image.rs

@@ -330,22 +330,18 @@ impl Image {
             PlanarConfiguration::Planar => samples,
         };
 
-        // Note: Not strictly according to specification. The specification says that tiled images
-        // must have their data encoded in `TileByteCounts` and `TileOffsets`. But `libtiff`
-        // processes tags one-by-one and with *either* of these it will set the flag for the image
-        // being tiled. Also it will just use whatever offsets are available. (I believe also using
-        // the first of the two tag pairs that's available—remember tags are encoded ascending).
-        let is_tiled = ifd.contains(Tag::TileWidth) || ifd.contains(Tag::TileLength);
-        let has_strips = ifd.contains(Tag::StripByteCounts) && ifd.contains(Tag::StripOffsets);
-        let has_tiles = ifd.contains(Tag::TileByteCounts) && ifd.contains(Tag::TileOffsets);
-
         let chunk_type;
         let chunk_offsets;
         let chunk_bytes;
         let strip_decoder;
         let tile_attributes;
-
+        match (
-        if !is_tiled && has_strips {
+            ifd.contains(Tag::StripByteCounts),
+            ifd.contains(Tag::StripOffsets),
+            ifd.contains(Tag::TileByteCounts),
+            ifd.contains(Tag::TileOffsets),
+        ) {
+            (true, true, false, false) => {
                 chunk_type = ChunkType::Strip;
 
                 chunk_offsets = tag_reader
@@ -386,20 +382,15 @@ impl Image {
                         },
                     ));
                 }
-        } else if is_tiled && (has_strips || has_tiles) {
+            }
+            (false, false, true, true) => {
                 chunk_type = ChunkType::Tile;
 
-            let tile_width = usize::try_from(tag_reader.require_tag(Tag::TileWidth)?.into_u32()?)?;
+                let tile_width =
+                    usize::try_from(tag_reader.require_tag(Tag::TileWidth)?.into_u32()?)?;
                 let tile_length =
                     usize::try_from(tag_reader.require_tag(Tag::TileLength)?.into_u32()?)?;
 
-            // Prioritize tiles over strips, tiles are what we expect.
-            let (tag_offsets, tag_counts) = if has_tiles {
-                (Tag::TileOffsets, Tag::TileByteCounts)
-            } else {
-                (Tag::StripOffsets, Tag::StripByteCounts)
-            };
-
                 if tile_width == 0 {
                     return Err(TiffFormatError::InvalidTagValueType(Tag::TileWidth).into());
                 } else if tile_length == 0 {
@@ -413,9 +404,14 @@ impl Image {
                     tile_width,
                     tile_length,
                 });
-
+                chunk_offsets = tag_reader
-            chunk_offsets = tag_reader.find_tag(tag_offsets)?.unwrap().into_u64_vec()?;
+                    .find_tag(Tag::TileOffsets)?
-            chunk_bytes = tag_reader.find_tag(tag_counts)?.unwrap().into_u64_vec()?;
+                    .unwrap()
+                    .into_u64_vec()?;
+                chunk_bytes = tag_reader
+                    .find_tag(Tag::TileByteCounts)?
+                    .unwrap()
+                    .into_u64_vec()?;
 
                 let tile = tile_attributes.as_ref().unwrap();
 
@@ -425,7 +421,8 @@ impl Image {
                     ));
                 }
 
-            if chunk_offsets.len() != tile.tiles_down() * tile.tiles_across() * planes as usize {
+                if chunk_offsets.len() != tile.tiles_down() * tile.tiles_across() * planes as usize
+                {
                     return Err(TiffError::FormatError(
                         TiffFormatError::IncorrectChunkCount {
                             expected: (tile.tiles_down() * tile.tiles_across() * planes as usize)
@@ -434,10 +431,12 @@ impl Image {
                         },
                     ));
                 }
-        } else {
+            }
+            (_, _, _, _) => {
                 return Err(TiffError::FormatError(
                     TiffFormatError::StripTileTagConflict,
-            ));
+                ))
+            }
         };
 
         let mut sample_format = sample_format;

src/decoder/mod.rs

@@ -857,11 +857,64 @@ impl TiffHeader {
 }
 
 impl<R: Read + Seek> Decoder<R> {
+    /// Parse the image header from the underlying file and create a decoder positioned before its
+    /// first directory.
+    ///
+    /// This method opens the file in a state _before_ the first (image) directory. You need to
+    /// call one of [`Self::next_image`], [`Self::next_directory`] or [`Self::seek_to_directory`]
+    /// to choose a directory before interacting with the tag methods.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use tiff::decoder::{Decoder, DecodingResult};
+    ///
+    /// let buffered_file = /* */
+    /// # std::io::Cursor::new(include_bytes!(concat!(
+    /// #   env!("CARGO_MANIFEST_DIR"), "/tests/images/tiled-gray-i1.tif"
+    /// # )));
+    ///
+    /// let mut decoder =  Decoder::open(buffered_file)?;
+    /// let mut img_buffer = DecodingResult::U8(vec![]);
+    ///
+    /// while decoder.more_images() {
+    ///     decoder.next_image()?;
+    ///     decoder.read_image_to_buffer(&mut img_buffer)?;
+    /// }
+    ///
+    /// # Ok::<_, tiff::TiffError>(())
+    /// ```
+    ///
+    /// # Relations
+    ///
+    /// The [`TiffHeader`] type allows opening a file with a custom offset.
+    pub fn open(mut r: R) -> TiffResult<Decoder<R>> {
+        let header = TiffHeader::parse(&mut r)?;
+        Ok(header.open(r))
+    }
+
     /// Create a decoder and attempt to parse the first image directory.
     ///
     /// To open a file without parsing any directory yet (for instance to retain a [`Decoder`] on
-    /// errors in the directory), use [`TiffHeader::parse`] and [`TiffHeader::open`] instead..
+    /// errors in the directory), use [`TiffHeader::parse`] and [`TiffHeader::open`] instead.
-    #[deprecated = "Prefer `Decoder::open`. These methods will become aliases in a future major release."]
+    ///
+    /// This method is equivalent to [`Decoder::open`] except it provides no decoder when the first
+    /// IFD errors due to not being an image or an IO problem. It can be replaced precisely with
+    /// this sequence:
+    ///
+    /// ```no_run
+    /// # use tiff::decoder::Decoder;
+    /// # let r = std::io::BufReader::new(std::io::empty());
+    /// let decoder = {
+    ///     let mut tmp = Decoder::open(r)?;
+    ///     tmp.next_image()?;
+    ///     tmp
+    /// };
+    ///
+    /// # Ok::<_, tiff::TiffError>(())
+    /// ```
+    ///
+    #[deprecated = "Prefer `Decoder::open` followed by an explicit `next_image` call. These methods will become aliases in a future major release."]
     pub fn new(mut r: R) -> TiffResult<Decoder<R>> {
         let header = TiffHeader::parse(&mut r)?;
         let mut decoder = header.open(r);
@@ -869,15 +922,6 @@ impl<R: Read + Seek> Decoder<R> {
         Ok(decoder)
     }
 
-    /// Parse the image header from the underlying file and create a decoder positioned before its
-    /// first directory.
-    ///
-    /// The [`TiffHeader`] type allows opening a file with a custom.
-    pub fn open(mut r: R) -> TiffResult<Decoder<R>> {
-        let header = TiffHeader::parse(&mut r)?;
-        Ok(header.open(r))
-    }
-
     /// Configure allocation limits for all subsequent operations.
     ///
     /// Limits apply to allocated return values such as from [`Decoder::read_image`] and

tests/decode_libtiffpic.rs

@@ -12,7 +12,9 @@ const REFERENCE_DIR: &str = "tests/libtiffpic";
 const FILES: &[(&str, bool)] = &[
     ("tests/libtiffpic/caspian.tif", true),
     ("tests/libtiffpic/cramps.tif", true),
-    ("tests/libtiffpic/cramps-tile.tif", true),
+    // FIXME: See <https://github.com/image-rs/image-tiff/issues/338>, this has tiles as
+    // StripOffsets and has a RowsPerStrip.
+    ("tests/libtiffpic/cramps-tile.tif", false),
     ("tests/libtiffpic/CVS/Entries", false),
     ("tests/libtiffpic/CVS/Repository", false),
     ("tests/libtiffpic/CVS/Root", false),
@@ -88,7 +90,9 @@ const FILES: &[(&str, bool)] = &[
     ("tests/libtiffpic/pc260001.tif", true),
     ("tests/libtiffpic/quad-jpeg.tif", true),
     ("tests/libtiffpic/quad-lzw.tif", true),
-    ("tests/libtiffpic/quad-tile.tif", true),
+    // FIXME: See <https://github.com/image-rs/image-tiff/issues/338>, this has tiles as
+    // StripOffsets and has a RowsPerStrip.
+    ("tests/libtiffpic/quad-tile.tif", false),
     ("tests/libtiffpic/README", false),
     ("tests/libtiffpic/smallliz.tif", true),
     ("tests/libtiffpic/strike.tif", true),