Add support for the InputCapture portal #96

whot · 2022-06-02T07:21:55Z

For the corresponding Portal PR, please see flatpak/xdg-desktop-portal#714

This is a draft PR to add support for the input capture portal. Most of it is straightforward portal implementation but this portal does have a larger surface and more state than most, which raises a few questions:

XdpSession is re-used for input capture portals now too. This makes namespacing (or the lack thereof) slightly awkward, e.g. the activated signal on the session is instead inputcapture-activated to avoid future conflicts, which then requires the other signals to be namespaced too.
- ~~ideally we'd have a XdpInputCaptureSession as subclass of XdpSession but the latter is declared final right now and I'm not 100% if that's an ABI break then.~~ edit: this was nacked in Subclass RemoteDesktop/ScreenCast sessions from XdpSession #102
XdpInputCaptureSession now contains an XdpSession and the API is namespaced properly.
The enable/disable/release methods are paired with their respective _finish, but tbh this is of questionable value. Even if the portal keeps these as requests, I'd argue hiding this away here would make for better API - we can use the session signals to notify the caller out-of-band. Update June 06: these are method calls now
the pointer barriers and capture zones have their own GObjects with properties and little other functionality. This makes for slightly more sensible API but it also conflicts a bit with e.g. XdpSession which doesn't have properties but setters/getters only.
the Disabled signal is still missing here I just noticed
there's a dbusmock/pytest based test suite included. ~~that won't run without patched dbus-python and dbusmock~~ but it makes testing so much easier. specifically, it allows to test libportal without an existing implementation of the actual portal. Merged to main in test: add a pytest/dbusmock-based test suite #99, this is just the inputcapture tests now

The above are the larger topics, I'd appreciate a high-level review of the API and suggestions for where to go with this.

cc @jadahl

whot · 2022-08-30T05:40:25Z

if anyone is following along: the most recent changes from today change the implementation so that we have an API that creates and takes XdpInputCaptureSession objects. Inside that object is an XdpSession object that represents the session, it's exposed to the user on request via xdp_input_capture_session_get_session() so the user can e.g. subscribe to closed.

This is basically a working implementation of #112 (while not touching the RD/SC API) and should eventually allow us to create an InputCapture session from an existing XdpSession object, see flatpak/xdg-desktop-portal#864.

jadahl

Some light review so far only, looks good to me so far.

libportal/portal-private.h

jadahl · 2022-09-02T09:04:54Z

libportal/inputcapture.c

+struct _XdpInputCaptureSession
+{
+  GObject parent_instance;
+  XdpSession *parent_session; /* strong ref */


IMO this approach to session object management makes more sense to me.

libportal/inputcapture.h

libportal/inputcapture-pointerbarrier.c

libportal/inputcapture.c

libportal/inputcapture-pointerbarrier.c

libportal/inputcapture-private.h

whot · 2022-09-07T01:22:33Z

Just FTR, this now sits on top of #114 because I need both to work at the same time and juggling branches is annoying.

whot · 2023-08-07T18:46:13Z

marking as ready, I stared at this for too long, I can no longer see any bugs...

Conan-Kudo · 2023-09-14T16:07:19Z

What's keeping this from landing at this point? It seems to have been sitting in a ready state for a month now...

GeorgesStavracas

One small comment. LGTM otherwise.

libportal/inputcapture.h

libportal/inputcapture-pointerbarrier.c

libportal/inputcapture-zone.c

libportal/inputcapture-pointerbarrier.c

ebassi · 2023-09-19T14:24:27Z

libportal/inputcapture.c

+  guint zone_set;
+};
+
+G_DEFINE_TYPE (XdpInputCaptureSession, xdp_input_capture_session, G_TYPE_OBJECT)


Suggested change

G_DEFINE_TYPE (XdpInputCaptureSession, xdp_input_capture_session, G_TYPE_OBJECT)

G_DEFINE_FINAL_TYPE (XdpInputCaptureSession, xdp_input_capture_session, G_TYPE_OBJECT)

This way, people will get a warning if they try subclassing the type by using the instance/class sizes known to the type system.

Undone, sorry, that breaks the build (CI) with Ubuntu 20.04.

ebassi · 2023-09-19T14:25:19Z

libportal/inputcapture-zone.c

+    gboolean is_valid;
+};
+
+G_DEFINE_TYPE (XdpInputCaptureZone, xdp_input_capture_zone, G_TYPE_OBJECT)


Suggested change

G_DEFINE_TYPE (XdpInputCaptureZone, xdp_input_capture_zone, G_TYPE_OBJECT)

G_DEFINE_FINAL_TYPE (XdpInputCaptureZone, xdp_input_capture_zone, G_TYPE_OBJECT)

This way, people using the type system to query the instance and class sizes in order to subclass this type will hit an assertion failure.

Done as well.

Undone, sorry, that breaks the build (CI) with Ubuntu 20.04.

libportal/inputcapture.c

ebassi · 2023-09-19T14:28:27Z

libportal/inputcapture.c

+      if (call->cancelled_id)
+        {
+          g_signal_handler_disconnect (g_task_get_cancellable (call->task), call->cancelled_id);
+          call->cancelled_id = 0;
+        }


Suggested change

if (call->cancelled_id)

{

g_signal_handler_disconnect (g_task_get_cancellable (call->task), call->cancelled_id);

call->cancelled_id = 0;

}

g_clear_signal_handler (&call->cancelled_id, g_task_get_cancellable (call->task));

Undone, sorry, that breaks the build (CI) with Ubuntu 20.04.

libportal/inputcapture.c

matthiasclasen · 2023-09-19T15:37:39Z

I had this on my list to look at today. Thanks for beating me to it, @ebassi

The Session is no longer unique to RemoteDesktop/ScreenCast and shouldn't be treated as such. Let's split this out so we can use the same object across other interfaces. The session state is a bit more difficult since it is also related to RD/SC only but it's more spaghettied in.

Disentangle the generic Session is-closed state from the ScreenCast/RemoteDesktop-specific case. The more detailed SessionState is specific to SC/RD - INITIAL and CLOSED is generic enough that we can make it a generic API.

ofourdan · 2023-10-04T07:50:52Z

libportal/session.c

@@ -50,6 +50,7 @@ static void
 xdp_session_finalize (GObject *object)
 {
  XdpSession *session = XDP_SESSION (object);
+  g_warning ("session finalize");


Spurious warning.

ofourdan · 2023-10-04T08:36:52Z

Many thanks for the review!

I have applied the changes and forced push to @whot's branch now.

Please also note that I had to drop a few suggested changes at it was breaking the build on Ubuntu 20.04 for the CI (I suspect the version of Glib is older on that target).

whot · 2023-10-09T22:45:48Z

ftr, I fixed a buggy test case, but this branch should still be ready to go.

This patch adds a new XdpSession type for the input capture protocol as well as as the methods provided by that portal to work on that session. Two helper objects are available now too: XdpInputCaptureZone and XdpInputCapturePointerBarrier

This also fixes the invocation of EmitSignalDetailed() which dated back to an earlier version of that dbusmock feature. Since we never used it, the wrong invocation got past the tests.

whot · 2023-10-17T08:43:46Z

gentle ping for merging :)

whot mentioned this pull request Jun 6, 2022

Add a InputCapture portal flatpak/xdg-desktop-portal#714

Merged

whot force-pushed the wip/inputcapture branch from b2d9cfe to e379f65 Compare June 27, 2022 08:48

whot force-pushed the wip/inputcapture branch 2 times, most recently from 31b8414 to 145192d Compare August 18, 2022 11:01

whot force-pushed the wip/inputcapture branch 3 times, most recently from e0650f3 to b4ab8ed Compare August 30, 2022 05:34

whot force-pushed the wip/inputcapture branch from b4ab8ed to 03f0b78 Compare August 30, 2022 09:26

jadahl reviewed Sep 2, 2022

View reviewed changes

whot force-pushed the wip/inputcapture branch 2 times, most recently from 6f3fa88 to e0ed195 Compare September 6, 2022 02:06

whot mentioned this pull request Sep 6, 2022

Subclass RemoteDesktop/ScreenCast sessions from XdpSession #102

Closed

whot force-pushed the wip/inputcapture branch from e0ed195 to d4b2375 Compare September 7, 2022 01:18

whot force-pushed the wip/inputcapture branch 2 times, most recently from 66a219c to 1f5a15e Compare November 28, 2022 21:49

whot force-pushed the wip/inputcapture branch from 4f748f8 to 3e9cf98 Compare December 6, 2022 04:56

whot force-pushed the wip/inputcapture branch from 3e9cf98 to 6943c9c Compare January 5, 2023 04:21

whot force-pushed the wip/inputcapture branch from 6943c9c to e1908e7 Compare January 20, 2023 04:54

p12tic mentioned this pull request Jan 21, 2023

Add libei backend support (and thus ability to run on Wayland) input-leap/input-leap#1594

Merged

whot force-pushed the wip/inputcapture branch from e1908e7 to 491be28 Compare April 20, 2023 04:27

whot force-pushed the wip/inputcapture branch from 491be28 to c9409a4 Compare July 21, 2023 05:44

whot force-pushed the wip/inputcapture branch from c9409a4 to 6ecf17f Compare August 7, 2023 18:45

whot marked this pull request as ready for review August 7, 2023 18:45

hunterzero99 mentioned this pull request Aug 10, 2023

Wayland support? [Donation target met] input-leap/input-leap#109

Open

GeorgesStavracas approved these changes Sep 19, 2023

View reviewed changes

libportal/inputcapture.h Outdated Show resolved Hide resolved

ebassi reviewed Sep 19, 2023

View reviewed changes

whot added 2 commits October 3, 2023 16:58

session: implement a generic close/closed handling

6f2efcd

Disentangle the generic Session is-closed state from the ScreenCast/RemoteDesktop-specific case. The more detailed SessionState is specific to SC/RD - INITIAL and CLOSED is generic enough that we can make it a generic API.

ofourdan force-pushed the wip/inputcapture branch from 6ecf17f to ef99c44 Compare October 4, 2023 07:43

ofourdan reviewed Oct 4, 2023

View reviewed changes

ofourdan force-pushed the wip/inputcapture branch 4 times, most recently from 52933d3 to 7371f61 Compare October 4, 2023 08:25

whot force-pushed the wip/inputcapture branch 3 times, most recently from b26f232 to 75e9aef Compare October 6, 2023 06:00

whot added 2 commits October 10, 2023 14:14

Implement support for the InputCapture portal

ce7fc03

This patch adds a new XdpSession type for the input capture protocol as well as as the methods provided by that portal to work on that session. Two helper objects are available now too: XdpInputCaptureZone and XdpInputCapturePointerBarrier

tests: add session close tests for the inputcapture portal

3e6c1bf

This also fixes the invocation of EmitSignalDetailed() which dated back to an earlier version of that dbusmock feature. Since we never used it, the wrong invocation got past the tests.

whot force-pushed the wip/inputcapture branch from 75e9aef to 3e6c1bf Compare October 10, 2023 04:14

GeorgesStavracas approved these changes Oct 18, 2023

View reviewed changes

GeorgesStavracas merged commit 1add346 into flatpak:main Oct 18, 2023
9 checks passed

Shuna322 mentioned this pull request Oct 28, 2023

Can't connect Wayland client to Windows server input-leap/input-leap#1718

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for the InputCapture portal #96

Add support for the InputCapture portal #96

whot commented Jun 2, 2022 •

edited

Loading

whot commented Aug 30, 2022

jadahl left a comment

jadahl Sep 2, 2022

whot commented Sep 7, 2022

whot commented Aug 7, 2023

Conan-Kudo commented Sep 14, 2023

GeorgesStavracas left a comment

ebassi Sep 19, 2023

ofourdan Oct 4, 2023

ofourdan Oct 4, 2023

ebassi Sep 19, 2023

ofourdan Oct 4, 2023

ofourdan Oct 4, 2023

ebassi Sep 19, 2023

ofourdan Oct 4, 2023

ofourdan Oct 4, 2023

matthiasclasen commented Sep 19, 2023

ofourdan Oct 4, 2023

ofourdan Oct 4, 2023

ofourdan commented Oct 4, 2023 •

edited

Loading

whot commented Oct 9, 2023

whot commented Oct 17, 2023

	G_DEFINE_TYPE (XdpInputCaptureSession, xdp_input_capture_session, G_TYPE_OBJECT)
	G_DEFINE_FINAL_TYPE (XdpInputCaptureSession, xdp_input_capture_session, G_TYPE_OBJECT)

	G_DEFINE_TYPE (XdpInputCaptureZone, xdp_input_capture_zone, G_TYPE_OBJECT)
	G_DEFINE_FINAL_TYPE (XdpInputCaptureZone, xdp_input_capture_zone, G_TYPE_OBJECT)

Add support for the InputCapture portal #96

Add support for the InputCapture portal #96

Conversation

whot commented Jun 2, 2022 • edited Loading

whot commented Aug 30, 2022

jadahl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whot commented Sep 7, 2022

whot commented Aug 7, 2023

Conan-Kudo commented Sep 14, 2023

GeorgesStavracas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthiasclasen commented Sep 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofourdan commented Oct 4, 2023 • edited Loading

whot commented Oct 9, 2023

whot commented Oct 17, 2023

whot commented Jun 2, 2022 •

edited

Loading

ofourdan commented Oct 4, 2023 •

edited

Loading