Re: [PATCH xserver 7/9] dix: Reallocate touchpoint buffer at input event time

2016-05-16 Thread Keith Packard
Peter Hutterer  writes:

> fwiw, that first sentence isn't correct anymore, you can drop it.
> rev-by still stands.

I've edited the comment, along with re-adding the UseSIGIO option to the
parsing code and pushed out an updated thread with all of your kind
Rb/Ab lines added. We have but the two giant patches left to see through
the review process before this series can be merged. I'd love to make
those simpler, but I can't imagine how...

Thanks again for helping get us this far.

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 7/9] dix: Reallocate touchpoint buffer at input event time

2016-05-15 Thread Peter Hutterer
On Wed, May 11, 2016 at 01:54:56PM -0700, Keith Packard wrote:
> Now that input is threaded, malloc can be used at event time to resize
> the touchpoint buffer as needed.x
> 
> Signed-off-by: Keith Packard 
> Reviewed-by: Peter Hutterer 
> ---
>  dix/touch.c | 89 
> +
>  1 file changed, 30 insertions(+), 59 deletions(-)
> 
> diff --git a/dix/touch.c b/dix/touch.c
> index 4c0412a..1d12d28 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

fwiw, that first sentence isn't correct anymore, you can drop it.
rev-by still stands.

Cheers,
   Peter

> + * 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(, 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, >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, >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 = >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 = >last.touches[i];
>  
> -if (!emulate_pointer && ti)
> +if (!emulate_pointer && ti)
> +break;
> +}
> +if (ti)
> +break;
> +if (!TouchResizeQueue(dev))
>  break;
>  }
>  
> @@ -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.8.0.rc3
> 
> 

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

2016-05-11 Thread Keith Packard
Now that input is threaded, malloc can be used at event time to resize
the touchpoint buffer as needed.x

Signed-off-by: Keith Packard 
Reviewed-by: Peter Hutterer 
---
 dix/touch.c | 89 +
 1 file changed, 30 insertions(+), 59 deletions(-)

diff --git a/dix/touch.c b/dix/touch.c
index 4c0412a..1d12d28 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(, 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, >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, >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 = >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 = >last.touches[i];
 
-if (!emulate_pointer && ti)
+if (!emulate_pointer && ti)
+break;
+}
+if (ti)
+break;
+if (!TouchResizeQueue(dev))
 break;
 }
 
@@ -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.8.0.rc3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 7/9] dix: Reallocate touchpoint buffer at input event time

2015-12-17 Thread Peter Hutterer
On Thu, Dec 17, 2015 at 04:11:42PM -0800, Keith Packard wrote:
> Now that input is threaded, malloc can be used at event time to resize
> the touchpoint buffer as needed.x

still not happy with the for (;;) instead of a while (1) but
Reviewed-by: Peter Hutterer 

needs your s-o-b btw

Cheers,
   Peter

> ---
>  dix/touch.c | 89 
> +
>  1 file changed, 30 insertions(+), 59 deletions(-)
> 
> diff --git a/dix/touch.c b/dix/touch.c
> index 4c0412a..1d12d28 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(, 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, >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, >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 = >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 = >last.touches[i];
>  
> -if (!emulate_pointer && ti)
> +if (!emulate_pointer && ti)
> +break;
> +}
> +if (ti)
> +break;
> +if (!TouchResizeQueue(dev))
>  break;
>  }
>  
> @@ -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.4
> 
> ___
> 

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

2015-12-17 Thread Keith Packard
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 4c0412a..1d12d28 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(, 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, >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, >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 = >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 = >last.touches[i];
 
-if (!emulate_pointer && ti)
+if (!emulate_pointer && ti)
+break;
+}
+if (ti)
+break;
+if (!TouchResizeQueue(dev))
 break;
 }
 
@@ -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.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel