GNOME Bugzilla – Bug 610901
RFE: g-s-d wacom configuration plugin.
Last modified: 2010-11-18 04:07:20 UTC
Created attachment 154562 [details] [review] 0001-Add-wacom-g-s-d-plugin.patch currently g-s-d manages keyboards, mice and touchpads but not wacom tablets. these tablets need some GUI integration, since they're actually quite likely to be changed frequently (often on a per-task basis). This is the start of a g-s-d wacom integration, works against xf86-input-wacom (not linuxwacom, it has no property support). So far, only three values are supported, but this will likely become more. There's no gnome-control-center capplet yet, so gconf-editor it is :) This bug is an RFE on whether this is wanted for g-s-d or whether it should be kept separate. Any comments? I'll keep the development of this on my wacom branch below. It will be rebased against master at some point (in fact the attached patch is already). http://cgit.freedesktop.org/~whot/gnome-settings-daemon/log/?h=wacom
RFE, I guess you mean RFC? :) A couple of questions: - Is the mouse plugin that bad that we need another one? - Does it work without the input extensions? Should it use XI2 directly? - What would the configuration UI for this look like? Do you have examples of what other OSes export to the user as configuration options? Or is it only applications that support them that allow configuration? - Would users ever want per-tablet configuration? - Do all the configuration options also pertain to touchscreens? Given that it's too late for GNOME 2.30 anyway, and we would be targetting GNOME 3.0, my guess is that this code should probably be using the GDK XI2-aware API instead (if the API allows it).
(In reply to comment #1) > RFE, I guess you mean RFC? :) hehe, yes. RFSomething. :) > A couple of questions: > - Is the mouse plugin that bad that we need another one? no but I expect the wacom plugin to become _very_ complicated and I'd rather not have it cluttering up the mouse code and leave it contained. see below for more details on why I think it'll be complicated. > - Does it work without the input extensions? Should it use XI2 directly? doesn't really matter at this point. the only difference here would be calls to XIQueryDevice instead of XListInputDevices and XIGetProperty instead of XGetDeviceProperty, etc. It's purely in the call naming, not in the actual functionality. XI is required, XI2 isn't. > - What would the configuration UI for this look like? Do you have examples of > what other OSes export to the user as configuration options? Or is it only > applications that support them that allow configuration? there will be a GUI but I haven't really started with it yet. This bug was mostly to scope out if we'd want support for wacom in g-s-d at all or whether we want to externalize it. this patch is a really early version, barely more than a stub and I haven't really laid out how everything will fit together in gconf and the GUI yet. http://www.gtk-apps.org/content/show.php/Wacom+Control+Panel?content=104309 has a gtk interface for wacom configuration (python, not well integrated though) that I might use as base interface because my glade skills leave something to be desired. OS X has a really nifty tool with a screenshot available here: http://image.versiontracker.com/scrnsht/55681/183919/574WacomTabletScreenshot.png i envision the configuration being done in the DE and the applications adjusting to that, more read-only than read-write. OS X provides application-specific configuration, but that seems only partially useful since you may want a configuration switch on _what_ you're doing, not _where_ you're doing it in (see task-dependent config below) > - Would users ever want per-tablet configuration? possibly, though I need to check in the driver how we can possibly identify different tablets. at this point, I don't think we can. we can identify different tools though. I've talked with ppl about wacom configuration and the one thing that came up is that there's a chance that we need task-dependent configuration. So a tablet may have a number of setting-combinations stored but only one is active. Once the user switches to e.g. inkscape, another configuration becomes active. (hence trying to be external to the mouse config tool) > - Do all the configuration options also pertain to touchscreens? not really, they are quite specific for wacom tablets or other tablets using the same driver. As you'll know, tablets serve a quite different usecase than touchscreen and if we want touchscreen configuration items, it'd probably be better to add them to the mouse plugin like we did with the touchpad items. > Given that it's too late for GNOME 2.30 anyway, and we would be targetting > GNOME 3.0, my guess is that this code should probably be using the GDK > XI2-aware API instead (if the API allows it). that should be possible. for this part, only the property calls are important and I hope they are exported :)
ping? any further comments? If not, I'll just go ahead and start hacking :)
No objections, here.
Created attachment 174302 [details] [review] wacom-g-s-d-plugin.diff This is the current state, it applies the properties correctly though some work is still needed. Can I ask for a high-level skim of the approach please? I'm not looking for in-depth code review at this point, there will be more cleanup. The layout of the gsettings keys is nested, so we have wacom/<generic keys> wacom/stylus/<stylus-specific keys> wacom/eraser/stylus-specific keys> I need to find something better for the button mapping, but at the moment my main concerns are whether the schemas are correctly named and placed, the general approach is correct, etc. gsettings messed a bit with my months-old patches :)
Review of attachment 174302 [details] [review]: Good first pass. ::: data/gnome-settings-daemon.convert @@ +107,3 @@ rgba-order = /desktop/gnome/font_rendering/rgba_order + +[org.gnome.settings-daemon.plugins.wacom] Nope. This file is for propagating existing values in GConf to GSettings. There was nothing before your GSettings code (that we shipped anyway), so you can remove all those changes. ::: data/gnome-settings-daemon.schemas.in @@ +536,3 @@ + <schema> + <key>/schemas/apps/gnome_settings_daemon/plugins/wacom/active</key> No, this schema file is gone in master. And you should do that in GSettings anyway. ::: data/org.gnome.settings-daemon.peripherals.wacom.gschema.xml.in.in @@ +12,3 @@ + </key> + </schema> + <schema gettext-domain="@GETTEXT_PACKAGE@" id="org.gnome.settings-daemon.peripherals.wacom" path="/org/gnome/settings-daemon/peripherals/wacom/"> You're missing a "child" definition somewhere. See org.gnome.settings-daemon.peripherals.gschema.xml.in.in @@ +23,3 @@ + <_description>Enable this to only move the cursor when the user touches the tablet.</_description> + </key> + <key name="tpcbutton" type="b"> No impossible acronyms please. "tablet-pc-button" is probably better. ::: plugins/wacom/gsd-wacom-manager.c @@ +232,3 @@ +/* Run through all devices and call the callback on each wacom device. */ +static void +wacom_foreach_device (void (*callback)(XDeviceInfo *info, XDevice* device, void *userdata), void *userdata) Please define the callback separately. And user Glib-style arguments (gpointer user_data, not void *userdata). @@ +341,3 @@ + } + + /* property is inx1,y1,x2,y2 */ Typo? @@ +552,3 @@ + i < 20 && (rc = XSetDeviceButtonMapping (GDK_DISPLAY_XDISPLAY (gdk_display_get_default()), device, (unsigned char*)map->map, map->nmap)) == MappingBusy; + ++i) { + g_usleep (300); Can you add the same comment as in the mouse plugin about this construct? ::: plugins/wacom/wacom.gnome-settings-plugin.in @@ +4,3 @@ +_Name=Wacom +_Description=Wacom plugin +Authors= Don't forget to add your name :)
Created attachment 174644 [details] [review] 0001-Add-wacom-g-s-d-plugin.patch Revamped, cleaned up and I think it addresses all of Bastien's comments. "And you should do that in GSettings anyway." I think I need more explanation here.
(In reply to comment #7) > Created an attachment (id=174644) [details] [review] > 0001-Add-wacom-g-s-d-plugin.patch > > Revamped, cleaned up and I think it addresses all of Bastien's comments. > > "And you should do that in GSettings anyway." > I think I need more explanation here. We don't use GConf for configuration storage anymore, but GSettings. http://library.gnome.org/devel/gio/stable/ch26.html (and the rest of the chapter) is a good read. But it looks like you already figured that out :) Remove the useless get/set_property and we're good to commit.
Created attachment 174749 [details] [review] 0001-Add-wacom-g-s-d-plugin.patch v2 Changes to v1: - get_property/set_property calls removed.
Committed without the get/set_property functions. Thanks!