Open MinaEnayat opened 2 months ago
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!
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.
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.
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.
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).
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...
This pull request has been mentioned on Image.sc Forum. There might be relevant details there:
https://forum.image.sc/t/how-to-mirror-images-on-omero-figure/105059/2
Hey Will,
I’ve added a new function to handle image flipping, including the following updates:
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