Bug: Miscellaneous OMETiffReader defects

[rleigh]

While doing the C++ conversion, I noticed a few small problems, mostly confined to initFile.

• "i" and "s" are both used to refer to the current series in a loop. s is assigned from i in the loop but both are identical. Suggestion: Remove s, rename i to s. Even better, rename i to series so it's obvious what it is.

• TiffData? processing has O(n!) complexity. Reduce to O(2n) by moving the plane clearing loop out of the TiffData? loop so it's two single passes only. In the C++ code this was done by changing exists from a boolean to an enum to differentiate between the states so we only clear unknown planes.

• In "verify that all planes are available" loop, if the reader is null and the planes are reinitialised, there's no explicit break to exit the loop and prevent the process happening again. Shouldn't in practice but it would make the intent clearer.

• At the end of initFile null series are removed. This breaks indexing when restoring dates, etc. This should be the last operation in initFile to avoid indexing errors.

• [zct]OneIndexed? only cope with indexing starting at zero or one. The code can be trivially generalised to support arbitrary index starts, done in the C++ code, which will make it a bit more robust.

• Several checks use data from IFD 0, even when they are doing a check for something in a completely different series, which could likely be using completely different values, so several things could break if using multi-series data where the series are not the same. To fix, it needs to use an IFD from the series in question.

[mlinkert]

Moving to 5.1.1 for triage.

[jburel]

Moving to 5.1.3 as part of the export as OME-Tiff effort.

[mlinkert]

Moving to 5.2.0, since none of this appears to be urgent. If there are test cases that highlight any of these problems, that would certainly be helpful.

[jamoore]

Splitting due to milestone decoupling.

[mlinkert]

No ongoing work here. As noted above, if test cases are available (or can be created) that highlight where the C++ is reader is correct but the Java one is incorrect, that would be a good first step.

The only point I don't completely agree with is this one:

• [zct]OneIndexed? only cope with indexing starting at zero or one. The code can be trivially generalised to support arbitrary index starts, done in the C++ code, which will make it a

bit more robust.

since the spec only allows zero-based indexing. Support for one-based indexing is a special case since that's a common mistake for other software writing OME-TIFFs, but unless the spec is being changed support for arbitrary indexing might be a little too lenient (but definitely tricky to draw the line for data that's technically invalid).