qtile / qtile

:cookie: A full-featured, hackable tiling window manager written and configured in Python (X11 + Wayland)
http://qtile.org
MIT License
4.79k stars 696 forks source link

Qtile screen order isn't matching output of xrandr --listmonitors #4693

Closed kkotyk closed 3 months ago

kkotyk commented 8 months ago

Issue description

Admittedly I have a complicated monitor setup with 4 monitors, a KVM and disconnect/reconnect monitors. But reloading qtile config or restarting qtile doesn't seem to recalculate the screen order listed by xrandr --listmonitors correctly once it has configured it first time:

Monitors: 4
 0: +*DP-0 2560/600x1440/340+1920+1440  DP-0
 1: +DP-4 1920/600x1080/340+4480+1440  DP-4
 2: +HDMI-0 1920/598x1080/336+0+1440  HDMI-0
 3: +DP-2 3440/800x1440/335+1480+0  DP-2

but qtile is giving:

> get_screens()
[{'gaps': {'bottom': (1920, 2850, 2560, 30),
           'left': None,
           'right': None,
           'top': None},
  'group': '2',
  'height': 1440,
  'index': 0,
  'width': 2560,
  'x': 1920,
  'y': 1440},
 {'gaps': {'bottom': (0, 2500, 1920, 20),
           'left': None,
           'right': None,
           'top': None},
  'group': '1',
  'height': 1080,
  'index': 1,
  'width': 1920,
  'x': 0,
  'y': 1440},
 {'gaps': {'bottom': (1480, 1410, 3440, 30),
           'left': None,
           'right': None,
           'top': None},
  'group': '8',
  'height': 1440,
  'index': 2,
  'width': 3440,
  'x': 1480,
  'y': 0},
 {'gaps': {'bottom': (4480, 2500, 1920, 20),
           'left': None,
           'right': None,
           'top': None},
  'group': '3',
  'height': 1080,
  'index': 3,
  'width': 1920,
  'x': 4480,
  'y': 1440}]

(monitor order 0,3,2,1 )

Version

0.23.0

Backend

X11 (default)

Config

No response

Logs

No response

Required

tych0 commented 8 months ago

This is unfortunately expected behavior. x11 doesn't strictly or stably order monitors, so there's no way for qtile to tell which is which with any kind of reliability. in my own code I've used monitor serial numbers to create a stable ordering, but they come out in qtile in a "random" order.

It would be kind of cool if you could optionally specify Screen(serial_no="...") and that screen's bars etc. would get bonded to the right screen.

kkotyk commented 8 months ago

Yes, I've looked at the internal Qtile code and it would be so much easier if the Screen() could take in an EDID to match off. Hell, it would be great if it could take in most xrandr params because then I can have qtile completely set up my monitors.

tych0 commented 8 months ago

I have wanted to add this API for a long time (the python code for interacting with x11 is mostly fleshed out in my script above), but I haven't really thought about a good design. e.g. how does one express LeftOf, etc in a platform agnostic way?

kkotyk commented 8 months ago

I would maybe start with a simpler, but more tedious way of just manually specifying x,y. I'd be perfectly happy using xrandr to initially figure out my layouts, but then manually mapping those x,y positions.

elParaguayo commented 8 months ago

I have wanted to add this API for a long time (the python code for interacting with x11 is mostly fleshed out in my script above), but I haven't really thought about a good design. e.g. how does one express LeftOf, etc in a platform agnostic way?

Could you do:

screen = [
    Screen(
        name="eDP-1",
        ...
    ),
    Screen(
        name="HDMI-1",
        left_of="eDP-1",
        ...
    )
]

Or did you mean something else?

kkotyk commented 8 months ago
@dataclass
class Monitor:
    edid: str
    resolution: str
    rate: str = "60.00"
    position: str = "0x0"
    #widgets: List[widget]
    is_primary: bool = False
    output: str = field(init=False)
    #screen: Screen = field(init=False)

    def __post_init__(self):
        self.output = self.get_monitor_connection()

    def get_monitor_connection(self):
        # Use xrandr to get detailed information about all connected monitors
        xrandr_output = subprocess.run(['xrandr', '--verbose'], stdout=subprocess.PIPE, text=True).stdout

        # Regular expression to find monitor connections and their corresponding EDID blocks
        monitor_info_regex = re.compile(r'(\S+) connected.*?EDID:\s+([0-9a-fA-F\n\s]+)', re.DOTALL)

        # Find all matches for the regex
        matches = monitor_info_regex.findall(xrandr_output)

        for match in matches:
            connection, edid_block = match
            # Remove whitespace and format the EDID block into a continuous string
            edid_formatted = ''.join(edid_block.split())

            if self.edid == edid_formatted:
                return connection

class MonitorManagement:
    def __init__(self, monitors) -> None:
        self.monitors = monitors

    def get_xrandr_command(self):
        """
        Generates an xrandr command to configure monitors based on a list of Monitor instances.

        :param monitors: List of Monitor dataclass instances
        :return: String containing the xrandr command
        """
        command_parts = ["xrandr"]
        for monitor in self.monitors:
            # Ensure the monitor has an output identifier
            if not monitor.output:
                continue

            # Basic configuration for each monitor
            config = f"--output {monitor.output} --mode {monitor.resolution} --rate {monitor.rate} --pos {monitor.position}"

            # If the monitor is the primary monitor, add the primary flag
            if monitor.is_primary:
                config += " --primary"

            command_parts.append(config)

        # Join all parts into a single command string
        return " ".join(command_parts)

monitors = [
            Monitor(edid=(
            "00ffffffffffff0004729004b6647070071b0104a53c227806ee91a3544c9926"
            "0f505421080001010101010101010101010101010101565e00a0a0a029503020"
            "350056502100001a000000ff002341534d79795a467a4b395864000000fd001e"
            "9022de3b010a202020202020000000fc00584232373148550a20202020200102"
            "020312412309070183010000654b040001015a8700a0a0a03b50302035005650"
            "2100001a5aa000a0a0a046503020350056502100001a6fc200a0a0a055503020"
            "350056502100001a22e50050a0a0675008203a0056502100001e1c2500a0a0a0"
            "11503020350056502100001a0000000000000000000000000000000000000019"),
            resolution="2560x1440",
            rate="144.00",
            position="1920x1440",
            is_primary=True),
            Monitor(edid=(
            "00ffffffffffff004c2d4810443757430d1f0103803c22782ae9e5aa534b9f24"
            "145054bfef80714f81c0810081809500a9c0b3000101023a801871382d40582c"
            "450056502100001e000000fd00304b1e5412000a202020202020000000fc0053"
            "3237523635780a2020202020000000ff00483454523330323037320a202001ec"
            "020324f14690041f131203230907078301000067030c0010008023681a000001"
            "01304b00023a80d072382d40102c458056502100001e011d007251d01e206e28"
            "550056502100001e011d00bc52d01e20b828554056502100001e2a4480a07038"
            "27403020350056502100001a8c0ad090204031200c40550056502100001800d9"),
            resolution="1920x1080",
            rate="60.00",
            position="0x1440",
            is_primary=False),
            Monitor(edid=(
            "00ffffffffffff001e6d4b7715c2000004200104b55021789ff675af4e42ab26"
            "0e50542109007140818081c0a9c0b300d1c08100d1cfdaa77050d5a034509020"
            "3a30204f3100001a000000fd003090e1e150010a202020202020000000fc004c"
            "4720554c545241474541520a000000ff003230344e545a4e31463638350a0257"
            "020330712309070747100403011f131283010000e305c000e2006ae606050161"
            "613d6d1a0000020530900004613d613d4ed470d0d0a0325030203a00204f3100"
            "001a000000000000000000000000000000000000000000000000000000000000"
            "00000000000000000000000000000000000000000000000000000000000000ee"
            "7012790000030114663801866f0def002f801f009f0545000200090000000000"
            "0000000000000000000000000000000000000000000000000000000000000000"
            "0000000000000000000000000000000000000000000000000000000000000000"
            "0000000000000000000000000000000000000000000000000000000000000b90"),
            resolution="3440x1440",
            rate="60.00",
            position="1480x0",
            is_primary=False),
            Monitor(edid=(
            "00ffffffffffff004c2d4a10443757430d1f0104a53c22783be9e5aa534b9f24"
            "145054bfef80714f81c0810081809500a9c0b3000101023a801871382d40582c"
            "450056502100001e000000fd00304b545412010a202020202020000000fc0053"
            "3237523635780a2020202020000000ff00483454523330323038300a2020017d"
            "02030ff142904523090707830100002a4480a070382740302035005650210000"
            "1a011d007251d01e206e2855000f282100001e00000000000000000000000000"
            "0000000000000000000000000000000000000000000000000000000000000000"
            "00000000000000000000000000000000000000000000000000000000000000d3"),
            resolution="1920x1080",
            rate="60.00",
            position="4480x1440",
            is_primary=False)
        ]

I've also started my own wrapper on top of qtile to be able to overcome this monitor problem. I'm being a bit a less agnostic than you by calling xrandr, but you can see what I'm thinking

tych0 commented 8 months ago

yeah, i'd rather use the x11 API directly. you also don't need all the edid info, there's a python package that will parse it and give you the serial number back that we could (optionally) depend on.

you also created a separate Monitor vs. Screen object; is there any reason? in my mind, ideally they'd be the same object.

tych0 commented 8 months ago

(but yes! something like that on Screen is what I'm after. if you want to do the API design, I'm happy to do the x11 plumbing.)

kkotyk commented 8 months ago

The reason I did EDID is because I believe any information I should need for monitor management should be easily found from xrandr --prop. EDID is kinda gross, but at least I don't have to further process it to get other information. I believe autorandr uses the same philosophy too?

I was also just building a thing for building on top of Screen() before I realized I should just bring this up with the devs. You'll see in my Monitor() class, I was actually thinking about including a Screen() object in there.

I'll sketch out some design docs knowing that you're interested in including this feature directly in Qtile.

tych0 commented 8 months ago

What I don't like about raw EDID is: how do i tell which monitor is which? If you use the serial number, usually I can look at my actual monitor on the back and see what its serial number is. If I have raw EDID in my config, I have to disambiguate by parsing it/some other magic. The serial number is what people actually know.

kkotyk commented 8 months ago

I'm fine with either way, and you are more involved with this project so you know what people need. We will just need to document a standard way of being able to query that serial number such that the x11l lib method you use always matches up. I would like to have a command line way to find that information that will always match what qtile will find.

tych0 commented 8 months ago

You should be able to get it via get-edid | parse-edid, but we could also add it to the info() method of Screen assuming the user has pyedid installed.

kkotyk commented 8 months ago
class Screen(CommandObject):
    r"""
    A physical screen, and its associated paraphernalia.

    Define a screen with a given set of :class:`Bar`\s of a specific geometry. Also,
    ``x``, ``y``, ``width``, and ``height`` aren't specified usually unless you are
    using 'fake screens'.

    The ``wallpaper`` parameter, if given, should be a path to an image file. How this
    image is painted to the screen is specified by the ``wallpaper_mode`` parameter. By
    default, the image will be placed at the screens origin and retain its own
    dimensions. If the mode is ``"fill"``, the image will be centred on the screen and
    resized to fill it. If the mode is ``"stretch"``, the image is stretched to fit all
    of it into the screen.

    The ``x11_drag_polling_rate`` parameter specifies the rate for drag events in the X11 backend. By default this is set to None, indicating no limit. Because in the X11 backend we already handle motion notify events later, the performance should already be okay. However, to limit these events further you can use this variable and e.g. set it to your monitor refresh rate. 60 would mean that we handle a drag event 60 times per second.

    """

    group: _Group
    index: int

    def __init__(
        self,
        top: BarType | None = None,
        bottom: BarType | None = None,
        left: BarType | None = None,
        right: BarType | None = None,
        wallpaper: str | None = None,
        wallpaper_mode: str | None = None,
        x11_drag_polling_rate: int | None = None,
        x: int | None = None,
        y: int | None = None,
        width: int | None = None,
        height: int | None = None,
        is_primary: bool | None = None, #<----------------------------------new
        refresh_rate: float | None = None, #<-------------------------------new
        serial_no: str | None = None, #<------------------------------------new
    ) -> None:
        self.top = top
        self.bottom = bottom
        self.left = left
        self.right = right
        self.wallpaper = wallpaper
        self.wallpaper_mode = wallpaper_mode
        self.x11_drag_polling_rate = x11_drag_polling_rate
        self.qtile: Qtile | None = None
        # x position of upper left corner can be > 0
        # if one screen is "right" of the other
        self.x = x if x is not None else 0
        self.y = y if y is not None else 0
        self.width = width if width is not None else 0
        self.height = height if height is not None else 0
        self.is_primary = is_primary if is_primary else False #<-------------------new
        self.refresh_rate = refresh_rate #<------------------------------------------new
        self.serial_no = serial_no  #<------------------------------------------new
        self.previous_group: _Group | None = None

So it looks like the base Screen object has fields we can use for x,y,width, and height but they are primarily used for the "fake_screen" objects. Perhaps we can extend that past fake screens and utilize that info plus the fields I added to pass into your methods for x11 screen management?

We could then modify:

def _process_screens(self, reloading: bool = False) -> None:
        current_groups = [s.group for s in self.screens]
        screens = []

        if hasattr(self.config, "fake_screens"):
            screen_info = [(s.x, s.y, s.width, s.height) for s in self.config.fake_screens]
            config = self.config.fake_screens
        else:
            # Alias screens with the same x and y coordinates, taking largest
            xywh = {}  # type: dict[tuple[int, int], tuple[int, int]]
            for sx, sy, sw, sh in self.core.get_screen_info():
                pos = (sx, sy)
                width, height = xywh.get(pos, (0, 0))
                xywh[pos] = (max(width, sw), max(height, sh))

            screen_info = [(x, y, w, h) for (x, y), (w, h) in xywh.items()]
            config = self.config.screens

        for i, (x, y, w, h) in enumerate(screen_info):
            if i + 1 > len(config):
                scr = Screen()
            else:
                scr = config[i]

            if not hasattr(self, "current_screen") or reloading:
                self.current_screen = scr
                reloading = False

            grp = None
            if i < len(current_groups):
                grp = current_groups[i]
            else:
                # We need to assign a new group
                # Get an available group or create a new one
                grp = self.get_available_group(i)
                if grp is None:
                    grp = self.add_autogen_group(i)

            # If the screen has changed position and/or size, or is a new screen then make sure that any gaps/bars
            # are reconfigured
            reconfigure_gaps = ((x, y, w, h) != (scr.x, scr.y, scr.width, scr.height)) or (
                i + 1 > len(self.screens)
            )

            if not hasattr(scr, "group"):
                # Ensure that this screen actually *has* a group, as it won't get
                # assigned one during `__init__` because they are created in the config,
                # where the groups also are. This lets us type `Screen.group` as
                # `_Group` rather than `_Group | None` which would need lots of other
                # changes to check for `None`s, and conceptually all screens should have
                # a group anyway.
                scr.group = grp

            scr._configure(self, i, x, y, w, h, grp, reconfigure_gaps=reconfigure_gaps)
            screens.append(scr)

        for screen in self.screens:
            if screen not in screens:
                for gap in screen.gaps:
                    if isinstance(gap, bar.Bar) and gap.window:
                        gap.kill_window()

        self.screens = screens

instead of looping over the screen info ordering, we could instead loop over qtile screens first and if the new fields are set then we can use your x11 code to manage screens or do a logical ordering. If not, we can fallback to the existing logic?

tych0 commented 8 months ago

we could instead loop over qtile screens first and if the new fields are set then we can use your x11 code to manage screens or do a logical ordering.

Sounds good to me. I also want physical resolution and virtual resolution (to implement DPI scaling).

Are you planning to add LeftOf() style combinators? That would be nicer than having people do the math.

tych0 commented 8 months ago

Also, what does is_primary represent here?

kkotyk commented 8 months ago

we could instead loop over qtile screens first and if the new fields are set then we can use your x11 code to manage screens or do a logical ordering.

Sounds good to me. I also want physical resolution and virtual resolution (to implement DPI scaling).

Are you planning to add LeftOf() style combinators? That would be nicer than having people do the math.

I could probably add that. I'm new to this project so I would like to start simple, but it seems extensible enough to be able to add that. Is there a developers guide for Qtile workflow and writing/running tests?

elParaguayo commented 7 months ago

You should be able to get it via get-edid | parse-edid, but we could also add it to the info() method of Screen assuming the user has pyedid installed.

Does this get us all the information we need about monitors? If not, we'll need to separate the retrieval of monitor info into the x11 and wayland backends and then map the Screen objects accordingly by the main core.

tych0 commented 7 months ago

Does this get us all the information we need about monitors?

The edid info itself is backend agnostic, but I think we have to query it per-backend anyway. At least for x11, it does not guarantee a stable ordering across reboots, so you have to look at the edid info to figure out which monitor you're actually talking to when you are talking to DP-1 or whatever.

If wayland guarantees a stable ordering relative to what sysfs does, we could get away with "generic" code for wayland. I don't know enough about it to say, though.

tych0 commented 7 months ago

Is there a developers guide for Qtile workflow and writing/running tests?

I do make check and fix things until it exit(0)s. For your particular case, I guess you'll want to end up creating a lot of little config files and check to see that the left_of() style combinators all work. One recent example is 59f91993fbb0ace6b2229aa2947bae637e397ad2

What would be really cool is if we could make e.g. xvfb lie about edid info too, so that you could test that part as well. I don't know if it's possible currently.

kkotyk commented 7 months ago

I will slowly pick away at it. Thanks.

github-actions[bot] commented 4 months ago

This issue is stale because it has been open 90 days with no activity. Remove the status: stale label or comment, or this will be closed in 30 days.