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

Foreground/background properties out of sync when changing modes #347

Open
Carifio24 opened this issue Feb 22, 2023 · 2 comments
Open

Foreground/background properties out of sync when changing modes #347

Carifio24 opened this issue Feb 22, 2023 · 2 comments

Comments

@Carifio24
Copy link
Member

@pkgw This issue is what we had briefly discussed yesterday. After looking at it again, I do think there's a problem (or at least some unintuitive behavior) here.

The issue is that the foreground and background member values can become out of sync with what's actually shown in the WWT viewer when switching modes. This is a particular problem with sky mode, though I don't think it's fundamentally restricted to that layer.

There's a video below that shows this in action, but the basic steps to reproduce are:

  • Create a WWT viewer (which will be in sky mode by default)
  • Switch the background and/or foreground to be different from the default
  • Switch to another mode
  • Switch back to sky mode

This will bring the viewer back to the default sky mode state, but the foreground and background member values will reflect what was chosen previously. If you want to restore your previous layer settings, you need to switch to something else, and then back to what you want. This is particularly noticeable if a user has a widget like the layer controls open. The reason for this is that the mode switching is internally adjusting WWT's foreground/background, but the Python-side properties don't know about that.

pywwt.mp4

I think there are probably a few different ways we could handle this, but as long as the Python properties are kept in sync with the WWT state I think it won't be confusing for a user.

@pkgw
Copy link
Contributor

pkgw commented Feb 27, 2023

Ah, I see what you mean. It sounds like this is actually a manifestation of a somewhat more serious architectural limitation that we have in pywwt right now. The pywwt code was basically written assuming that it fully controls the WWT instance, and that nothing happens in WWT without going through pywwt. This assumption always had a few potential issues, and they really get exposed in the JupyterLab "app" model where different clients might all be interacting with the same app.

To really fix this, the pywwt implementation would need some serious restructuring, and the messaging protocol would need to gain a lot of additional features to more thoroughly synchronize state between the app and clients like pywwt. For instance, when pywwt connects to the app, it assumes that no layers have been loaded, but if the app has already been running, there might already be various imagesets and tables loaded that in principle pywwt should learn about and expose on the Python side. Pywwt should also be prepared to update its list of known imagesets to match the app, which might learn about new ones if/when other clients ask it to load WTML files.

I think that this is how pywwt should ideally be operating, since I think the long-lived app model is really the right way to go with the overall UX in this area. But it would take some effort to really get all of this right. This particular issue might be a good place to move the ball forward, potentially.

Basically we need to deal with asynchronous state synchronization in some generic way. When pywwt starts up, it needs the app to tell it about all aspects of its state (at least, all aspects that pywwt cares about), like foreground/background, available imagesets, active layers, etc. And then whenever any of those aspects change, the app needs to notify pywwt, and pywwt needs to listen to those notifications. And all of this needs to be done knowing that messages are sent and received asynchronously and may even arrive out of order, get lost, etc. So it's a gnarly problem. Unfortunately I am not sure if there's a way to tackle this particular issue that isn't just making our lives more difficult for the time when we sit down to solve the bigger problem "for real".

@Carifio24
Copy link
Member Author

That's a really good point about the JupyterLab "app" context, and the fact that we are no longer expecting pywwt to "own" the WWT instance it's manipulating. I agree that it's not worth doing small band-aid fixes before thinking about how to really solve this problem.

FWIW, the original thing that led me to be looking into this was stuff related to this sort of state syncing in glue-wwt, since one can recreate exactly the same pattern as in the above video using the glue interface. glue-wwt has a pretty restricted use case: there one can reasonably assume that glue-wwt "owns" the WWT instance, and the foreground/background traitlets will definitely reference sky imagesets. For that sort of restricted context, this particular issue can be solved in a hacky way by explicitly calling the traitlet observers when the mode changes to sky mode. But that obviously doesn't work well in a more general context.

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

No branches or pull requests

2 participants