On Thu, Jul 26, 2012 at 1:08 PM, Jason Gerecke <killert...@gmail.com> wrote:
> One should not have to keep in mind the historic quirk that
> mouse buttons 4-7 have when dealing with buttons events generated
> by the tablet. This patch isolates conversion between physical
> and X11 numbering to the single place it has historically
> mattered: the button property getters/setters.
>
> Signed-off-by: Jason Gerecke <killert...@gmail.com>
Acked-by: Ping Cheng <pingli...@gmail.com> for the whole set.
I read through the patchset. But, I did not look closely into the
changes. Plus, I didn't test any of the changes. That's why I am only
tagging it with an "Acked-by".
I think using original tablet button numbers, instead of map/remap X11
buttons, is a great idea. I also like the idea of isolating conversion
between physical and X11 numbering. Nice job!
Since the change touched many parts of the codebase, I hope more
people can take a look or test the changes with different tablet
models.
Ping
> ---
> src/wcmCommon.c | 7 +++----
> src/wcmXCommand.c | 59
> +++++++++++++++++++++++++++++++++++++------------------
> 2 files changed, 43 insertions(+), 23 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index e81a80c..144dd20 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -286,16 +286,15 @@ static void sendAButton(InputInfoPtr pInfo, int button,
> int mask,
> #ifdef DEBUG
> WacomCommonPtr common = priv->common;
> #endif
> - int mapped_button = button > 2 ? button + 4 : button; /* maintain
> prior "dead button" behavior */
>
> DBG(4, priv, "TPCButton(%s) button=%d state=%d\n",
> common->wcmTPCButton ? "on" : "off", button, mask);
>
> - if (!priv->keys[mapped_button][0])
> + if (!priv->keys[button][0])
> return;
>
> - sendAction(pInfo, (mask != 0), priv->keys[mapped_button],
> - ARRAY_SIZE(priv->keys[mapped_button]),
> + sendAction(pInfo, (mask != 0), priv->keys[button],
> + ARRAY_SIZE(priv->keys[button]),
> first_val, num_val, valuators);
> }
>
> diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> index 49a336d..c3a2a7c 100644
> --- a/src/wcmXCommand.c
> +++ b/src/wcmXCommand.c
> @@ -122,12 +122,11 @@ static void wcmResetButtonAction(InputInfoPtr pInfo,
> int button, int nbuttons)
> WacomDevicePtr priv = (WacomDevicePtr) pInfo->private;
> unsigned int new_action[256] = {};
> int x11_button = priv->button_default[button];
> - int index = button < 3 ? button : button + 4;
> char name[64];
>
> sprintf(name, "Wacom button action %d", button);
> new_action[0] = AC_BUTTON | AC_KEYBTNPRESS | x11_button;
> - wcmResetAction(pInfo, name, index, priv->btn_actions, priv->keys,
> &new_action, prop_btnactions, nbuttons);
> + wcmResetAction(pInfo, name, button, priv->btn_actions, priv->keys,
> &new_action, prop_btnactions, nbuttons);
> }
>
> static void wcmResetStripAction(InputInfoPtr pInfo, int index)
> @@ -209,7 +208,6 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
> WacomDevicePtr priv = (WacomDevicePtr) pInfo->private;
> WacomCommonPtr common = priv->common;
> int values[WCM_MAX_BUTTONS];
> - int nbuttons;
> int i;
>
> DBG(10, priv, "\n");
> @@ -282,9 +280,8 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
> values[0] = MakeAtom(pInfo->type_name, strlen(pInfo->type_name),
> TRUE);
> prop_tooltype = InitWcmAtom(pInfo->dev, WACOM_PROP_TOOL_TYPE,
> XA_ATOM, 32, 1, values);
>
> - nbuttons = min(max(priv->nbuttons + 4, 7), WCM_MAX_BUTTONS);
> memset(values, 0, sizeof(values));
> - prop_btnactions = InitWcmAtom(pInfo->dev, WACOM_PROP_BUTTON_ACTIONS,
> XA_ATOM, 32, nbuttons, values);
> + prop_btnactions = InitWcmAtom(pInfo->dev, WACOM_PROP_BUTTON_ACTIONS,
> XA_ATOM, 32, priv->nbuttons, values);
> for (i = 0; i < priv->nbuttons; i++)
> wcmResetButtonAction(pInfo, i, priv->nbuttons);
>
> @@ -503,29 +500,35 @@ static int wcmSetActionsProperty(DeviceIntPtr dev, Atom
> property,
>
> for (i = 0; i < prop->size; i++)
> {
> + int index = i;
> Atom subproperty = ((Atom*)prop->data)[i];
> XIPropertyValuePtr subprop;
>
> + if (property == prop_btnactions)
> + { /* Driver uses physical -- not X11 -- button
> numbering internally */
> + if (i < 3)
> + index = i;
> + else if (i < 7)
> + continue;
> + else
> + index = i - 4;
> + }
> +
> if (subproperty == 0)
> { /* Interpret 'None' as meaning 'reset' */
> if (property == prop_btnactions)
> - {
> - if (i < 3)
> - wcmResetButtonAction(pInfo,
> i, size);
> - else if (i > 6)
> - wcmResetButtonAction(pInfo,
> i-4, size);
> - }
> + wcmResetButtonAction(pInfo, index,
> size);
> else if (property == prop_strip_buttons)
> - wcmResetStripAction(pInfo, i);
> + wcmResetStripAction(pInfo, index);
> else if (property == prop_wheel_buttons)
> - wcmResetWheelAction(pInfo, i);
> + wcmResetWheelAction(pInfo, index);
>
> - if (subproperty != handlers[i])
> - subproperty = handlers[i];
> + if (subproperty != handlers[index])
> + subproperty = handlers[index];
> }
>
> XIGetDeviceProperty(dev, subproperty, &subprop);
> - wcmSetActionProperty(dev, subproperty, subprop,
> checkonly, &handlers[i], &actions[i]);
> + wcmSetActionProperty(dev, subproperty, subprop,
> checkonly, &handlers[index], &actions[index]);
> }
> }
>
> @@ -804,7 +807,7 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property,
> XIPropertyValuePtr prop,
> #endif
> } else if (property == prop_btnactions)
> {
> - int nbuttons = min(max(priv->nbuttons + 4, 7),
> WCM_MAX_BUTTONS);
> + int nbuttons = priv->nbuttons < 4 ? priv->nbuttons :
> priv->nbuttons + 4;
> return wcmSetActionsProperty(dev, property, prop, checkonly,
> nbuttons, priv->btn_actions, priv->keys);
> } else
> {
> @@ -845,10 +848,28 @@ int wcmGetProperty (DeviceIntPtr dev, Atom property)
> }
> else if (property == prop_btnactions)
> {
> - int nbuttons = min(max(priv->nbuttons + 4, 7),
> WCM_MAX_BUTTONS);
> + /* Convert the physical button representation used internally
> + * to the X11 button representation we've historically used.
> + * To do this, we need to skip X11 buttons 4-7 which would be
> + * used by a scroll wheel rather than an actual button.
> + */
> + int nbuttons = priv->nbuttons < 4 ? priv->nbuttons :
> priv->nbuttons + 4;
> + Atom x11_btn_actions[nbuttons];
> + int i;
> +
> + for (i = 0; i < nbuttons; i++)
> + {
> + if (i < 3)
> + x11_btn_actions[i] = priv->btn_actions[i];
> + else if (i < 7)
> + x11_btn_actions[i] = 0;
> + else
> + x11_btn_actions[i] = priv->btn_actions[i-4];
> + }
> +
> return XIChangeDeviceProperty(dev, property, XA_ATOM, 32,
> PropModeReplace, nbuttons,
> - priv->btn_actions, FALSE);
> + x11_btn_actions, FALSE);
> }
> else if (property == prop_strip_buttons)
> {
> --
> 1.7.11.1
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Linuxwacom-devel mailing list
> Linuxwacom-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
