Skip to content

Conversation

@sbesson
Copy link
Member

@sbesson sbesson commented Jan 7, 2026

Aims to address the issue detected in ome/bioformats#4394 (comment) as a follow-up of #226

As shown in the failing tests https://github.com/ome/ome-model/blob/master/specification/samples/2016-06/folders-larger-taxonomy.ome.xml is a good way to reproduce the IndexOutOfBoundsException via showinf -omexml

@sbesson
Copy link
Member Author

sbesson commented Jan 13, 2026

While investigating further, I identified the cause for one of the failures in the Bio-Formats repository tests with org.openmicroscopy:ome-xml:6.5.2. The Zeiss LSM reader makes successive calls to setImageROIRef(reference, imageIndex, roiIndex) using sequential values of roiIndex which might start from a non-zero value - see
https://github.com/ome/bioformats/blob/8ba547afad0d419516ba30728c8a8127356f3f01/components/formats-gpl/src/loci/formats/in/ZeissLSMReader.java#L1585 and https://github.com/ome/bioformats/blob/8ba547afad0d419516ba30728c8a8127356f3f01/components/formats-gpl/src/loci/formats/in/ZeissLSMReader.java#L1635 where calling setImageROIRef(roi, imageIndex, roiRefIndex).

The changes in #226 implemented a behavior similar to other setters which append an ROI link if roiRefIndex == sizeOfROIList() and result in an IndexOutOfBoundsException if roiRefIndex > sizeOfROIList. Previously, the implementation would ignore the value of roiRefIndex entirely and defer the addition of references until resolveReferences was called.

c9966ef is one way to restore some form of backwards compatibility by deferring the resolution of references if roiRefIndex > sizeOfROIList(). de97c1a adds tests covering scenarios where ROI references are added either sequentially starting with a non-zero index or non-sequentially. Note in particular that the order of the references in testAppendImageROIReferencesNonSequential would be different from org.openmicroscopy:ome-xml:6.5.1.

A few additional/alternate thoughts:

  • the failures also highlighted some issues with the the Folder/Folder references which will need separate investigation
  • as a possible alternative to this PR or a follow-up change, we could decide that the reference indices for these setter APIs must be provided in the [0 sizeOfLinkedROIList()] range. This would align closely with other setter calls but would certainly need to be communicated and handled as a backwards-incompatible API change
  • at the Bio-Formats level, it might be worth auditing the usage of reference setters. Independently of the decision on the API contract, it feels we should ensure that indices should always be supplied sequentially starting with 0 as this is the order where the behavior is be unambiguous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant