On Tue, Dec 13, 2011 at 01:59:02PM -0800, Jason Gerecke wrote:
> Replaces sendWheelStripEvents and getWheelButton with several
> small functions to reduce unnecessary code duplication.
>
> As a side-effect, it is now possible for the driver to handle
> simultaneous scrolls on multiple axes. Previously, getWheelButton
> would go through all the axes but was limited to returning a
> single button/action.
>
> Signed-off-by: Jason Gerecke <killert...@gmail.com>
thanks! this patch makes it messy, but at least removes the copy/paste.
aside from the comments below, please reshuffle this one so it applies
_before_ the other one in the series. short story: this way we can bisect
down to check if this patch broke anything on its own before the new feature
was added.
coincidentally, writing tests for these so they keep exhibiting the same
behaviour would be great...
> ---
> src/wcmCommon.c | 206
> ++++++++++++++++++++++++++++++++-----------------------
> 1 files changed, 121 insertions(+), 85 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index 0e719cf..170d1d0 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -317,98 +317,106 @@ static void sendAButton(InputInfoPtr pInfo, int
> button, int mask,
> first_val, num_val, valuators);
> }
>
> -/*****************************************************************************
> - * getWheelButton --
> - * Get the wheel button to be sent for the current device state.
> -
> ****************************************************************************/
> -
> -static int getWheelButton(InputInfoPtr pInfo, const WacomDeviceState* ds,
> - unsigned int **fakeKey)
> +/**
> + * Get the distance an axis was scrolled. This function is aware
> + * of the different ways different scrolling axes work and strives
> + * to produce a common representation of relative change.
> + *
> + * @param current Current value of the axis
> + * @param old Previous value of the axis
> + * @param wrap Value after which wraparound occurs (0 if axis does not
> wrap)
> + * @param invert Scroll axis is inverted
> + * @param bitwise Scroll axis values change in a bitwise manner
> + * @return Relative change in axis value
> + */
> +static int getScrollDelta(int current, int old, int wrap, Bool invert, Bool
> bitwise)
I'd really like either a usage of flags or enums here so that the code is
easier to read.
delta = getScrollDelta(ds->stripx, priv->oldStripX, 0, TRUE, TRUE);
is not at all obvious, getScrollDelta(..., INVERT|BITWISE) would be.
> {
> - WacomDevicePtr priv = (WacomDevicePtr) pInfo->private;
> - int fakeButton = 0, value = 0;
> + int delta;
>
> - /* emulate events for relative wheel */
> - if ( ds->relwheel )
> + if (bitwise)
> {
> - value = ds->relwheel;
> - fakeButton = (value > 0) ? priv->relup : priv->reldn;
> - *fakeKey = (value > 0) ? priv->wheel_keys[0+1] :
> priv->wheel_keys[1+1];
> + current = (int)log2(current);
> + old = (int)log2(old);
> }
>
> - /* emulate events for absolute wheel when it is a touch ring (on pad) */
> - if ( (ds->abswheel != priv->oldWheel) && IsPad(priv) &&
> - (priv->oldProximity == ds->proximity))
> - {
> - int wrap_delta;
> - value = priv->oldWheel - ds->abswheel;
> -
> - /* Wraparound detection. If the distance oldvalue..value is
> - * larger than the oldvalue..value considering the
> - * wraparound, assume wraparound and readjust */
> - if (value < 0)
> - wrap_delta = ((MAX_PAD_RING + 1) + priv->oldWheel) -
> ds->abswheel;
> - else
> - wrap_delta = priv->oldWheel - ((MAX_PAD_RING + 1) +
> ds->abswheel);
> + delta = current - old;
>
> - DBG(12, priv, "wrap detection for %d (old %d): %d (wrap %d)\n",
> - ds->abswheel, priv->oldWheel, value, wrap_delta);
> + if (invert)
> + delta = -delta;
>
> - if (abs(wrap_delta) < abs(value))
> - value = wrap_delta;
> -
> - fakeButton = (value > 0) ? priv->wheelup : priv->wheeldn;
> - *fakeKey = (value > 0) ? priv->wheel_keys[2+1] :
> priv->wheel_keys[3+1];
> - }
> -
> - /* emulate events for 2nd absolute wheel when it is a touch ring (on
> pad) */
> - if ( (ds->abswheel2 != priv->oldWheel2) && IsPad(priv) &&
> - (priv->oldProximity == ds->proximity))
> + if (wrap != 0)
> {
> + /* Wraparound detection. If the distance old..current
> + * is larger than the old..current considering the
> + * wraparound, assume wraparound and readjust */
> int wrap_delta;
> - value = priv->oldWheel2 - ds->abswheel2;
>
> - /* Wraparound detection. If the distance oldvalue..value is
> - * larger than the oldvalue..value considering the
> - * wraparound, assume wraparound and readjust */
> - if (value < 0)
> - wrap_delta = ((MAX_PAD_RING + 1) + priv->oldWheel2) -
> ds->abswheel2;
> + if (delta < 0)
> + wrap_delta = (wrap + 1) + delta;
> else
> - wrap_delta = priv->oldWheel2 - ((MAX_PAD_RING + 1) +
> ds->abswheel2);
> + wrap_delta = -((wrap + 1) - delta);
>
> - DBG(12, priv, "wrap detection for %d (old %d): %d (wrap %d)\n",
> - ds->abswheel2, priv->oldWheel2, value, wrap_delta);
> + if (abs(wrap_delta) < abs(delta))
> + delta = wrap_delta;
> + }
>
> - if (abs(wrap_delta) < abs(value))
> - value = wrap_delta;
> + return delta;
> +}
>
> - fakeButton = (value > 0) ? priv->wheel2up : priv->wheel2dn;
> - *fakeKey = (value > 0) ? priv->wheel_keys[4+1] :
> priv->wheel_keys[5+1];
> - }
> +/**
> + * Get the scroll button/action to send given the delta of
> + * the scrolling axis and the possible events that can be
> + * sent.
> + *
> + * @param delta Amount of change in the scrolling axis
> + * @param button_up Button event to send on scroll up
> + * @param button_dn Button event to send on scroll down
> + * @param action_up Action to send on scroll up
> + * @param action_dn Action to send on scroll down
> + * @param[out] action Action that should be performed
> + * @return Button that should be pressed
> + */
> +static int getWheelButton(int delta, int button_up, int button_dn,
> + unsigned int *action_up, unsigned int *action_dn,
> + unsigned int **action)
> +{
> + int button = 0;
> + *action = NULL;
>
> - /* emulate events for left strip */
> - if ( ds->stripx != priv->oldStripX )
> + if (delta)
> {
> - value = ds->stripx - priv->oldStripX;
> -
> - fakeButton = (value < 0) ? priv->striplup : priv->stripldn;
> - *fakeKey = (value < 0) ? priv->strip_keys[0+1] :
> priv->strip_keys[1+1];
> + button = delta > 0 ? button_up : button_dn;
> + *action = delta > 0 ? action_up : action_dn;
> }
>
> - /* emulate events for right strip */
> - if ( ds->stripy != priv->oldStripY )
> - {
> - value = ds->stripy - priv->oldStripY;
> + return button;
> +}
>
> - fakeButton = (value < 0) ? priv->striprup : priv->striprdn;
> - *fakeKey = (value < 0) ? priv->strip_keys[2+1] :
> priv->strip_keys[3+1];
> - }
> +/**
> + * Send button or actions for a scrolling axis.
> + *
> + * @param button X button number to send if no action is defined
> + * @param action Action to send
> + * @param pInfo
> + * @param first_val
> + * @param num_vals
> + * @param valuators
> + */
> +static void sendWheelStripEvent(int button, unsigned int *action,
> InputInfoPtr pInfo,
> + int first_val, int num_vals, int *valuators)
> +{
> + WacomDevicePtr priv = (WacomDevicePtr) pInfo->private;
>
> - DBG(10, priv, "send fakeButton %x with value = %d \n",
> - fakeButton, value);
> + unsigned int button_action[1] = {button | AC_BUTTON | AC_KEYBTNPRESS};
> + if (!action || !(*action)) {
> + DBG(10, priv, "No wheel/strip action set; sending button %d
> (action %d).\n", button, button_action[0]);
> + action = &button_action[0];
> + }
>
> - return fakeButton;
> + sendAction(pInfo, 1, action, ARRAY_SIZE(action), first_val, num_vals,
> valuators);
> + sendAction(pInfo, 0, action, ARRAY_SIZE(action), first_val, num_vals,
> valuators);
> }
> +
>
> /*****************************************************************************
> * sendWheelStripEvents --
> * Send events defined for relative/absolute wheels or strips
> @@ -418,31 +426,59 @@ static void sendWheelStripEvents(InputInfoPtr pInfo,
> const WacomDeviceState* ds,
> int first_val, int num_vals, int *valuators)
> {
> WacomDevicePtr priv = (WacomDevicePtr) pInfo->private;
> - int fakeButton = 0;
> + int fakeButton = 0, delta = 0;
> unsigned int *fakeKey = NULL;
>
> DBG(10, priv, "\n");
>
> - fakeButton = getWheelButton(pInfo, ds, &fakeKey);
> + /* emulate events for left strip */
> + delta = getScrollDelta(ds->stripx, priv->oldStripX, 0, TRUE, TRUE);
> + if (delta && IsPad(priv) && priv->oldProximity == ds->proximity)
> + {
> + DBG(10, priv, "Left touch strip scroll delta = %d\n", delta);
> + fakeButton = getWheelButton(delta, priv->striplup,
> priv->stripldn,
> + priv->strip_keys[0+1],
> priv->strip_keys[1+1], &fakeKey);
urgh, named defines for the indices please. which would prevent...
> + sendWheelStripEvent(fakeButton, fakeKey, pInfo, first_val,
> num_vals, valuators);
> + }
>
> - if (!fakeButton && (!fakeKey || !(*fakeKey)))
> - return;
> + /* emulate events for right strip */
> + delta = getScrollDelta(ds->stripy, priv->oldStripY, 0, TRUE, TRUE);
> + if (delta && IsPad(priv) && priv->oldProximity == ds->proximity)
> + {
> + DBG(10, priv, "Right touch strip scroll delta = %d\n", delta);
> + fakeButton = getWheelButton(delta, priv->striprup,
> priv->striprdn,
> + priv->strip_keys[2+1],
> priv->strip_keys[3+1], &fakeKey);
> + sendWheelStripEvent(fakeButton, fakeKey, pInfo, first_val,
> num_vals, valuators);
> + }
>
> - if (!fakeKey || !(*fakeKey))
> + /* emulate events for relative wheel */
> + delta = getScrollDelta(ds->relwheel, 0, 0, FALSE, FALSE);
> + if (delta && IsCursor(priv) && priv->oldProximity == ds->proximity)
> {
> - /* send both button on/off in the same event for pad */
> - xf86PostButtonEventP(pInfo->dev, is_absolute(pInfo), fakeButton
> & AC_CODE,
> - 1, first_val, num_vals, VCOPY(valuators,
> num_vals));
> + DBG(10, priv, "Relative wheel scroll delta = %d\n", delta);
> + fakeButton = getWheelButton(delta, priv->relup, priv->reldn,
> + priv->wheel_keys[0+1],
> priv->wheel_keys[1+1], &fakeKey);
> + sendWheelStripEvent(fakeButton, fakeKey, pInfo, first_val,
> num_vals, valuators);
> + }
confusion here, the relwheel has the same index as the lstrip? and the lring
the same as the rstrip?
the diff is virtually so this may be correct, but having named defines with
comments would help greatly here.
Cheers,
Peter
>
> - xf86PostButtonEventP(pInfo->dev, is_absolute(pInfo), fakeButton
> & AC_CODE,
> - 0, first_val, num_vals, VCOPY(valuators,
> num_vals));
> + /* emulate events for left touch ring */
> + delta = getScrollDelta(ds->abswheel, priv->oldWheel, MAX_PAD_RING,
> TRUE, FALSE);
> + if (delta && IsPad(priv) && priv->oldProximity == ds->proximity)
> + {
> + DBG(10, priv, "Left touch wheel scroll delta = %d\n", delta);
> + fakeButton = getWheelButton(delta, priv->wheelup, priv->wheeldn,
> + priv->wheel_keys[2+1],
> priv->wheel_keys[3+1], &fakeKey);
> + sendWheelStripEvent(fakeButton, fakeKey, pInfo, first_val,
> num_vals, valuators);
> }
> - else
> +
> + /* emulate events for right touch ring */
> + delta = getScrollDelta(ds->abswheel2, priv->oldWheel2, MAX_PAD_RING,
> TRUE, FALSE);
> + if (delta && IsPad(priv) && priv->oldProximity == ds->proximity)
> {
> - sendAction(pInfo, 1, fakeKey, ARRAY_SIZE(priv->wheel_keys[0]),
> - first_val, num_vals, valuators);
> - sendAction(pInfo, 0, fakeKey, ARRAY_SIZE(priv->wheel_keys[0]),
> - first_val, num_vals, valuators);
> + DBG(10, priv, "Right touch wheel scroll delta = %d\n", delta);
> + fakeButton = getWheelButton(delta, priv->wheel2up,
> priv->wheel2dn,
> + priv->wheel_keys[4+1],
> priv->wheel_keys[5+1], &fakeKey);
> + sendWheelStripEvent(fakeButton, fakeKey, pInfo, first_val,
> num_vals, valuators);
> }
> }
>
> --
> 1.7.7.3
------------------------------------------------------------------------------
Learn Windows Azure Live! Tuesday, Dec 13, 2011
Microsoft is holding a special Learn Windows Azure training event for
developers. It will provide a great way to learn Windows Azure and what it
provides. You can attend the event by watching it streamed LIVE online.
Learn more at http://p.sf.net/sfu/ms-windowsazure
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
