! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
hacky/incomplete fix for invalid xrandr settings
Status:
RESOLVED: FIXED
Product:
Xfce4-settings
Component:
General

Comments

Description Sean 2011-03-13 22:12:12 CET
Without this patch, it is possible to get an invalid xrandr configuration (well, it's valid, but not in a useful way, and it causes things to break).

To reproduce without the patch:

(1) Enable two monitors
(2) Disable the left monitor in xfce4-settings
(3) Notice that the xrandr dekstop size is still as wide as when both monitors were enabled, even though only one is in use; the panel switcher misrenders its windows, xfdesktop horrifically misrenders the desktop, and killing/restarting xfdesktop can then result in an X server crash (on Fedora Rawhide).

This patch is a hack that simply tries to determine what the best layout/position for the enabled monitors is in conjunction with the values in xfconf.

The real fix would be to have the display configuration panel to actually let the user position their monitors and to generate the proper layout there.

With this patch, it's possible to end up with both monitors at position 0,0 which basically means they end up cloned.  Since the display panel is borderline useless, there's no way to actually change/undo this without editing xfconf settings directly.

Still, that's better than X server crashes and the other bugs that can be caused by having one monitor floating out in the middle of the virtual desktop.
Comment 1 Jérôme Guelfucci editbugs 2011-03-14 16:42:28 CET
Hi Sean,

I would be pleased to have a look at this but the attachment is missing.

Cheers,

Jérôme
Comment 2 Sean 2011-03-14 23:20:00 CET
Created attachment 3563 
hacky fix patch
Comment 3 Lionel Le Folgoc 2011-03-16 22:09:29 CET
Created attachment 3569 
Updated proposed patch

Your patch fixes the issue and works fine here, thanks!
I attach a slightly updated version to fix the weird indent, and add a test to compute the mode size only for outputs that won't be disabled (it's a waste of time to do that for disabled outputs, as their size doesn't matter).

I couldn't reproduce your issue with wnck though. It's possible that wnck receives outdated info from the x server; this could be fixed by blocking events until we finished enabling/disabling outputs and setting the screen size. Could you try to add a call to XGrabServer() or gdk_x11_grab_server() before the second loop, and one to XUngrabServer() or gdk_x11_ungrab_server() after the screen size is set?
Comment 4 Jérôme Guelfucci editbugs 2011-03-19 14:23:42 CET
I just pushed the patch updated by Lionel. Lionel told me the xrandr tool does the same thing so it should be safe, thus I also pushed it to the 4.8 branch: it will be able in the next stable release.

Sean, it would be cool if you could test Lionel's proposal to fix the panel switcher issue. Thanks in advance!

I also want to thank you and Lionel for your contribution! As far as the display settings dialog UI is concerned, adding support for setting the monitors layout is a goal for 4.10. If you feel like giving it a try, Lionel and I can give a hand and review/push your work. If not, it'll wait a bit more ^^

Cheers,

Jérôme
Comment 5 Sean 2011-03-19 21:49:34 CET
No problem, I'm pulling the patch into my git tree now and I'll test things out fully when I get back from my errands (out to give some interviews for game artists this afternoon -- busy busy).

I do plan on giving the UI a whack, mostly just by copying what the GNOME guys did in their dialog.  (Heck, maybe just ripping the code, depending on how much control-center stuff their randr UI directly depends on.)

After that I'd like to clean it up a bit.  I don't like how the GNOME setting tries to identity monitors, which seems to be based on the idea that everybody is using a laptop and an external monitor (and hence have different names like "LVDS" and "External Monitor").  The Windows way is much better as it works for that case and works for people who have two identical monitors.  The Windows UI just puts a number on each display (in addition to the name), and the identification UI just renders a big huge copy of the number on each monitor, which is easy enough to do with some simple shaped and/or ARGB windows and cairo.

A final feature I may like to give a shot at is being able to let a panel be assigned to the "primary display" rather than a specific display, and then letting the randr config UI set the user's preferred primary monitor (which I need anyway because the stupid ati Linux drivers enumerate monitors in the opposite order that their Catalyst drivers do on Windows), so that switching primary monitors will also automatically move default panels between the displays.  That would help me out when I set up new boxes.

... all of that commentary probably belongs in its own bug.
Comment 6 Jérôme Guelfucci editbugs 2011-03-20 15:48:34 CET
Looks like a good plan!

I like the idea of being able to quickly identify monitors by displaying an identifier on each of them, that's quite intuitive.

Re primary display, I think this will require some changes in the panel itself but it should not be very difficult.

I'm looking forward to seeing this coming, don't hesitate if you need any information.
Comment 7 Sean 2011-03-20 20:47:27 CET
Lionel's patch seems to be working just fine for me.

I'll try to get to the randr stuff this week.  My Roku is off for RMA so I'm going to need to rely on my second monitor for Hulu watching this week, so now I have a super-critical reason to have the thing working well.  ;)
Comment 8 Jérôme Guelfucci editbugs 2011-03-21 09:27:36 CET
Ok, marking this particular bug as fixed then. Thanks for testing!

Bug #7413

Reported by:
Sean
Reported on: 2011-03-13
Last modified on: 2011-03-21

People

Assignee:
Jérôme Guelfucci
CC List:
4 users

Version

Version:
unspecified

Attachments

hacky fix patch (3.67 KB, patch)
2011-03-14 23:20 CET , Sean
no flags
Updated proposed patch (5.70 KB, patch)
2011-03-16 22:09 CET , Lionel Le Folgoc
no flags

Additional information