[PATCH v2 23/42] dix: add touch event history helper functions

[Chase Douglas]

On 12/20/2011 07:21 AM, walter harms wrote:
> 
> 
> Am 20.12.2011 04:22, schrieb Peter Hutterer:
>> If touch client has not registered for ownership events and a grab above
>> that client is rejected, the client needs to receive the complete event
>> history.
>>
>> The history currently doesn't really do fancy overflow handling. We assume
>> that the first TOUCH_HISTORY_SIZE events are the important ones and anything
>> after that is dropped. If that is a problem, fix the client that takes > 100
>> event to decide whether to accept or reject.
>>
>> Events marked with TOUCH_CLIENT_ID or TOUCH_REPLAYING must not be stored in
>> the history, they are events created by the DIX to comply with the protocol.
>> Any such event should already be in the history anyway.
>>
>> A fixme in this patch: we don't have a function to actually deliver the
>> event yet.
>>
>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>> ---
>> Changes to v1:
>> - add DebugF for history overflow
>>
>>  dix/touch.c     |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/input.h |    4 ++
>>  2 files changed, 113 insertions(+), 0 deletions(-)
>>
>> diff --git a/dix/touch.c b/dix/touch.c
>> index dfb7ff0..f741207 100644
>> --- a/dix/touch.c
>> +++ b/dix/touch.c
>> @@ -35,6 +35,9 @@
>>  #include "eventstr.h"
>>  #include "exevents.h"
>>  
>> +#define TOUCH_HISTORY_SIZE 100
>> +
>> +
>>  /* If a touch queue resize is needed, the device id's bit is set. */
>>  static unsigned char resize_waiting[(MAXDEVICES + 7)/8];
>>  
>> @@ -394,6 +397,112 @@ TouchEndTouch(DeviceIntPtr dev, TouchPointInfoPtr ti)
>>      ti->num_grabs = 0;
>>      ti->client_id = 0;
>>  
>> +    TouchEventHistoryFree(ti);
>> +
>>      valuator_mask_zero(ti->valuators);
>>  }
>>  
>> +/**
>> + * Allocate the event history for this touch pointer. Calling this on a
>> + * touchpoint that already has an event history does nothing but counts as
>> + * as success.
>> + *
>> + * @return TRUE on success, FALSE on allocation errors
>> + */
>> +Bool
>> +TouchEventHistoryAllocate(TouchPointInfoPtr ti)
>> +{
>> +    if (ti->history)
>> +        return TRUE;
>> +
>> +    ti->history = calloc(TOUCH_HISTORY_SIZE, sizeof(*ti->history));
>> +    ti->history_elements = 0;
>> +    if (ti->history)
>> +        ti->history_size = TOUCH_HISTORY_SIZE;
>> +    return ti->history != NULL;
>> +}
> 
> just for the beauty of the code i would change the order a bit like:
> 
>  ti->history_elements = 0;
>  ti->history_size = 0;
>   ti->history = calloc(TOUCH_HISTORY_SIZE, sizeof(*ti->history));
>    if (ti->history)
> 	ti->history_size = TOUCH_HISTORY_SIZE;
>   return ti->history != NULL;
> }

For me personally, this doesn't improve nor detract. I don't see much of
a difference. I suggest leaving it as is due to time constraints.

> you could also make the code void * and return ti->history
> (That is what i would expect from *alloc*() ).

This isn't a typical allocate function. It makes sense to me how it is
written. It isn't allocating blocks of memory and returning them to the
caller, it is allocating memory and storing it inside the passed in
structure.

> btw:
> Would it make sence to have a dynamical buffer in future ?
> if yes you could add a second argument (TOUCH_HISTORY_SIZE)
> and replace calloc with realloc().
> (just an idea, i am not a big fan of #define something 100 because
> things tend to change in future).

There are possibilities, such as using specs like touch screen data rate
or setting an input device property. However, we believe 100 events
should be reasonable in all circumstances, so it makes sense to start
with that and fix it later if need be.

-- Chase