On Thu, Mar 3, 2011 at 4:56 PM, Peter Hutterer <[email protected]>wrote:
> Instead of returning rather ambiguous 0, 1 and 2, return enums with
> readable
> descriptions.
>
> No functional changes, other than that we now skip a few tests in
> wcmCheckSuppress if we already have the result.
>
> Signed-off-by: Peter Hutterer <[email protected]>
> ---
> src/wcmCommon.c | 101
> ++++++++++++++++++++++++++++++----------------------
> src/xf86Wacom.h | 6 +++
> test/wacom-tests.c | 14 +++++++
> 3 files changed, 78 insertions(+), 43 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index 0a5a8ac..16d850e 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -53,8 +53,10 @@ static int *VCOPY(const int *valuators, int nvals)
>
> ****************************************************************************/
>
> static int applyPressureCurve(WacomDevicePtr pDev, const
> WacomDeviceStatePtr pState);
> -static void commonDispatchDevice(WacomCommonPtr common, unsigned int
> channel,
> - const WacomChannelPtr pChannel, int suppress);
> +static void commonDispatchDevice(WacomCommonPtr common,
> + unsigned int channel,
> + const WacomChannelPtr pChannel,
> + enum WacomSuppressMode suppress);
> static void sendAButton(InputInfoPtr pInfo, int button, int mask,
> int first_val, int num_vals, int *valuators);
>
> @@ -766,50 +768,65 @@ void wcmSendEvents(InputInfoPtr pInfo, const
> WacomDeviceState* ds)
> }
> }
>
>
> -/*****************************************************************************
> - * wcmCheckSuppress --
> - * Determine whether device state has changed enough - return 0
> - * if not.
> -
> ****************************************************************************/
> -
> -static int wcmCheckSuppress(WacomCommonPtr common,
> - const WacomDeviceState* dsOrig,
> - WacomDeviceState* dsNew)
> +/**
> + * Determine whether device state has changed enough to warrant further
> + * processing. The driver's "suppress" setting decides how much
> + * movement/state change must occur before we process events to avoid
> + * overloading the server with minimal changes (and getting fuzzy events).
> + * wcmCheckSuppress ensures that events meet this standard.
> + *
> + * @param dsOrig Previous device state
> + * @param dsNew Current device state
> + *
> + * @retval SUPPRESS_ALL Ignore this event completely.
> + * @retval SUPPRESS_NONE Process event normally.
> + * @retval SUPPRESS_NON_MOTION Suppress all data but motion data.
> + */
> +static enum WacomSuppressMode
> +wcmCheckSuppress(WacomCommonPtr common,
> + const WacomDeviceState* dsOrig,
> + WacomDeviceState* dsNew)
> {
> int suppress = common->wcmSuppress;
> - /* NOTE: Suppression value of zero disables suppression. */
> - int returnV = 0;
> + enum WacomSuppressMode returnV = SUPPRESS_NONE;
>
> /* Ignore all other changes that occur after initial out-of-prox. */
> if (!dsNew->proximity && !dsOrig->proximity)
> - return 0;
> + return SUPPRESS_ALL;
>
> /* Never ignore proximity changes. */
> - if (dsOrig->proximity != dsNew->proximity) returnV = 1;
> -
> - if (dsOrig->buttons != dsNew->buttons) returnV = 1;
> - if (dsOrig->stripx != dsNew->stripx) returnV = 1;
> - if (dsOrig->stripy != dsNew->stripy) returnV = 1;
> - if (abs(dsOrig->tiltx - dsNew->tiltx) > suppress) returnV = 1;
> - if (abs(dsOrig->tilty - dsNew->tilty) > suppress) returnV = 1;
> - if (abs(dsOrig->pressure - dsNew->pressure) > suppress) returnV =
> 1;
> - if (abs(dsOrig->capacity - dsNew->capacity) > suppress) returnV =
> 1;
> - if (abs(dsOrig->throttle - dsNew->throttle) > suppress) returnV =
> 1;
> + if (dsOrig->proximity != dsNew->proximity) goto out;
> +
> + if (dsOrig->buttons != dsNew->buttons) goto out;
> + if (dsOrig->stripx != dsNew->stripx) goto out;
> + if (dsOrig->stripy != dsNew->stripy) goto out;
> + if (abs(dsOrig->tiltx - dsNew->tiltx) > suppress) goto out;
> + if (abs(dsOrig->tilty - dsNew->tilty) > suppress) goto out;
> + if (abs(dsOrig->pressure - dsNew->pressure) > suppress) goto out;
> + if (abs(dsOrig->capacity - dsNew->capacity) > suppress) goto out;
> + if (abs(dsOrig->throttle - dsNew->throttle) > suppress) goto out;
> if (abs(dsOrig->rotation - dsNew->rotation) > suppress &&
> - (1800 - abs(dsOrig->rotation - dsNew->rotation)) >
> suppress) returnV = 1;
> + (1800 - abs(dsOrig->rotation - dsNew->rotation)) > suppress)
> goto out;
>
> /* look for change in absolute wheel position
> * or any relative wheel movement
> */
> if ((abs(dsOrig->abswheel - dsNew->abswheel) > suppress)
> - || (dsNew->relwheel != 0)) returnV = 1;
> + || (dsNew->relwheel != 0))
> + goto out;
>
> - /* cursor moves or not? */
> + returnV = SUPPRESS_ALL;
> +
> +out:
> + /* Special handling for cursor: if nothing else changed but the
> + * pointer x/y, suppress all but cursor movement. This return value
> + * is used in commonDispatchDevice to short-cut event processing.
> + */
> if ((abs(dsOrig->x - dsNew->x) > suppress) ||
> (abs(dsOrig->y - dsNew->y) > suppress))
> {
> - if (!returnV) /* need to check if cursor moves or not */
> - returnV = 2;
> + if (returnV == SUPPRESS_ALL)
> + returnV = SUPPRESS_NON_MOTION;
> }
> else /* don't move cursor */
> {
> @@ -833,7 +850,7 @@ void wcmEvent(WacomCommonPtr common, unsigned int
> channel,
> WacomDeviceState* pLast;
> WacomDeviceState ds;
> WacomChannelPtr pChannel;
> - int suppress = 0;
> + enum WacomSuppressMode suppress;
> WacomDevicePtr priv = common->wcmDevices;
> pChannel = common->wcmChannel + channel;
> pLast = &pChannel->valid.state;
> @@ -895,12 +912,10 @@ void wcmEvent(WacomCommonPtr common, unsigned int
> channel,
> common->wcmModel->FilterRaw(common,pChannel,&ds);
> }
>
> - /* Discard unwanted data */
> + /* skip event if we don't have enough movement */
>
Maybe you want to add this info to the comments: Suppress is not aimed at
skipping the event due to no movement. The skipping part was a minor
feature. The main purpose was to discard small (x,y) movement (less than
wcmSuppress) to avoid jitter when user holding the tool on the tablet.
> suppress = wcmCheckSuppress(common, pLast, &ds);
> - if (!suppress)
> - {
> + if (suppress == SUPPRESS_ALL)
> return;
> - }
>
> /* JEJ - Do not move this code without discussing it with me.
> * The device state is invariant of any filtering performed below.
> @@ -1148,7 +1163,8 @@ setPressureButton(const WacomDevicePtr priv, const
> WacomDeviceState *ds)
> }
>
> static void commonDispatchDevice(WacomCommonPtr common, unsigned int
> channel,
> - const WacomChannelPtr pChannel, Bool
> suppress)
> + const WacomChannelPtr pChannel,
> + enum WacomSuppressMode suppress)
> {
> InputInfoPtr pInfo = NULL;
> WacomToolPtr tool = NULL;
> @@ -1260,20 +1276,19 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
> deltx *= priv->factorX;
> delty *= priv->factorY;
>
> + /* less than one device coordinate movement? */
>
Sorry to bother you again. I am still trying to figure out how this check
works. I must have missed the concept. What is a device coordinate? Tool
(x,y), I guess. How do we get a less than one device coordinate movement? I
thought it is the screen movement that we cares about, not the tool
movement. A device coordinate
change that causes a deltx/y >=1 may not lead to a cursor movement.
If the if-statement doesn't avoid discarding non-cursor movement events due
to the factor that we do not have enough information to make the decision
(lack of screen size/resolution, etc), removing this block of code might be
a better option.
I need to understand the concept first though. Please shed light on me.
Ping
if (abs(deltx)<1 && abs(delty)<1)
> {
> - /* don't move the cursor */
> - if (suppress == 1)
> - {
> - /* send other events, such as button/wheel
> */
> - filtered.x = priv->oldX;
> - filtered.y = priv->oldY;
> - }
> - else /* no other events to send */
> + /* We have no other data in this event, skip */
> + if (suppress == SUPPRESS_NON_MOTION)
> {
> DBG(10, common, "Ignore non-movement
> relative data \n");
> return;
> }
> +
> + /* send other events, such as button/wheel */
> + filtered.x = priv->oldX;
> + filtered.y = priv->oldY;
> }
> }
>
> diff --git a/src/xf86Wacom.h b/src/xf86Wacom.h
> index 5ddb4e4..ac012f4 100644
> --- a/src/xf86Wacom.h
> +++ b/src/xf86Wacom.h
> @@ -175,6 +175,12 @@ extern WacomCommonPtr wcmRefCommon(WacomCommonPtr
> common);
> extern void wcmFreeCommon(WacomCommonPtr *common);
> extern WacomCommonPtr wcmNewCommon(void);
>
> +enum WacomSuppressMode {
> + SUPPRESS_NONE = 8, /* Process event normally */
> + SUPPRESS_ALL, /* Supress and discard the whole event */
> + SUPPRESS_NON_MOTION /* Supress all events but x/y motion */
> +};
> +
>
>
> /****************************************************************************/
> #endif /* __XF86WACOM_H */
>
> diff --git a/test/wacom-tests.c b/test/wacom-tests.c
> index 2d48e18..b8ad63c 100644
> --- a/test/wacom-tests.c
> +++ b/test/wacom-tests.c
> @@ -216,6 +216,19 @@ test_initial_size(void)
>
> }
>
> +static void
> +test_suppress(void)
> +{
> + enum WacomSuppressMode rc;
> + WacomCommonRec common = {0};
> + WacomDeviceState old = {0},
> + new = {0};
> +
> + common.wcmSuppress = 2;
> +
> + rc = wcmCheckSuppress(&common, &old, &new);
> + g_assert(rc == SUPPRESS_ALL);
> +}
>
> int main(int argc, char** argv)
> {
> @@ -223,6 +236,7 @@ int main(int argc, char** argv)
> g_test_add_func("/common/refcounting", test_common_ref);
> g_test_add_func("/common/rebase_pressure", test_rebase_pressure);
> g_test_add_func("/common/normalize_pressure",
> test_normalize_pressure);
> + g_test_add_func("/common/test_suppress", test_suppress);
> g_test_add_func("/xfree86/initial_size", test_initial_size);
> return g_test_run();
> }
> --
> 1.7.4
>
>
>
> ------------------------------------------------------------------------------
> What You Don't Know About Data Connectivity CAN Hurt You
> This paper provides an overview of data connectivity, details
> its effect on application quality, and explores various alternative
> solutions. http://p.sf.net/sfu/progress-d2d
> _______________________________________________
> Linuxwacom-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>
------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
