-
Notifications
You must be signed in to change notification settings - Fork 30
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
confine projection #305
confine projection #305
Conversation
That second commit handles the scenario at ome/omero-web#115 (comment) where we enable/disable projection when user turns channels on/off. |
Yes, I think so. Keep simple |
Interesting side-question: Multi-t and multi-z images. Atm, I am gettting a multi-t projections when I have multi-t images with (orginally) multi-z. See for example user-3 https://merge-ci.openmicroscopy.org/web/webclient/?show=image-100718 |
Yes, the behaviour here is as expected. See the under-size-limit images from previous comment and a over-limit-image-when-all-channels-on in https://merge-ci.openmicroscopy.org/web/webclient/?show=image-100727 But I guess it would be better to not to introduce complex logic with "when channels on..." etc. |
I think it is better to stick to sizeC instead of active channels. |
All looks good. Checked iviewer, and also "old" viewer with images from https://merge-ci.openmicroscopy.org/web/webclient/?show=project-15107 Ready to merge fmpov |
@@ -517,7 +517,13 @@ export default class DimensionSlider { | |||
*/ | |||
getZProjectionDisabled(handle, forwards) { | |||
let dims = this.image_config.image_info.dimensions; | |||
return (handle !== null && forwards || (dims.max_x * dims.max_y) > UNTILED_RETRIEVAL_LIMIT); | |||
let tiled = (dims.max_x * dims.max_y) > UNTILED_RETRIEVAL_LIMIT; | |||
let bytes_per_pixel = Math.ceil(Math.log2(this.image_config.image_info.range[1]) / 8.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may want to add a TODO
and create an issue so we can use the omero-model
property when available ome/omero-model#47
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah interesting, so: see if someone set a value explicitly for iviewer, then check the server default, then use a client fallback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently we don't have the config available from the server.
When that is available, we could teach iviewer to use that as a default value, but if it is set in iviewer itself then use that. That could allow iviewer to be more restrictive than the server (but not less restrictive since the server would enforce it's own limit).
Maybe we should make this configurable within iviewer itself while we wait for configuration to be available on the server. I'd hate for someone to upgrade and then be blocked from viewing images that they have previously viewed OK. |
That will be certainly a good option |
1024 * 1024 * 256, | ||
int, | ||
("Maximum bytes of raw pixel data allowed for Z-projection. " | ||
"Above this threshold, Z-projection is disabled")], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe indicate that this value will be ignored if the limit is greater that the one set server-side.
Server side is work-in-progress but when it is in-place that will be the rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems wrong for the documentation to describe future functionality.
Since I'll need to open a PR to update the functionality, I can change the description at the time to match (depending on what we end up with).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can review the wording later on when we implement the size limit from server
My point is that if people start setting a high limit, this will be ignored later on and will default to the server one. This might come at a surprise for that.
See ome/omero-web#115
This ports the same logic to the iviewer to restrict the size of projections.
To test:
Z * X * Y * C * bits-per-pixel
is > 1024 * 1024 * 256.