[PATCH v2 2/2] Populate touch emulated button state separately

[Chase Douglas]

On 07/11/2012 11:34 PM, Peter Hutterer wrote:
> On Mon, Jul 09, 2012 at 05:12:44PM -0700, Chase Douglas wrote:
>> When a touch begin is pointer emulated, do not include touch state when
>> calculating the button state for the initial motion and button press
>> events. This change ensures the logical state of the buttons prior to the
>> events is set properly for emulated events.
>>
>> Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
>> ---
>>   Xi/exevents.c    |   15 +++++++++++++++
>>   dix/events.c     |    2 ++
>>   dix/inpututils.c |    4 ----
>>   3 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/Xi/exevents.c b/Xi/exevents.c
>> index c1fcc55..0341ad5 100644
>> --- a/Xi/exevents.c
>> +++ b/Xi/exevents.c
>> @@ -1445,6 +1445,13 @@ DeliverTouchEmulatedEvent(DeviceIntPtr dev, TouchPointInfoPtr ti,
>>       event_set_state(dev, kbd, &ptrev->device_event);
>>       ptrev->device_event.corestate = event_get_corestate(dev, kbd);
>>
>> +    /* Add button 1 emulated state only for touch update and touch end events.
>> +     * The state confers the *previous* state of the button. */
>> +    if (ev->any.type != ET_TouchBegin) {
>> +        SetBit(ptrev->device_event.buttons, dev->button->map[1]);
>> +        ptrev->device_event.corestate |= (Button1Mask >> 1) << dev->button->map[1];
>> +    }
>> +
>
> can we move this into event_get_corestate?

In order to do that in a way that satisfies both blocks above and below, 
we would have to extend the function like this:

event_get_corestate(DeviceIntPtr dev, DeviceIntPtr kbd,
                     bool force_primary_button);

Which would be called like this for the above and below blocks, 
respectively:

event_get_corestate(dev, kbd, ev->any.type != ET_TouchBegin);

event_get_corestate(dev, kbd,
                     (event->flags & XIPointerEmulated) && mouse &&
                     mouse->touch && mouse->touch->buttonsDown > 0);

The same would apply for event_set_state(), and everywhere other than 
these two blocks the forcing parameter would be passed in as FALSE.

Does that work for you?

>>       if (grab) {
>>           /* this side-steps the usual activation mechansims, but... */
>>           if (ev->any.type == ET_TouchBegin && !dev->deviceGrab.grab)
>> @@ -1701,6 +1708,14 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr device)
>>       corestate = event_get_corestate(mouse, kbd);
>>       event_set_state(mouse, kbd, event);
>>
>> +    /* Touch button state is already set for pointer emulated events. */
>> +    if (!(event->flags & XIPointerEmulated)) {
>> +        if (mouse && mouse->touch && mouse->touch->buttonsDown > 0) {
>> +            SetBit(event->buttons, mouse->button->map[1]);
>> +            event->corestate |= (Button1Mask >> 1) << mouse->button->map[1];
>> +        }
>> +    }
>> +
>
> same here, let's move this into event_set_state()? i feel a bit queasy about
> having a function that returns us the core state, only to have that state
> messed with on returning.
>
> Cheers,
>    Peter
>
>>       ret = UpdateDeviceState(device, event);
>>       if (ret == DONT_PROCESS)
>>           return;
>> diff --git a/dix/events.c b/dix/events.c
>> index 86336fe..00ae8a9 100644
>> --- a/dix/events.c
>> +++ b/dix/events.c
>> @@ -5135,6 +5135,8 @@ ProcQueryPointer(ClientPtr client)
>>       rep.type = X_Reply;
>>       rep.sequenceNumber = client->sequence;
>>       rep.mask = event_get_corestate(mouse, keyboard);
>> +    if (mouse && mouse->touch)
>> +        rep.mask |= mouse->touch->state;
>>       rep.length = 0;
>>       rep.root = (GetCurrentRootWindow(mouse))->drawable.id;
>>       rep.rootX = pSprite->hot.x;
>> diff --git a/dix/inpututils.c b/dix/inpututils.c
>> index 223d547..3ae41cc 100644
>> --- a/dix/inpututils.c
>> +++ b/dix/inpututils.c
>> @@ -671,7 +671,6 @@ event_get_corestate(DeviceIntPtr mouse, DeviceIntPtr kbd)
>>                    kbd->key) ? XkbStateFieldFromRec(&kbd->key->xkbInfo->
>>                                                     state) : 0;
>>       corestate |= (mouse && mouse->button) ? (mouse->button->state) : 0;
>> -    corestate |= (mouse && mouse->touch) ? (mouse->touch->state) : 0;
>>
>>       return corestate;
>>   }
>> @@ -685,9 +684,6 @@ event_set_state(DeviceIntPtr mouse, DeviceIntPtr kbd, DeviceEvent *event)
>>           if (BitIsOn(mouse->button->down, i))
>>               SetBit(event->buttons, mouse->button->map[i]);
>>
>> -    if (mouse && mouse->touch && mouse->touch->buttonsDown > 0)
>> -        SetBit(event->buttons, mouse->button->map[1]);
>> -
>>       if (kbd && kbd->key) {
>>           XkbStatePtr state;
>>
>> --
>> 1.7.10.4
>>
>