-
Notifications
You must be signed in to change notification settings - Fork 243
Increase maximum group attribute for ROI from 255 to MAX_INTEGER
#289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
cad2608 to
ec04c7d
Compare
|
@rasband What do you think? Do you like this change? |
|
There’s a potential issue with this change: the group number is stored as a byte in the preferences file, and removing this limitation would not be straightforward. |
|
@rasband I looked at the ROI binary format, and had a brief conversation with Claude.ai about it:
I agree with Claude that this approach could work. Thoughts? |
|
Claude’s approach will probably work after the usual testing and debugging. Could we change the group number to a short to reduce the space used in header2? 64k groups should be sufficient. I’m impressed by how knowledgeable Claude is about ImageJ programming. Can you get it to make the necessary changes? |
Increments ROI format version to 229 and uses header2 offset 52 for groups > 255. Maintains backwards compatibility by using byte 30 as marker (value 0 indicates extended group at offset 52). Groups <= 255 continue to use byte 30 for compatibility with older readers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@rasband I asked Claude to implement the fix and make a git commit. Then I reviewed the result (which looks correct to me) and pushed the result to this PR. Edit: I also pushed a followup commit decreasing the |
This is a proposal to lift the maximum value for the
groupattribute of ROIs, from currently 255 toInteger.MAX_VALUEThis attribute is used for instance to group ROIs into categories, ROIs with the same group number would belong to the same category and will be depicted with the same color.
Currently, the group is limited to a max value of 255, I think because the original implementation was using a 8-bit LUT to get the color for a given ROI group. It turns out the Glasbey LUT used to depict the ROI can yield distinct color beyond this range (see (
glasbeyLut.getRGB(group)inRoi.getGroupColor).The rest of the code seems to also deal with the higher range without any issue.
The current implementation also allows storing corresponding names for the group. Those are stored in a
String[]. The maximum size of an array is alsoInteger.MAX_VALUE, right ?This proposal was motivated by people I had a chat with, who had the idea to use the group attribute to group ROIs across multiples Z-planes into pseudo 3D Rois.
But I think there could be other valid use cases where the current limitation of 255 might be problematic.