[Snowcap] Server-side object handle feedback

[Ottatop]

With the work in #414, we're going to need to expose handles to server-side objects. The tasklist (and perhaps decorations) will need some form of ForeignToplevelHandle, and workspaces will need ExtWorkspaceHandles, names up for bikeshedding.

Currently, Pinnacle uses a pull-based API for all handles, with signals providing push-based information updates. I am considering a more push-based event stream to implement Snowcap handles with an eye towards maybe moving the Pinnacle API to something similar. This would be similar to event streams implemented in niri and hyprland—a single stream from which all object creation, update, and destruction events flow through. Currently with Pinnacle there are a bunch of different RPCs for single properties and I believe consolidating everything under a single source of truth is better long-term.

To implement this, I propose a single EventStream RPC:

message Event {
    oneof event {
        ForeignToplevelCreated foreign_toplevel_created = 1;
        ForeignToplevelUpdated foreign_toplevel_updated = 2;
        ForeignToplevelDestroyed foreign_toplevel_destroyed = 3;
        // the other stuff
    }
}

message EventStreamRequest {}
message EventStreamResponse {
    repeated Event events = 1;
}

service EventService {
    rpc EventStream(EventStreamRequest) returns (stream EventStreamResponse);
}

Each handle has its own create, update, and destroy messages.

However, there are a few questions that need answering:

1. What exactly should those messages look like?

   A rough translation of the create, update, and destroy events initially implemented in Snowcap: Add WlrTaskList support #415 is this:

   message ForeignToplevelCreated {
       string identifier = 1;
   
       optional string app_id = 1;
       optional string title = 2;
       bool maximized = 3;
       bool minimized = 4;
       bool activated = 5;
       bool fullscreen = 6;
       repeated string output_names = 7;
       optional string parent_identifier = 8;
   }
   message ForeignToplevelUpdated {
       string identifier = 1;
   
       optional string app_id = 1;
       optional string title = 2;
       optional bool maximized = 3;
       optional bool minimized = 4;
       optional bool activated = 5;
       optional bool fullscreen = 6;
       optional string output_name_entered = 7;
       optional string output_name_left = 8;
       oneof parent {
           string parent_identifier = 9;
           google.protobuf.Empty unset = 10;
       }
   }
   message ForeignToplevelDestroyed {
       string identifier = 1;
   }

   This makes the update message look a little funky, especially when dealing with setting explicit nulls, but this isn't a dealbreaker.

   We could also do a more Wayland-style approach and have Created just supply the identifier and split all of the fields of Update into their own message. I'm not sure how good of an idea that is.

2. I'm not sure if we should also roll in all the other stream-type RPCs we have like widget events and the recently-merged layer and popup events.

@Ph4ntomas What do you think?

[Ph4ntomas]

Just a quick answer with my first thought

names up for bikeshedding.

I like the idea of keeping wayland names :)

I am considering a more push-based event stream to implement Snowcap handles with an eye towards maybe moving the Pinnacle API to something similar. [...] I believe consolidating everything under a single source of truth is better long-term

To implement this, I propose a single EventStream RPC:

I'm all for a push based API, since it's makes it easy to track lifetime & perform cleanup.

I'm not sure however about a single stream, and the code snippet seems to point in that direction. More on that below.

We could also do a more Wayland-style approach and have Created just supply the identifier and split all of the fields of Update into their own message. I'm not sure how good of an idea that is.

I think both approach have their pros & cons.

The issue I see with Wayland-style events is that some protocols have explicit synchronization messages to have atomic behavior (e.g. frame or done) which may or may not come with a 'cancel' event or some other requirement like the need to discard changes if the object is destroyed. These synchronization message may also be coming from other protocol altogether (e.g. https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/5c575b6fc49525603d08d5ac67f5e80f607d8839/staging/ext-foreign-toplevel-state/ext-foreign-toplevel-state-v1.xml#L158-170).
IMO this complexity should be handled server-side instead of needing to deal with it explicitly for every object that want to deal with a particular event.

IMO as funky as it is, bunching updates together enforce proper behavior from the config and remove the need for client to essentially re-implement a protocol handler.
That being said, since this is essentially protocol-dependent, we could just have for a policy that we proxy wayland messages if the protocol doesn't care for atomicity, and have synthetic updates events otherwise, but I feel it's a bad idea to have different way of solving the same issue.

I'm not sure if we should also roll in all the other stream-type RPCs we have like widget events and the recently-merged layer and popup events.

This come back to the remark regarding single v multiple streams for user-facing API. With #430 we're using the widget event stream closing as a signal for the compositor discarding the surface. It's perfectly fine to rely on an explicit close event, but I'd rather keep that event as an implementation details instead of expecting the user to actually handle the event and call some cleanup-code.
We can however send a synthetic signal/event so clients can deal with its own cleanup, or even better provide a way to register a cleanup callback.

---

I have one remark with the current RPC:

message EventStreamRequest {}
message EventStreamResponse {
    repeated Event events = 1;
}

service EventService {
    rpc EventStream(EventStreamRequest) returns (stream EventStreamResponse);
}

With the current design, I understand the config will have only one active stream that will always receive all events, regardless of whether they will be used. Even if we have a single stream, I think it would be best to have a way to request specific events we're interested in so the config isn't spammed needlessly.

This proposal doesn't (yet) address how to deal with the initial state for a given handle. For example, WlrForeignToplevel will send Created event only once per Manager, but the config may want these events more than once (e.g. I want a tasklist per output for the duration of the session, but I may need some transient tasklists to implement alt-tab behavior with preview using set_rectangle).

The current design seems to imply it's up to the config-side to store the state for every possible protocol, regardless of whether it's currently used (I think that was the idea behind the Source trait).
I feel this is "wasteful" since the server will need to keep that data around to call methods on wayland objects and to send the initial state when the event stream is open the first time. Since we'll need the state server-side anyway, I feel it should be the responsibility of the Server to maintain the state an re-send synthetic created events for the initial state for objects.

[Ph4ntomas]

Okay I have a feeling I misunderstood what the EventStream is meant to be, but I'm not 100% sure :)

Is the idea to have a single RPC to create the stream, but then one stream per handle?
With the flow being something like the following (for WlrForeignToplevelHandle):

1. API receives the handle identifier from a handle to wlr_foreign_toplevel::Manager via its internal stream.
2. API initialize the wlr_foreign_toplevel::Handle:
   a. store the identifier inside the newly created object
   b. create a new stream for that handle
   c. wait for the initial state
3. API sends wlr_foreign_toplevel::Handle to user-code via a (user-facing) event/signal.

If that's the case we'll want the EventStreamRequest to have a bit more parameters for filtering.

If the idea is to have a single event stream my initial comment mostly address that :)

[Ottatop]

If the idea is to have a single event stream my initial comment mostly address that :)

Yea it's a single event stream

The issue I see with Wayland-style events is that some protocols have explicit synchronization messages to have atomic behavior (e.g. frame or done) which may or may not come with a 'cancel' event or some other requirement like the need to discard changes if the object is destroyed. These synchronization message may also be coming from other protocol altogether (e.g. https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/5c575b6fc49525603d08d5ac67f5e80f607d8839/staging/ext-foreign-toplevel-state/ext-foreign-toplevel-state-v1.xml#L158-170).
IMO this complexity should be handled server-side instead of needing to deal with it explicitly for every object that want to deal with a particular event.

IMO as funky as it is, bunching updates together enforce proper behavior from the config and remove the need for client to essentially re-implement a protocol handler.
That being said, since this is essentially protocol-dependent, we could just have for a policy that we proxy wayland messages if the protocol doesn't care for atomicity, and have synthetic updates events otherwise, but I feel it's a bad idea to have different way of solving the same issue.

We can supply atomicity by batch sending all of the separate messages within one single EventStreamResponse and treating it as if it has an implicit done event. If the compositor gets a cancel event/the object is destroyed we don't have to send any events that were "pending".

This come back to the remark regarding single v multiple streams for user-facing API. With #430 we're using the widget event stream closing as a signal for the compositor discarding the surface. It's perfectly fine to rely on an explicit close event, but I'd rather keep that event as an implementation details instead of expecting the user to actually handle the event and call some cleanup-code.
We can however send a synthetic signal/event so clients can deal with its own cleanup, or even better provide a way to register a cleanup callback.

Maybe I'm not understanding this correctly, but whether the Closing signal gets emitted on stream drop or from a close event is the implementation detail, isn't it? I'm not sure what this "synthetic signal/event" is supposed to be for.

With the current design, I understand the config will have only one active stream that will always receive all events, regardless of whether they will be used. Even if we have a single stream, I think it would be best to have a way to request specific events we're interested in so the config isn't spammed needlessly.

I think for now it's easier to just send everything unconditionally. If we determine that this is a problem we can do a backward-compatible change to add event toggling.

This proposal doesn't (yet) address how to deal with the initial state for a given handle. For example, WlrForeignToplevel will send Created event only once per Manager, but the config may want these events more than once (e.g. I want a tasklist per output for the duration of the session, but I may need some transient tasklists to implement alt-tab behavior with preview using set_rectangle).

The current design seems to imply it's up to the config-side to store the state for every possible protocol, regardless of whether it's currently used (I think that was the idea behind the Source trait).
I feel this is "wasteful" since the server will need to keep that data around to call methods on wayland objects and to send the initial state when the event stream is open the first time. Since we'll need the state server-side anyway, I feel it should be the responsibility of the Server to maintain the state an re-send synthetic created events for the initial state for objects.

Hm, re-sending the created events to bring a new object up to speed is pretty much just a getter for the current state. I don't know how complicated it would be to support "replaying" created events nor if making a straight-up getter works without missing updates coming from the stream. My original idea was, as you said, storing handles and state config-side, where getting the current state is a simple clone of that stored state.

---

Given that either way the config is going to store some data, I think I'm going to move forward with keeping state config-side. I'm also going to keep the event stream to just handles for now, other stuff can be rolled in if desired. In the interest of encoding the required initial state of a handle in the protobuf type itself, let's go with the initial Created message I talked about, but I'm probably going to split the Update message so there aren't multiple ways to batch updates together (both within a message and within a response).

[Ottatop]

but I'm probably going to split the Update message so there aren't multiple ways to batch updates together (both within a message and within a response).

Actually something like

message ForeignToplevelUpdated {
    message AppId { string app_id = 1; }
    message Title { string title = 1; }
    message Parent { optional parent_id = 1; }

    oneof property {
        AppId app_id = 1;
        Title title = 2;
        Parent parent = 3;
    }
}

is probably better

[Ph4ntomas]

We can supply atomicity by batch sending all of the separate messages within one single EventStreamResponse and treating it as if it has an implicit done event. If the compositor gets a cancel event/the object is destroyed we don't have to send any events that were "pending".

Actually something like

[...]

is probably better

Yeah that make sense, and is way easier to read than the merged update

[Ph4ntomas]

This come back to the remark regarding single v multiple streams for user-facing API. With #430 we're using the widget event stream closing as a signal for the compositor discarding the surface. It's perfectly fine to rely on an explicit close event, but I'd rather keep that event as an implementation details instead of expecting the user to actually handle the event and call some cleanup-code.
We can however send a synthetic signal/event so clients can deal with its own cleanup, or even better provide a way to register a cleanup callback.

Maybe I'm not understanding this correctly, but whether the Closing signal gets emitted on stream drop or from a close event is the implementation detail, isn't it?

Yes, my point was that dealing with the close event itself should not be up to the user-code (i.e. snowcap-api does its cleanup regardless of what the user does.

I'm not sure what this "synthetic signal/event" is supposed to be for.

This is what we do for surface. We're emitting a Closing event even though we never technically got one. The user is free to ignore that event, but the surface internal state (i.e. the program) is dropped by the API.

[Ph4ntomas]

Hm, re-sending the created events to bring a new object up to speed is pretty much just a getter for the current state. I don't know how complicated it would be to support "replaying" created events nor if making a straight-up getter works without missing updates coming from the stream. My original idea was, as you said, storing handles and state config-side, where getting the current state is a simple clone of that stored state.

I was speaking of sending the server-side state as a Created event. This is already what should be done, since:

• the config start asynchronously so the server can't just send event before the EventStream RPC is called.
• the Created event contains complete object i.e. past the initial done message that mark the end of configuration.

A getter would just be a mess to deal with IMO since that would cause a lot of synchronization issues.

[Ph4ntomas]

I think for now it's easier to just send everything unconditionally. If we determine that this is a problem we can do a backward-compatible change to add event toggling.

FWIW, this means we'll have at least 3 instances of the same data (compositor-side, snowcap-server side & config side). I know it's only once more than what you get if you run e.g. waybar, but since the event-stream and snowcap will be set-up on config start everything will always be loaded and stored.

Could we maybe just keep the logic of sending everything unconditionally, but delay when wayland globals are bound by snowcap ? E.g have an additional RPC

message ServiceStartRequest {
 // some enum to identify the service we want to start.
}
message ServiceStartResponse {}

service EventService {
  rpc StartService(ServiceStartRequest) returns (ServiceStartResponse);
}

Server-side, the implementation just bind to the corresponding global, then the services start sending every event through the (already open) EventStream.

This keep the logic simple, and avoid having to deal with the initial state, too

[Ottatop]

I was speaking of sending the server-side state as a Created event. This is already what should be done, since:

• the config start asynchronously so the server can't just send event before the EventStream RPC is called.
• the Created event contains complete object i.e. past the initial done message that mark the end of configuration.

A getter would just be a mess to deal with IMO since that would cause a lot of synchronization issues.

Yes, sending an initial flood of created events to synchronize new clients with the current server state is the plan. From your messages before and after this I think your view is to have individual sources/objects handling stream events? That way the config doesn't have to maintain a master copy of the server state? As I mentioned, this then requires additional floods of created events to every new object that wants the current server state, which is sort of like a getter and would require a request from the config. And yes, one of my concerns is that an actual getter doesn't synchronize with the event stream.

Could we maybe just keep the logic of sending everything unconditionally, but delay when wayland globals are bound by snowcap ? E.g have an additional RPC

message ServiceStartRequest {
 // some enum to identify the service we want to start.
}
message ServiceStartResponse {}

service EventService {
  rpc StartService(ServiceStartRequest) returns (ServiceStartResponse);
}

Server-side, the implementation just bind to the corresponding global, then the services start sending every event through the (already open) EventStream.

This keep the logic simple, and avoid having to deal with the initial state, too

Assuming I'm correct about your views above, this only simplifies the first object that wants the initial state, no? Every object after that still has the same issue. This is why I want to have a config-side master copy of the server state.

But also I might be totally wrong in which case ignore everything I just said

[Ph4ntomas]

From your messages before and after this I think your view is to have individual sources/objects handling stream events? > That way the config doesn't have to maintain a master copy of the server state?

Well there are several concerns overall:

• I don't like the config being spammed since I'd like to have pinnacle run on some low-power devices (c.f. the e-paper tablet I'm using to implement touch support on discord dev channel).
• I don't like the config needing to hold some state for objects no plugins will ever use for the same reason (I had to rebuild the kernel for said tablet to be able to compile pinnacle, because there wasn't enough memory without ZRAM. The memory is that scarce).
• I'm not a huge fan of duplicating state between server-side and config.

OTOH, you mentioned the config will have to hold some state anyway (which I may or may not have overlooked before ...), and I do agree it's better to hold a single instance per server-side objects. The last message was actually with that in mind.

Assuming I'm correct about your views above, this only simplifies the first object that wants the initial state, no? Every object after that still has the same issue. This is why I want to have a config-side master copy of the server state.

But also I might be totally wrong in which case ignore everything I just said

Delaying when global are bounds means the config doesn't need to keep track of stuff that won't be needed, and it remove the need for some code just to send the initial state when the stream is created since we can ensure it will always be created before handlers.

Instead of having them all handler on startup and sending events as soon as the event-stream is available, my proposal is to require the config to create the event-stream and then lazily add the handlers to calloop if/when the config request them.

[Ottatop]

• I don't like the config being spammed since I'd like to have pinnacle run on some low-power devices (c.f. the e-paper tablet I'm using to implement touch support on discord dev channel).

I mean, realistically how many updates are we going to get from a workspace or foreign toplevel? I'd assume the event stream for this would look fairly sparse, it's not like we're sending pointer events or the like.

• I don't like the config needing to hold some state for objects no plugins will ever use for the same reason (I had to rebuild the kernel for said tablet to be able to compile pinnacle, because there wasn't enough memory without ZRAM. The memory is that scarce).

In a similar vein, realistically how much memory are workspace and foreign toplevel objects going to take? A foreign toplevel is what, 2 strings, a list of output ids, an array of bools for state, and a parent id? That's not much memory. At even a large 100 bytes per toplevel and 50 open windows, that's only 5 kilobytes. If we were talking about an nrf chip or something with only 256 kilobytes of RAM, then yes this is an issue. But if it can compile Pinnacle, even with kernel mods, then it probably has on the order of gigabytes of RAM i.e. enough RAM for it not to make a noticeable difference. The decorations on a window alone are on the order of megabytes, scaling with the window size (yes I'd like to figure out a solution for that).

• I'm not a huge fan of duplicating state between server-side and config.

This is fundamentally incompatible with a purely push-based API. If you can't store the state the server gives you, the only other way to get that state is by pulling it from the server.

Delaying when global are bounds means the config doesn't need to keep track of stuff that won't be needed, and it remove the need for some code just to send the initial state when the stream is created since we can ensure it will always be created before handlers.

Yes, this removes logic for sending the initial state for the first client. But what happens when another client is run, when the global is already bound? There still needs to be code in place to send that initial state. Pinnacle is, for the most part, built to handle multiple config clients, and I'd like Snowcap to be similar. Reloading the config is the same: the global will have already been bound by the time the config restarts.

Instead of having them all handler on startup and sending events as soon as the event-stream is available, my proposal is to require the config to create the event-stream and then lazily add the handlers to calloop if/when the config request them.

I would prefer to benchmark and performance test before doing this. We would also have to figure out when to actually add the handler. Is it when the user calls something that wants to retrieve some handles? Then it now needs to update the event stream and then wait for a response before continuing. That's added complexity and I don't want to do that for minute gains.

[Ph4ntomas]

Yes, this removes logic for sending the initial state for the first client. But what happens when another client is run, when the global is already bound? There still needs to be code in place to send that initial state. Pinnacle is, for the most part, built to handle multiple config clients, and I'd like Snowcap to be similar. Reloading the config is the same: the global will have already been bound by the time the config restarts.

Okay, I did not think about the config restarting. I'm not sure it's a good idea to have multiple clients right now since there's no code server-side to handle that and they end up overriding each other configs but if it's the general direction you want to follow, it makes sense too.

We would also have to figure out when to actually add the handler. Is it when the user calls something that wants to retrieve some handles? Then it now needs to update the event stream and then wait for a response before continuing.

That was my idea yes, although I don't feel it represent that much added complexity.

You still need to deal with the following:

• the config wants a handle before the server sent the initial create event
• the initial create event never arrives because the compositor doesn't support the given protocol (once snowcap is truly standalone)
• the config receives a bunch of update before it get a Created event (in case of multi-client or a restart)

I'd argue it's less complex with service on demand because you can have a clear sync point:

1. When a new service is needed that the client doesn't have, the client send a start service. The return value of this start service tells the client whether the service is supported at all.
   If it isn't the first service config, the server knows it need to send some Created events.
2. The client now wait for the RPC to complete and Created events:
   2a. If it's supported, the client knows it should receive a Created events with the initial state any-time
   or
   2b. The client receive the initial Created events as part of the answer, and receive update on the stream.

I would go with 2a tho, because it's easy for the client to deal with state synchronization (just drop events you don't care about/until you get a created event) and it's easier for the server to deal with ordering, so we can ensure any update received before the Created event can safely be dropped since they are redundant.

---

Regarding performance & memory gain, if we only have to deal with workspaces and toplevels, it's marginal.

I'm a bit more concerned about adding in stuff like D-Bus protocols (assuming you still plan to have it server-side as you mentioned in #403, but IMO that has more cons than pros) since those can become chatty.
EDIT: I initially mentioned surface/layer events, but that's only a concern for multi-clients. Since it can easily be avoided (by not doing multi-client), I'm removing it.

But alright, we'll get there when we get there.

[Ottatop]

• the config wants a handle before the server sent the initial create event

From the config's viewpoint the handle doesn't exist until it gets a create event; there's nothing to "want".

• the initial create event never arrives because the compositor doesn't support the given protocol (once snowcap is truly standalone)

I don't see what there is to deal with here; if the compositor doesn't support a protocol Snowcap just isn't going to send any events related to it.

• the config receives a bunch of update before it get a Created event (in case of multi-client or a restart)

I don't see how this can happen. When a client connects a new event stream is created, and when it is Snowcap can send created events for all current server objects before any updates. The event stream isn't a transparent proxy for Wayland events, we have the ability to buffer and order things the way we want.

I'd argue it's less complex with service on demand because you can have a clear sync point:

1. When a new service is needed that the client doesn't have, the client send a start service. The return value of this start service tells the client whether the service is supported at all.
   If it isn't the first service config, the server knows it need to send some Created events.

I'd argue this is more complex because we already have to support clients after the first one, and implementing this just means there is now special infrastructure in place for the initial client. Though there is something to be said about capabilities and how the config should work when we don't have a specific protocol. I don't know how that's gonna look.

[Ph4ntomas]

From the config's viewpoint the handle doesn't exist until it gets a create event; there's nothing to "want".

From snowcap API that may be true, not necessarily from the config viewpoint. As an example, a worskpace widget will
"want" a handle to the group containing the output it's on. Even if events are push-based, user-defined Program will still need to get handles before they can be created. It's possible to get those at runtime, but then we're just offloading the complexity to the user code.

I don't see what there is to deal with here; if the compositor doesn't support a protocol Snowcap just isn't going to send any events related to it.

There are also protocol where you need to actively request an object to the protocol's global before receiving any event (e.g. ext-data-control-v1). I guess we can expose the global via a Created event and a special handle in that case.

I don't see how this can happen. When a client connects a new event stream is created, and when it is Snowcap can send created events for all current server objects before any updates.

If that can be done atomically, it's fine. If not, the client could still get some events before the initial state that are being broadcasted to already connected clients.

The event stream isn't a transparent proxy for Wayland events, we have the ability to buffer and order things the way we want.

Oh yeah, I know that.

I'd argue this is more complex because we already have to support clients after the first one, and implementing this just means there is now special infrastructure in place for the initial client.

The special infrastructure I had in mind (server side) was a simple option to be able to load services on demand, nothing fancy.

With or without that, you still need to deal with sending Created to all connected clients after the initial configuration, then once per new clients. The main difference might be that you could track which clients requested to receive a fresh state, but we could just as well broadcast the Created event and let the clients drop them if they don't care about it.

[Ottatop]

From snowcap API that may be true, not necessarily from the config viewpoint. As an example, a worskpace widget will
"want" a handle to the group containing the output it's on. Even if events are push-based, user-defined Program will still need to get handles before they can be created. It's possible to get those at runtime, but then we're just offloading the complexity to the user code.

I'd argue that this is the wrong direction to think about this. In your example the workspace widget assumes that an output will have a workspace group ready to pull. This should be framed the other way around: workspace groups are pushed to a Snowcap config and it will create a workspace widget once that happens. Alternatively, a workspace widget is created eagerly on an output and doesn't display anything until it receives a workspace group that has entered that output.

The special infrastructure I had in mind (server side) was a simple option to be able to load services on demand, nothing fancy.

It is easy to throw the word "simple" around before implementation, but from what I am picturing this now requires plumbing to start services along with server-side storage and checks on what services are active, as well as config-side logic to actually start those services, on top of all the stuff we already have to do to handle new clients.

But perhaps I am overblowing the complexity of this and maybe it's not as bad as I thought. Don't get me wrong, this is objectively more complex considering either way we still have to implement new client handling, but I'm going to attempt to implement some form of delayed service starting and see how it goes. But none of those delayed global shenanigans, that just doesn't seem helpful in any way.