i have a nvidia card, with nv driver, hence no RANDR support : Xlib: extension "RANDR" missing on display ":0.0". xfce4-display-settings segfaults at startup : (gdb) bt full #0 0x0b10a3a5 in _XRRGetScreenInfo () from /usr/X11R6/lib/libXrandr.so.6.0 No symbol table info available. #1 0x0b10a44c in XRRGetScreenInfo () from /usr/X11R6/lib/libXrandr.so.6.0 No symbol table info available. #2 0x1c003f4a in xfce_randr_legacy_new (display=0x85f1e018) at xfce-randr-legacy.c:80 legacy = (XfceRandrLegacy *) 0x89fa50a0 n = 0 num_screens = 1 screen = (GdkScreen *) 0x87004030 root_window = (GdkWindow *) 0x7f2f8020 xdisplay = (Display *) 0x89fa4800 rotation = 0 #3 0x1c002e50 in main (argc=1, argv=0xcfbd504c) at main.c:701 dialog = (GtkWidget *) 0x1c004500 gxml = (GladeXML *) 0x795d8f3 error = (GError *) 0x0 display = (GdkDisplay *) 0x85f1e018
Nick, can you look into this? Since the dialog requires XRandR the best way probably is to detect whether it's available plus display an error dialog and quit if it's not.
Yup, will look in to it this weekend.
Created attachment 2114 Patch This patch should fix the problem, but it does add a few new string :-(. Of course i can just exit the application without segfaulting, but that will leave the user with no clue what went wrong... So (Brian, Stephan, Jannis?), what do we do with this? Personally i'd say we should just commit it and tell the i18n list. They are only warnings and normally they won't show up (ie. no _need_ to translate them...). If so, a string review would be nice...
(In reply to comment #3) > Created an attachment (id=2114) [details] > Patch > > This patch should fix the problem, but it does add a few new string :-(. Of > course i can just exit the application without segfaulting, but that will leave > the user with no clue what went wrong... > > So (Brian, Stephan, Jannis?), what do we do with this? Personally i'd say we > should just commit it and tell the i18n list. They are only warnings and > normally they won't show up (ie. no _need_ to translate them...). > If so, a string review would be nice... Personally, I'd prefer this patch instead of the console-only error you committedo to SVN. But we're in string freeze and about to release 4.6 in two weeks, so it's rather unlikely that the new strings will be translated in time. I suggest we keep the patch in mind and apply it for 4.8.
(In reply to comment #4) > (In reply to comment #3) > > Created an attachment (id=2114) [details] [details] > > Patch > > > > This patch should fix the problem, but it does add a few new string :-(. Of > > course i can just exit the application without segfaulting, but that will leave > > the user with no clue what went wrong... > > > > So (Brian, Stephan, Jannis?), what do we do with this? Personally i'd say we > > should just commit it and tell the i18n list. They are only warnings and > > normally they won't show up (ie. no _need_ to translate them...). > > If so, a string review would be nice... > > Personally, I'd prefer this patch instead of the console-only error you > committedo to SVN. But we're in string freeze and about to release 4.6 in two > weeks, so it's rather unlikely that the new strings will be translated in time. > I suggest we keep the patch in mind and apply it for 4.8. Oh, I didn't read the commit carefully enough. It was a commit to fix the settings helper, not the dialog itself. So, yeah, I'd vote for including this patch before 4.6. IMHO the strings could be improved a bit. I'll attach a patch for this in a minute.
Created attachment 2116 Like the previous patch but with different strings I don't think we need to tell people that the application is going to quit. They'll notice that anyway. We should tell them which version of RandR they need though.
Guys, you need 2 things here: 1. Compile-time detection of libXrandr, to protect all randr code with an #ifdef. 2. Run-time detection using XRRQueryExtension(). You've got #2, but this'll fail to compile on any system without even the xrandr client lib. So: bad. Anyway, string is fine in the dialog. But why not use xfce_message_dialog()? Also, use GTK_STOCK_QUIT for the dialog button.
(In reply to comment #7) > Guys, you need 2 things here: > > 1. Compile-time detection of libXrandr, to protect all randr code with an > #ifdef. > > 2. Run-time detection using XRRQueryExtension(). > > You've got #2, but this'll fail to compile on any system without even the > xrandr client lib. So: bad. xrandr is a dependency. The whole display plugin is useless without it. If you run xfce 4.6 you can update to a decent version of xorg too. > Anyway, string is fine in the dialog. But why not use xfce_message_dialog()? > Also, use GTK_STOCK_QUIT for the dialog button. message dialog is fine, gtk_stock_quit is a good idea.
I've committed the whole thing in revision 29350.
Sorry, but I don't think we want to require libXrandr in Xfce. We like to be able to run on older X servers... and non-Xorg X servers as well. Olivier, thoughts?
Ok, I'll make xrandr an optional dependency, but then the display plugin won't be compiled at all. Is that a problem? IIRC it always used xrandr for applying the settings.
I agree with brian here, libXrandr should be an optional dependency, but keep in mind the case when it's present (and then compiled in) but not available due to lack of support in the driver (like nv)... but i think the way it is in svn already handles that case correctly.
Yup, only a #if #endif patch is needed. Will create one this evening.
Created attachment 2119 Optional support for Xrandr This patch allows xfce4-settings compilation without the randr dependency (--disable-xrandr).
Do we want to apply this patch before the final release? If so, does it look ok?
Yeah, that looks right to me, though I haven't tested it. Assuming you have, I'd say yeah, we do want it in for the next RC.
Committed in revision 29413. Compiles fine with --disable-xrandr.