On Tue, Mar 11, 2014 at 2:10 PM, Egbert Eich <e...@freedesktop.org> wrote:
> From: Egbert Eich <e...@suse.com>
>
> Worn out devices send a non-zero pressure even when not in contact
> with the tablet. To compensate for this the driver detects if the
> pressure sent by the device immediately after indicating proximity
> is non-zero. It substracts this value from any pressure value sent
> later and rescales the pressure range to the full device range.
> If it later on sees the pressure value fall below this inital value
> it will readjust it this lower value.
> The downside of this is that when the pen is pushed onto the tablet
> really fast the initial pressure reading may be non-zero also the
> pen isn't worn. This can lead to lost events.
> This patch tries to address this:
> If the first pressure reading is != 0 it is recorded. If the recorded
> maximum value is >0 but a later pressure reading is higher the maximum
> value is replaced . If no button press event is generated the 'normal'
> way it is checked of the recorded maximum would trigger one when
> minPressure decreases.
> Once a 'normal' button event is generated or minPressure doesn't
> decrease any more the recorded maximum is set to 0 which will disable
> the checks until the next prox in.
>
> Signed-off-by: Egbert Eich <e...@suse.com>
> ---
Looking good -- the logic seems sound, though there are two questions
I have (and one nitpick)
> v2: After a 'regular' button event occured don't consider MaxRawPressure
> until next prox in.
> Don't consider MaxRawPressure any more when it has triggered a button
> event but minPressure
> doesn't drop any more.
>
> src/wcmCommon.c | 68
> +++++++++++++++++++++++++++++++++++++++++++++--------
> src/xf86Wacom.h | 2 +-
> src/xf86WacomDefs.h | 2 ++
> test/wacom-tests.c | 11 ++++-----
> 4 files changed, 65 insertions(+), 18 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index afc09f9..b39be08 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1066,11 +1066,11 @@ rebasePressure(const WacomDevicePtr priv, const
> WacomDeviceState *ds)
> * @see rebasePressure
> */
> TEST_NON_STATIC int
> -normalizePressure(const WacomDevicePtr priv, const WacomDeviceState *ds)
> +normalizePressure(const WacomDevicePtr priv, const int raw_pressure)
> {
> WacomCommonPtr common = priv->common;
> double pressure;
> - int p = ds->pressure;
> + int p = raw_pressure;
> int range_left;
>
> /* normalize pressure to 0..FILTER_PRESSURE_RES */
> @@ -1094,16 +1094,16 @@ normalizePressure(const WacomDevicePtr priv, const
> WacomDeviceState *ds)
> * Returns the state of all buttons, but buttons other than button 1 are
> * unmodified.
> */
> +#define PRESSURE_BUTTON 1
> static int
> -setPressureButton(const WacomDevicePtr priv, const WacomDeviceState *ds)
> +setPressureButton(const WacomDevicePtr priv, int buttons, const int pressure)
> {
> WacomCommonPtr common = priv->common;
> - int button = 1;
> - int buttons = ds->buttons;
> + int button = PRESSURE_BUTTON;
>
> /* button 1 Threshold test */
> /* set button1 (left click) on/off */
> - if (ds->pressure < common->wcmThreshold)
> + if (pressure < common->wcmThreshold)
> {
> buttons &= ~button;
> if (priv->oldButtons & button) /* left click was on */
> @@ -1111,7 +1111,7 @@ setPressureButton(const WacomDevicePtr priv, const
> WacomDeviceState *ds)
> /* don't set it off if it is within the tolerance
> and threshold is larger than the tolerance */
> if ((common->wcmThreshold > THRESHOLD_TOLERANCE) &&
> - (ds->pressure > common->wcmThreshold -
> THRESHOLD_TOLERANCE))
> + (pressure > common->wcmThreshold -
> THRESHOLD_TOLERANCE))
> buttons |= button;
> }
> }
> @@ -1169,6 +1169,7 @@ static void commonDispatchDevice(WacomCommonPtr common,
> unsigned int channel,
> WacomDeviceState* ds = &pChannel->valid.states[0];
> WacomDevicePtr priv = NULL;
> WacomDeviceState filtered;
> + int raw_pressure = 0;
>
> /* device_type should have been retrieved and set in the respective
> * models, wcmISDV4.c or wcmUSB.c. Once it comes here, something
> @@ -1242,11 +1243,58 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
>
> if ((IsPen(priv) || IsTouch(priv)) && common->wcmMaxZ)
> {
> + int prev_min_pressure = priv->oldProximity ?
> priv->minPressure : 0;
> +
> detectPressureIssue(priv, common, &filtered);
> +
> + raw_pressure = filtered.pressure;
> + if (!priv->oldProximity)
> + priv->maxRawPressure = raw_pressure;
> +
> priv->minPressure = rebasePressure(priv, &filtered);
> - filtered.pressure = normalizePressure(priv, &filtered);
> - if (IsPen(priv))
> - filtered.buttons = setPressureButton(priv, &filtered);
> +
> + filtered.pressure = normalizePressure(priv,
> filtered.pressure);
> + if (IsPen(priv)) {
> + filtered.buttons = setPressureButton(priv,
> + filtered.buttons,
> +
> filtered.pressure);
> +
> + /* Here we run a heustic to avoid the loss of button
> events if the
Nit: s/heustic/heuristic/
> + * pen gets pushed onto the tablet so quickly that
> the first pressure
> + * event read is non-zero and is thus interpreted as
> a pressure bias */
> + if (filtered.buttons & PRESSURE_BUTTON) {
> + /* If we triggered 'normally' reset max
> pressure to
> + * avoid to trigger again while this device
> is in proximity */
> + priv->maxRawPressure = 0;
> + } else if (priv->maxRawPressure) {
> + /* If we haven't triggered normally we record
> the maximal pressure
> + * and see if this would have triggered with
> a lowered bias. */
> + if (priv->maxRawPressure < raw_pressure &&
> + priv->prevRawPressure <= raw_pressure )
> + priv->maxRawPressure = raw_pressure;
> + if (priv->maxRawPressure) {
This check appears to be unnecessary? priv->maxRawPressure was
non-zero upon entering this block, and the 'if' statement immediately
above will never decrease its value.
> + int norm_max_pressure =
> normalizePressure(priv,
> +
> priv->maxRawPressure);
> + filtered.buttons =
> setPressureButton(priv,
> +
> filtered.buttons,
> +
> norm_max_pressure);
> + /* If minPressure is not decrementing
> any more
> + * reset maxRawPressure to avoid that
> worn devices won't
> + * report a button release until
> going out of proximity */
> + if (filtered.buttons &
> PRESSURE_BUTTON &&
> + priv->minPressure ==
> prev_min_pressure)
> + priv->maxRawPressure = 0;
> + }
> + /* minPressure returned to 0. If
> maxRawPressure would ever
> + * trigger a button press it just has. Reset
> maxRawPressure now. */
> + if (!priv->minPressure)
> + priv->maxRawPressure = 0;
> + }
> + /* We need to record the pressure for the next time
> we come here to make
> + * sure we only record maxRawPressure on an inclining
> slope. We cannot use
> + * maxRawPressure as this may have been reset */
> + priv->prevRawPressure = raw_pressure;
If maxRawPressure is zero for any reason, the block of code that bumps
up its value will never be called. There should be no need for
prevRawPressure from what I understand.
> + }
> filtered.pressure = applyPressureCurve(priv,&filtered);
> }
> else if (IsCursor(priv) && !priv->oldCursorHwProx)
> diff --git a/src/xf86Wacom.h b/src/xf86Wacom.h
> index c0448f2..c96e2ee 100644
> --- a/src/xf86Wacom.h
> +++ b/src/xf86Wacom.h
> @@ -205,7 +205,7 @@ extern int wcmSetType(InputInfoPtr pInfo, const char
> *type);
> extern int getScrollDelta(int current, int old, int wrap, int flags);
> extern int getWheelButton(int delta, int action_up, int action_dn);
> extern int rebasePressure(const WacomDevicePtr priv, const WacomDeviceState
> *ds);
> -extern int normalizePressure(const WacomDevicePtr priv, const
> WacomDeviceState *ds);
> +extern int normalizePressure(const WacomDevicePtr priv, const int
> raw_pressure);
> extern enum WacomSuppressMode wcmCheckSuppress(WacomCommonPtr common,
> const WacomDeviceState*
> dsOrig,
> WacomDeviceState* dsNew);
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index afe6543..356cd0c 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -305,6 +305,8 @@ struct _WacomDeviceRec
> int minPressure; /* the minimum pressure a pen may hold */
> int oldMinPressure; /* to record the last minPressure before
> going out of proximity */
> unsigned int eventCnt; /* count number of events while in proximity
> */
> + int maxRawPressure; /* maximum 'raw' pressure seen until first
> button event */
> + int prevRawPressure; /* 'raw' pressure send by the previous event
> */
See above.
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
> WacomToolPtr tool; /* The common tool-structure for this
> device */
>
> int isParent; /* set to 1 if the device is not
> auto-hotplugged */
> diff --git a/test/wacom-tests.c b/test/wacom-tests.c
> index f44d3ab..a245021 100644
> --- a/test/wacom-tests.c
> +++ b/test/wacom-tests.c
> @@ -179,7 +179,6 @@ test_normalize_pressure(void)
> InputInfoRec pInfo = {0};
> WacomDeviceRec priv = {0};
> WacomCommonRec common = {0};
> - WacomDeviceState ds = {0};
> int pressure, prev_pressure = -1;
> int i, j;
>
> @@ -198,9 +197,9 @@ test_normalize_pressure(void)
>
> for (i = 0; i <= common.wcmMaxZ; i++)
> {
> - ds.pressure = i;
> + pressure = i;
>
> - pressure = normalizePressure(&priv, &ds);
> + pressure = normalizePressure(&priv, pressure);
> assert(pressure >= 0);
> assert(pressure <= FILTER_PRESSURE_RES);
>
> @@ -216,14 +215,12 @@ test_normalize_pressure(void)
> * minPressure and ignores actual pressure. This would be a bug in the
> * driver code, but we might as well test for it. */
> priv.minPressure = 10;
> - ds.pressure = 0;
>
> - prev_pressure = normalizePressure(&priv, &ds);
> + prev_pressure = normalizePressure(&priv, 0);
> for (i = 0; i < priv.minPressure; i++)
> {
> - ds.pressure = i;
>
> - pressure = normalizePressure(&priv, &ds);
> + pressure = normalizePressure(&priv, i);
>
> assert(pressure >= 0);
> assert(pressure < FILTER_PRESSURE_RES);
> --
> 1.8.4.5
>
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
