Added basic tile decoding

[to-mas-kral]

Hi there ! This is my first PR to an open-source Rust repo after quite some time so bear with me please :-)

Tile-based decoding isn't much different from strip-based, it's just a matter of trying to fit it nicely into the existing codebase, which I tried really hard to do, but I didn't really come up with elegant code... I basically had to duplicate the whole expand_strip method.

I didn't add incremental decoding, because I wasn't sure about the API, but that can be easily done later.

I left a few TODOs in the code. Firstly, I need a good way of skipping some bytes from the Reader to skip padded tiles, but there doesn't seem to be a good way of doing it, so I just allocated one Vec, that I reuse for the whole image.

The other TODO is about errors. I'm not sure how pedantic the decoder should be. For example, the spec says "do not use both strip-oriented fields and tile-oriented fields in the same image", but I noticed that some libtiff test images contain both.

I also tried to add tile support for the JPEG decoder, but the JPEG implementation doesn't work for my test images #124 (comment), so I'll wait until that gets fixed.

[HeroicKatora]

I think most of this looks very solid. Where are the images from, so as not to run into any copyright issues on work committed to git? (Their size is okay).

Firstly, I need a good way of skipping some bytes from the Reader to skip padded tiles, but there doesn't seem to be a good way of doing it, so I just allocated one Vec, that I reuse for the whole image.

This is fine by me. I'd suppose we could add something to the EndianReader here at some point. You could open an issue about this inefficiency.

I'm not sure how pedantic the decoder should be.

A good baseline is to try and accept anything that other libraries deal with, if there is no ambiguity. There's a larger question of how to report such warnings or violations to the user. In any case, one justification in the code and a FIXME comment is good enough, we can always relax this later if it appears to be an issue in practice.

[to-mas-kral]

I don't like that I duplicated some code in the expand_strip/tile methods, so I'll try to refactor that.

The new test photos are mine, so there shouldn't be any problem about that.

How do you think the incremental API should work ?
The strip incremental API lets you read a few strips on their own and then read the rest of the image at once (at least I think that's how it works). The problem with this API for Tiles is that if the user doesn't read all of the tiles in a row, then they would get like an L-shaped result, which probably doesn't make sense. So I'd let the user read one tile at the time, but make it so the next call to read_image() returns the whole image.

[HeroicKatora]

How do you think the incremental API should work ?

What do you think about actually returning an error when calling read_strip on a tiled image? The method is about the ability to efficiently access the underlying encoding and I'd argue it is the resposibility of the caller to ensure that this is the correct encoding. For tiles you could add a similar read_tile method.

[to-mas-kral]

Sure, I added new error types for that.

Btw, I'm happy to see that @fintelia is working on simplifying the expand_strip function. I'll probably wait until that gets merged and try to mirror that for expand_tile.

[HeroicKatora]

@TomasKralCZ Both of these have been merged, so you can reuse the refactored methods :)

[to-mas-kral]

I think the code looks pretty good now.
One thing I'm not sure about is why @fintelia removed color inversion for signed formats. Is that non-standard behaviour ? I remember there was a comment about how libtiff interprets that in a certain way but it got removed.

[fintelia]

There's some duplicated code that could probably be factored out, but that can happen in a followup.