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

Add Horizontal and Vertical Image Flipping Functionality #591

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

MinaEnayat
Copy link
Contributor

@MinaEnayat MinaEnayat commented Sep 20, 2024

Hey Will,

I’ve added a new function to handle image flipping, including the following updates:

  • Horizontal Flip: Mirrors the image along the X-axis (left to right).
  • Vertical Flip: Mirrors the image along the Y-axis (top to bottom).
  • Updated the image viewer panel: Added controls for both horizontal and vertical flipping, allowing users to easily adjust the orientation of images.
  • Updated the export functionality to ensure that flipped images retain their orientation when exported (PDF or TIFF), ensuring that any flipped transformations made within the viewer are preserved in exported documents.

Known Issue:
When using the rotation slider while the image is flipped, the slider temporarily rotates the unflipped version of the image. Once the slider is released, the image is rotated based on the flipped version. This inconsistency between flipping and rotation will need to be addressed to ensure smooth interaction between the two features.

With the collaboration of @Tom-TBT

Fixes #552

@will-moore
Copy link
Member

will-moore commented Sep 25, 2024

This is working really nicely. Looking good.
I haven't tried the Figure export script yet.

Small point: I have used custom padding on the Z-projection button above, and it's also nested in a .btn-group (I'm not sure why now). So the alignment, spacing and size doesn't quite match the new buttons. I don't have a strong preference for which buttons get updated, but it would be nice to have them consistent. Maybe adding btn-group class to your .flipping div, or adding custom margin/padding etc?

It took me a while to find a couple of minor issues: The dragging to pan images is working under most conditions except if I have only 1 axis flipped AND I have a rotation of e.g. 90 degrees. This is quite tricky, and a bit edge-case so don't worry about it too much.
Also, when you're dragging the rotation slider, the preview viewport ignores the flip, and this reverts back to correct orientation when you stop dragging.

Even more edge-case is if you select 2 images and only 1 of them is flipped, then the panning in the viewport is "wrong" for one of the images (since they appear overlaid with the same flip applied when dragging, then the same pan direction is applied to both instead of opposites). Really not worried about this one!

@will-moore
Copy link
Member

The build is failing above because the export script is failing with KeyError:

if panel['horizontal_flip']:

You'll need to use if panel.get('horizontal_flip', False): everywhere to handle figure JSON that doesn't include these new attributes. Same for vertical_flip I guess.

@will-moore
Copy link
Member

I see now that you've bumped the var VERSION = 9; and added the horizontal_flip and vertical_flip attributes to every panel when you load the figure.
But I don't think you really need to do this. There are various optional attributes for each panel, and adding these shouldn't require a bump to the version as they're not breaking changes.
As long as the script doesn't require them (see previous comment) then we don't need to ensure that they are present on every panel. If we simply omit optional parameters from the JSON model, it reduces the size/complexity when they're not used.

@will-moore
Copy link
Member

The PDF export is looking good and the TIFF export is almost there. I just noticed that a Rectangle is being mis-placed for panels with rotation and only 1 axis flipped, e.g. see the selected (centre) panel of this figure, and the bottom-centre and bottom-right panels.

Screenshot 2024-09-26 at 15 20 09

@Tom-TBT
Copy link
Contributor

Tom-TBT commented Oct 7, 2024

Hi Will,

I see now that you've bumped the var VERSION = 9; and added the horizontal_flip and vertical_flip attributes to every panel when you load the figure.

I reverted those changes. I included them because I thought this would solve the test errors, but handling the optional properties also in python is smarter.

@MinaEnayat also resolved the export to tiff and the preview panel display (the right panel preview was not flipped correctly when using the rotation slider).

@will-moore
Copy link
Member

Thanks for the updates on this and other PRs. Just a heads up that I'm pretty tied up over the next week or so - Afraid I might not get time to review etc for a bit...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image Flipping - Similar to iViewer
3 participants