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

resolution update handling #3367

Open
jpouellet opened this Issue Dec 6, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@jpouellet
Contributor

jpouellet commented Dec 6, 2017

Qubes OS version:

R4-rc3

...continuing discussion from QubesOS/qubes-gui-daemon#17

It's not clear to me that core-admin-client should be involved in resolution updates at all. It seems like having the gui-daemon handle resolution changes directly would:

  • be simpler (less moving parts = less fragile)
  • eliminate a cross-repo dependency (which seems to be the root cause of why resolution updating is broken right now)
  • and be more flexible (not requiring a 1:1 mapping of user login sessions (for qvm-start-gui pidfiles) and X server instances with given outputs/resolutions)

The only legitimate reason for core-admin-client being involved right now seems to be to check for the no-monitor-layout feature to decide whether or not to send resolution updates, but that would be easy enough to just pass along as a flag to qubes-guid like we currently do for the rpc-clipboard feature.

I would like to simplify the whole resolution update handling situation by making the gui-daemon responsible for handling randr events, querying the new resolution, and dispatching qubes.SetMonitorLayout, removing the need for core-admin to know or care about resolutions at all.

Since this is not a trivial patch, I figured I'd ask if there's some fundamental reason why core-admin should be involved that I'm missing before going ahead and spending the time to implement it.

So... thoughts? Am I missing some reason that core-admin should be involved?

There's also:

        .. event:: monitor-layout-change (subject, event, monitor_layout)

            Desktop layout was changed, probably because a display was plugged
            in or out.

            :param subject: Event emitter (the qube object)
            :param event: Event name (``'monitor-layout-change'``)
            :param monitor_layout: The new layout

in core-admin/qubes/vm/qubesvm.py, except it's not actually implemented or used anywhere, nor do I see what would be improved by it being so.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Dec 6, 2017

Member

.. event:: monitor-layout-change (subject, event, monitor_layout)

This is indeed unused, leftover from pre-AdminAPI core3.

As to the merit: moving resolution handling to qubes-gui-daemon repository makes sense. But I prefer to not add additional code to the qubes-guid tool itself - it is sufficiently complex already, modifications require substantial X11 knowledge and in many cases have side effects. So, if possible better have it in separate tool, so its future modifications will not impact qubes-guid code directly.

TBH, moving the whole qvm-start-gui tool to qubes-gui-daemon repo would be more logical, because it is tightly bound to qubes-guid command line.

Member

marmarek commented Dec 6, 2017

.. event:: monitor-layout-change (subject, event, monitor_layout)

This is indeed unused, leftover from pre-AdminAPI core3.

As to the merit: moving resolution handling to qubes-gui-daemon repository makes sense. But I prefer to not add additional code to the qubes-guid tool itself - it is sufficiently complex already, modifications require substantial X11 knowledge and in many cases have side effects. So, if possible better have it in separate tool, so its future modifications will not impact qubes-guid code directly.

TBH, moving the whole qvm-start-gui tool to qubes-gui-daemon repo would be more logical, because it is tightly bound to qubes-guid command line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment