Kitware / UPennContrast

UPenn ?
https://upenn-contrast.netlify.com/
Apache License 2.0
8 stars 6 forks source link

Snapshot #664

Closed bruyeret closed 4 months ago

bruyeret commented 4 months ago

This pull request fixes and improve snapshots

Close #650 Close #640 Close #623 Close #615

The way that "screenshots" are made have been reworked The ImageOverview component (the minimap) has changed thanks to some code factorization, and this fixes some issues with Z max merge, but the scale of the minimap is now wrong I have tried a lot of different things but nothing worked I now use the endpoint ${apiRoot}/item/${itemId}/tiles/region instead of ${apiRoot}/item/${itemId}/tiles/zxy/{z}/{x}/{y} and this is likely the reason why scaling is messed up

We could disable the minimap for now, merge this and wait for @manthey to come back to give us some advice What do you think @arjunrajlab?

arjunrajlab commented 4 months ago

I have also noticed, even before this PR, that the little square in the minimap doesn't move around properly anymore. Like, if you click and drag the square in the minimap, the image moves but the square doesn't move.

bruyeret commented 4 months ago

Oh yes I noticed that too, it should be fixed by this PR

arjunrajlab commented 4 months ago

Also, I tried the Z-merge snapshot feature, but that didn't seem to work properly.

arjunrajlab commented 4 months ago

Download all does work nicely, though!

bruyeret commented 4 months ago

@arjunrajlab is it better now? Dataset name is shown and the snapshots already saved the layers but didn't apply them when being loaded

arjunrajlab commented 4 months ago

Actually, could you enable the minimap, even if it is not quite right? I think people might like to have it as a way to get a general sense of where they are in the image.

bruyeret commented 4 months ago

I put it back! If you are ok, I can merge right now 😄

arjunrajlab commented 4 months ago

Yes, let's merge, thank you!