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

Source Info Tool Bar and Location Card #167

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

trautmane
Copy link

@trautmane trautmane commented Oct 26, 2023

Hi @tpietzsch and @axtimwalde ,

This is the current draft of the Source Info Tool Bar and Location Card UI additions I started working on in Brno. There is more to be done but I wanted to get feedback from you before I continued development.

Screenshot 2023-10-26 at 12 00 48 PM

Some key design questions I have are:

  1. Should I try to add some type of "smart-formatting" logic that considers the window size and the visible global coordinate range to determine what precision to display (e.g. the %n.mf format strings) in the toolbar and/or the editable location text fields? If so, what do you recommend?
  2. Does this division/combination of read-only toolbar with editable location card make sense? Anything else you'd like to see formatted/arranged differently?
  3. Is it possible to have a mix of 3D and 2D sources? If so, I need to address that. The "global" ViewerOptions.is2D implies to me you only do 2D or 3D for all sources.

Of course, I'm also happy to address any code suggestions you might have.
My hope is to iterate and improve things on this branch based upon your feedback.

Thanks!

In case it helps, I've been using this test case to review the UI changes.

@tpietzsch
Copy link
Member

Thanks a lot @trautmane, this looks great already!

Some key design questions I have are:

  1. Should I try to add some type of "smart-formatting" logic that considers the window size and the visible global coordinate range to determine what precision to display (e.g. the %n.mf format strings) in the toolbar and/or the editable location text fields? If so, what do you recommend?

I think this would be nice, but I would post-pone it. This can be done later, in the next step after this PR is merged.

Probably, it would involve finding the min/max of the union of all sources transformed into the world coordinate system. One can of zoom out further, such that this "filled world" only fills a few screen pixels, and in that situation, I would adapt the min/max to reflect that.

  1. Is it possible to have a mix of 3D and 2D sources? If so, I need to address that. The "global" ViewerOptions.is2D implies to me you only do 2D or 3D for all sources.

It is fine to assume, that everything is either 3D or 2D.

But... this made me think, what about other axis?

Programmatically, Sources are always 3D. "2D sources" are 3D sources with a Z size of 1. And ViewerOptions.is2D only sets up navigation actions such that one cannot move away from the Z=0 plane.

You can add "normal" 3D sources to a Viewer which is2D. You only see the Z=0 slice then, with no way to move to other slices with the UI (you can of course move to other slices by programmatically setting the viewer transform). Maybe it would be nice to be able to do this using the location card sliders? This would be already halfway to having a more "ImageJ-like" viewer behavior.

What about time? It would be cool to have a time field and slider in the location card, and also to support setting the timepoint by pasting in 4D coordinates.

Also, there have been several requests for adding a standard way to support sliders for higher dimensions of >4D sources. The location card could be a good place to add this. This change goes deeper, of course. Probably it involves modifying the Source interface to set higher dimension coordinates, and querying the number of dimensions, and extents in the higher dimensions. (@axtimwalde, maybe you have opinions/ideas there?)

What do you think?

  1. Does this division/combination of read-only toolbar with editable location card make sense? Anything else you'd like to see formatted/arranged differently?

I have not looked at the code much yet, but I will do this ASAP. I have run the GridViewerTest and played with the UI.

  • I think the toolbar/card split makes sense.
  • When I start BDV (e.g., 'GridViewerTest'), I get an exception:
    java.lang.NullPointerException
      at bdv.ui.appearance.AppearanceIO$AppearanceConstructor$ConstructAppearance.construct(AppearanceIO.java:182)
      ...
    Error while reading appearance settings file /Users/pietzsch/Library/Application   Support/sc.fiji.bigdataviewer/appearance.yaml. Using defaults.
    
    This happens because existing appearance.yaml config is missing the "showSourceInfoToolBar" key. This should be make backwards compatible. Probably you just need to set a default value when the "showSourceInfoToolBar" key is missing in bdv.ui.appearance.AppearanceIO$AppearanceConstructor$ConstructAppearance.construct.
  • When starting BDV, the button in the location toolbar has the focus. So the BDV shortcuts don't work until you click into the viewer canvas. This should be avoided. Probably by making the toolbar and button not focusable.
  • The connection between sliders and viewer panel is glitchy. I don't know what causes this, I'll try to find out when I dig into the code. In general, I think the viewer transform should be updated continuously when dragging a slider, instead of just at the end when the slider is released. Another option would be to have no sliders, just the fields. But I like the sliders for the possibility to include time and higher dimensions.
    I had some situations where:
    • the sliders do not update when changing the viewer transform with the mouse,
    • the viewer transform does not update when the sliders are changed,
    • editing the viewer transform with the mouse doesn't work anymore.
  • I don't like that the "Sources" and "Groups" cards are always reset to collapsed when showing the side panel through clicking the edit symbol. I guess the motivation for this is to make sure the "Locations" panel is visible. We should add functionality to the side-panel to make sure that a particular card is visible (or use that functionality if it exists already -- I don't remember whether there is something like that).
  • What is the idea behind slider ranges? It looks like you adapt it according to the interval of the current source. This allows to basically focus each "corner" of the source in the center of the screen. E.g., in the GridViewerTest like this:
    00
    10
    01
    However, note that the interval is in source coordinates, while the location is in world coordinates. Each Source has a transformation into world space (for example, angles with lightsheet data). I pushed a modified GridViewerTest where this is illustrated. The source interval doesn't make sense anymore for the sliders in that situation.
    A possibility is to always use the min/max of the union of all sources transformed into the world coordinate system. Or use the min.max of the current source transformed into the world coordinate system.

Of course, I'm also happy to address any code suggestions you might have. My hope is to iterate and improve things on this branch based upon your feedback.

That sounds good. Please let me know your thoughts about the points above!

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.

2 participants