wayland: Add support for the session management protocol #10883
Conversation
Add session management protocol support for saving/restoring toplevel window state across runs.
| if (session_id && *session_id == '\0') { | ||
| session_id = NULL; | ||
| } | ||
| data->xdg_session_v1 = xdg_session_manager_v1_get_session(data->xdg_session_manager_v1, XDG_SESSION_MANAGER_V1_REASON_LAUNCH, session_id); |
There was a problem hiding this comment.
I imagine you want to change the reason based on whether we have a session id or not.
If we have an existing session ID, we're in reason = session_restore
There was a problem hiding this comment.
In almost all cases though, we're creating a session due to being launched normally. The 'restore' reason seems to be somewhat ambiguous in the spec:
A app instance is restored, for example part of a restored session, or restored from having been temporarily terminated due to resource constraints.
My interpretation of this was that it should be used if recovering from some kind of non-fatal abnormal behavior, such as if an app can reinitialize to gracefully recover from an out-of-memory or GPU device lost scenario (versus 'recover' in the case of a hard crash).
| } | ||
| if (window_id) { | ||
| data->window_id = SDL_strdup(window_id); | ||
| Wayland_CreateSession(c); |
There was a problem hiding this comment.
This seems the wrong place. We can have N windows in a session.
We want this somewhere global in SDL_WaylandVideo.
There was a problem hiding this comment.
This just defers the creation of the session object until a window with a valid ID actually requires it. This removes the requirement that the client set the session ID property before initializing the video subsystem, as clients tend to just initialize all of their required subsystems at once during early initialization, and it may need the IO subsystem, or others, initialized to retrieve the serialized session ID string from storage.
|
|
||
| static void handle_xdg_session_replaced(void *data, struct xdg_session_v1 *xdg_session_v1) | ||
| { | ||
| // NOP |
There was a problem hiding this comment.
SDL_SetStringProperty(SDL_GetGlobalProperties(), SDL_PROP_GLOBAL_VIDEO_WAYLAND_SESSION_ID_STRING, "");
As we're going to have local data storage too based on the session ID string, if we're replaced, we don't want to overwrite our local data storage with data from the now-replaced client.
There was a problem hiding this comment.
Yes, any existing objects associated with the session also need to be destroyed here as well, as the spec states that they become inert if this happens.
There was a problem hiding this comment.
They should. But if they're inert it means the compositor has to ignore it, so clients should be able to just do whatever is easiest for them.
|
The restriction regarding allowing a toplevel to only be added to a session before the first commit to the underlying wl_surface is a problem, as SDL destroys and recreates the Wayland toplevel objects when hiding/showing windows, while the surface persists for the life of the SDL_Window object. I raised the issue in the protocol discussion. Other than that, the implementation seems to work on the experimental GNOME implementation with some small fixes. |
Add session management protocol support for saving/restoring toplevel window state across runs.
Draft pull for https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/18