search

ome / omero-figure

An OMERO.web app for creating Figures from images in OMERO
http://figure.openmicroscopy.org
GNU Affero General Public License v3.0
15 stars 30 forks source link

Crop rotation fixes #410

Closed will-moore closed 3 years ago

will-moore commented 3 years ago

Fixes #408 - copy rotated crop region and paste to ROI. Also fixes #324

Rectangle ROIs don't support rotation, but this fixes the issue above by creating a Polygon to simulate a rotated Rectangle.

To test:

Screenshot 2020-12-08 at 21 55 42

Screenshot 2020-12-15 at 14 28 10

imagesc-bot commented 3 years ago

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/omero-figure-box-annotations-rotated-incorrectly/45787/4

pwalczysko commented 3 years ago

Copy the rotated Crop region, select the uncropped (unrotated) Panel and click Paste to paste the rotated crop region. The rotation should also be applied to the panel so that both images show the same rotated viewport.

Up till this point all worked. The above one I am not sure what to do with - where is the unrotated uncropped Panel ? I just used it in the previous step, and pasted the ROI on it, and it (the ROI, not the panel onto which I pasted) was rotated.... as I would expect. This seems to me like a repetition of the previous test, but now, surprisingly, a new result expectation (rotation of the unrotated panel) seems not reasonable - certainly there is some misunderstanding.

will-moore commented 3 years ago

Copy the rotated Crop region, select the uncropped (unrotated) Panel and click Paste to paste the rotated crop region.

This means to use the "Paste" button at the bottom of the "Preview" Tab, to apply the same crop and rotation to the selected panel. This is NOT Pasting to create a new ROI as in previous steps.

pwalczysko commented 3 years ago

Copy the rotated Crop region, select the uncropped (unrotated) Panel and click Paste to paste the rotated crop region.

This means to use the "Paste" button at the bottom of the "Preview" Tab, to apply the same crop and rotation to the selected panel. This is NOT Pasting to create a new ROI as in previous steps.

Thanks, got this one, and it works as expected.

pwalczysko commented 3 years ago

Crop the rotated panel using the "Crop" button. The 'initial' crop region should match the current viewport (see screenshot) and the updated crop region should be correctly applied to the panel.

Sorry, struggling with this one too. Using the Crop button is clear, but should I first rotate and then Crop ? This means I should not zoom first ? Should I copy and paste the ROI somewhere ? If I do as told here, I am ending up with see screenshot below, how do I get the ROI displayed on the rotated image as shown on yours ?

Screenshot 2021-01-11 at 18 05 12

pwalczysko commented 3 years ago

Crop the rotated panel using the "Crop" button. The 'initial' crop region should match the current viewport (see screenshot) and the updated crop region should be correctly applied to the panel.

After clarification, I was able to verify the above workflow too, it works fine.

Nevertheless, in this workflow, if the ROI does not come from the zoomed-in region but, is copied from another place in Figure (screenshot 1 below) or comes from OMERO (screenshot 2 below), this workflow still fails, as the copied or "from OMERO" ROIs are displayed unrotated.

Screenshot 1 (ROI copied from somewhere else in Figure and pasted (option "From Clipboard")

Screenshot 2021-01-12 at 11 17 17

Screenshot 2 (ROI from OMERO in Crop dialog, ROI should be rotated as the image is rotated)

Screenshot 2021-01-12 at 11 18 19

pwalczysko commented 3 years ago

Would it be possible to fix also the Edit ROI panel view for rotated images ?

Workflow: Rotate an image in Figure. Click on "Labels" tab and "Edit" in the "ROI" row. Observe that the view in the Edit ROI dialog is unrotated, the image inside the panel on the canvas is rotated though.

Screenshot 2021-01-12 at 11 25 02

pwalczysko commented 3 years ago

In summary, 3 RFEs : 2 RFEs regarding the crop dialog and rotation https://github.com/ome/omero-figure/pull/410#issuecomment-758591609 and one regarding the Edit ROI dialog https://github.com/ome/omero-figure/pull/410#issuecomment-758593581

will-moore commented 3 years ago

@pwalczysko I'm afraid fixing the "Edit ROIs" dialog to handle rotated images isn't easy since we'd need drawing tools to allow you to draw rotated Rectangles which I don't have (hand-coded drawing tools).

But the other 2 RFEs above should be fixed:

Screenshot 2021-01-22 at 10 36 09

pwalczysko commented 3 years ago

@pwalczysko I'm afraid fixing the "Edit ROIs" dialog to handle rotated images isn't easy since we'd need drawing tools to allow you to draw rotated Rectangles which I don't have (hand-coded drawing tools).

I think this is fine. I would suggest a (couple of) short video(s) for the solution of the rotation and crop workflows would be very handy, as these workflows are very popular, but as the testing shows, not easy to get right on the first try.

Would you like to list for testing next week with my name ?

pwalczysko commented 3 years ago

But the other 2 RFEs above should be fixed:

Open the crop dialog, having either copied a Crop Region from another Panel (with or without rotation) OR having copied a Rectangle. Also nice to have Rectangle ROIs saved on the Image in OMERO, and/or Rectangle ROIs on the Figure panel (see screenshot)
Check that each of these thumbnails shows the correct region of the image.
Clicking either the Rectangle ROIs from OMERO or the Rectangle on the figure panel (top and bottom sections of thumbnails) will set the Panel rotation to zero. Clicking the "clipboard" thumbnail will apply a rotation IF you copied the crop region from a Panel that was rotated.
When you hit "OK", any rotation that got applied should be saved (so that the crop region you selected was correctly applied to the image).

This works as expected. Ready to merge fmpov.

will-moore commented 3 years ago

That last commit fixes an issue I noticed while preparing a demo movie: If you paste the crop region from a panel with 0 rotation, it didn't apply it to the new panel (it didn't remove the rotation on the target panel).

will-moore commented 3 years ago

I made a demo movie for this PR:

https://user-images.githubusercontent.com/900055/106256644-731b4600-6213-11eb-90d1-e3c4173590b9.mp4

pwalczysko commented 3 years ago

@will-moore great movie, thanks, these are precisely the workflows which were picked upon in the recent workshop again, and the difference of those two Paste buttons is nicely shown

jburel commented 3 years ago

This could probably go to the official youtube channel otherwise it will be lost

will-moore commented 3 years ago

Yes, can make the video more "public" when we release. The last commit needs a test before merging: Copy the crop region from a non-rotated image (zero rotation) and paste as a crop region onto a rotated image. The rotation should be set to zero.

pwalczysko commented 3 years ago

Copy the crop region from a non-rotated image (zero rotation) and paste as a crop region onto a rotated image. The rotation should be set to zero.

Tested When I use the Paste buttion from Labels pane, the rotation is not set to zero. When I use the Paste button from Preview pane, the rotation is not set to zero. I guess this is the expected behaviour ?

will-moore commented 3 years ago

When I use the Paste button from Preview pane, the rotation is not set to zero. I guess this is the expected behaviour ?

No. When I use the Paste button from Preview pane, the rotation should be set to zero.

pwalczysko commented 3 years ago

When I use the Paste button from Preview pane, the rotation is not set to zero. I guess this is the expected behaviour ?

No. When I use the Paste button from Preview pane, the rotation should be set to zero.

Yes, the rotation on Pate is indeed set to zero. Sorry, typo (in my original report, there are two "no"s for both of the panes, in truth, I shoiuld have written "y" for the Preview pane, as this is what I saw.

Sorry

pwalczysko commented 3 years ago

Inside the Crop dialog, copied a Rrectangle ROI from another panel which was rotated so that it is in Clipboard now.

the desciption above says

Clicking the "clipboard" thumbnail will apply a rotation IF you copied the crop region from a Panel that was rotated.

But when I click the Clipboard thumb it does not apply any rotation, instead, it unrotates the main panel in the Crop dialog and pastes the ROI correctly. Did this behaviour change (i.e. unitended change) or is it a misunderstanding ?

will-moore commented 3 years ago

You copied a Rectangle ROI, but my text says "IF you copied the crop region..." Rectangles can't have any rotation, but a crop region can. The behaviour hasn't changed.

Does the thumbnail representation that you clicked look like the crop outline that is generated when you click it?

pwalczysko commented 3 years ago

You copied a Rectangle ROI, but my text says "IF you copied the crop region..." Rectangles can't have any rotation, but a crop region can. The behaviour hasn't changed.

Does the thumbnail representation that you clicked look like the crop outline that is generated when you click it?

thanks for explaining. No, the crop outline looks differently, the crop dialog is not showing the right thing now imho when Clipboard is selected.

Screenshot 2021-02-09 at 11 45 08

pwalczysko commented 3 years ago

Compare the situation inside Crop dialog described in previous comment with the situation when I paste the crop region without using the Crop dialog on the same image.

Screenshot 2021-02-09 at 11 48 20

This behaviour seems right here, whereas the Crop dialog above https://github.com/ome/omero-figure/pull/410#issuecomment-775880924 shows the wrong thumb and when thumb is clicked, a wrong rotation and position is shown in main panel.

will-moore commented 3 years ago

Sorry @pwalczysko I can't seem to reproduce the bug you're seeing. Can you point me at an example figure and exact steps to reproduce? Thanks.

pwalczysko commented 3 years ago

The image used is (user-3)

https://merge-ci.openmicroscopy.org/web/webclient/?show=image-140172

The figure saved is

https://merge-ci.openmicroscopy.org/web/figure/file/70451

We clarified the workflow on a call - basically, taking the crop region from the zoomed in image, then selecting the non-zoomed non-rotated original image in the same figure and clicking on Crop button. In the crop dialog, the thumb of the Clipboard is wrong, as well as when this wrong thumb is applied onto the image, it is not where it should be either.

pwalczysko commented 3 years ago

The problem https://github.com/ome/omero-figure/pull/410#issuecomment-778292041 is fixed now.

Ready to merge fmpov.