search

ome / omero-iviewer

An OMERO.web app allowing to view images
https://www.openmicroscopy.org/omero/iviewer/
Other
19 stars 30 forks source link

improved mirroring #484

Closed barrettMCW closed 1 week ago

barrettMCW commented 2 months ago

Fixed the jitter from panning while rotated, and it seems at some point in the pr process drawing the rois while mirrored broke. Fixed it up. Still couldn't get projections to work. I do think that would be the most robust way to handle this but I'll take another crack at projections next time something breaks. Thanks y'all!

will-moore commented 1 month ago

Thanks for this - apologies for the delayed response. It's working nicely and fixes the crazy jitter on dragging when rotated. It also fixes the ROI drawing when flipped. However, the dragging of the image when flipped is opposite to the expected direction (the flipping is not taken into account to reverse the direction of drag).

NB: I also found it a little tricky to know when the image was flipped as there's no indicator. This is outside the scope of the PR so no need to address it now unless you want to, e.g. add some visual clue to the flip buttons when they are enabled?

barrettMCW commented 1 month ago

That's strange... I added a mitigation specifically to fix that on my end line 80 Can you give me some more info about how you're testing it? I'm viewing it with npm run dev on chrome. I like the flipped indicator idea! I'll add it quick.

will-moore commented 1 month ago

That's weird. I'm building with npm run debug or npm run prod then opening iviewer from webclient. However, if I use npm run dev then I don't see the bug (same as you).

barrettMCW commented 1 month ago

added flip indicator, not pretty lol but distinct between flipped and not flipped. the prod vs dev is a real head scratcher though, I'll keep looking into it

barrettMCW commented 1 month ago

Think I might have gotten it figured out? I was looking at the name of the constructor which gets minified in prod. Hopefully that was it, I don't have an easy way of testing this out, let me know if it's still an issue then I'll set up a proper dev environment to test this more thoroughly. Thanks!

barrettMCW commented 1 month ago

Great catch! Thanks! (didn't know you could do that lol) It's mostly fixed. It captures extents properly but for whatever reason it doesn't render when only X is mirrored. When just Y or X and Y are mirrored it works perfectly but X is misbehaving. Still captures properly so I think it's better than it was.

barrettMCW commented 1 month ago

Still haven't figured out roi popups yet. I'll skip that this PR, but if there's anything else you find, let me know. Thanks!

Tom-TBT commented 1 month ago

Hello, I tried it out and it works quite well now, congrats.

A few things that I spotted.

When you click a button, it stays selected until something elsewhere is clicked. I find it slightly annoying. It would be nice if it could keep the "unselected" look right after clicking on it. image

About the button color, I tried with different blues. I could suggest the "dodgerblue". But if I'm being honest my visual tastes can be poor. Up to you to accept the suggestion or not. image

To finish, I found an actual bug: the text of the label ROIs are flipped too 😅 image

Cheers, Tom

barrettMCW commented 1 month ago

@Tom-TBT Thanks a lot! I appreciate it! I like the color change, I'll add that. I was aware of the other two issues, the button thing is just weird and the text thing seems tricky, but I'll take a peak. The button issue can probably get solved, but the text issue seems like it will need the proper projection based flipping to keep the position accurate while displaying the text right side up, although I'll take a stab at it.

barrettMCW commented 1 month ago

Sorry for the delay. Changed the color but I don't think I'll be able to fix the text issue or the selected issue. The text is ol native, so I'm not sure how to go about extending or modifying that and the selected issue seems to effect all the controls, such as zoom and birdseye, even on the IDR. I created #487 to track it. But thanks for spotting those @Tom-TBT!

barrettMCW commented 1 month ago

Just discovered that the scalebar can be dragged, and of course, it drags wrong when flipped, checking it out now.

barrettMCW commented 1 week ago

Sorry, got busy and this fell off, won't be able to put time into this for a bit. I think this is a good place to call it for this pr. Thanks for all the help y'all!

will-moore commented 1 week ago

This contains a bunch of fixes so it would be good to get this in. Although some minor issues were identified during the testing, they are not sufficient to block this and can be tackled in future when time allows.