On Fri, Aug 3, 2012 at 11:58 AM, Ping Cheng <pingli...@gmail.com> wrote: > 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 > Thanks Ping -- it's a lot of code to go over, and I wasn't sure if I'd get an ACK out of anyone :) I pushed them up to the repo yesterday after a week of silence to expose them to a wider testing audience. There's ~2 weeks until the next release is scheduled so there should be enough time to shake out anything I missed.
Jason --- Day xee-nee-svsh duu-'ushtlh-ts'it; nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it. Huu-chan xuu naa~-gha. >> --- >> 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