GNOME Bugzilla – Bug 705943
OLEDs on Intuos4 dead again
Last modified: 2013-10-04 11:40:31 UTC
git bisect tells me that this patch: commit df161dba1845e79bb83498ba0c8d193b4c742e43 wacom: Use a GtkListBox for the buttons' mapping instead of a GtkTreeView that is a part of fix for https://bugzilla.gnome.org/show_bug.cgi?id=703148 killed OLED icons on Intuos4. Should I try to fix it or it's a part of ongoing work and the OLED icons feature is on the way back to the code? some bits in the code before the patch has been applied: [gnomedev@localhost gnome-control-center]$ git grep g_settings_set_string df161dba^ | grep wacom df161dba^:panels/wacom/cc-wacom-page.c: g_settings_set_string (button->settings, OLED_LABEL, str); df161dba^:panels/wacom/cc-wacom-page.c: g_settings_set_string (button->settings, CUSTOM_ACTION_KEY, str); df161dba^:panels/wacom/cc-wacom-page.c: g_settings_set_string (button->settings, CUSTOM_ACTION_KEY, ""); df161dba^:panels/wacom/cc-wacom-page.c: g_settings_set_string (button->settings, OLED_LABEL, ""); df161dba^:panels/wacom/cc-wacom-page.c: g_settings_set_string (button->settings, OLED_LABEL, WACOM_C(action_table[type].action_name)); df161dba^:panels/wacom/cc-wacom-page.c: g_settings_set_string (button->settings, OLED_LABEL, ""); df161dba^:panels/wacom/cc-wacom-page.c: g_settings_set_string (priv->wacom_settings, "rotation", rotation); are currently gone: [gnomedev@localhost gnome-control-center]$ git grep g_settings_set_string | grep wacom panels/wacom/cc-wacom-button-row.c: g_settings_set_string (button->settings, CUSTOM_ACTION_KEY, custom_key); panels/wacom/cc-wacom-page.c: g_settings_set_string (priv->wacom_settings, "rotation", rotation);
Created attachment 251961 [details] [review] restore OLED handling on Intuos4 - patch for gnome-settings-daemon That patch fixes a regression introduced by commit df161dba1845e79bb8 and should be applied to gnome-settings-daemon. The remaining part of OLED handling (in gnome-control-center and gnome-settings-daemon as well) has to be reviewed as it might require some cleaning - I have not traced what changes were made during the button mapping window re-design. There might be a better way to fix the regression, unfortunately I won't be able to work on it for a while.
thanks for the patches
Review of attachment 251961 [details] [review]: ::: plugins/wacom/Makefile.am @@ +168,3 @@ gsd-wacom-button-editor.c \ + gsd-wacom-oled.h \ + gsd-wacom-oled.c \ This should go in now, in a separate patch. ::: plugins/wacom/gsd-wacom-osd-window.c @@ +1042,2 @@ static gchar * +get_escaped_accel_shortcut (const gchar *accel, I don't like the settings being modified in something that shouldn't modify any settings. You can return another string instead: static gchar * get_escaped_accel_shortcut (const gchar *accel, char **oled_label) @@ +1076,2 @@ + if (type == GSD_WACOM_ACTION_TYPE_HELP) { + g_settings_set_string (button->settings, OLED_LABEL, C_("Action type", "Show On-Screen Help")); That's not going to fit on the OLED screen. @@ +1081,2 @@ + if (type == GSD_WACOM_ACTION_TYPE_SWITCH_MONITOR) { + g_settings_set_string (button->settings, OLED_LABEL, C_("Action type", "Switch Monitor")); This neither.
Created attachment 252470 [details] "Switch Monitor" and "Show On Screen Help" on Wacom Intuos4 Wireless Part of Wacom Intuos4 Wireless tablet with OLED display showing "Switch Monitor" and "Show On Screen Help"
(In reply to comment #3) > Review of attachment 251961 [details] [review]: > > ::: plugins/wacom/Makefile.am > @@ +168,3 @@ > gsd-wacom-button-editor.c \ > + gsd-wacom-oled.h \ > + gsd-wacom-oled.c \ > > This should go in now, in a separate patch. OK > > ::: plugins/wacom/gsd-wacom-osd-window.c > @@ +1042,2 @@ > static gchar * > +get_escaped_accel_shortcut (const gchar *accel, > > I don't like the settings being modified in something that shouldn't modify any > settings. You can return another string instead: > static gchar * > get_escaped_accel_shortcut (const gchar *accel, char **oled_label) Yes, that's ugly. I'll make the suggested changes. > @@ +1076,2 @@ > + if (type == GSD_WACOM_ACTION_TYPE_HELP) { > + g_settings_set_string (button->settings, OLED_LABEL, C_("Action type", > "Show On-Screen Help")); > > That's not going to fit on the OLED screen. It almost fits (I'd say "acceptable" just to keep text the same as on the screen. However I might make a new translation context and cut it down to "On Screen Help" - check here how it currently looks on wacom intuos4 wireless: https://bugzilla.gnome.org/attachment.cgi?id=252470 > @@ +1081,2 @@ > + if (type == GSD_WACOM_ACTION_TYPE_SWITCH_MONITOR) { > + g_settings_set_string (button->settings, OLED_LABEL, C_("Action type", > "Switch Monitor")); > > This neither. That one fits nicely - check same attachement.
Fine then. Please add translator comments to keep the strings as short as possible.
Created attachment 253028 [details] [review] restore OLED handling on Intuos4 - patch for gnome-settings-daemon v2 I hope that this is a better solution however string handling could be probably done in a better way. The patch works, but I can't test it fully due to some strange problems that are showing up as: (gnome-settings-daemon:32037): Gdk-CRITICAL **: gdk_window_invalidate_rect_full: assertion 'GDK_IS_WINDOW (window)' failed (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve '#LabelB' position (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve #LabelB position (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve '#LabelC' position (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve #LabelC position (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve '#LabelD' position (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve #LabelD position (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve '#LabelE' position and so on. The osd window does show up, but all elements (buttons, labels) are on position 0,0.
(In reply to comment #7) > Created an attachment (id=253028) [details] [review] > restore OLED handling on Intuos4 - patch for gnome-settings-daemon v2 > > I hope that this is a better solution however string handling could be probably > done in a better way. The patch works, but I can't test it fully due to some > strange problems that are showing up as: > (gnome-settings-daemon:32037): Gdk-CRITICAL **: > gdk_window_invalidate_rect_full: assertion 'GDK_IS_WINDOW (window)' failed commit 1b095209a80b032622a60a889c0ca85f75b2d3bf Author: Bastien Nocera <hadess@hadess.net> Date: Fri Oct 4 09:12:10 2013 +0200 wacom: Fix potential warning on startup When the window isn't setup yet, no need to try and invalidate the whole drawing area. > (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve > '#LabelB' position > > (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve > #LabelB position > > (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve > '#LabelC' position > > (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve > #LabelC position > > (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve > '#LabelD' position > > (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve > #LabelD position > > (gnome-settings-daemon:32037): wacom-plugin-WARNING **: Failed to retrieve > '#LabelE' position > > and so on. The osd window does show up, but all elements (buttons, labels) are > on position 0,0. commit 67ddb3d6a07e6246ad66a4b0880e9dc8631f8250 Author: Bastien Nocera <hadess@hadess.net> Date: Fri Oct 4 09:41:13 2013 +0200 wacom: Make OSD work again The recent lirsvg security changes made the include portion of the SVG used for the OSD fail to parse. Set the base URI for the layouts before loading the SVG data to fix the new strict loading policy (#691708). https://bugzilla.gnome.org/show_bug.cgi?id=709350
Created attachment 256457 [details] [review] wacom: restore OLED handling on Intuos4 tablets This patch restores OLED handling that was broken by recent button mapping changes.
Attachment 256457 [details] pushed as d97d540 - wacom: restore OLED handling on Intuos4 tablets