Skip to content

Ensure projection parameters are within bounds in loadJSON and setID#665

Merged
will-moore merged 7 commits into
ome:masterfrom
Tom-TBT:z-end
Feb 27, 2026
Merged

Ensure projection parameters are within bounds in loadJSON and setID#665
will-moore merged 7 commits into
ome:masterfrom
Tom-TBT:z-end

Conversation

@Tom-TBT
Copy link
Copy Markdown
Contributor

@Tom-TBT Tom-TBT commented Feb 2, 2026

A figure from 2022 on my server had an issue when loaded. I found that z-end == sizeZ in the JSON.

TLDR to reproduce:

  • create a new figure for an image
  • export the JSON and import JSON after setting z-end>=sizeZ
  • try to do something with the panel, it will stop responding

Also projection was set to true, but out of bound z-end is enough to make the panel fail (not the whole figure fails. Just the panel is greyed out, guessing this comes from panel_model.validate).

What's curious is that these images have a single Z-slice, so weird that projection == true.

I added code in the JSON loading to set bounds to z-start and z-end so that it never goes out. Also tried to do it in a conservative way that:

  • it doesn't create variables if it's not necessary (when all three are missing)
  • it keeps z-start and z-end unchanged, unless it's out of bounds.

Writting the PR, I realize that I could reproduce the issue by setting the ID of a stack to a smaller stack. So I also added the bounding logic to setId.

Copilot AI review requested due to automatic review settings February 2, 2026 14:39
@Tom-TBT
Copy link
Copy Markdown
Contributor Author

Tom-TBT commented Feb 2, 2026

Also I must say I am unsure the code is in the right place for loadJSON. I don't think it's a bug tied to a specific OMERO.figure model version, so I instead check that the z-projection parameters are always in bound when figure is loaded from JSON.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an issue where z-projection parameters (z_start, z_end, z_projection) can be out of bounds when loading figures from JSON or switching to images with fewer Z slices, causing panels to become unresponsive. The fix adds bounds checking in two locations to ensure these parameters are always valid.

Changes:

  • Added bounds checking logic in load_from_JSON to sanitize z-projection parameters when loading figures
  • Added bounds checking logic in setId to handle switching to images with different Z dimensions
  • Removed trailing whitespace in multiple files

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/js/models/figure_model.js Adds bounds checking for z_start, z_end, z_projection when loading panels from JSON; handles sizeZ=1 case and ensures start<=end; also removes trailing whitespace
src/js/models/panel_model.js Adds bounds checking for z_start, z_end, z_projection in setId function when switching images; also removes trailing whitespace

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js/models/figure_model.js Outdated
Comment thread src/js/models/figure_model.js
Comment thread src/js/models/panel_model.js Outdated
Comment thread src/js/models/panel_model.js Outdated
@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-plugins-push#41. See the console output for more details.
Possible conflicts:

--conflicts

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-plugins-push#42. See the console output for more details.
Possible conflicts:

--conflicts

@will-moore
Copy link
Copy Markdown
Member

Just tried testing on merge-ci before I realised this didn't get merged! Certainly managed to reproduce bugs anyway...

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-plugins-push#56. See the console output for more details.
Possible conflicts:

--conflicts

@will-moore will-moore added this to the 7.4.0 milestone Feb 23, 2026
@snoopycrimecop
Copy link
Copy Markdown
Member

snoopycrimecop commented Feb 23, 2026

Conflicting PR. Removed from build OMERO-plugins-push#67. See the console output for more details.
Possible conflicts:

  • PR Handle many channels  #664 Nirkan 'Handle many channels '
    • src/js/models/panel_model.js
  • Upstream changes
    • src/js/models/panel_model.js

--conflicts Conflict resolved in build OMERO-plugins-push#72. See the console output for more details.

@snoopycrimecop snoopycrimecop mentioned this pull request Feb 23, 2026
Copy link
Copy Markdown
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thx

@will-moore will-moore merged commit d004baf into ome:master Feb 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants