QubesOS / qubes-video-companion

MIT License
20 stars 10 forks source link

Allow monitor choice #24

Closed ben-grande closed 3 months ago

ben-grande commented 3 months ago

The client can choose the monitor, which will be prompted in the Qrexec dialog. The monitor can be enforced overridden by the target by environment variable. Useful on a multi-monitor setup, especially when Dom0 is the target.

Fixes: https://github.com/QubesOS/qubes-issues/issues/9275

ben-grande commented 3 months ago

Possibly requires https://github.com/QubesOS/qubes-video-companion/pull/23 to avoid merge conflicts on the DBUS variable.

marmarek commented 3 months ago

I don't think it should be client selecting the monitor. Besides the policy aspect (who control what is shared), the display names may not match (or be known at all) to the source qube - see xrandr output in VM. And even when known, I'm not sure if they are guaranteed to be compatible with qrexec argument (like if it contains . or maybe there could be :?). Maybe there could be a zenity (or gtk directy?) prompt to select which monitor on the service side?

Later it could be extended to include also a screenshot, or even selecting individual window (the latter might be more problematic, and may deserve separate separate service if a protocol with dynamic frame size would be needed).

ben-grande commented 3 months ago

I don't think it should be client selecting the monitor. Besides the policy aspect (who control what is shared), the display names may not match (or be known at all) to the source qube - see xrandr output in VM.

The monitor names are not known by the source. I just allow the client to specify the target because then it can be shown in the Qrexec dialog, but in any case, the target can always override the monitor name.

You following suggestions seems promising, especially the combination of dialog box with screenshot of the monitors.

And even when known, I'm not sure if they are guaranteed to be compatible with qrexec argument (like if it contains . or maybe there could be :?).

I believe the monitor names can only have these characeters: [0-9A-Za-z].

See the output of:

xrandr | awk '/ (dis)?connected /{print $1}'

I didn't read the xrandr source code to verify and did not find information in the manual page.

Qrexec arguments can have the following characters: [0-9A-Za-z+_.].

Maybe there could be a zenity (or gtk directy?) prompt to select which monitor on the service side?

I thought about this, but then thought there should be less dialogs for UX reasons, but if you prefer a dialog, I can do that instead.

Later it could be extended to include also a screenshot, or even selecting individual window (the latter might be more problematic, and may deserve separate separate service if a protocol with dynamic frame size would be needed).

A screenshot of the screens would be nice, I thought of this to be more explicit but then went in the simple route.

zenity doesn't allow embedding images. yad does and it uses GTK+ behind the scenes. Can be GTK directly also, but that is something I have to learn and examples uses in Qubes for reference would be nice.

ben-grande commented 3 months ago

yad does seen to have some dependencies:

Installing:
 yad                    x86_64   9.3-4.fc37          qubes-dom0-cached   235 k
Installing dependencies:
 gspell                 x86_64   1.12.1-1.fc37       qubes-dom0-cached   106 k
 gtksourceview3         x86_64   3.24.11-8.fc37      qubes-dom0-cached   581 k
 javascriptcoregtk4.0   x86_64   2.42.2-1.fc37       qubes-dom0-cached   8.4 M
 webkit2gtk4.0          x86_64   2.42.2-1.fc37       qubes-dom0-cached    24 M

Possibly GTK is better because there is a high chance everything is already installed.

marmarek commented 3 months ago

It's also okay to start with zenity (which AFAIR is already pulled in by something else) for the list-only option, and later replace with something else for list+screenshots. @marmarta can help with GTK dialog.

ben-grande commented 3 months ago

Zenity dialog ready for review. Sorry about many pushes, had some troubles in the rebase and merge.

ben-grande commented 3 months ago

Updated commit message.

ben-grande commented 3 months ago

@marmarta

I have no prior experience with Gtk other than this PR, but I can grasp some examples that I've read about screenshots with pixbuf and combobox.

One design idea is taking screenshot of monitors using Gdk.pixbuf_get_from_window and setting the geometry to the monitor one. Then a combobox to select the monitor and when an option is selected, show screenshot of wanted monitor. Do not show screenshot of all monitors at the same time as many monitor can coexist and would have to reduce image resolution.

window = Gdk.get_default_root_window()
x, y, width, height = window.get_geometry()

monitor_pixbuf = Gdk.pixbuf_get_from_window(window, x, y, width, height)

if monitor_pixbuf:
    monitor_pixbuf.savev("monitor_NAME.png", "png", (), ())
else:
    ## ERROR OUT

Another option is a design similar to the KDE screen arrangement would be one option and the user click on the display or on a combobox that selects the display per name.