[PATCH xserver 6/6] dix: Reallocate touchpoint buffer at input event time

[Peter Hutterer]

On Tue, Dec 08, 2015 at 03:44:54PM -0800, Keith Packard wrote:
> Now that input is threaded, malloc can be used at event time to resize
> the touchpoint buffer as needed.x
> ---
>  dix/touch.c | 89 +++++++++++++++++++++----------------------------------------
>  1 file changed, 30 insertions(+), 59 deletions(-)
> 
> diff --git a/dix/touch.c b/dix/touch.c
> index eee110d..591f776 100644
> --- a/dix/touch.c
> +++ b/dix/touch.c
> @@ -42,9 +42,6 @@
>  
>  #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];
> -
>  /**
>   * Some documentation about touch points:
>   * The driver submits touch events with it's own (unique) touch point ID.
> @@ -74,47 +71,28 @@ static unsigned char resize_waiting[(MAXDEVICES + 7) / 8];
>   * @return Always True. If we fail to grow we probably will topple over soon
>   * anyway and re-executing this won't help.
>   */
> +
>  static Bool
> -TouchResizeQueue(ClientPtr client, void *closure)
> +TouchResizeQueue(DeviceIntPtr dev)
>  {
> -    int i;
> -
> -    input_lock();
> -
> -    /* first two ids are reserved */
> -    for (i = 2; i < MAXDEVICES; i++) {
> -        DeviceIntPtr dev;
> -        DDXTouchPointInfoPtr tmp;
> -        size_t size;
> -
> -        if (!BitIsOn(resize_waiting, i))
> -            continue;
> +    DDXTouchPointInfoPtr tmp;
> +    size_t size;
>  
> -        ClearBit(resize_waiting, i);
> +    /* Need to grow the queue means dropping events. Grow sufficiently so we
> +     * don't need to do it often */
> +    size = dev->last.num_touches + dev->last.num_touches / 2 + 1;
>  
> -        /* device may have disappeared by now */
> -        dixLookupDevice(&dev, i, serverClient, DixWriteAccess);
> -        if (!dev)
> -            continue;
> -
> -        /* Need to grow the queue means dropping events. Grow sufficiently so we
> -         * don't need to do it often */
> -        size = dev->last.num_touches + dev->last.num_touches / 2 + 1;
> -
> -        tmp = reallocarray(dev->last.touches, size, sizeof(*dev->last.touches));
> -        if (tmp) {
> -            int j;
> -
> -            dev->last.touches = tmp;
> -            for (j = dev->last.num_touches; j < size; j++)
> -                TouchInitDDXTouchPoint(dev, &dev->last.touches[j]);
> -            dev->last.num_touches = size;
> -        }
> +    tmp = reallocarray(dev->last.touches, size, sizeof(*dev->last.touches));
> +    if (tmp) {
> +        int j;
>  
> +        dev->last.touches = tmp;
> +        for (j = dev->last.num_touches; j < size; j++)
> +            TouchInitDDXTouchPoint(dev, &dev->last.touches[j]);
> +        dev->last.num_touches = size;
> +        return TRUE;
>      }
> -    input_unlock();
> -
> -    return TRUE;
> +    return FALSE;
>  }
>  
>  /**
> @@ -172,14 +150,20 @@ TouchBeginDDXTouch(DeviceIntPtr dev, uint32_t ddx_id)
>      if (TouchFindByDDXID(dev, ddx_id, FALSE))
>          return NULL;
>  
> -    for (i = 0; i < dev->last.num_touches; i++) {
> -        /* Only emulate pointer events on the first touch */
> -        if (dev->last.touches[i].active)
> -            emulate_pointer = FALSE;
> -        else if (!ti)           /* ti is now first non-active touch rec */
> -            ti = &dev->last.touches[i];
> +    for (;;) {
> +        for (i = 0; i < dev->last.num_touches; i++) {
> +            /* Only emulate pointer events on the first touch */
> +            if (dev->last.touches[i].active)
> +                emulate_pointer = FALSE;
> +            else if (!ti)           /* ti is now first non-active touch rec */
> +                ti = &dev->last.touches[i];
>  
> -        if (!emulate_pointer && ti)
> +            if (!emulate_pointer && ti)
> +                break;
> +        }
> +        if (ti)
> +            break;
> +        if (!TouchResizeQueue(dev))
>              break;
>      }

this hunk took me a while, wouldn't it be easier do do something like:
    do {
       for (i = 0, ...) {
           ...
       }

       if (!ti)
           TouchResizeQueue(dev))
    } while (!ti); 


Cheers,
   Peter
  
> @@ -194,21 +178,8 @@ TouchBeginDDXTouch(DeviceIntPtr dev, uint32_t ddx_id)
>              next_client_id = 1;
>          ti->client_id = client_id;
>          ti->emulate_pointer = emulate_pointer;
> -        return ti;
>      }
> -
> -    /* If we get here, then we've run out of touches and we need to drop the
> -     * event (we're inside the SIGIO handler here) schedule a WorkProc to
> -     * grow the queue for us for next time. */
> -    ErrorFSigSafe("%s: not enough space for touch events (max %u touchpoints). "
> -                  "Dropping this event.\n", dev->name, dev->last.num_touches);
> -
> -    if (!BitIsOn(resize_waiting, dev->id)) {
> -        SetBit(resize_waiting, dev->id);
> -        QueueWorkProc(TouchResizeQueue, serverClient, NULL);
> -    }
> -
> -    return NULL;
> +    return ti;
>  }
>  
>  void
> -- 
> 2.6.2
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>