Closed barrettMCW closed 1 year ago
Hi, apologies for not reviewing sooner. Can you briefly say what I'm expecting to see when mirroring is enabled? A button or popup option or is the mirroring simply enabled always? Thanks,
No problem! Enabling mirroring adds two buttons to the top left set of buttons, under the plus and minus. These buttons will flip the viewport in the x or y axis with css and modifies pointer event coords as needed. The mirroring buttons themselves are enabled by adding ENABLE_MIRRORING=true to initparams or setting omero.web.iviewer.enable_mirror=true in configs(url)
Ah, yes - it's working now. I though I had the config set but it turned out I didn't....
In general this looks great...
The only significant issue I've seen so far is that when I click a different image thumbnail on the left to open a new Image, that doesn't show the mirror control buttons. It's like those buttons only get shown correctly when the page loads, not when each image loads.
I also noticed that when I drag (pan) the image while the Shape pop-up is showing, the Shape pop-up movement is not reflected, so if the image is reflected then the pop-up flies in the wrong direction. But this is very minor.
Good catch! I figured out how to properly add a control (with globals). I have been using BirdsEye as a guide, but I can't seem to figure out what's going on there. As far as I can tell birdseye is a non-default control, but I can't find where it gets added to the MDI images (or any image that isn't the init_requested.) Any ideas on where that happens or another place I should add the control? I was not able to replicate the shape popup issues. I used to have them (when I recorded the gif) but I added some code to hopefully fix that before my initial commit. Is it persisting on your end? Sorry about the true != 'True' thing, it got me too lol
found it!
Ah I see what you mean. I wasn't able to find an obvious place to fix. I'll make a pr if I find something in the future. Thanks!
I have a problem with a follow-up function which is not taking into account the flipped image.
Not sure that this is a blocker, but a user will certainly be surprised.
Similarly to https://github.com/ome/omero-iviewer/pull/439#issuecomment-1381897677 , when a Projection is made out of the flipped image, the Projection is non-flipped (although the Preview of it is flipped)
Thinking and playing further, I think that the Save Viewport... behaviour is not very good (say, worse than the projection) in this sense:
Thus, some controls in the top-left get respected, some ignored... Not explicable in intuitive way...
Great finds! I agree, save viewport should do that. I got saving mirrored canvases to work, so that will be coming. Gonna finish up that then I'll download some z-stack data to look at projections.
Saving mirrored viewport works now! I was looking at the projections and it seems doable but I think maybe we shouldn't save it as mirrored directly. The other controls do not effect how the projection saves. I want to add a "ground truth" option in the future that saves an orientation (zoom, rotation, mirror) to move to when opening that image. ( so once we coreg we view the slides in the proper orientation by default ) I think it would be better to add that and copy over current orientation as ground truth?
Saving mirrored viewport works now! I was looking at the projections and it seems doable but I think maybe we shouldn't save it as mirrored directly. The other controls do not effect how the projection saves. I want to add a "ground truth" option in the future that saves an orientation (zoom, rotation, mirror) to move to when opening that image. ( so once we coreg we view the slides in the proper orientation by default ) I think it would be better to add that and copy over current orientation as ground truth?
Thank you @barrettMCW , this all sounds very reasonable and encouraging to me.
And yes, the Save Viewport...
is working as expected. I am suggesting that this great improvement should be merged as is - the bird's eye view is of course good to have, but if it is not soon-to-solve problem, it seems to me that the benefits outweight the drawbacks.
Thank you! Do you mean shape popup? I didn't see any problems with birdseye.
Thank you! Do you mean shape popup? I didn't see any problems with birdseye.
No, I mean bridseye view, bottom-right in central pane of iviewer (on many of the screenshots here in this PR, the birdseye view is collapsed, you just have to click onto the arrow in bottom-right to expand it).
You can see that if I flip the image, the birdseye is still in original orientation. Now, when I use the birdseye to "jump" between places on a large image, the discrepancy between the two views is of course confusing. I have also checked that e.g. rotation of the image rotates the birdseye view accordingly. I was thinking you were discussing it with @will-moore in https://github.com/ome/omero-iviewer/pull/439#issuecomment-1379526368 ?
I was looking at the non-flipped birdseye as a feature rather than a bug, as the red box is correct, although now seeing that rotating the image rotates the entire birdseye makes me feel I should flip it. Shouldn't be hard
Ahh, that was me talking about trying to copy how birdseye was implemented to make sure it's always added. But yeah I'll change that quick, good catch
Ummm, really sorry for not discoverign it earlier. When I rotate&flip horizontally then the panning can become very hard. I can record a video if need be, but maybe you can reproduce ? Just rotate some 45 degrees, then flip horizontally and try to pan please.
ickyyyy, i see it. Birdseye is almost good then I'll fix that. I've got a couple ideas churning so I think it'll be doable
I have working mirrored rotation but I am not happy about it. I unrotate the current and previous coords, mirror new coords and redo the rotation on new coords. It works, barring light stuttering and the occasional jump to a side then jump back. I'm sure there's a faster way to calculate it without unrotating it but I only took trig once in highschool :/ There is an if statement so performance should only suffer while rotated
@barrettMCW I just found this: https://stackoverflow.com/questions/63638347/inverting-the-y-axis, and the codesandbox link there is working. I wonder if that approach for flipping will address some of these issues you're seeing since OpenLayers may understand at a lower level that the image is inverted/mirrored?
I wouldn't worry about save Projection handling the flipped image - that seems like really edge case.
A couple of other places you may want to handle the mirrored state:
But these are not blockers, so it's up to you (could also come in a follow-up PR)
I remember seeing that codepen from way back when I started this, but I couldn't figure it out. I tried again, but no luck. I think the problem is that I'm doing addCoordinateTransforms(view.getProjection(), new Projection...) instead of (tiles.getProjection(), view.getProjection()), but I cannot find the OmeroImage or Ol3.Tile projections. If our image layer doesn't have a projection I'm not sure that method will work. Cache and Viewport links are good adds, as that's probably the mechanism I'll use for ground truth. I'll get started on that shortly
I think this is it. Cached settings works. ViewportAsUrl probably works. I didn't build and put it on a test server to test the url itself but it recognizes the index-dev.html initparams and builds a url with mirror parameters, which I imagine means it will work. I discovered rotating is a little wonky as well. I think it's much more manageable than the other things I correct for. I think we just leave it for now and I'll take another crack at mirroring with openlayers when I start base truth.
I think this is it. Cached settings works. ViewportAsUrl probably works. I didn't build and put it on a test server to test the url itself but it recognizes the index-dev.html initparams and builds a url with mirror parameters, which I imagine means it will work. I discovered rotating is a little wonky as well. I think it's much more manageable than the other things I correct for. I think we just leave it for now and I'll take another crack at mirroring with openlayers when I start base truth.
Thanks a lot. Testing on our server has shown that:
Testing with the dev build
enableMirror: "True"
initialFlipX: true
initialFlipY: null
and when built, with URL containing &fx=true&fy=false
enableMirror: "True"
initialFlipX: "true"
initialFlipY: "false"
So it looks like you need to convert the "true"
string into true
boolean
Sorry about that, using a string bool for initparam works. Thanks for your help guys!
Confirming that Get link to viewport
now works with the mirroring as expected.
Ready to merge fmpov.
@barrettMCW Apologies for the delay, but this has now been released in omero-iviewer 0.13.0
.
Thanks for your contribution!
We co-register our WSIs against MRIs. Being able to mirror the image is pretty important for that. I did a little CSS trickery then modified pixel values based off what way it's flipped. DragPan, ShapePopup, and Drawing in general work identically when mirrored. (as far as I can tell) Thanks