GNOME Bugzilla – Bug 690548
Make modeswitch buttons work in the OSD
Last modified: 2013-01-09 11:00:09 UTC
So you can cycle through, to find which modeswitch you assigned a particular button to for example
Yes, makes a lot of sense, even more with newer tablet that may not have a LED to show the current mode.
Created attachment 232978 [details] [review] Proposed Proposed patch to implement the requested functionality
Review of attachment 232978 [details] [review]: "Proposed Proposed patch" is "reviewed reviewed". ::: plugins/wacom/gsd-wacom-manager.c @@ +1263,3 @@ if (wbutton->type == WACOM_TABLET_BUTTON_TYPE_HARDCODED) { int new_mode; + gboolean update_osd = (manager->priv->osd_window != NULL); You only ever use it once, so you can do the check in the if () statement directly. ::: plugins/wacom/gsd-wacom-osd-window.h @@ +56,3 @@ GtkDirectionType dir, gboolean active); +void gsd_wacom_osd_window_set_mode (GsdWacomOSDWindow *osd_window, Split this off in a separate patch.
(In reply to comment #3) > Review of attachment 232978 [details] [review]: > > "Proposed Proposed patch" is "reviewed reviewed". lol :) > ::: plugins/wacom/gsd-wacom-manager.c > @@ +1263,3 @@ > if (wbutton->type == WACOM_TABLET_BUTTON_TYPE_HARDCODED) { > int new_mode; > + gboolean update_osd = (manager->priv->osd_window != NULL); > > You only ever use it once, so you can do the check in the if () statement > directly. Sure, was a leftover from a previous implementation, no reason to keep a var for this. > ::: plugins/wacom/gsd-wacom-osd-window.h > @@ +56,3 @@ > > GtkDirectionType dir, > gboolean > active); > +void gsd_wacom_osd_window_set_mode > (GsdWacomOSDWindow *osd_window, > > Split this off in a separate patch. Not sure I understand why this would need to be in a separate patch. This is really the API that allows the wacom settings manager to update the OSD window on mode change, it's really part of the patch. There are admitedly things that could be separate from the patch, like for example the space before parenthesis added here: @@ -884,7 +917,7 @@ gsd_wacom_osd_window_draw (GtkWidget *widget, gsd_wacom_osd_window_adjust_cairo (osd_window, cr); /* And render the tablet layout */ - gsd_wacom_osd_window_update(osd_window); + gsd_wacom_osd_window_update (osd_window); rsvg_handle_render_cairo (osd_window->priv->handle, cr); or even the comment being misleading here: @@ -1263,15 +1263,16 @@ filter_button_events (XEvent *xevent, if (wbutton->type == WACOM_TABLET_BUTTON_TYPE_HARDCODED) { int new_mode; - /* Update OSD window if shown */ - if (osd_window_update_viewable (manager, wbutton, dir, xiev)) + /* We switch modes on key press */ + if (xiev->evtype == XI_ButtonRelease) { + osd_window_update_viewable (manager, wbutton, dir, xiev); return GDK_FILTER_REMOVE; - - /* We switch modes on key release */ - if (xiev->evtype == XI_ButtonRelease) - return GDK_FILTER_REMOVE; - + } Because the code switches modes on key press and not on key release as stated in the original comment.
Created attachment 233048 [details] [review] [PATCH 1/4] wacom: add API to update modes while OSD is active Updated and split up patches after review.
Created attachment 233052 [details] [review] [PATCH 2/4] wacom: allow switching modes while OSD is active
Created attachment 233053 [details] [review] [PATCH 3/4] wacom: Coding style fix
Created attachment 233054 [details] [review] [PATCH 4/4] wacom: fix misleading comment
Review of attachment 233048 [details] [review]: Looks fine.
Review of attachment 233052 [details] [review]: Looks good.
Review of attachment 233053 [details] [review]: ++
Review of attachment 233054 [details] [review]: ++
Thanks! Patches pushed to git head, closing.