Skip to content
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

Rotate image panel by 90° #595

Merged
merged 18 commits into from
Nov 28, 2024
Merged

Rotate image panel by 90° #595

merged 18 commits into from
Nov 28, 2024

Conversation

Rdornier
Copy link
Contributor

@Rdornier Rdornier commented Oct 1, 2024

Fixes #484

The rotation button at the bottom rotates the image/canvas by steps of 90°

image

@will-moore
Copy link
Member

will-moore commented Oct 3, 2024

The display looks a bit strange for me:

Screenshot 2024-10-03 at 12 32 18

Maybe try to remove height: 200px from the .rotation-controls-shown styles and remove absolute from the new button.

There's a space between the 90 and ° which makes the button bigger than it needs to be. Also I wonder if a different icon would distinguish it from the rotation icon above? (see below). Or possibly just a + 90° is better?

Screenshot 2024-10-03 at 12 55 53

E.g.

--- a/src/css/figure.css
+++ b/src/css/figure.css
@@ -650,11 +650,9 @@
     }
 
     .rotation-controls-shown .panel-rotation {
-        margin-top: 170px;
+        margin-top: 160px;
         padding: 2px;
-        min-width: 50px;
         display: block;
-        position: absolute;
         pointer-events: all;
     }
 
@@ -884,7 +882,6 @@
         background-color: #FAFAFA;
         border: 1px solid #CCCCCC;
         border-radius: 3px;
-        height: 200px;
         padding: 2px;
         position: absolute;
         z-index: 100;
diff --git a/src/templates/image_display_options.template.html b/src/templates/image_display_options.template.html
index 420c4f04..5369a15b 100644
--- a/src/templates/image_display_options.template.html
+++ b/src/templates/image_display_options.template.html
@@ -8,8 +8,8 @@
 
         <input type="range" class="rotation-slider" max="360" value="<%= rotation %>"></input>
         <button type="button" class="btn btn-outline-secondary btn-sm panel-rotation" title="Panel rotation">
-            <i class="bi-arrow-clockwise" style="font-size: 14px"></i>
-            <span>90</span> &deg;
+            <i class="bi-arrow-90deg-right" style="font-size: 14px"></i>
+            <span>90</span>&deg;
         </button>

@will-moore
Copy link
Member

will-moore commented Oct 3, 2024

There's some strange behaviour when the panel is rectangular.
I didn't notice that you're also updating with width and height until I tried a rectangular panel...
But when I try to Undo the change, the width is restored but not the height.

E.g. if I take a panel like on the left and I rotate it 90° it looks like the panel in the centre. Then if I Undo that change, it looks like the panel on the right. The Width is undone, but not the height, and the rotation has jumped to 270°.

Screenshot 2024-10-03 at 13 04 41

var offset_x = viewport.x-offset
var offset_y = viewport.y+offset

this.cropToRoi({'x': offset_x, 'y': offset_y, 'width': viewport.height, 'height': viewport.width, 'rotation': panelRotationAngle})
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that you don't need this cropToRoi function here?, and removing it fixes the Undo issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found it necessary for rectangular big images. If we don't crop and simply update width/height, then the image appears more zoomed-in, but for non-big images, then removing this method works properly (see below). As I already had some weird issues, regarding big images, which you couldn't reproduce, can you test on a rectangular big image if you have the same issue ?

image

@snoopycrimecop
Copy link
Member

snoopycrimecop commented Oct 4, 2024

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

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

@will-moore
Copy link
Member

It looks like cropToRoi() was just needed for getting the new zoom, so I copied the lines for that and used them directly.

This is working for me with rectangular images etc.

+++ b/src/js/models/panel_model.js
@@ -303,15 +303,14 @@
             var rotation = this.get('rotation') || "0"
             var panelRotationAngle =  (parseInt(rotation) + 90) % 360
 
+            // For rectangular images we need to recalculate the zoom;
             var viewport = this.getViewportAsRect()
             var width = this.get('width')
             var height = this.get('height')
-            var offset = (viewport.height - viewport.width)/2
-            var offset_x = viewport.x-offset
-            var offset_y = viewport.y+offset
-
-            this.cropToRoi({'x': offset_x, 'y': offset_y, 'width': viewport.height, 'height': viewport.width, 'rotation': panelRotationAngle})
-            this.save({'rotation': panelRotationAngle, 'height': width, 'width': height});
+            var xPercent = this.get('orig_width') / viewport.height;
+            var yPercent = this.get('orig_height') / viewport.width;
+            var zoom = Math.min(xPercent, yPercent) * 100;
+            this.save({'rotation': panelRotationAngle, 'height': width, 'width': height, 'zoom': zoom});
 
             return panelRotationAngle

@Rdornier
Copy link
Contributor Author

Rdornier commented Oct 8, 2024

You're right, it is the zoom level that causes the issue. I've corrected it. Thanks !

@snoopycrimecop
Copy link
Member

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

--conflicts

@snoopycrimecop
Copy link
Member

snoopycrimecop commented Nov 27, 2024

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

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

@will-moore
Copy link
Member

Both those conflicting PRs are merged now @Rdornier

@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@ome ome deleted a comment from snoopycrimecop Nov 27, 2024
@Rdornier
Copy link
Contributor Author

Thanks @will-moore for merging the PRs.
I've solved the conflicts now

@will-moore will-moore added this to the 7.2.0 milestone Nov 27, 2024
Copy link
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.

Working fine on merge-ci now, Thanks

@will-moore will-moore merged commit 1dfb138 into ome:master Nov 28, 2024
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.

Rotate 90° without cropping
3 participants