search

flatpak / libportal

libportal - Flatpak portal library
https://libportal.org
GNU Lesser General Public License v3.0
82 stars 40 forks source link

Subclass RemoteDesktop/ScreenCast sessions from XdpSession #102

Closed whot closed 2 years ago

whot commented 2 years ago

XdpSession is currently a mixed struct, comprising of both the bits required for a Session in general and the bits required for a ScreenCast session and/or a RemoteDesktop Session (the latter is usually a ScreenCast session too but does not have to be).

This works though only because we effectively only have one real subtype. With InputCapture using sessions as well we're getting namespace crowding and collisions. Fix this by switching XdpSession to be a derivable object and then subclass XdpScreenCastSession from it and then XdpRemoteDesktopSession from that.

API-wise nothing changes, the returned objects are now of the right subclass but the API itself remains unchanged. Ideally we could deprecate the current ones and switch the others to "type-safe" APIs but that's mostly busywork and I can't come up with a non-clashing nice enough name for the create_... calls.

cc @jadahl

whot commented 2 years ago

Any chance we get can get this merged? I have other branches depending on this (inputcapture in particular)

whot commented 2 years ago

For the screencast/remote desktop case, this seems doable, but what do we do about other "ad hoc" capabilities, would such be added?

I would argue that a more future-proof approach would be session bundling rather than just extending existing session implementations. SC/RD can be grandfathered in because they're too intertwined.

I'd argue for that primarily because it'll become impossible to figure out the right combinations of interactions - iirc there's one call in screencast/remotedesktop that can switch the session from one to the other, but that call needs to happen at the right session state. In the Clipboard case here, does Start affect what you can do? If not, then they're independent, if so then, whoah, we're in for some spec writing pain if we start adding InputCapture too :)

Maybe we should just have a generic org.freedesktop.portal.Session.Create() instead of per-portal ones and then use the session as-is? AFAICT nothing happens on session creation anyway, all the interaction is later when we have portal-specific calls. And with "bundling" I mean - some way of combining several independent sessions into one logical entity that can be Closed as one?

jadahl commented 2 years ago

I would argue that a more future-proof approach would be session bundling rather than just extending existing session implementations.

Mh, not so sure about that, the point of sharing the session is that it's a single "handle" for an activity that was granted permission for; by making them two, it allows for them to work independently (except if we say they can't, but well, why have them separate if they can't be used separately?). For example, in the RD/SC, you set up your expectations for a session, then "start" it, and in response to the start call, a dialog handling all the things is presented. How do you deal with that if you have to start 2 sessions for the sake of what should really be one? "Bundling" them sounds like it'd still be a single session, containing two sessions, so in SC/RD, we went from 1 to 2, or 3 if the "bundled" session is a "parent" session, and adding clipboard, we end up with 3 (or 4).

Maybe it's just wrong to think of sessions as being tied to an "object' ala object oriented programming, and more about some "thing" that the user asked for special permissions for, that was granted.

Maybe we should just have a generic org.freedesktop.portal.Session.Create() instead of per-portal ones and then use the session as-is?

That'd work too, IMO, the SC/RD CreateSession doesn't really do anything anyway. It'd kind of go in another direction than this PR though.

whot commented 2 years ago

Maybe it's just wrong to think of sessions as being tied to an "object' ala object oriented programming, and more about some "thing" that the user asked for special permissions for, that was granted.

There's a Draft API proposal in #112 now on how this could look like, together with https://github.com/flatpak/xdg-desktop-portal/pull/864

whot commented 2 years ago

Closing this one, this is clearly a dead end.

112 has the initial draft of how detangling SC/RD from the session namespace looks like and #96 has the implementation for InputCapture, it's already using the new XdpSession-inside-other-session approach.