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

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

Open
2 tasks done
kkotyk opened this issue Feb 13, 2024 · 20 comments
Open
2 tasks done

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

kkotyk opened this issue Feb 13, 2024 · 20 comments

Comments

@kkotyk
Copy link

kkotyk commented Feb 13, 2024

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

  • I have searched past issues to see if this bug has already been reported, and it hasn't been.
  • I understand that people give their precious time for free, and thus I've done my very best to make this problem as easy as possible to investigate.
@tych0
Copy link
Member

tych0 commented Feb 13, 2024

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
Copy link
Author

kkotyk commented Feb 13, 2024

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
Copy link
Member

tych0 commented Feb 13, 2024

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
Copy link
Author

kkotyk commented Feb 13, 2024

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
Copy link
Member

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
Copy link
Author

kkotyk commented Feb 13, 2024

@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
Copy link
Member

tych0 commented Feb 13, 2024

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
Copy link
Member

tych0 commented Feb 13, 2024

(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
Copy link
Author

kkotyk commented Feb 13, 2024

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
Copy link
Member

tych0 commented Feb 13, 2024

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
Copy link
Author

kkotyk commented Feb 14, 2024

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
Copy link
Member

tych0 commented Feb 14, 2024

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
Copy link
Author

kkotyk commented Feb 14, 2024

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
Copy link
Member

tych0 commented Feb 14, 2024

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
Copy link
Member

tych0 commented Feb 14, 2024

Also, what does is_primary represent here?

@kkotyk
Copy link
Author

kkotyk commented Feb 14, 2024

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
Copy link
Member

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
Copy link
Member

tych0 commented Feb 15, 2024

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
Copy link
Member

tych0 commented Feb 18, 2024

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 59f9199

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
Copy link
Author

kkotyk commented Feb 18, 2024

I will slowly pick away at it. Thanks.

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

No branches or pull requests

3 participants