Skip to content

Commit

Permalink
meta-monitor-manager-xrandr.c: DeleteMonitor before SetMonitor (#693)
Browse files Browse the repository at this point in the history
A somewhat similar fix appears in mutter [1].

The RandR protocol [2] states concerning RRSetMonitor that:

> If 'name' matches an existing Monitor on the screen, the
> existing one will be deleted as if RRDeleteMonitor were called.

Unfortunately, this behavior is not the case in practice, and if an
existing monitor with the same name exists the server generates a
BadValue error [3].  While this was fixed very recently in the Xorg
xserver [3], it is unclear when these changes will make it to most
distributions due to how long it has been since a formal Xorg xserver
major release.  Therefore, we must ourselves prevent muffin from
aborting if a monitor with the same name already exists, a common
condition upon restarting cinnamon or calling `cinnamon --replace`.

The mutter fix [1] solves the problem of preventing mutter from aborting
on the error, but the request still fails to set the monitor's outputs.
The mutter fix also uses an API which is not in muffin yet.

Therefore, we take the approach of using xcb to first delete any monitor
with the name of the one which we wish to set.  Note that this request
can also fail if no such monitor exists, so we explicitly ignore this
error.

A word of warning to those who may wish to use xcb to also replace the
SetMonitor call:  a crashing bug was only recently fixed in libxcb [4]
which caused all calls to xcb_randr_set_monitor() to crash or otherwise
cause the calling program to behave in an undefined manner.  While the
fix is released in libxcb 1.17, this version is not even available in
Ubuntu 24.04, for instance.  Therefore, it may be some time before we
can reliably call xcb_randr_set_monitor().

[1] https://gitlab.gnome.org/GNOME/mutter/-/commit/8d3696f39a0b3af725b7615f7e2ac74ce5e0bcbf
[2] https://cgit.freedesktop.org/xorg/proto/randrproto/tree/randrproto.txt
[3] https://gitlab.freedesktop.org/xorg/xserver/-/commit/146bb9b2c19fb75b7629b65d5871969b7fcef97a
[4] https://gitlab.freedesktop.org/xorg/lib/libxcb/-/commit/038636786ad1914f3daf3503ae9611f40dffbb8f
  • Loading branch information
jknockel authored May 21, 2024
1 parent c1a29cb commit 199284c
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/backends/x11/meta-monitor-manager-xrandr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,8 @@ meta_monitor_manager_xrandr_tiled_monitor_added (MetaMonitorManager *manager,
GList *outputs;
GList *l;
int i;
xcb_connection_t *xcb_conn;
xcb_void_cookie_t cookie;

if (!(meta_monitor_manager_get_capabilities (manager) &
META_MONITOR_MANAGER_CAPABILITY_TILING))
Expand Down Expand Up @@ -1117,6 +1119,11 @@ meta_monitor_manager_xrandr_tiled_monitor_added (MetaMonitorManager *manager,
xrandr_monitor_info->outputs[i] = output->winsys_id;
}

xcb_conn = XGetXCBConnection (manager_xrandr->xdisplay);
cookie = xcb_randr_delete_monitor_checked (xcb_conn,
DefaultRootWindow (manager_xrandr->xdisplay),
name_atom);
free (xcb_request_check (xcb_conn, cookie)); /* ignore DeleteMonitor errors */
XRRSetMonitor (manager_xrandr->xdisplay,
DefaultRootWindow (manager_xrandr->xdisplay),
xrandr_monitor_info);
Expand Down

1 comment on commit 199284c

@LinuxOnTheDesktop
Copy link

Choose a reason for hiding this comment

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

This apparently very well considered patch seems welcome, because Mint has long suffered from seemingly random chaos when one attaches a second monitor. That chaos has made it an anxious business to use Mint when giving presentations. See the following bug reports, some of which, hopefully, can be deemed fixed after this the present patch: #532; #578; #593; linuxmint/cinnamon#10892; linuxmint/cinnamon#11889.

Please sign in to comment.