GNOME Bugzilla – Bug 773779
Implement drawing tablet support on x11
Last modified: 2016-11-07 14:53:24 UTC
Currently we only have mutter managing tablet configuration on wayland, management on X11 is still done by gnome-settings-daemon, based on "old" settings. I'm attaching some patches to make mutter take over X11 tablet configuration too, and we can finally remove the wacom tablet management in g-s-d. This is based on the Wacom input driver at the moment, the libinput driver is not the fallback driver yet for tablets and pads, so that is left as another bridge to cross (hopefully soon). That said, porting from Wacom to libinput will be mostly straightforward on top of these patches.
Created attachment 338891 [details] [review] Revert "backends: Use g-s-d settings for tablet configuration" This reverts commit b52f304f9db590220555cb3a04a3c31f32ee7f19.
Created attachment 338892 [details] [review] clutter/x11: Implement XTest-based ClutterVirtualInputDevice This will be used too on X11 in order to implement the button-to-keycombo mapping in pad devices.
Created attachment 338893 [details] [review] clutter/x11: Add minimal support for pad devices We most notably handle button events (acquired through a passive grab on all device buttons) which are translated to CLUTTER_PAD_BUTTON* events, so there is generic handling of pad actions on X11.
Created attachment 338894 [details] [review] core: Handle special actions associated to pad buttons in generic code And remove the wayland-specific handling. This works for both Wayland and X11 (provided the compositor receives pad events through a passive grab there).
Created attachment 338895 [details] [review] backends: Add missing pad button-to-keycombo translation
Created attachment 338896 [details] [review] clutter: Add ClutterDeviceManager::tool-changed signal This signal will notify whenever a device changed tool.
Created attachment 338897 [details] [review] clutter/evdev: Emit ClutterDeviceManager::tool-changed We do so whenever a tool enters or leaves proximity. We now also ensure that last_tool is NULL after it leaves proximity, although the CLUTTER_PROXIMITY_OUT event itself should still contain tool information.
Created attachment 338898 [details] [review] clutter/evdev: Take over stylus configuration Stylus configuration (stylus buttons, pressure) was handled at the very high level, doing the button and pressure translations right before sending these to wayland clients. However, it makes more sense to store these settings into the ClutterInputDeviceTool itself, and have clutter apply the config at the lower level so 1) the settings actually apply desktop-wide, not just in clients and 2) X11 and wayland may share similar configuration paths. The settings are now just applied whenever the tool enters proximity, in reaction to ClutterDeviceManager::tool-changed. This commit moves all handling of these two settings to the clutter level, and removes the wayland-specific paths
Created attachment 338899 [details] [review] backends: Remove ToolSettings struct Its only purpose was caching settings applying to an stylus/tool, this is now handled through ClutterInputDeviceTool evdev specific API, or X device properties, so is not needed anymore.
Created attachment 338900 [details] [review] backends: extend tablet device checks The Clutter X11 backend can't drop CLUTTER_PEN_DEVICE and CLUTTER_ERASER_DEVICE in favor of CLUTTER_TABLET_DEVICE without losing information (as the driver will create one device for each). So make MetaInputSettings cater for both sets of device types.
Created attachment 338901 [details] [review] backends/x11: Implement tablet settings based on the Wacom driver This is a stopgap solution until libinput is the fallback driver handling tablets and pads.
Created attachment 338902 [details] [review] clutter/x11: Implement ClutterInputDeviceTool This is implemented using Wacom-driver specific properties at the moment, until libinput becomes the fallback driver handling tablet and pad management. Whenever a tool becomes in proximity, a new ClutterInputDeviceToolXI2 will be created (if it wasn't created previously) for the given serial number. This tool will be set in all events send from the device.
Review of attachment 338891 [details] [review]: Sure
Review of attachment 338892 [details] [review]: OK
Review of attachment 338893 [details] [review]: OK
Review of attachment 338894 [details] [review]: Nice
Review of attachment 338895 [details] [review]: OK
Review of attachment 338896 [details] [review]: OK
Review of attachment 338897 [details] [review]: OK
Review of attachment 338899 [details] [review]: Yay
Review of attachment 338900 [details] [review]: ::: src/backends/meta-input-settings.c @@ +875,3 @@ + clutter_input_device_get_device_type (device) != CLUTTER_PEN_DEVICE && + clutter_input_device_get_device_type (device) != CLUTTER_ERASER_DEVICE && + clutter_input_device_get_device_type (device) != CLUTTER_PAD_DEVICE) Is PAD_DEVICE needed? If yes, can it really be omitted from the other conditions?
Review of attachment 338902 [details] [review]: ::: clutter/clutter/Makefile.am @@ +414,3 @@ x11/clutter-device-manager-xi2.c \ x11/clutter-input-device-xi2.c \ + x11/clutter-input-device-tool-xi2.c \ Looks like you forgot to git-add those ::: clutter/clutter/x11/clutter-device-manager-xi2.c @@ +967,3 @@ + prop = XInternAtom (backend_x11->xdpy, "Wacom Serial IDs", True); + if (prop == None) + return FALSE; IMHO serial_id or 0 would be clearer here
Review of attachment 338901 [details] [review]: ::: src/backends/x11/meta-input-settings-x11.c @@ +703,3 @@ + return; + + /* Grab the puke bucket! */ Again? We already got one from above!!1!
Review of attachment 338898 [details] [review]: OK
(In reply to Florian Müllner from comment #22) > Review of attachment 338902 [details] [review] [review]: > > ::: clutter/clutter/Makefile.am > @@ +414,3 @@ > x11/clutter-device-manager-xi2.c \ > x11/clutter-input-device-xi2.c \ > + x11/clutter-input-device-tool-xi2.c \ > > Looks like you forgot to git-add those Darn > > ::: clutter/clutter/x11/clutter-device-manager-xi2.c > @@ +967,3 @@ > + prop = XInternAtom (backend_x11->xdpy, "Wacom Serial IDs", True); > + if (prop == None) > + return FALSE; > > IMHO serial_id or 0 would be clearer here Indeed, seems a brain fart, a harmless one for a change :).
Created attachment 339134 [details] [review] clutter/x11: Implement ClutterInputDeviceTool This is implemented using Wacom-driver specific properties at the moment, until libinput becomes the fallback driver handling tablet and pad management. Whenever a tool becomes in proximity, a new ClutterInputDeviceToolXI2 will be created (if it wasn't created previously) for the given serial number. This tool will be set in all events send from the device.
Review of attachment 339134 [details] [review]: OK (Disclaimer: all the patches are untested on my part, not least due to missing hardware)
(In reply to Florian Müllner from comment #21) > Review of attachment 338900 [details] [review] [review]: > > ::: src/backends/meta-input-settings.c > @@ +875,3 @@ > + clutter_input_device_get_device_type (device) != CLUTTER_PEN_DEVICE && > + clutter_input_device_get_device_type (device) != > CLUTTER_ERASER_DEVICE && > + clutter_input_device_get_device_type (device) != CLUTTER_PAD_DEVICE) > > Is PAD_DEVICE needed? If yes, can it really be omitted from the other > conditions? Yes, and yes. pad is only affected by the left-handed setting here (because buttons change position if you flip the tablet 180º), the other settings are about abs coordinates, so only affect styli. (In reply to Florian Müllner from comment #27) > Review of attachment 339134 [details] [review] [review]: > > OK > > (Disclaimer: all the patches are untested on my part, not least due to > missing hardware) I assumed as much :), took good care of that myself. Equally appreciated anyway!
Attachment 338891 [details] pushed as db9d8fc - Revert "backends: Use g-s-d settings for tablet configuration" Attachment 338892 [details] pushed as 674a483 - clutter/x11: Implement XTest-based ClutterVirtualInputDevice Attachment 338893 [details] pushed as 9abf689 - clutter/x11: Add minimal support for pad devices Attachment 338894 [details] pushed as 1831a1d - core: Handle special actions associated to pad buttons in generic code Attachment 338895 [details] pushed as cea7d62 - backends: Add missing pad button-to-keycombo translation Attachment 338896 [details] pushed as bd83873 - clutter: Add ClutterDeviceManager::tool-changed signal Attachment 338897 [details] pushed as 75c3f0f - clutter/evdev: Emit ClutterDeviceManager::tool-changed Attachment 338898 [details] pushed as b252771 - clutter/evdev: Take over stylus configuration Attachment 338899 [details] pushed as 6257f11 - backends: Remove ToolSettings struct Attachment 338900 [details] pushed as b35b531 - backends: extend tablet device checks Attachment 338901 [details] pushed as ff97536 - backends/x11: Implement tablet settings based on the Wacom driver Attachment 339134 [details] pushed as ea4dbdd - clutter/x11: Implement ClutterInputDeviceTool
This causes an issue compiling mutter git. x11/clutter-input-device-tool-xi2.c:30:43: error: no previous declaration for ‘clutter_input_device_tool_xi2_get_type’ [-Werror=missing-declarations] Editing the makefile to make it wno error makes it compile.
It's now fixed in master, sorry about that. And strange I wasn't getting the error using "jhbuild make", however plain autogen+make got me there.