Open will-moore opened 4 months ago
Bug: The panel in the newly created figure is shifted with respect to the ROIs.
RFE:
Some misformatting of the dialog - note the inverted commas are only on one side of the ID (there probably should not be any inverted commas)
To comment further on the ROI-shifting problem https://github.com/ome/omero-iviewer/pull/465#issuecomment-2036930397 - this behaviour is present only when working with figures created by the feature implemented in the present PR https://github.com/ome/omero-iviewer/pull/465 - the figures which are created within OMERO.figure itself do not have any problems with shifted ROIs, i.e. the ongoing work on https://github.com/ome/omero-figure/pull/477 does not seem to be cause of this, cc @jburel
@pwalczysko thanks for the testing. Re: https://github.com/ome/omero-iviewer/pull/465#issuecomment-2036930397 - I think that what is going on here is that OMERO.figure is trying not to add duplicate ROIs on top of one-another. If a Shape already exists when you try to add one, the Shape is automatically offset to the lower-right. In the same way that when you copy and paste a Shape it is offset to the lower-right. If you first delete the Shapes in OMERO.figure before loading and adding them, you should see that they are added in the right place.
@pwalczysko thanks for the testing. Re: #465 (comment) - I think that what is going on here is that OMERO.figure is trying not to add duplicate ROIs on top of one-another. If a Shape already exists when you try to add one, the Shape is automatically offset to the lower-right. In the same way that when you copy and paste a Shape it is offset to the lower-right. If you first delete the Shapes in OMERO.figure before loading and adding them, you should see that they are added in the right place.
Ah, sorry, thanks for explanation. Indeed, this is as you say, the shapes are shifted only because they are already added once. This behaviour is not different from what is in Figure by default and is not introduced by this PR.
I will wait for tomorrow's build for retest of the double-quotes https://github.com/ome/omero-iviewer/pull/465/commits/efda99710532f9743896717e88f0e68ca23ec3b2
This pull request has been mentioned on Image.sc Forum. There might be relevant details there:
https://forum.image.sc/t/omero-iviewer-multiple-images-views-exporting-to-omero-figure/94675/2
Double-quotes are fixed.
The only remaining issue is that points are ignored. As this is done without warning, I wonder if we should add one ? If points are not supported in Figure, then this is fine... but still they are supported in iviewer - if someone gets into great lenght and creates a lot of points, then they might be unpleasantly surprised not to find them in figure later...
@pwalczysko Points are now converted to Ellipses in figure...
@pwalczysko Points are now converted to Ellipses in figure...
I am really not sure about this. If somebody has already some Ellipses (which were meant to be ellipses) on an image, and adds points, then, in Figure, they get a surprise.
Maybe we can discuss in a meeting tomorrow - my preference would be to drop the points if there are any and warn the user. Offering some distortion of the ROIs by default is not the best practice imho.
@jburel Just to clarify, do we need to have Points support in OMERO.figure before this PR is approved?
@pwalczysko suggested at https://github.com/ome/omero-iviewer/pull/465#issuecomment-2051806129 that a warning might be enough, although I think he's changing his mind?!
If we need to wait for Points in OMERO.figure, that needs the Vite PR to be merged, then a follow-up PR. Then iviewer will depend on Points support in OMERO.figure, so we'll need to only enable "Save as Figure" if we have the most recent version of Figure. This will have to be detected in some manual way (we don't want to add omero-figure as a python dependency of iviewer). Currently we detect if Figure is installed and don't show the "Save As Figure" option if it's not found, but detecting the version will be a bit harder.
I don't think we are in any rush to have this PR included and release. My preference, as mentioned earlier today, will be to:
While using figure with this PR, I don't think it will solve the problems we are currently facing with big images
I feel that going back and forth between the apps to manipulate the images is not going to work for big images unfortunately
@jburel There is no cropping here. All panels in Figure can be zoomed and panned over the whole image. You may get the impression of a crop when you pan the viewport, but that is just because the rendered region is restricted around the viewport and doesn't cover the whole image. When you pan and drop, it re-renders around the new viewport region.
It's possible to create an Inset panel in the normal way (maybe even with a shortcut in the new workflow) to show where the viewport is, but that is outside the scope of this PR.
To test: