On Tue, Mar 30, 2010 at 04:18:26PM -0700, Ping Cheng wrote:
> From b31371bdc655d554074fe0f3982498d2d4c062e0 Mon Sep 17 00:00:00 2001
> From: Ping Cheng <[email protected]>
> Date: Tue, 30 Mar 2010 15:40:49 -0700
> Subject: [PATCH] Normalize pressure sensitivity
>
> Instead of reporting the raw pressure, the normalized pressure from
> 0 to FILTER_PRESSURE_RES (which is 2048) is reported. This is mainly
> to deal with the case where heavily used stylus may have a "pre-loaded"
> initial pressure. This patch checks the in-prox pressure and subtract
> it from the raw pressure to prevent a potential left-click before the
> pen touches the tablet.
>
> Left click threshold and pressure curve are updated accordingly.
>
> Signed-off-by: Ping Cheng <[email protected]>
Finally got through this review, took me a bit to understand it but this is
a great commit message, thank you.
> ---
> src/wcmCommon.c | 56
> +++++++++++++++++++++++++++++++--------------------
> src/wcmXCommand.c | 2 +-
> src/xf86Wacom.c | 4 +-
> src/xf86WacomDefs.h | 1 +
> 4 files changed, 38 insertions(+), 25 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index f5a91d4..e9822de 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1445,18 +1445,39 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
>
> if (IsStylus(priv) || IsEraser(priv))
> {
can you copy the commit message in here (maybe slightly rephrased) so that a
reader of the code immediately knows what's going on here.
> + double tmpP;
> +
> + /* set the minimum pressure when in prox */
> + if (!priv->oldProximity)
> + priv->minPressure = filtered.pressure;
> + else if (priv->minPressure > filtered.pressure)
> + priv->minPressure = filtered.pressure;
> +
> + /* normalize pressure to FILTER_PRESSURE_RES */
> + tmpP = (filtered.pressure - priv->minPressure)
> + * FILTER_PRESSURE_RES;
> + tmpP /= common->wcmMaxZ - priv->minPressure;
> + filtered.pressure = (int)tmpP;
> +
> /* set button1 (left click) on/off */
> - if (filtered.pressure >= common->wcmThreshold)
> - filtered.buttons |= button;
> - else
> + if (filtered.pressure < common->wcmThreshold)
> {
> - /* threshold tolerance */
> - int tol = common->wcmMaxZ / 250;
> - if (strstr(common->wcmModel->name, "Intuos4"))
> - tol = common->wcmMaxZ / 125;
> - if (filtered.pressure < common->wcmThreshold -
> tol)
> - filtered.buttons &= ~button;
> + filtered.buttons &= ~button;
> + if (priv->oldButtons & button) /* left click
> was on */
> + {
> + /* threshold tolerance */
> + int tol = FILTER_PRESSURE_RES / 125;
why is this hardcoded as 125? can we have a define for this please?
> +
> + /* don't set it off if it is within the
> tolerance
> + and the tolerance is larger than
> threshold */
> + if ((common->wcmThreshold > tol) &&
> + (filtered.pressure >
> common->wcmThreshold - tol))
> + filtered.buttons |= button;
> + }
> }
> + else
> + filtered.buttons |= button;
> +
> /* transform pressure */
> transPressureCurve(priv,&filtered);
> }
> @@ -1601,10 +1622,8 @@ int wcmInitTablet(LocalDevicePtr local, const char*
> id, float version)
> if (common->wcmThreshold <= 0)
> {
> /* Threshold for counting pressure as a button */
> - if (strstr(common->wcmModel->name, "Intuos4"))
> - common->wcmThreshold = common->wcmMaxZ * 3 / 25;
> - else
> - common->wcmThreshold = common->wcmMaxZ * 3 / 50;
> + common->wcmThreshold = FILTER_PRESSURE_RES / 75;
> +
same here, the / 75 shouldn't be a magic number, but a magic define instead.
> xf86Msg(X_PROBED, "%s: using pressure threshold of %d for
> button 1\n",
> local->name, common->wcmThreshold);
> }
> @@ -1648,18 +1667,11 @@ static void transPressureCurve(WacomDevicePtr pDev,
> WacomDeviceStatePtr pState)
> int p = pState->pressure;
>
> /* clip */
> - p = (p < 0) ? 0 : (p > pDev->common->wcmMaxZ) ?
> - pDev->common->wcmMaxZ : p;
> -
> - /* rescale pressure to FILTER_PRESSURE_RES */
> - p = (p * FILTER_PRESSURE_RES) / pDev->common->wcmMaxZ;
> + p = (p < 0) ? 0 : (p > FILTER_PRESSURE_RES) ?
> + FILTER_PRESSURE_RES : p;
urgh, no double nested ternaries please, especially when they go across two
lines. how about using min/max instead?
int p = min(FILTER_PRESSURE_RES, max(0, pState->pressure));
(we might need min/max macros for this but I think the server exports them
through misc.h)
>
> /* apply pressure curve function */
> p = pDev->pPressCurve[p];
> -
> - /* scale back to wcmMaxZ */
> - pState->pressure = (p * pDev->common->wcmMaxZ) /
> - FILTER_PRESSURE_RES;
> }
> }
>
> diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> index c8e4f5f..40207a3 100644
> --- a/src/wcmXCommand.c
> +++ b/src/wcmXCommand.c
> @@ -636,7 +636,7 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property,
> XIPropertyValuePtr prop,
>
> value = *(CARD32*)prop->data;
>
> - if ((value < 1) || (value > common->wcmMaxZ))
> + if ((value < 1) || (value > FILTER_PRESSURE_RES))
> return BadValue;
>
> if (!checkonly)
> diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c
> index efca491..3f951df 100644
> --- a/src/xf86Wacom.c
> +++ b/src/xf86Wacom.c
> @@ -767,12 +767,12 @@ static int wcmRegisterX11Devices (LocalDevicePtr local)
> /* Rotation rotates the Max X and Y */
> wcmRotateTablet(local, common->wcmRotate);
>
> - /* pressure */
> + /* pressure normalized to FILTER_PRESSURE_RES */
> InitValuatorAxisStruct(local->dev, 2,
> #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) >= 7
> XIGetKnownProperty(AXIS_LABEL_PROP_ABS_PRESSURE),
> #endif
> - 0, common->wcmMaxZ, 1, 1, 1);
> + 0, FILTER_PRESSURE_RES, 1, 1, 1);
>
> if (IsCursor(priv))
> {
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index 4691a58..b22f78e 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -240,6 +240,7 @@ struct _WacomDeviceRec
> /* JEJ - filters */
> int* pPressCurve; /* pressure curve */
> int nPressCtrl[4]; /* control points for curve */
> + int minPressure; /* the minimum pressure a pen may hold */
>
> WacomToolPtr tool; /* The common tool-structure for this device
> */
> WacomToolAreaPtr toolarea; /* The area defined for this device */
> --
> 1.6.6.1
>
Cheers,
Peter
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
