-
Notifications
You must be signed in to change notification settings - Fork 79
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 with glue Affine transform #1551
Changes from all commits
37f07f1
64591dc
92acfe0
9f0b3a3
5af35fc
ac7f5c1
6c57f28
5b29340
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
from .rotate_image import * # noqa |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import math | ||
|
||
from traitlets import Any | ||
|
||
from jdaviz.core.registries import tray_registry | ||
from jdaviz.core.template_mixin import TemplateMixin, ViewerSelectMixin | ||
|
||
|
||
@tray_registry('imviz-rotate-image', label="Simple Image Rotation") | ||
class RotateImageSimple(TemplateMixin, ViewerSelectMixin): | ||
template_file = __file__, "rotate_image.vue" | ||
|
||
angle = Any(0).tag(sync=True) | ||
|
||
def __init__(self, *args, **kwargs): | ||
super().__init__(*args, **kwargs) | ||
self._theta = 0 # degrees, clockwise | ||
|
||
def vue_rotate_image(self, *args, **kwargs): | ||
# We only grab the value here to avoid constantly updating as | ||
# user is still entering or updating the value. | ||
try: | ||
self._theta = float(self.angle) | ||
except Exception: | ||
return | ||
|
||
viewer = self.app._viewer_by_id(self.viewer_selected) | ||
|
||
# Rotate selected viewer canvas. | ||
# TODO: Translation still a bit broken if zoomed in. | ||
viewer.state.rotation = math.radians(self._theta) | ||
|
||
# TODO: Zoom box in Compass still broken, how to fix? If not, we need to disable it. | ||
# Update Compass plugin. | ||
viewer.on_limits_change() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do I get viewer limits that is consistent with the rotation, so I can show a sane zoom box in Compass plugin when image is rotated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you going to show the image rotated in the compass plugin as well? Or are you going to show it in the original/native orientation and want to show the box rotated? (since it matters for the limits) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking it is easier in Compass if we do not rotate the image but only rotate the zoom box. The Compass display real estate is already pretty small. To rotate the image in Compass would make the image display even smaller when rotated. Also even if not rotated, images can have wild orientations after you link them by WCS. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
<template> | ||
<j-tray-plugin> | ||
<v-row> | ||
<j-docs-link :link="'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#simple-image-rotation'">Rotate image.</j-docs-link> | ||
</v-row> | ||
|
||
<plugin-viewer-select | ||
:items="viewer_items" | ||
:selected.sync="viewer_selected" | ||
label="Viewer" | ||
hint="Select viewer." | ||
/> | ||
|
||
<v-row> | ||
<v-col> | ||
<v-text-field | ||
v-model="angle" | ||
type="number" | ||
label="Angle" | ||
hint="Rotation angle in degrees clockwise" | ||
></v-text-field> | ||
</v-col> | ||
</v-row> | ||
|
||
<v-row justify="end"> | ||
<v-btn color="primary" text @click="rotate_image">Rotate</v-btn> | ||
</v-row> | ||
|
||
</j-tray-plugin> | ||
</template> |
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.
Works but pretty sure it is clockwise again... Is this expected?
Also, the center translation is still a bit off when I zoom in first and then rotate on a rectangular image. If I zoom in on a star, I would expect rotation around the star. But the star moves quite dramatically when I rotate.
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.
Apparently applying the transform directly on
x
,y
in thepretransform.__call__()
seems to have it operate the other way around again, but that is easily fixed, even if I cannot follow the full chain of events.I think for finding the right centre of rotation setting
xy
from(x_min + x_max) / 2
rather thanshape[0] / 2
etc. is still the better option (x_min
,y_min
may be nonzero even if not zoomed in); at least that that keeps the viewport centred and rotated around the original zoom region when zooming in first and then rotating. Unfortunately with the same modification, zooming in after rotation is completely off – apparently this is setting the limits from the unrotated frame, I recon this is failing due to the data values not being returned from the transformed pixel coordinates just as with the mouseover.I would expect that once the latter problem is solved, the zoom will also operate on the correct subset, so the alternative above still looks a step forward.
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.
Derek, all this will be fixed upstream, yes?
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.
Not sure which of the "all" is fixable upstream. I have a patch for the sense of direction, and keeping the centre of rotation in the current viewport centre, but the mouseover stats, active selection area and zoom-after-rotation remain of.
I think I'll work on that next, but it sounds like this will still require some adaption from Imviz to use those
viewer.state
methods.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.
If it helps, the Imviz mouseover logic is here:
jdaviz/jdaviz/configs/imviz/plugins/viewers.py
Line 97 in c2a9b82
Out of scope now but if you are curious, Cubeviz version is here:
jdaviz/jdaviz/configs/cubeviz/plugins/viewers.py
Line 65 in c2a9b82
They both send info to the same plugin here:
https://github.com/spacetelescope/jdaviz/blob/main/jdaviz/configs/imviz/plugins/coords_info/coords_info.py
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.
Yes; looked at the above, but can I find any more info on what type of object the
data
is that is passed toon_mouse_or_key_event
? I've been struggling to find out what its'domain'
item is or where it is set.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.
That I am not sure... Could be glue-jupyter , bqplot-image-gl , bqplot ... 🤷
Hopefully @maartenbreddels or @astrofrog can tell us.