Skip to content

Conversation

@sbesson
Copy link
Member

@sbesson sbesson commented Jan 7, 2026

Fixes 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 using Bio-Formats showinf -ome-xml and org.openmicroscopy:ome-xml 6.5.2.

Taking setImageROIRef(roiReference, imageIndex, roiRefIndex) as an example, the current logic following the changes in #226 is the following,

  1. if the ROI referred by roiReference does not exist, the addition of the ROI reference for image imageIndex is deferred under resolveReferences is called
  2. if the ROI referred by roiReference exists and roiRefIndex == getRoiRefCount, the ROI reference is appended to the list of references for image imageIndex
  3. if the ROI referred by roiReference exists and roiRefIndex < getRoiRefCount, the ROI reference for image imageIndex at roiRefIndex is replaced by roiReference

This Pull Requests adds a fourth additional conditional behavior allowing to restore backwards-compatibility

  1. if the ROI referred by roiReference exists and roiRefIndex > getRoiRefCount, the addition of the ROI reference for image imageIndex is deferred under resolveReferences is called

Unit tests are also added to cover the different failing scenarios found in our repository tests i.e.:

  • Image ROI references added with roiRefIndex starting with a non-zero offset
  • Folder Folder references added with a mixture of references pointing to existing and non-existing folders

@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.

Define a FolderFolderRef before the target Folder is declared and
confirm that the reference addition is deferred
@sbesson
Copy link
Member Author

sbesson commented Jan 19, 2026

With some additional debugging, the sequence of calls leading to the IndexOutOfBoundsException when reading https://github.com/ome/ome-model/blob/master/specification/samples/2016-06/folders-larger-taxonomy.ome.xml with org.openmicroscopy:ome-xml:6.5.2 is the following:

dest.setFolderID(Folder:13,0)
dest.setFolderID(Folder:30,1)
dest.setFolderID(Folder:16,2)
dest.setFolderID(Folder:31,3)
dest.setFolderID(Folder:6,4)
dest.setFolderID(Folder:4,5)
dest.setFolderID(Folder:17,6)
dest.setFolderID(Folder:22,7)
dest.setFolderID(Folder:29,8)
dest.setFolderID(Folder:25,9)
dest.setFolderID(Folder:15,10)
dest.setFolderID(Folder:3,11)
dest.setFolderID(Folder:1,12)
dest.setFolderFolderRef(Folder:2,12,0)
dest.setFolderFolderRef(Folder:30,12,1)

Here, the exception is caused by the fact that Folder:2 is not defined yet so the first call to setFolderFolderRef is deferred and the second call tries to access folderRefIndex 1. Handling folderRefIndex > getFolderRefCount() to defer the reference addition also suffices to fix the metadata processing. 946e787 updates the OME-XML unit tests to also capture this scenario.

Moving this PR out of draft so that it is included in the nightly builds before assigning reviewers.

@sbesson sbesson marked this pull request as ready for review January 19, 2026 10:00
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