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

marker for specviz coordinate mouseover #8

Closed

Conversation

kecnry
Copy link

@kecnry kecnry commented Dec 29, 2022

This is the most basic implementation of a marker in the spectrum viewer that indicates the position of the coordinates display. Before merging, we may want to consider some way to disable this (or disable when other tools - ie zoom or subset creation - are active).

At the least, this might help to test and debug behavior of spacetelescope#1894.

Screen.Recording.2022-12-30.at.8.47.19.AM.mov
Screen.Recording.2022-12-30.at.8.48.50.AM.mov

TODO:

  • option to disable or automatically disable when other tools are active in the viewer?

@pllim
Copy link
Owner

pllim commented Dec 29, 2022

FYI -- I rebased your PR on top of my new changes over at spacetelescope#1894 . Please double check the diff. Thanks!

@kecnry
Copy link
Author

kecnry commented Dec 30, 2022

Diff still looks good, feel free to keep rebasing until we decide whether to adopt this or not. I also updated the description with videos that show the behavior for multiple spectra and support for as_steps.

@kecnry
Copy link
Author

kecnry commented Dec 30, 2022

The latest commit tweaks the logic to hide both the marker and the actual spectrum information when a tool is active (with the exception of any "default tool" like slice in cubeviz). I think this provides more useful information when trying to draw a region for a spectral subset or zoom region and also minimizes the annoyance of the marker in these cases. It wouldn't be hard to put a toggle somewhere for the marker as well, but I'm not sure it makes sense to show that as a tool since that would then conflict with the logic of the slice default tool in cubeviz 🤔

Screen.Recording.2022-12-30.at.9.54.46.AM.mov

@pllim
Copy link
Owner

pllim commented Dec 30, 2022

Can't we just hide the marker but leave the coord info alone? I think it is quite confusing the format change suddenly when you select subset.

@kecnry
Copy link
Author

kecnry commented Dec 30, 2022

Can't we just hide the marker but leave the coord info alone? I think it is quite confusing the format change suddenly when you select subset.

We can, but I thought this was more useful - happy to hear what everyone else thinks though! My thinking was that if I'm drawing a range I want to know the exact coordinates of the mouse, as that is what gets applied to the box zoom or spectral subset - but I likely no longer care about inspecting the closest point in the nearest spectrum layer. And I think without the marker, the closest point information can be deceptive 🤷

We might want to reformat the case during drawing so that it still shows wave/flux instead of pixel, but still corresponding to the exact mouse position (which is indicated by the layer icon and marker disappearing).

* only used in spectrum profile viewers
* currently ALWAYS on
(or default tool - slice in cubeviz, for example)
@kecnry kecnry force-pushed the specviz-mouseover-marker branch from d7dd13f to eb24ed4 Compare December 30, 2022 18:24
@Jenneh
Copy link

Jenneh commented Jan 3, 2023

The marker seems to bounce around a lot. If zoomed in, does it appear to move more smoothly over the line?

@pllim
Copy link
Owner

pllim commented Jan 3, 2023

Probably the same problem in Imviz where event x/y inaccurate when zoomed out a lot. I don't think that's something we can ever fix, so what about disabling marker unless zoomed in at a certain level or more?

@kecnry
Copy link
Author

kecnry commented Jan 3, 2023

The marker seems to bounce around a lot. If zoomed in, does it appear to move more smoothly over the line?

I don't think that's something we can ever fix, so what about disabling marker unless zoomed in at a certain level or more?

The marker will show whatever information is shown in the mouseover display, which by specifications is the location of the nearest data point, and so I think the "bouncing" is intentional. If that is visually distracting and we want to allow to disable the visual feedback, we can do that, but we are intentionally not interpolating so it moves smoothly along the line.

I also intentionally showed screen recordings where it switches back and forth between the two different spectra.... we could perhaps add more logic so that it is more "resistant to change", but I worry that might just feel confusing and really only happens in this edge case where you intentionally mouseover near the midway point of two spectra.

@Jenneh
Copy link

Jenneh commented Jan 3, 2023

It seems like it is more useful to keep even with the bouncing around. But a scientist would probably have a stronger opinion than me.

@kecnry
Copy link
Author

kecnry commented Jan 3, 2023

It sounds like the consensus from POs is that pros of markers outweigh the cons. I will be taking over the work on spacetelescope#1894, but do not have merge permission here, so will close this PR without merging and push this and any follow-up improvements to spacetelescope#1894 directly. Once that PR is merged, we can later iterate on options to manually select the "active layer" and toggle the visibility of the marker.

@kecnry kecnry closed this Jan 3, 2023
@kecnry kecnry deleted the specviz-mouseover-marker branch January 3, 2023 19:28
@pllim
Copy link
Owner

pllim commented Jan 4, 2023

Sounds good. Thanks!

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.

3 participants