Menu
▾
▴
Re: [Linuxwacom-devel] [PATCH 4 of 4] xf86-input-wacom: Centralize TPCButton/Touch/Gesture initialization
|
From: Peter H. <pet...@wh...> - 2010-04-29 04:59:45
|
On Sun, Apr 25, 2010 at 12:00:35AM -0700, Ping Cheng wrote:
> From 6e43f315c038a0c046db1fb2badc5487ee3f5ca2 Mon Sep 17 00:00:00 2001
> From: Ping Cheng <pin...@gm...>
> Date: Sat, 24 Apr 2010 23:27:54 -0700
> Subject: [PATCH 4/4] Centralize TPCButton/Touch/Gesture initialization during wcmPreInit
>
> Moved TPCButton initialization from other routines to wcmParseOptions
> of wcmValidateDevice.c by checking tablet_type. For hotplugged tablets,
> TPCButton will be initialized once since wcmParseOptions will only be
> called once. For tools defined through xorg.conf, we only allow stylus
> to define this option.
>
> Moved wcmTouch inside ISBITSET BTN_TOOL_DOUBLETAP check since only
> the devices that support touch would have this option. Same rule
> applies to wcmGesture. For hotplugged tablets, these two options
> will be initialized once since wcmParseOptions will only be called
> once. For tools defined through xorg.conf, only touch can change
> these options.
>
> Common options would not be overridden in xorg.conf as long as the
> options are the same for all the tools that are associated with the
> same tablet. Common options set through xsetwacom apply to the whole
> tablet.
>
> Signed-off-by: Ping Cheng <pi...@wa...>
> ---
> src/wcmConfig.c | 2 +-
> src/wcmISDV4.c | 7 -------
> src/wcmUSB.c | 7 -------
> src/wcmValidateDevice.c | 35 +++++++++++++++++++++--------------
> src/xf86Wacom.c | 3 ---
> src/xf86Wacom.h | 2 +-
> 6 files changed, 23 insertions(+), 33 deletions(-)
>
> diff --git a/src/wcmConfig.c b/src/wcmConfig.c
> index ec1ca67..113bc81 100644
> --- a/src/wcmConfig.c
> +++ b/src/wcmConfig.c
> @@ -411,7 +411,7 @@ static LocalDevicePtr wcmPreInit(InputDriverPtr drv, IDevPtr dev, int flags)
>
> /* Process the common options. */
> xf86ProcessCommonOptions(local, local->options);
> - if (!wcmParseOptions(local))
> + if (!wcmParseOptions(local, need_hotplug))
> goto SetupProc_fail;
>
> /* mark the device configured */
> diff --git a/src/wcmISDV4.c b/src/wcmISDV4.c
> index b63bc9f..4b8f803 100644
> --- a/src/wcmISDV4.c
> +++ b/src/wcmISDV4.c
> @@ -172,13 +172,6 @@ static Bool isdv4Init(LocalDevicePtr local, char* id, float *version)
> /*set the model */
> common->wcmModel = &isdv4General;
>
> - /* Tablet PC Button is on by default */
> - common->wcmTPCButtonDefault = 1;
> -
> - /* check if TPCButton was turned off by user for stylus */
> - if (priv->flags & STYLUS_ID)
> - common->wcmTPCButton = xf86SetBoolOption(local->options,
> - "TPCButton", common->wcmTPCButtonDefault);
> return Success;
> }
>
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 1d4ccdd..470dc7a 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -445,13 +445,6 @@ static Bool usbWcmInit(LocalDevicePtr local, char* id, float *version)
> common->wcmResolX = WacomModelDesc [i].xRes;
> common->wcmResolY = WacomModelDesc [i].yRes;
> }
> -
> - if (common->wcmModel && (common->tablet_type & WCM_TPC))
> - {
> - /* For penabled Tablet PCs, Tablet PC Button
> - * are on by default */
> - common->wcmTPCButtonDefault = 1;
> - }
> }
>
> if (!common->wcmModel)
> diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c
> index 9d0509a..0d98bd0 100644
> --- a/src/wcmValidateDevice.c
> +++ b/src/wcmValidateDevice.c
> @@ -381,7 +381,7 @@ int wcmNeedAutoHotplug(LocalDevicePtr local, const char **type)
> return 1;
> }
>
> -int wcmParseOptions(LocalDevicePtr local)
> +int wcmParseOptions(LocalDevicePtr local, int need_hotplug)
the name "need_hotplug" doesn't really make sense here. This is how it's
called in the caller for a reason, but here a name like
"force_features" or something more creative and descriptive seems more
appropriate. Once we enter wcmParseOptions, hotplugging or not is
irrelevant.
see my commend below though.
> {
> WacomDevicePtr priv = (WacomDevicePtr)local->private;
> WacomCommonPtr common = priv->common;
> @@ -566,11 +566,16 @@ int wcmParseOptions(LocalDevicePtr local)
> if (xf86SetBoolOption(local->options, "ButtonsOnly", 0))
> priv->flags |= BUTTONS_ONLY_FLAG;
>
> - /* Tablet PC button applied to the whole tablet. Not just one tool */
> - if ( priv->flags & STYLUS_ID )
> - common->wcmTPCButton = xf86SetBoolOption(local->options,
> - "TPCButton",
> - common->wcmTPCButtonDefault);
> + /* Tablet PC */
> + if (common->tablet_type & WCM_TPC)
> + {
> + common->wcmTPCButtonDefault = 1;
> +
> + /* check if hover was turned off by user */
> + if (need_hotplug || (priv->flags & STYLUS_ID))
> + common->wcmTPCButton = xf86SetBoolOption(local->options,
> + "TPCButton", common->wcmTPCButtonDefault);
> + }
something strikes me as odd here. The result of this code is that TPCButton
is set for the first device, and then also for the stylus.
IIRC it doesn't apply to any other device, so couldn't we just remove the
check and always check for the option?
Cheers,
Peter
>
> /* a single touch device */
> if (ISBITSET (common->wcmKeys, BTN_TOOL_DOUBLETAP))
> @@ -578,6 +583,11 @@ int wcmParseOptions(LocalDevicePtr local)
> /* TouchDefault was off for all devices
> * except when touch is supported */
> common->wcmTouchDefault = 1;
> +
> + /* check if touch was turned off by user */
> + if (need_hotplug || (priv->flags & TOUCH_ID))
> + common->wcmTouch = xf86SetBoolOption(local->options,
> + "Touch", common->wcmTouchDefault);
> }
>
> /* 2FG touch device */
> @@ -586,15 +596,12 @@ int wcmParseOptions(LocalDevicePtr local)
> /* GestureDefault was off for all devices
> * except when multi-touch is supported */
> common->wcmGestureDefault = 1;
> - }
> -
> - /* check if touch was turned off in xorg.conf */
> - common->wcmTouch = xf86SetBoolOption(local->options, "Touch",
> - common->wcmTouchDefault);
>
> - /* Touch gesture applies to the whole tablet */
> - common->wcmGesture = xf86SetBoolOption(local->options, "Gesture",
> - common->wcmGestureDefault);
> + /* check if gesture was turned off by user */
> + if (need_hotplug || (priv->flags & TOUCH_ID))
> + common->wcmGesture = xf86SetBoolOption(local->options,
> + "Gesture", common->wcmGestureDefault);
> + }
>
> /* Touch capacity applies to the whole tablet */
> common->wcmCapacity = xf86SetBoolOption(local->options, "Capacity", common->wcmCapacityDefault);
> diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c
> index e71990d..424b820 100644
> --- a/src/xf86Wacom.c
> +++ b/src/xf86Wacom.c
> @@ -953,9 +953,6 @@ static Bool wcmOpen(LocalDevicePtr local)
> /* only ISDV4 are supported on X server 1.7 and later */
> common->wcmForceDevice=DEVICE_ISDV4;
> common->wcmDevCls = &gWacomISDV4Device;
> -
> - /* Tablet PC buttons on by default */
> - common->wcmTPCButtonDefault = 1;
> }
> else
> {
> diff --git a/src/xf86Wacom.h b/src/xf86Wacom.h
> index b522bd7..4e9325e 100644
> --- a/src/xf86Wacom.h
> +++ b/src/xf86Wacom.h
> @@ -152,7 +152,7 @@ extern void wcmHotplugOthers(LocalDevicePtr local);
> extern int wcmAutoProbeDevice(LocalDevicePtr local);
>
> /* setup */
> -extern int wcmParseOptions(LocalDevicePtr local);
> +extern int wcmParseOptions(LocalDevicePtr local, int need_hotplug);
> extern void wcmInitialCoordinates(LocalDevicePtr local, int axes);
> extern void wcmInitialScreens(LocalDevicePtr local);
> extern void wcmInitialScreens(LocalDevicePtr local);
> --
> 1.6.6.1
>
|