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

remote-desktop: add the ability to communicate via an EIS socket #762

Merged
merged 3 commits into from
Jul 11, 2023

Conversation

whot
Copy link
Contributor

@whot whot commented Apr 1, 2022

This is intended as replacement for the NotifyFoo methods. libei
provides a more flexible and powerful method of sending input events to
the compositor.

A new method ConnectToEIS requests a file descriptor from the compositor
which can then be plugged into libei.

Once established, the communication between compositor and application
is direct, without the need to go through the portal process(es).

To avoid ambiguities between NotifyFoo and input events sent via libei,
any application that uses an EIS connection cannot use the NotifyFoo
methods.

One notable comment: I decided to pass the EIS connection down through the impl.portal instead of pipewire's approach of having the portal connect directly. Unlike the single pipewire daemon, an EIS implementation can be provided by multiple processes, so I figured it's easier to pass this down as close as possible to the entity that knows who the EIS implementation

Eventually (and once libreis is ready) this will have the hooks to auto-set the app-id on the EIS connection as well though strictly speaking it's not necessary since the app-id is passed down to whoever needs to set up the socket anyway.

Noteworthy: there is no keysym event in libei at this point, so there's no equivalent to NotifyKeyboardKeysym

cc @jadahl

whot referenced this pull request Nov 28, 2022
When connecting to EIS, this test can never be TRUE, considering that
the value is set a few lines below.
@whot whot mentioned this pull request Nov 28, 2022
@whot
Copy link
Contributor Author

whot commented Dec 12, 2022

ftr, we have a working (though not yet complete) use-case in this input-leap PR

@whot whot force-pushed the wip/remote-desktop-eis branch 2 times, most recently from b77ebcb to 16981d3 Compare March 13, 2023 01:20
@whot whot force-pushed the wip/remote-desktop-eis branch 3 times, most recently from 23f46c2 to a83dc1a Compare April 20, 2023 03:05
@whot whot marked this pull request as ready for review April 20, 2023 03:05
@whot
Copy link
Contributor Author

whot commented Apr 20, 2023

Marking this as ready. There's no immediate libei dependency other than the socket being passed, so even if libei takes another few weeks for the official "it's stable" API this bit is ready.

Squashed the commits together, there is no purpose keeping the various bugfixes we had in the git history after the merge.

@whot
Copy link
Contributor Author

whot commented May 17, 2023

ftr, libei is now declared stable. Not that it matters for this PR since we don't actually do anything with libei directly.

data/org.freedesktop.portal.RemoteDesktop.xml Outdated Show resolved Hide resolved
data/org.freedesktop.portal.RemoteDesktop.xml Outdated Show resolved Hide resolved
tests/test_remotedesktop.py Show resolved Hide resolved
30s is apparently not enough on the runners.
This is intended as replacement for the NotifyFoo methods. libei
provides a more flexible and powerful method of sending input events to
the compositor.

A new method ConnectToEIS requests a file descriptor from the compositor
which can then be plugged into libei.

Once established, the communication between compositor and application
is direct, without the need to go through the portal process(es).

To avoid ambiguities between NotifyFoo and input events sent via libei,
any application that uses an EIS connection may not use the NotifyFoo
methods.

Co-authored-by: Olivier Fourdan <ofourdan@redhat.com>
Copy link
Collaborator

@jadahl jadahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All lgtm, I think this is ready to land. Can consider tweaking the error message, but nut super urgent.

tests/test_remotedesktop.py Outdated Show resolved Hide resolved
The check_notify() helper returns false for a number of reasons -
session in the wrong state, using EIS when a Notify method is called,
etc. Adjust the error message a bit to be more generic.
@GeorgesStavracas GeorgesStavracas added this to the 1.18 milestone Jul 11, 2023
@GeorgesStavracas GeorgesStavracas merged commit b9693c3 into flatpak:main Jul 11, 2023
3 checks passed
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

Successfully merging this pull request may close these issues.

None yet

4 participants