Re: [PATCH 0/27] Input cleanup and smooth scrolling support

2011-06-06 Thread Peter Hutterer
On Fri, Jun 03, 2011 at 03:54:58PM +0100, Daniel Stone wrote:
> There's a few patches in this series, destined for 1.12.  Most notable
> is the last (but arguably least pleasant) of the series, which adds
> support for smooth scrolling by way of new scroll valuators, which
> emulate button events, and vice-versa for older drivers too.
> 
> Along the way, we convert pretty much all co-ordinates in the input path
> to use doubles, including ValuatorMask, and extend the usage of masks
> within GetPointerEvents's children.  This works fine for me on a variety
> of touchpads and mice, but in all honesty, I've not tested absolute
> devices in a few rebases, so I can't vouch for them.



Reviewed-by: Peter Hutterer  for patches
#1, #3, #5, #6, #8, #9, #10, #11, #12, #13, #14, #17 (see comment below),
#18, #19, #20, #21, #22, #23, #24, #25, #26 (given a rename) and all given
the trunc() changes of course.

Simon already pointed out #16, reviewed-and-gone-crosseyed otherwise though.

However, some of the patches will conflict with a set I have in my tree
http://patchwork.freedesktop.org/patch/5648/. Your #17 replicates some of
that, so it should be easy to resolve.

There are quite a few patches that could go in right now to give them a few
months of testing before we merge the new features. Interested in that? 

In fact, all except the POINTER_EMULATED patch and the actual integration of
the smooth scrolling.  None of the functionality should change here after
all.

Cheers,
  Peter
___
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


Re: [PATCH 27/27] Input: Add smooth-scrolling support to GetPointerEvents

2011-06-06 Thread Peter Hutterer
On Fri, Jun 03, 2011 at 04:00:03PM +0100, Daniel Stone wrote:
> For scroll wheel support, we used to send buttons 4/5 and 6/7 for
> horizontal/vertical positive/negative scroll events.  For touchpads, we
> really want more fine-grained scroll values.  GetPointerEvents now
> accepts both old-school scroll button presses, and new-style scroll axis
> events, while emitting both types of events to support both old and new
> clients.
> 
> Signed-off-by: Daniel Stone 
> ---
>  dix/getevents.c |  108 --
>  1 files changed, 104 insertions(+), 4 deletions(-)
> 
> diff --git a/dix/getevents.c b/dix/getevents.c
> index c1ebbb2..f8be73c 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -2,6 +2,7 @@
>   * Copyright © 2006 Nokia Corporation
>   * Copyright © 2006-2007 Daniel Stone
>   * Copyright © 2008 Red Hat, Inc.
> + * Copyright © 2011 The Chromium OS Authors
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a
>   * copy of this software and associated documentation files (the "Software"),
> @@ -60,11 +61,13 @@
>  #include 
>  #include 
>  #include 
> +#include 

does it scroll smoother if you include it twice? :)

>  #include 
>  #include "exglobals.h"
>  #include "exevents.h"
>  #include "extnsionst.h"
>  #include "listdev.h" /* for sizing up DeviceClassesChangedEvent */
> +#include "xserver-properties.h"
>  
>  /* Number of motion history events to store. */
>  #define MOTION_HISTORY_SIZE 256
> @@ -622,8 +625,10 @@ GetMaximumEventsNum(void) {
>  /* One raw event
>   * One device event
>   * One possible device changed event
> + * Lots of possible separate scroll events (horiz + vert)

"button scroll events" is less ambiguous now that we have two scrolling
methods.

> + * Lots of possible separate raw scroll event (horiz + vert)
>   */
> -return 3;
> +return 51;
>  }
>  
>  
> @@ -1199,7 +1204,12 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr 
> pDev, int type, int buttons
>int flags, const ValuatorMask *mask_in)

GPE has a rather extensive comment describing what it does. You could
continue this tradition ;)

>  {
>  CARD32 ms = GetTimeInMillis();
> -int num_events = 0;
> +int num_events = 0, nev_tmp;
> +int i;
> +ValuatorMask mask;
> +Atom h_scroll_label = XIGetKnownProperty(AXIS_LABEL_PROP_REL_HSCROLL);
> +Atom v_scroll_label = XIGetKnownProperty(AXIS_LABEL_PROP_REL_VSCROLL);
> +int h_scroll_axis = -1, v_scroll_axis = -1;
>  
>  /* refuse events from disabled devices */
>  if (!pDev->enabled)
> @@ -1210,8 +1220,98 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr 
> pDev, int type, int buttons
>  
>  events = UpdateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT,
>&num_events);
> -num_events += _GetPointerEvents(events, pDev, type, buttons, ms, flags,
> -mask_in);
> +
> +valuator_mask_copy(&mask, mask_in);
> +
> +/* Find the vertical and horizontal scroll axes, if any. */
> +for (i = 0; i < pDev->valuator->numAxes; i++)
> +{
> +if (h_scroll_label && pDev->valuator->axes[i].label == 
> h_scroll_label)
> +h_scroll_axis = i;
> +else if (v_scroll_label &&
> + pDev->valuator->axes[i].label == v_scroll_label)
> +v_scroll_axis = i;
> +}

this may be better cached in the DeviceIntRec instead of recalculated on
every single pointer event.

> +
> +/* Turn a scroll button press into a smooth-scrolling event. */
> +if (type == ButtonPress &&
> +((v_scroll_axis != -1 && (buttons == 4 || buttons == 5)) ||
> + (h_scroll_axis != -1 && (buttons == 6 || buttons == 7
> +{
> +double val;
> +
> +type = MotionNotify;
> +buttons = 0;
> +if (buttons == 4)

how about switch(buttons) {...}

> +{
> +val = valuator_mask_get_double(&mask, v_scroll_axis) + 1.0;
> +valuator_mask_set_double(&mask, v_scroll_axis, val);
> +}
> +else if (buttons == 5)
> +{
> +val = valuator_mask_get_double(&mask, v_scroll_axis) - 1.0;
> +valuator_mask_set_double(&mask, v_scroll_axis, val);
> +}
> +else if (buttons == 6)
> +{
> +val = valuator_mask_get_double(&mask, v_scroll_axis) + 1.0;
> +valuator_mask_set_double(&mask, h_scroll_axis, val);
> +}
> +else if (buttons == 7)
> +{
> +val = valuator_mask_get_double(&mask, v_scroll_axis) - 1.0;
> +valuator_mask_set_double(&mask, h_scroll_axis, val);
> +}
> +}
> +
> +/* Return the original event set, plus potential legacy scrolling 
> events. */
> +nev_tmp = _GetPointerEvents(events, pDev, type, buttons, ms, flags,
> +&mask);
> +events += nev_tmp;
> +num_events += nev_tmp;
> +

Re: [PATCH 27/27] Input: Add smooth-scrolling support to GetPointerEvents

2011-06-06 Thread Peter Hutterer
On Mon, Jun 06, 2011 at 12:20:21PM +0100, Daniel Stone wrote:
> Hi,
> 
> On Sun, Jun 05, 2011 at 08:49:43PM +0200, Simon Thum wrote:
> > On 06/03/2011 05:00 PM, Daniel Stone wrote:
> > > For scroll wheel support, we used to send buttons 4/5 and 6/7 for
> > > horizontal/vertical positive/negative scroll events.  For touchpads, we
> > > really want more fine-grained scroll values.  GetPointerEvents now
> > > accepts both old-school scroll button presses, and new-style scroll axis
> > > events, while emitting both types of events to support both old and new
> > > clients.
> > 
> > I guess apps that want the best support across server generations are
> > supposed to know they can ignore legacy scroll by virtue of the
> > POINTER_EMULATED flag? If yes, shouldn't the flag be always set on
> > either the axis updates or the button events?
> 
> Yeah -- there's a missing inputproto part that I'll send out with the
> second round of patches.  Here's the applicable text:
>  PointerEmulated means that the event has been emulated from another
>  XI 2.x event for legacy client support, and that this event should
>  be ignored if the client listens for these events.  This flag will
>  be set on scroll ButtonPress events (buttons 4, 5, 6 and 7) if a
>  smooth-scrolling event on the Rel Vert Scroll or Rel Horiz Scroll
>  axis events was also generated.
> 
> You'll note that the GetPointerEvents helper does indeed always set
> POINTER_EMULATED on buttons 4, 5, 6 and 7.

if XIPointerEmulated is set on an event, will there be a corresponding
RawEvent? This needs to be pointed out here. If yes, the raw event needs a
flag as well.

Cheers,
  Peter

> > I further guess that the non-raw scroll events will still be adding up,
> > is that intentional?
> 
> Crap, good catch.  Guess that's what you get for playing around with
> smooth scrolling and a bunch of accel changes at the same time ... I'll
> probably have to fix that by adding a new axis mode, I guess (relative
> but not additive).
> 
> Cheers,
> Daniel
 
___
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


Re: [PATCH 26/27] Input: Split GetPointerEvents body into a helper function

2011-06-06 Thread Peter Hutterer
On Fri, Jun 03, 2011 at 04:00:02PM +0100, Daniel Stone wrote:
> For smooth-scrolling support, we want GetPointerEvents to generate
> multiple events, so split the body of the function out into a helper
> function in order to call it multiple times.
> 
> Signed-off-by: Daniel Stone 
> ---
>  dix/getevents.c |   59 --
>  1 files changed, 43 insertions(+), 16 deletions(-)
> 
> diff --git a/dix/getevents.c b/dix/getevents.c
> index ad99cae..c1ebbb2 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -1065,24 +1065,18 @@ QueuePointerEvents(DeviceIntPtr device, int type,
>   *
>   * @return the number of events written into events.
>   */
> -int
> -GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type, int 
> buttons,
> - int flags, const ValuatorMask *mask_in) {
> +static int
> +_GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type,
> +  int buttons, CARD32 ms, int flags,
> +  const ValuatorMask *mask_in)

"create_pointer_events" maybe?

getevents.c is one of the few files that is consistent with static
functions being lowercase, even though they use foo_bar and fooBar style.

wouldn't mind switching all of them to foo_bar at some point.

Cheers,
  Peter

> +{
>  int num_events = 1, i;
> -CARD32 ms;
>  DeviceEvent *event;
> -RawDeviceEvent*raw;
> +RawDeviceEvent *raw;
>  double screenx = 0.0, screeny = 0.0;
>  ScreenPtr scr = miPointerGetScreen(pDev);
>  ValuatorMask mask;
>  
> -/* refuse events from disabled devices */
> -if (!pDev->enabled)
> -return 0;
> -
> -if (!scr)
> -return 0;
> -
>  switch (type)
>  {
>  case MotionNotify:
> @@ -1098,10 +1092,6 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr 
> pDev, int type, int buttons
>  return 0;
>  }
>  
> -ms = GetTimeInMillis(); /* before pointer update to help precision */
> -
> -events = UpdateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT, 
> &num_events);
> -
>  raw = &events->raw_event;
>  events++;
>  num_events++;
> @@ -1189,6 +1179,43 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr 
> pDev, int type, int buttons
>  }
>  
>  /**
> + * Generate a series of InternalEvents (filled into the EventList)
> + * representing pointer motion, or button presses.
> + *
> + * events is not NULL-terminated; the return value is the number of events.
> + * The DDX is responsible for allocating the event structure in the first
> + * place via InitEventList() and GetMaximumEventsNum(), and for freeing it.
> + *
> + * In the generated events rootX/Y will be in absolute screen coords and
> + * the valuator information in the absolute or relative device coords.
> + *
> + * last.valuators[x] of the device is always in absolute device coords.
> + * last.valuators[x] of the master device is in absolute screen coords.
> + *
> + * master->last.valuators[x] for x > 2 is undefined.
> + */
> +int
> +GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type, int 
> buttons,
> +  int flags, const ValuatorMask *mask_in)
> +{
> +CARD32 ms = GetTimeInMillis();
> +int num_events = 0;
> +
> +/* refuse events from disabled devices */
> +if (!pDev->enabled)
> +return 0;
> +
> +if (!miPointerGetScreen(pDev))
> +return 0;
> +
> +events = UpdateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT,
> +  &num_events);
> +num_events += _GetPointerEvents(events, pDev, type, buttons, ms, flags,
> +mask_in);
> +return num_events;
> +}
> +
> +/**
>   * Generate internal events representing this proximity event and enqueue
>   * them on the event queue.
>   *
> -- 
> 1.7.5.3

___
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


[PULL] few minor input fixes

2011-06-06 Thread Peter Hutterer
Only important patch here is the first, it causes the 0.10.x builds to
break on some machines.

The following changes since commit c5b72fd350bbdfd1facd0ddd5085f238c4cf252a:

  DIX: Set backgroundState correctly for root window (2011-06-01 18:42:23 -0700)

are available in the git repository at:
  git://people.freedesktop.org/~whot/xserver.git for-keith

Peter Hutterer (8):
  test: don't test for double alignment on i386. (#36986)
  dix: fix an error message.
  test: fix memset size for WindowRec (#37801)
  dix: don't pass x/y to transformAbsolute
  dix: drop x/y back into the right valuators after transformation.
  dix: use xi2_get_type instead of manual typecast
  Xi: use __func__ instead of function name.
  dix: fix crashers with floating device.

 Xi/exevents.c  |4 ++--
 dix/events.c   |6 +++---
 dix/getevents.c|   43 ---
 test/input.c   |3 +++
 test/xi2/protocol-common.c |2 +-
 5 files changed, 37 insertions(+), 21 deletions(-)
___
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


Re: [PATCH 17/27] Input: Remove x and y from moveAbsolute/moveRelative

2011-06-06 Thread Peter Hutterer
On Fri, Jun 03, 2011 at 03:59:53PM +0100, Daniel Stone wrote:
> Both these functions modify the mask and
> pDev->last.{valuators,remainder} in-place now, so there's no need to
> pass in pointers to local x and y values.
> 
> Signed-off-by: Daniel Stone 
> ---
>  dix/getevents.c |   34 --
>  1 files changed, 8 insertions(+), 26 deletions(-)
> 
> diff --git a/dix/getevents.c b/dix/getevents.c
> index 2f8e137..eff277b 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -715,15 +715,12 @@ UpdateFromMaster(InternalEvent* events, DeviceIntPtr 
> dev, int type, int *num_eve
>   * Move the device's pointer to the position given in the valuators.
>   *
>   * @param dev The device whose pointer is to be moved.
> - * @param x Returns the x position of the pointer after the move.
> - * @param y Returns the y position of the pointer after the move.
>   * @param mask Valuator data for this event.
>   */
>  static void
> -moveAbsolute(DeviceIntPtr dev, int *x_out, int *y_out, ValuatorMask *mask)
> +moveAbsolute(DeviceIntPtr dev, ValuatorMask *mask)
>  {
>  int i;
> -double x = dev->last.valuators[0], y = dev->last.valuators[1];
>  
>  for (i = 0; i < valuator_mask_size(mask); i++)
>  {
> @@ -737,21 +734,16 @@ moveAbsolute(DeviceIntPtr dev, int *x_out, int *y_out, 
> ValuatorMask *mask)
>  dev->last.remainder[i] = val - floor(val);
>  valuator_mask_set_double(mask, i, val);
>  }
> -
> -*x_out = dev->last.valuators[0];
> -*y_out = dev->last.valuators[1];
>  }
>  
>  /**
>   * Move the device's pointer by the values given in @valuators.
>   *
>   * @param dev The device whose pointer is to be moved.
> - * @param x Returns the x position of the pointer after the move.
> - * @param y Returns the y position of the pointer after the move.
>   * @param mask Valuator data for this event.
>   */
>  static void
> -moveRelative(DeviceIntPtr dev, int *x_out, int *y_out, ValuatorMask *mask)
> +moveRelative(DeviceIntPtr dev, ValuatorMask *mask)
>  {
>  int i;
>  Bool clip_xy = IsMaster(dev) || !IsFloating(dev);
> @@ -773,9 +765,6 @@ moveRelative(DeviceIntPtr dev, int *x_out, int *y_out, 
> ValuatorMask *mask)
>  dev->last.remainder[i] = val - round_towards_zero(val);
>  }
>  }
> -
> -*x_out = dev->last.valuators[0];
> -*y_out = dev->last.valuators[1];
>  }
>  
>  /**
> @@ -1185,24 +1174,17 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr 
> pDev, int type, int buttons
>  }
>  
>  transformAbsolute(pDev, &mask);
> -/* x and y are only set, but not used, by moveAbsolute */
> -moveAbsolute(pDev, &x, &y, &mask);
> +moveAbsolute(pDev, &mask);
>  } else {
>  if (flags & POINTER_ACCELERATE)
>  accelPointer(pDev, &mask, ms);
> -moveRelative(pDev, &x, &y, &mask);
> +moveRelative(pDev, &mask);
>  }
>  
> -if (valuator_mask_isset(&mask, 0))
> -{
> -x_frac = valuator_mask_get_double(&mask, 0);
> -x_frac -= round_towards_zero(x_frac);
> -}
> -if (valuator_mask_isset(&mask, 1))
> -{
> -y_frac = valuator_mask_get_double(&mask, 1);
> -y_frac -= round_towards_zero(y_frac);
> -}
> +x = pDev->last.valuators[0];
> +x_frac = pDev->last.remainder[0];
> +y = pDev->last.valuators[1];
> +y_frac = pDev->last.remainder[1];
  
I think it'd be easier to understand to get these two out of the valuator
mask instead out of the device. AIUI, both should have the same value
anyway.

Cheers,
  Peter

>  set_raw_valuators(raw, &mask, raw->valuators.data,
>raw->valuators.data_frac);
> -- 
> 1.7.5.3
> 
___
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


Re: [PATCH libXi 3/4] Added XIGrabDeviceWithConfine.

2011-06-06 Thread Jeremy Huddleston

On Jun 6, 2011, at 11:48 PM, Peter Hutterer wrote:

>> 
>> Or to really be picky:
>> buff = calloc(req->mask_len, 4);
> 
> amended locally, thanks.

or to be even pickier:

buff = calloc(4, req->mask_len);

b/c:
void * calloc(size_t count, size_t size);

;)

___
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


Re: [PATCH inputproto v2 2/4] XI2.1: Add confine_to to XIGrabDevice.

2011-06-06 Thread Jeremy Huddleston
Yeah, I think the v1->v2 changes make sense.

On Jun 6, 2011, at 10:27 PM, Peter Hutterer wrote:

> From: Philipp Reh 
> 
> Updated the specs for XIGrabDevice which now
> sends an additional confine_to parameter after the mask.
> 
> The confine_to behaviour is the same as the core event behaviour: if the
> device is a pointer, the confine_to window determines the window the
> cursor is confined to during the grab.
> 
> Changes in protocol behaviour to XI 2.0:
> XIGrabDevice may return NotViewable if the confine_to is not viewable.
> 
> Signed-off-by: Philipp Reh 
> Reviewed-by: Peter Hutterer 
> Signed-off-by: Peter Hutterer 
> Reviewed-by: Jeremy Huddleston 
> ---
> Changes to v1:
> - slave devices can have confine_to as well. add the documentation
> 
> I've thought about the valuator range confinement and IMO we should just
> leave it at the current behaviour which proportionally clips it into the
> confine_to range. XI2 valuators are mainly there to give high precision data
> to the clients and if the pointer is confined, this data is confined too.
> Sending non-confined valuator ranges during a confine grab would put the
> burden of mapping the ranges into the confine_to dimensions on the client.
> 
> specs/XI2proto.txt |   16 +++-
> 1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/specs/XI2proto.txt b/specs/XI2proto.txt
> index 5abf9d4..a777d9a 100644
> --- a/specs/XI2proto.txt
> +++ b/specs/XI2proto.txt
> @@ -39,6 +39,7 @@ device information in each event (with the exception of 
> core events).
> Changes introduced by version 2.1
> 
> - RawEvents are sent regardless of the grab state.
> +- XIGrabDevice takes a confine_to parameter
> 
> //❧❧❧
> 
> @@ -822,10 +823,13 @@ Return the current focus window for the given device.
> cursor:  Cursor
> mask_len:CARD16
> masks:   SETofEVENTMASK
> +confine_to*: Window
> ▶
> status:  Success, AlreadyGrabbed, Frozen, InvalidTime, 
> NotViewable
> └───
> 
> +* since XI 2.1
> +
> This request actively grabs control of the specified input device. Further
> input events from this device are reported only to the grabbing client.
> This request overides any previous active grab by this client for this
> @@ -851,6 +855,8 @@ device.
> Length of mask in 4 byte units.
> mask
> Event mask. An event mask for an event type T is defined as (1 << T).
> +confine_to
> +The window to confine the pointer to. Can be None.
> status
> Success or the reason why the grab could not be established.
> 
> @@ -887,10 +893,18 @@ devices are frozen; they are simply queued for later 
> processing.
> If the cursor is not None and the device is a master pointer device, the
> cursor will be displayed until the device is ungrabbed.
> 
> +If the client supports XI 2.1 and confine_to is not None and the device
> +is a master pointer device or a slave device, the device will be confined
> +to that window. Reported root-relative coordinates are restricted to the
> +confine_to window. If the device reports valuators within an axis range,
> +the reported valuators are proportionally restricted to the confine_to
> +window.
> +
> This request fails and returns:
> 
> AlreadyGrabbed: If the device is actively grabbed by some other client.
> -NotViewable: If grab-window is not viewable.
> +NotViewable: If grab-window or the confine_to window (if any) is not
> + viewable.
> InvalidTime: If the specified time is earlier than the last-grab-time for
>  the specified device or later than the current X server time.
>  Otherwise, the last-grab-time for the specified device is set
> -- 
> 1.7.5.1
> 
> ___
> 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

___
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

Re: [PATCH] dix: fix crashers with floating device.

2011-06-06 Thread Jeremy Huddleston
Looks right...

Reviewed-by: Jeremy Huddleston 

On Jun 6, 2011, at 9:01 PM, Peter Hutterer wrote:

> dc57f89959e549403f8488eb9f23425bd7118b22 accidentally reversed the
> conditions.
> 
> in dix/events.c we try to detach floating devices. This leads to a
> NULL-dereference on GetMaster()->id.
> 
> in dix/getevents.c we try to get the master device for the floating slave
> and dereference it.
> 
> Signed-off-by: Peter Hutterer 
> ---
> dix/events.c|2 +-
> dix/getevents.c |4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/dix/events.c b/dix/events.c
> index 6660504..6b74b1a 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -1421,7 +1421,7 @@ CheckGrabForSyncs(DeviceIntPtr thisDev, Bool thisMode, 
> Bool otherMode)
> static void
> DetachFromMaster(DeviceIntPtr dev)
> {
> -if (!IsFloating(dev))
> +if (IsFloating(dev))
> return;
> 
> dev->saved_master_id = GetMaster(dev, MASTER_ATTACHED)->id;
> diff --git a/dix/getevents.c b/dix/getevents.c
> index 1352a81..c935c97 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -864,7 +864,7 @@ positionSprite(DeviceIntPtr dev, int mode,
>  * to the current screen. */
> miPointerSetPosition(dev, mode, screenx, screeny);
> 
> -if(!IsMaster(dev) || !IsFloating(dev)) {
> +if(!IsMaster(dev) && !IsFloating(dev)) {
> DeviceIntPtr master = GetMaster(dev, MASTER_POINTER);
> master->last.valuators[0] = *screenx;
> master->last.valuators[1] = *screeny;
> @@ -911,7 +911,7 @@ updateHistory(DeviceIntPtr dev, ValuatorMask *mask, 
> CARD32 ms)
> return;
> 
> updateMotionHistory(dev, ms, mask, dev->last.valuators);
> -if(!IsMaster(dev) || !IsFloating(dev))
> +if(!IsMaster(dev) && !IsFloating(dev))
> {
> DeviceIntPtr master = GetMaster(dev, MASTER_POINTER);
> updateMotionHistory(master, ms, mask, dev->last.valuators);
> -- 
> 1.7.5.1
> 
> ___
> 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
> 

___
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


Re: [PATCH 07/27] Input: Add vertical and horizontal scroll axes

2011-06-06 Thread Peter Hutterer
On Fri, Jun 03, 2011 at 03:59:43PM +0100, Daniel Stone wrote:
> To be used for smooth scrolling with future driver APIs.
> 
> Signed-off-by: Daniel Stone 
> ---
>  Xi/xiproperty.c  |5 +
>  include/xserver-properties.h |2 ++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
[...]
> diff --git a/include/xserver-properties.h b/include/xserver-properties.h
> index 2b1feab..b3764c5 100644
> --- a/include/xserver-properties.h
> +++ b/include/xserver-properties.h
> @@ -74,6 +74,8 @@
>  #define AXIS_LABEL_PROP_REL_DIAL"Rel Dial"
>  #define AXIS_LABEL_PROP_REL_WHEEL   "Rel Vert Wheel"
>  #define AXIS_LABEL_PROP_REL_MISC"Rel Misc"
> +#define AXIS_LABEL_PROP_REL_VSCROLL "Rel Vert Scroll"
> +#define AXIS_LABEL_PROP_REL_HSCROLL "Rel Horiz Scroll"
>  
>  /*
>   * Absolute axes
> -- 
> 1.7.5.3

Should a mouse wheel use Rel Horiz Wheel or Rel Horiz Scroll now? Might be
worth adding a bit to the commit message to explain the semantics of this
new set of properties.

aiui, adding this means that clients can rely that these axes are an
emulated scroll axis whereas wheel would be a physical axis that can be
interpreted as whatever by the client. either way, clients need to know and
handle both.
 
Cheers,
  Peter
___
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


Re: [PATCH 04/27] Input: Add double-precision valuator_mask API

2011-06-06 Thread Peter Hutterer
On Fri, Jun 03, 2011 at 03:59:40PM +0100, Daniel Stone wrote:
> Add API for valuator_mask that accepts and returns doubles, rather than
> ints.  No double API is provided for set_range at the moment.
> 
> Signed-off-by: Daniel Stone 
> ---
>  dix/inpututils.c |   28 +---
>  include/input.h  |5 +
>  test/input.c |   21 +++--
>  3 files changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/dix/inpututils.c b/dix/inpututils.c
> index 085a6b4..07adbb5 100644
> --- a/dix/inpututils.c
> +++ b/dix/inpututils.c
> @@ -497,7 +497,7 @@ valuator_mask_isset(const ValuatorMask *mask, int 
> valuator)
>  }
>  
>  /**
> - * Set the valuator to the given data.
> + * Set the valuator to the given integer data.
>   */
>  void
>  valuator_mask_set(ValuatorMask *mask, int valuator, int data)
> @@ -508,8 +508,20 @@ valuator_mask_set(ValuatorMask *mask, int valuator, int 
> data)
>  }
>  
>  /**
> - * Return the requested valuator value. If the mask bit is not set for the
> - * given valuator, the returned value is undefined.
> + * Set the valuator to the given floating-point data.
> + */
> +void
> +valuator_mask_set_double(ValuatorMask *mask, int valuator, double data)
> +{
> +mask->last_bit = max(valuator, mask->last_bit);
> +SetBit(mask->mask, valuator);
> +mask->valuators[valuator] = data;
> +}

given that the function does a bit more than just a single assignment, it
may be worth changing valuator_mask_set to call valuator_mask_set_double
instead of duplicating the three lines.

Reviewed-by: Peter Hutterer  either way though.

Cheers,
  Peter

> +
> +/**
> + * Return the requested valuator value as an integer, rounding towards zero.
> + * If the mask bit is not set for the given valuator, the returned value is
> + * undefined.
>   */
>  int
>  valuator_mask_get(const ValuatorMask *mask, int valuator)
> @@ -518,6 +530,16 @@ valuator_mask_get(const ValuatorMask *mask, int valuator)
>  }
>  
>  /**
> + * Return the requested valuator value as a double. If the mask bit is not
> + * set for the given valuator, the returned value is undefined.
> + */
> +double
> +valuator_mask_get_double(const ValuatorMask *mask, int valuator)
> +{
> +return mask->valuators[valuator];
> +}
> +
> +/**
>   * Remove the valuator from the mask.
>   */
>  void
> diff --git a/include/input.h b/include/input.h
> index a15623a..db56b25 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -585,6 +585,9 @@ extern _X_EXPORT void 
> valuator_mask_set_range(ValuatorMask *mask,
>  extern _X_EXPORT void valuator_mask_set(ValuatorMask *mask,
>  int valuator,
>  int data);
> +extern _X_EXPORT void valuator_mask_set_double(ValuatorMask *mask,
> +   int valuator,
> +   double data);
>  extern _X_EXPORT void valuator_mask_zero(ValuatorMask *mask);
>  extern _X_EXPORT int valuator_mask_size(const ValuatorMask *mask);
>  extern _X_EXPORT int valuator_mask_isset(const ValuatorMask *mask, int bit);
> @@ -593,6 +596,8 @@ extern _X_EXPORT int valuator_mask_num_valuators(const 
> ValuatorMask *mask);
>  extern _X_EXPORT void valuator_mask_copy(ValuatorMask *dest,
>   const ValuatorMask *src);
>  extern _X_EXPORT int valuator_mask_get(const ValuatorMask *mask, int valnum);
> +extern _X_EXPORT double valuator_mask_get_double(const ValuatorMask *mask,
> + int valnum);
>  
>  extern _X_EXPORT double round_towards_zero(double val);
>  
> diff --git a/test/input.c b/test/input.c
> index ac37d67..0d72fb8 100644
> --- a/test/input.c
> +++ b/test/input.c
> @@ -1089,12 +1089,16 @@ static void dix_input_valuator_masks(void)
>  {
>  ValuatorMask *mask = NULL, *copy;
>  int nvaluators = MAX_VALUATORS;
> -int valuators[nvaluators];
> +double valuators[nvaluators];
> +int val_ranged[nvaluators];
>  int i;
>  int first_val, num_vals;
>  
>  for (i = 0; i < nvaluators; i++)
> -valuators[i] = i;
> +{
> +valuators[i] = i + 0.5;
> +val_ranged[i] = i;
> +}
>  
>  mask = valuator_mask_new(nvaluators);
>  assert(mask != NULL);
> @@ -1104,9 +1108,10 @@ static void dix_input_valuator_masks(void)
>  for (i = 0; i < nvaluators; i++)
>  {
>  assert(!valuator_mask_isset(mask, i));
> -valuator_mask_set(mask, i, valuators[i]);
> +valuator_mask_set_double(mask, i, valuators[i]);
>  assert(valuator_mask_isset(mask, i));
> -assert(valuator_mask_get(mask, i) == valuators[i]);
> +assert(valuator_mask_get(mask, i) == 
> round_towards_zero(valuators[i]));
> +assert(valuator_mask_get_double(mask, i) == valuators[i]);
>  assert(valuator_mask_size(mask) == i + 1);
>  assert(valuator_mask_num_valuators(mask) == i + 1);
>  }
> @@ -

Re: [PATCH xserver 02/12] Added processing of XI grabs with confine.

2011-06-06 Thread Peter Hutterer
On Mon, Jun 06, 2011 at 02:29:48PM +0100, Daniel Stone wrote:
> Hi,
> 
> On Thu, Jun 02, 2011 at 05:13:36PM +1000, Peter Hutterer wrote:
> > +if(HasConfineTo(client) == TRUE)
> > +memcpy((unsigned char *)&confine_to, (char*)&stuff[1] + 
> > stuff->mask_len * 4, 4);
> 
> The first cast here is unnecessary.

just for the archives, this line was obsoleted with version 2 of the patch
(adding the get_confine_to)

http://patchwork.freedesktop.org/patch/5810/

Cheers,
  Peter
___
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


Re: [PATCH libXi 3/4] Added XIGrabDeviceWithConfine.

2011-06-06 Thread Peter Hutterer
On Mon, Jun 06, 2011 at 12:57:17PM +0100, Daniel Stone wrote:
> On Thu, Jun 02, 2011 at 09:59:22AM -0700, Jeremy Huddleston wrote:
> > > @@ -65,10 +65,15 @@ XIGrabDevice(Display* dpy, int deviceid, Window 
> > > grab_window, Time time,
> > > buff = calloc(1, len * 4);
> > > memcpy(buff, mask->mask, mask->mask_len);
> > > 
> > > +len++; /* for the confine_to */
> > > +
> > > SetReqLen(req, len, len);
> > > -Data(dpy, buff, len * 4);
> > > +Data(dpy, buff, (len - 1) * 4);
> > > free(buff);
> > > 
> > > +/* put the confine_to window at the end */
> > > +Data32(dpy, &confine_to, 4);
> > > +
> > > if (_XReply(dpy, (xReply *)&reply, 0, xTrue) == 0)
> > >   reply.status = GrabSuccess;
> > 
> > This feels cleaner to me:
> > 
> > len = req->mask_len + 1;
> > buff = calloc(1, req->mask_len * 4);
> > memcpy(buff, mask->mask, mask->mask_len);
> > 
> > SetReqLen(req, len, len);
> > Data(dpy, buff, req->mask_len * 4);
> > free(buff);
> > 
> > Data32(dpy, &confine_to, 4);
> 
> Or to really be picky:
> buff = calloc(req->mask_len, 4);

amended locally, thanks.

Cheers,
  Peter
___
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


[PATCH inputproto v2 3/4] XI2.1: Add confine_to to XIPassiveGrabDevice

2011-06-06 Thread Peter Hutterer
Enter and button grabs can specify an additional confine_to window to which
the pointer is confined to during the grab. If the confine_to window is not
None the grab does not activate if the confine_to window is not viewable.

Changes in protocol behaviour to XI 2.0:
XIPassiveGrabDevice may return BadWindow if the confine_to window does
not specify a valid window.

Signed-off-by: Peter Hutterer 
CC: Philipp Reh 
Reviewed-by: Jeremy Huddleston 
---
Changes to v1:
- allow for confine_to on slave devices

 specs/XI2proto.txt |   17 -
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/specs/XI2proto.txt b/specs/XI2proto.txt
index a777d9a..fdbcc72 100644
--- a/specs/XI2proto.txt
+++ b/specs/XI2proto.txt
@@ -39,7 +39,7 @@ device information in each event (with the exception of core 
events).
 Changes introduced by version 2.1
 
 - RawEvents are sent regardless of the grab state.
-- XIGrabDevice takes a confine_to parameter
+- XIGrabDevice and XIPassiveGrabDevice take a confine_to parameter
 
 //❧❧❧
 
@@ -1047,8 +1047,11 @@ you pass to the event-mode argument:
 ▶
 num_modifiers_return:INT16
 modifiers_return:GRABMODIFIERINFO
+confine_to*: Window
 └───
 
+* since XI 2.1
+
 GRABTYPE { GrabtypeButton, GrabtypeKeycode, GrabtypeEnter,
GrabtypeFocusIn}
 
@@ -1096,6 +1099,8 @@ on the specified input device.
 Number of elements in modifiers_return
 modifiers_return
 XKB modifier state that could not be grabbed.
+confine_to
+The window to confine the pointer to. Can be None.
 
 If owner-events is False, input events generated from this device are
 reported with respect to grab-window, and are only reported if
@@ -1127,6 +1132,7 @@ device is actively grabbed if:
 - the grab_window contains the pointer, and
 - a passive grab on the same button/keycode + modifier
   combination does not exist on an ancestor of grab_window.
+- the confine_to window (if any) is viewable
 
 Otherwise, if grab_type is GrabtypeEnter or GrabtypeFocusIn, the
 device is actively grabbed if:
@@ -1139,6 +1145,7 @@ device is actively grabbed if:
   grab_window or a descendant of grab_window,
 - a passive grab of the same grab_type + modifier combination does not
   does not exist on an ancestor of grab_window.
+- the confine_to window (if any) is viewable
 
 A modifier of GrabAnyModifier is equivalent to issuing the request for
 all possible modifier combinations (including no modifiers). A client
@@ -1186,6 +1193,14 @@ grab deactivates, addional LeaveNotify events with mode
 XIPassiveUngrabNotify are generated and sent to the grabbing client
 before the grab deactivates.
 
+If the client supports XI 2.1 and the grab type is GrabtypeButton or
+GrabtypeEnter and confine_to is not None and the device is a
+master pointer device or a slave device, the device will be confined
+to that window for the duration of the grab. Reported root-relative
+coordinates are restricted to the confine_to window. If the device reports
+valuators within an axis range, the reported valuators are proportionally
+restricted to the confine_to window.
+
 ┌───
 XIPassiveUngrabDevice
 deviceid:DEVICEID
-- 
1.7.5.1

___
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

[PATCH inputproto v2 2/4] XI2.1: Add confine_to to XIGrabDevice.

2011-06-06 Thread Peter Hutterer
From: Philipp Reh 

Updated the specs for XIGrabDevice which now
sends an additional confine_to parameter after the mask.

The confine_to behaviour is the same as the core event behaviour: if the
device is a pointer, the confine_to window determines the window the
cursor is confined to during the grab.

Changes in protocol behaviour to XI 2.0:
XIGrabDevice may return NotViewable if the confine_to is not viewable.

Signed-off-by: Philipp Reh 
Reviewed-by: Peter Hutterer 
Signed-off-by: Peter Hutterer 
Reviewed-by: Jeremy Huddleston 
---
Changes to v1:
- slave devices can have confine_to as well. add the documentation

I've thought about the valuator range confinement and IMO we should just
leave it at the current behaviour which proportionally clips it into the
confine_to range. XI2 valuators are mainly there to give high precision data
to the clients and if the pointer is confined, this data is confined too.
Sending non-confined valuator ranges during a confine grab would put the
burden of mapping the ranges into the confine_to dimensions on the client.

 specs/XI2proto.txt |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/specs/XI2proto.txt b/specs/XI2proto.txt
index 5abf9d4..a777d9a 100644
--- a/specs/XI2proto.txt
+++ b/specs/XI2proto.txt
@@ -39,6 +39,7 @@ device information in each event (with the exception of core 
events).
 Changes introduced by version 2.1
 
 - RawEvents are sent regardless of the grab state.
+- XIGrabDevice takes a confine_to parameter
 
 //❧❧❧
 
@@ -822,10 +823,13 @@ Return the current focus window for the given device.
 cursor:  Cursor
 mask_len:CARD16
 masks:   SETofEVENTMASK
+confine_to*: Window
 ▶
 status:  Success, AlreadyGrabbed, Frozen, InvalidTime, 
NotViewable
 └───
 
+* since XI 2.1
+
 This request actively grabs control of the specified input device. Further
 input events from this device are reported only to the grabbing client.
 This request overides any previous active grab by this client for this
@@ -851,6 +855,8 @@ device.
 Length of mask in 4 byte units.
 mask
 Event mask. An event mask for an event type T is defined as (1 << T).
+confine_to
+The window to confine the pointer to. Can be None.
 status
 Success or the reason why the grab could not be established.
 
@@ -887,10 +893,18 @@ devices are frozen; they are simply queued for later 
processing.
 If the cursor is not None and the device is a master pointer device, the
 cursor will be displayed until the device is ungrabbed.
 
+If the client supports XI 2.1 and confine_to is not None and the device
+is a master pointer device or a slave device, the device will be confined
+to that window. Reported root-relative coordinates are restricted to the
+confine_to window. If the device reports valuators within an axis range,
+the reported valuators are proportionally restricted to the confine_to
+window.
+
 This request fails and returns:
 
 AlreadyGrabbed: If the device is actively grabbed by some other client.
-NotViewable: If grab-window is not viewable.
+NotViewable: If grab-window or the confine_to window (if any) is not
+ viewable.
 InvalidTime: If the specified time is earlier than the last-grab-time for
  the specified device or later than the current X server time.
  Otherwise, the last-grab-time for the specified device is set
-- 
1.7.5.1

___
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

Re: [PATCH inputproto 2/4] XI2.1: Add confine_to to XIGrabDevice.

2011-06-06 Thread Peter Hutterer
On Mon, Jun 06, 2011 at 12:53:33PM +0100, Daniel Stone wrote:
> Hi,
> 
> On Thu, Jun 02, 2011 at 05:13:28PM +1000, Peter Hutterer wrote:
> > +If the client supports XI 2.1 and confine_to is not None and the device is 
> > a
> > +master pointer device, the cursor will be confined to that window.
> 
> If the device is an SD, won't its motion also be confined? As far as I
> can tell, we call ConfineCursorToWindow pretty unconditionally, and it
> does look like it would get clipped ...

this is the inputproto patch, so the server behaviour doesn't necessarily
matter here (also, i just found the server crashes if you try to confine_to
a floating slave ;)

Anyway. Currently, slave devices are _not_ confined to a window. This
isn't directly documented but simply explained - confine_to previously only
existed on core requests, so only master pointers could be confined by a
client, for all others confine_to was None.
And a side effect I just noticed is that the XI2 valuator range is
confined proportionately as well, that shouldn't happen IMO.

It makes sense to confine floating slaves, there may be use-cases (gimp
map-to-canvas?). attached slaves are detached on a direct grab anyway.
I'll try to get out more patches then.

Cheers,
  Peter
___
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


[PATCH] dix: fix crashers with floating device.

2011-06-06 Thread Peter Hutterer
dc57f89959e549403f8488eb9f23425bd7118b22 accidentally reversed the
conditions.

in dix/events.c we try to detach floating devices. This leads to a
NULL-dereference on GetMaster()->id.

in dix/getevents.c we try to get the master device for the floating slave
and dereference it.

Signed-off-by: Peter Hutterer 
---
 dix/events.c|2 +-
 dix/getevents.c |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/dix/events.c b/dix/events.c
index 6660504..6b74b1a 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -1421,7 +1421,7 @@ CheckGrabForSyncs(DeviceIntPtr thisDev, Bool thisMode, 
Bool otherMode)
 static void
 DetachFromMaster(DeviceIntPtr dev)
 {
-if (!IsFloating(dev))
+if (IsFloating(dev))
 return;
 
 dev->saved_master_id = GetMaster(dev, MASTER_ATTACHED)->id;
diff --git a/dix/getevents.c b/dix/getevents.c
index 1352a81..c935c97 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -864,7 +864,7 @@ positionSprite(DeviceIntPtr dev, int mode,
  * to the current screen. */
 miPointerSetPosition(dev, mode, screenx, screeny);
 
-if(!IsMaster(dev) || !IsFloating(dev)) {
+if(!IsMaster(dev) && !IsFloating(dev)) {
 DeviceIntPtr master = GetMaster(dev, MASTER_POINTER);
 master->last.valuators[0] = *screenx;
 master->last.valuators[1] = *screeny;
@@ -911,7 +911,7 @@ updateHistory(DeviceIntPtr dev, ValuatorMask *mask, CARD32 
ms)
 return;
 
 updateMotionHistory(dev, ms, mask, dev->last.valuators);
-if(!IsMaster(dev) || !IsFloating(dev))
+if(!IsMaster(dev) && !IsFloating(dev))
 {
 DeviceIntPtr master = GetMaster(dev, MASTER_POINTER);
 updateMotionHistory(master, ms, mask, dev->last.valuators);
-- 
1.7.5.1

___
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


Re: [PATCH inputproto 4/4] inputproto 2.1

2011-06-06 Thread Peter Hutterer
On Mon, Jun 06, 2011 at 12:54:49PM +0100, Daniel Stone wrote:
> Hi,
> 
> On Thu, Jun 02, 2011 at 05:13:30PM +1000, Peter Hutterer wrote:
> > And revert the warnings and hoops we made the user jump through. They'll
> > come back for XI 2.2.
> > 
> > Revert "Put a #warning and #error in to avoid unsuspecting XI 2.1 users."
> > 
> > This reverts commit 811a643d3eadb4baf4d21f5fef7b4f0c17318167.
> > 
> > Revert "Require configure flag to build this proto version."
> > 
> > This reverts commit 875879114d1681f79bd6bcf7d5bdf49f841da1c9
> > 
> > Signed-off-by: Peter Hutterer 
> 
> Other than my comment on #2, which also applies for #3, consider the
> whole series:
> Reviewed-by: Daniel Stone 

thanks. On that note, I think it might be safer to bump to 2.0.99.1 in
configure.ac for now to avoid the usual screwups. 

Cheers,
  Peter
___
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


[PATCH v3 3/12] Xi: Add support for confine_to on XI 2.1 XIPassiveGrab requests.

2011-06-06 Thread Peter Hutterer
Signed-off-by: Peter Hutterer 
Reviewed-by: Jeremy Huddleston 
---
Changes to v2:
- rename HasConfineTo to XI2ClientSendsConfineTo() to avoid too much
  ambiguity. this also required the obvious fix to 2/12.
- extra space removed from get_confine_to

On Mon, Jun 06, 2011 at 02:34:20PM +0100, Daniel Stone wrote:
> >
> > +get_confine_to(const xXIPassiveGrabDeviceReq *req)
>
> Disambiguating this name would be pretty keen as well, since we now have
> two get_confine_tos.  Also, there's an extraneous space in the return
> statement.

I think we should be fine here. both get_confine_to are static and local to
the file and take a typed request pointer that should make it obvious which
request to pass in.

 Xi/xigrabdev.c |2 +-
 Xi/xigrabdev.h |2 ++
 Xi/xipassivegrab.c |   40 
 3 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/Xi/xigrabdev.c b/Xi/xigrabdev.c
index 8b394c9..4bf68f2 100644
--- a/Xi/xigrabdev.c
+++ b/Xi/xigrabdev.c
@@ -58,7 +58,7 @@ get_confine_to(xXIGrabDeviceReq *req)
 return (Window*)data;
 }
 
-static Bool
+Bool
 XI2ClientSendsConfineTo(ClientPtr client)
 {
 XIClientPtr pXIClient;
diff --git a/Xi/xigrabdev.h b/Xi/xigrabdev.h
index 08309c9..d4eb9cf 100644
--- a/Xi/xigrabdev.h
+++ b/Xi/xigrabdev.h
@@ -38,4 +38,6 @@ int SProcXIUngrabDevice(ClientPtr client);
 
 void SRepXIGrabDevice(ClientPtr client, int size, xXIGrabDeviceReply * rep);
 
+Bool XI2ClientSendsConfineTo(ClientPtr client);
+
 #endif /* XIGRABDEV_H */
diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
index ae43433..a18a169 100644
--- a/Xi/xipassivegrab.c
+++ b/Xi/xipassivegrab.c
@@ -45,6 +45,23 @@
 #include "dixgrabs.h"
 #include "misc.h"
 
+#include "xigrabdev.h" /* HasConfineTo */
+
+/**
+ * The confine_to window in XI2.1 trails the request. It is the first 4
+ * bytes after the fixed request + the mask bytes + modifier bytes.
+ * Mask and modifiers are both in 4-byte units.
+ *
+ * @return A pointer to the 4 bytes that represent the confine_to on the
+ * wire.
+ */
+static Window*
+get_confine_to(const xXIPassiveGrabDeviceReq *req)
+{
+char *data = (char*)&req[1] + req->mask_len * 4 + req->num_modifiers * 4;
+return (Window*)data;
+}
+
 int
 SProcXIPassiveGrabDevice(ClientPtr client)
 {
@@ -60,6 +77,10 @@ SProcXIPassiveGrabDevice(ClientPtr client)
 swapl(&stuff->cursor, n);
 swapl(&stuff->time, n);
 swapl(&stuff->detail, n);
+
+if(XI2ClientSendsConfineTo(client) == TRUE)
+swapl(get_confine_to(stuff), n);
+
 swaps(&stuff->mask_len, n);
 swaps(&stuff->num_modifiers, n);
 
@@ -89,6 +110,7 @@ ProcXIPassiveGrabDevice(ClientPtr client)
 void *tmp;
 int mask_len;
 int n;
+Window confine_to;
 
 REQUEST(xXIPassiveGrabDeviceReq);
 REQUEST_AT_LEAST_SIZE(xXIPassiveGrabDeviceReq);
@@ -160,6 +182,24 @@ ProcXIPassiveGrabDevice(ClientPtr client)
 if (status != Success)
return status;
 
+/* If the client has version 2_1 or higher, it will send the confine_to 
window
+ * at the end of the request */
+confine_to = None;
+
+if(XI2ClientSendsConfineTo(client) == TRUE) {
+Window *wire_confine_to = get_confine_to(stuff);
+confine_to = *wire_confine_to;
+}
+
+if (confine_to != None)
+{
+status = dixLookupWindow((WindowPtr*)&tmp, confine_to, client, 
DixSetAttrAccess);
+if (status != Success)
+return status;
+}
+
+param.confineTo = confine_to;
+
 status = CheckGrabValues(client, ¶m);
 if (status != Success)
 return status;
-- 
1.7.5.1

___
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


Re: [PATCH xserver 05/12] dix: split client list retrieval out of DeliverEventToClients

2011-06-06 Thread Peter Hutterer
On Mon, Jun 06, 2011 at 02:36:23PM +0100, Daniel Stone wrote:
> Hi,
> 
> On Thu, Jun 02, 2011 at 05:13:39PM +1000, Peter Hutterer wrote:
> > +static int
> > +GetClientsForDelivery(DeviceIntPtr dev, WindowPtr win,
> > +  xEvent *events, Mask filter, InputClients **clients)
> >  {
> > -int attempt;
> > -enum EventDeliveryState rc = EVENT_SKIP;
> > -InputClients *other;
> > +int rc = 0;
> >  
> >  [...]
> >  
> > +rc = 1;
> > +out:
> > +return rc;
> > +}
> > 
> > [...]
> > 
> > +if (!GetClientsForDelivery(dev, win, events, filter, &other))
> > +goto out;
> 
> This would probably be better off as Bool than int.

amended locally, thanks.

Cheers,
  Peter
___
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


Re: [PATCH libXi 1/4] Add XI2 library-internal array offsets to XIint.h

2011-06-06 Thread Peter Hutterer
On Mon, Jun 06, 2011 at 02:27:38PM +0100, Daniel Stone wrote:
> Hi,
> 
> On Mon, Jun 06, 2011 at 10:19:10PM +1000, Peter Hutterer wrote:
> > On 6/06/11 21:56 , Daniel Stone wrote:
> > >On Thu, Jun 02, 2011 at 05:13:31PM +1000, Peter Hutterer wrote:
> > >>These defines are currently defined in XI.h and XI2.h. Their only use is 
> > >>as
> > >>offset into a library-internal array. Move them to XIint.h so they may be
> > >>removed from the protocol headers with the next revision.
> > >
> > >Hmm, I'm kind of uneasy about having new protocol headers break builds
> > >with old libraries. :\
> > 
> > I'm pretty sure we discussed this once before and the outcome was:
> > the protocol headers can't be changed, the defines need to stay
> > around.
> 
> Yep, sounds familiar.
> 
> > what we can do however is _add_ them to libXi and ifdef them out
> > accordingly. this also means that the ifndef will never be hit, but
> > it's still easier to read this way.
> > 
> > so there is no matching patch for inputproto for this change, we
> > keep the defines around for backwards compatibility. the new
> > additions however are only in libXi.
> 
> Hmm, well in that case I guess the commit message at least needs to be
> fixed, but if we're (quite sensibly) not adding any new defines going
> forward, and these will never be used as they're just duplicating
> existing defines, I'm not really sure I see the point? Maybe just a
> comment saying 'see XI.h for previous values' or similar?

we're talking about two different defines here I think.

inputproto provides defines like XI_Add_DevicePresenceNotify_Major.
These describe the _protocol_ version and are in  minor/major format.
For device presence, protocol version is 1.4

libXi provides defines like XInput_Add_DevicePresenceNotify. These are
monotonous and are simply offsets into a libXi-internal array.
For device presence, this offset is 5. These defines is used on every
_XiCheckExtInt() call.
[this patch is about these defines, they currently live in inputproto]

For the protocol, I don't want to provide more defines for XI2.1, 2.2 etc
and just use the version numbers. For the library, these offsets need to be
defined though. so that we still do the right version number comparisons
internally in Xlib. So there will be additions to the static versions[]
array, and they need that offset defined somewhere so that we don't need to
guess that XI 2.1 is 8 and XI 2.2 will (likely) be 9.

Having said all that, the alternative to this patchset is to change
_XiCheckExtInit to take major.minor instead. Not sure if that counts as an
ABI break though. Technically we already had this ABI break when we made it
_X_HIDDEN so now we should be safe to change it.

Cheers,
  Peter
___
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


Re: [PATCH 1/7] man: Fix typo.

2011-06-06 Thread Cyril Brulebois
Alan Coopersmith  (06/06/2011):
> While I've heard "disconnects" used as a noun, in this case it should
> be consistent with the other uses, so the change is good.

Thanks, I'll amend the commit to mention that instead (with
that = “consistency++”).

Mraw,
KiBi.


signature.asc
Description: Digital signature
___
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

Re: [PATCH 3/7] man: Fix case for MIT-unspecified.

2011-06-06 Thread Alan Coopersmith
On 06/ 5/11 02:32 PM, Cyril Brulebois wrote:
> include/site.h says that COMPILEDDISPLAYCLASS is MIT-unspecified, rather
> than MIT-Unspecified. Fix the manpage accordingly.
> 
> Signed-off-by: Cyril Brulebois 
> ---
>  man/Xserver.man |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/man/Xserver.man b/man/Xserver.man
> index d239cf3..204caf4 100644
> --- a/man/Xserver.man
> +++ b/man/Xserver.man
> @@ -356,7 +356,7 @@ ends.
>  .B \-class \fIdisplay-class\fP
>  XDMCP has an additional display qualifier used in resource lookup for
>  display-specific options.  This option sets that value, by default it
> -is "MIT-Unspecified" (not a very useful value).
> +is "MIT-unspecified" (not a very useful value).
>  .TP 8
>  .B \-cookie \fIxdm-auth-bits\fP
>  When testing XDM-AUTHENTICATION-1, a private key is shared between the

Reviewed-by: Alan Coopersmith 

-- 
-Alan Coopersmith-alan.coopersm...@oracle.com
 Oracle Solaris Platform Engineering: X Window System

___
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


Re: [PATCH 1/7] man: Fix typo.

2011-06-06 Thread Alan Coopersmith
On 06/ 5/11 02:32 PM, Cyril Brulebois wrote:
> As far as I can tell, disconnect is a verb, not a noun.
> 
> Signed-off-by: Cyril Brulebois 
> ---
>  man/Xserver.man |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/man/Xserver.man b/man/Xserver.man
> index b725949..2c016c2 100644
> --- a/man/Xserver.man
> +++ b/man/Xserver.man
> @@ -89,7 +89,7 @@ This option exists primarily for running test suites 
> remotely.
>  .B \-audit \fIlevel\fP
>  sets the audit trail level.  The default level is 1, meaning only connection
>  rejections are reported.  Level 2 additionally reports all successful
> -connections and disconnects.  Level 4 enables messages from the
> +connections and disconnections.  Level 4 enables messages from the
>  SECURITY extension, if present, including generation and revocation of
>  authorizations and violations of the security policy.
>  Level 0 turns off the audit trail.

While I've heard "disconnects" used as a noun, in this case it should
be consistent with the other uses, so the change is good.

Reviewed-by: Alan Coopersmith 

-- 
-Alan Coopersmith-alan.coopersm...@oracle.com
 Oracle Solaris Platform Engineering: X Window System

___
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


[PATCH bigreqsproto] Install target dbs alongside generated documents

2011-06-06 Thread Gaetan Nadon
This matches a change in xorg-sgml-docs whereby the masterdb will look for
the target dbs into the same location as the generated documents.

The target dbs are now installed alongside the generated documents.
Previously they are installed in $prefix/sgml/X11/dbs alongside masterdb which
has the potential of installing outside the package prefix and cause
distcheck to fail when user does not have write permission in this package.

Signed-off-by: Gaetan Nadon 
---

Requires xorg-sgml-doctools patch and libX11 to test it out
The docs have olinks to libX11

 docbook.am |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docbook.am b/docbook.am
index 3cf21e8..5864c9e 100644
--- a/docbook.am
+++ b/docbook.am
@@ -71,7 +71,7 @@ if HAVE_STYLESHEETS
 if HAVE_XSLTPROC
 
 # DocBook/XML generated document cross-reference database
-sgmldbs_DATA = $(docbook:.xml=.html.db) $(docbook:.xml=.fo.db)
+shelf_DATA += $(docbook:.xml=.html.db) $(docbook:.xml=.fo.db)
 
 # Generate DocBook/XML document cross-reference database
 # Flags for the XSL Transformation processor generating xref target databases
@@ -92,4 +92,4 @@ XSLTPROC_FLAGS =  \
 endif HAVE_XSLTPROC
 endif HAVE_STYLESHEETS
 
-CLEANFILES = $(shelf_DATA) $(sgmldbs_DATA)
+CLEANFILES = $(shelf_DATA)
-- 
1.7.4.1

___
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


[PATCH libX11] Install target dbs alongside generated documents

2011-06-06 Thread Gaetan Nadon
This matches a change in xorg-sgml-docs whereby the masterdb will look for
the target dbs into the same location as the generated documents.

The target dbs are now installed alongside the generated documents.
Previously they are installed in $prefix/sgml/X11/dbs alongside masterdb which
has the potential of installing outside the package prefix and cause
distcheck to fail when user does not have write permission in this package.

Signed-off-by: Gaetan Nadon 
---

Requires xorg-sgml-doctools patch and bigreqsproto to test it out
The docs have end links from bigreqsproto


 docbook.am |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docbook.am b/docbook.am
index 6937083..2ffb7e6 100644
--- a/docbook.am
+++ b/docbook.am
@@ -72,7 +72,7 @@ if HAVE_STYLESHEETS
 if HAVE_XSLTPROC
 
 # DocBook/XML generated document cross-reference database
-sgmldbs_DATA = $(docbook:.xml=.html.db) $(docbook:.xml=.fo.db)
+shelf_DATA += $(docbook:.xml=.html.db) $(docbook:.xml=.fo.db)
 
 # Generate DocBook/XML document cross-reference database
 # Flags for the XSL Transformation processor generating xref target databases
@@ -94,4 +94,4 @@ endif HAVE_XSLTPROC
 endif HAVE_STYLESHEETS
 endif HAVE_XMLTO
 
-CLEANFILES = $(shelf_DATA) $(sgmldbs_DATA)
+CLEANFILES = $(shelf_DATA)
-- 
1.7.4.1

___
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


[PATCH sgml-doctools] Masterdb: read the target dbs from the usual doc directory

2011-06-06 Thread Gaetan Nadon
This is a coordinated change with the packages building DocBook/XML.
The target dbs are now installed alongside the generated documents.
Currently they are installed in $prefix/sgml/X11/dbs alongside masterdb which
has the potential of installing outside the package prefix and cause
distcheck to fail when user does not have write permision in this package.

The master db xi:include statement is changed. It uses a fully qualified
path but could also have used a relative path although it is difficult to
ensure it would work reliably. The generated html/pdf link are still relative.

The README elaborates on requirements which previously existed and were not
introduced by this patch.

Signed-off-by: Gaetan Nadon 
---
 README|7 +++
 masterdb/Makefile.am  |   12 +++--
 masterdb/masterdb.xml |  126 
 3 files changed, 78 insertions(+), 67 deletions(-)

diff --git a/README b/README
index 07e760b..e057938 100644
--- a/README
+++ b/README
@@ -64,6 +64,13 @@ configure option. For external references to work, it is 
assumed that
 the each package installs the documentation using the relative
 doc/${PACKAGE_TARNAME} location.
 
+It is also a requirement that all packages containing DocBook/XML documentation
+be installed under the same read-only architecture independent installation
+directory as evidenced by the DATAROOTDIR package value. This value should also
+be the same for the xorg-sgml-doctools package. The purpose of this requirement
+is to form a document tree of a known structure such that stylesheets,
+masterdb, target dbs and other files can be reached.
+
 The Docbook stylesheet technology will create references with paths relative
 to this location. It will navigate up to "doc" using ../ and then navigate
 down to the document it refers to.
diff --git a/masterdb/Makefile.am b/masterdb/Makefile.am
index b967d68..4b8044d 100644
--- a/masterdb/Makefile.am
+++ b/masterdb/Makefile.am
@@ -11,16 +11,20 @@ CLEANFILES = $(sgmldbs_DATA)
 
 %.html.xml: %.xml
$(AM_V_GEN)$(SED) -e 's|__ext__|html|g' \
-   -e 's|__db__|html|g' < $< > $@
+   -e 's|__db__|html|g' \
+   -e 's|@datarootdir[@]|$(datarootdir)|g' < $< > $@
 
 %.txt.xml: %.xml
$(AM_V_GEN)$(SED) -e 's|__ext__|txt|g' \
-   -e 's|__db__|html|g' < $< > $@
+   -e 's|__db__|html|g' \
+   -e 's|@datarootdir[@]|$(datarootdir)|g' < $< > $@
 
 %.pdf.xml: %.xml
$(AM_V_GEN)$(SED) -e 's|__ext__|pdf|g' \
-   -e 's|__db__|fo|g' < $< > $@
+   -e 's|__db__|fo|g' \
+   -e 's|@datarootdir[@]|$(datarootdir)|g' < $< > $@
 
 %.ps.xml: %.xml
$(AM_V_GEN)$(SED) -e 's|__ext__|ps|g' \
-   -e 's|__db__|fo|g' < $< > $@
+   -e 's|__db__|fo|g' \
+   -e 's|@datarootdir[@]|$(datarootdir)|g' < $< > $@
diff --git a/masterdb/masterdb.xml b/masterdb/masterdb.xml
index 914f756..4860103 100644
--- a/masterdb/masterdb.xml
+++ b/masterdb/masterdb.xml
@@ -38,24 +38,24 @@ listed here. This allows crosslinking between documents.
 
   
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
   
   
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
@@ -63,7 +63,7 @@ listed here. This allows crosslinking between documents.
   
 
   
-http://www.w3.org/2001/XInclude";>
+http://www.w3.org/2001/XInclude";>
   
 
   
@@ -71,21 +71,21 @@ listed here. This allows crosslinking between documents.
 
   
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
   
   
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
   
   
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
@@ -93,14 +93,14 @@ listed here. This allows crosslinking between documents.
 
 
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
 
 
 
-  http://www.w3.org/2001/XInclude";>
+  http://www.w3.org/2001/XInclude";>
 
   
 
@@ -108,74 +108,74 @@ listed here. This allows crosslinking between do

Re: [PATCH] randr: void function cannot return value

2011-06-06 Thread Keith Packard
On Mon, 6 Jun 2011 16:53:41 +0200, Nicolas Kaiser  wrote:

> Providing an argument to return in a function with void return type
> is not allowed by the C standard, and makes the Sun compilers unhappy.

Merged.
   feab043..21eec36  master -> master

-- 
keith.pack...@intel.com


pgpxXm9dVIPYG.pgp
Description: PGP signature
___
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

Re: [PATCH] randr: void function cannot return value

2011-06-06 Thread walter harms


Am 06.06.2011 16:53, schrieb Nicolas Kaiser:
> Providing an argument to return in a function with void return type
> is not allowed by the C standard, and makes the Sun compilers unhappy.
> 
> Signed-off-by: Nicolas Kaiser 
> Reviewed-by: Jeremy Huddleston 
> Reviewed-by: Alan Coopersmith 
> ---
>  randr/rrcrtc.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
> index d4d8f2a..0437795 100644
> --- a/randr/rrcrtc.c
> +++ b/randr/rrcrtc.c
> @@ -632,7 +632,7 @@ RRModeGetScanoutSize (RRModePtr mode, PictTransformPtr 
> transform,
>  void
>  RRCrtcGetScanoutSize(RRCrtcPtr crtc, int *width, int *height)
>  {
> -return RRModeGetScanoutSize (crtc->mode, &crtc->transform, width, 
> height);
> +RRModeGetScanoutSize (crtc->mode, &crtc->transform, width, height);
>  }
>  
>  /*


Is the function really helpful ? maybe it be removed altogether ?

re,
 wh
___
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


Re: [PATCH 2/4] Remove the cacheing of the last scratch PixmapRec

2011-06-06 Thread Keith Packard
On Mon, 06 Jun 2011 14:29:55 +0100, Chris Wilson  
wrote:

> The critical notification that is missing is the one from
> FreeScratchPixmapHeader(). Adding a call to screen->ModifyPixmapHeader
> and listening for that in the ddx is sufficient for us to insert the
> appropriate barriers.

That's what I figured.

> I think using CreatePixmap(usage_hint=SCRATCH_HEADER) is a better
> solution, both from the perspective of the ddx and the core server. As
> there are no released drivers that depend upon that notification yet
> (otherwise this issue would have been raised earlier), let's fix it in
> a single step, at the beginning of the next cycle.

If we can offer an API compatible solution in 1.11, that would permit
drivers to be back-ported to 1.11, which would be nice. Not critical, of
course, just means that drivers would have to support both the 1.12 and
1.11 interfaces for a bit longer.

However, it sounds like these are two slightly different solutions to
the same problem, right?

-- 
keith.pack...@intel.com


pgpWG2aKIO8KG.pgp
Description: PGP signature
___
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

Re: [PATCH 01/14] doc: Fix typo.

2011-06-06 Thread Cyril Brulebois
Jamey Sharp  (05/06/2011):
> I've commented on patches 4 and 10.

Amended locally, thanks. (D'oh @ 10…)

> Otherwise, for patches 1-12:
> 
> Reviewed-by: Jamey Sharp 

[and Alan and Daniel…]

I'll post a follow-up for 14 in a moment.

Thanks everyone.

Mraw,
KiBi.


signature.asc
Description: Digital signature
___
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

[PATCH] randr: void function cannot return value

2011-06-06 Thread Nicolas Kaiser
Providing an argument to return in a function with void return type
is not allowed by the C standard, and makes the Sun compilers unhappy.

Signed-off-by: Nicolas Kaiser 
Reviewed-by: Jeremy Huddleston 
Reviewed-by: Alan Coopersmith 
---
 randr/rrcrtc.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index d4d8f2a..0437795 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -632,7 +632,7 @@ RRModeGetScanoutSize (RRModePtr mode, PictTransformPtr 
transform,
 void
 RRCrtcGetScanoutSize(RRCrtcPtr crtc, int *width, int *height)
 {
-return RRModeGetScanoutSize (crtc->mode, &crtc->transform, width, height);
+RRModeGetScanoutSize (crtc->mode, &crtc->transform, width, height);
 }
 
 /*
-- 
1.7.5.3
___
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


[PULL] xkb case check fixes for master

2011-06-06 Thread Cyril Brulebois
Hi Keith,

the following changes since commit feab04397de2684568ded8f299cac9f44f8b:

  XQuartz: AIGLX: Remove unnecessary includes in indirect.c (2011-06-03 
16:01:18 -0400)

are available in the git repository at:
  git://people.freedesktop.org/~kibi/xserver master

Cyril Brulebois (3):
  xkb: Fix case checks for Latin 1.
  xkb: Fix case checks for Latin 2.
  xkb: Fix case checks for Latin 4.

 xkb/xkbfmisc.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

Mraw,
KiBi.


signature.asc
Description: Digital signature
___
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

Re: [PATCH 2/3] xkb: Fix case checks for Latin 2.

2011-06-06 Thread Daniel Stone
Hi,

On Mon, Jun 06, 2011 at 04:25:26PM +0200, Cyril Brulebois wrote:
> Daniel Stone  (06/06/2011):
> > On Sun, Jun 05, 2011 at 03:27:08AM +0200, Cyril Brulebois wrote:
> > > Those ones were getting _XkbKSLower for no reasons:
> > >   XK_ogonek: U+02DB OGONEK
> > >   XK_doubleacute: U+02DD DOUBLE ACUTE ACCENT
> > 
> > I think this one also needs a check to exclude XK_breve.
> 
> keysymdef.h has:
> | #define XK_Aogonek   0x01a1  /* U+0104 LATIN CAPITAL 
> LETTER A WITH OGONEK */
> | #define XK_breve 0x01a2  /* U+02D8 BREVE */
> | #define XK_Lstroke   0x01a3  /* U+0141 LATIN CAPITAL 
> LETTER L WITH STROKE */
> 
> xkbfmisc.c has:
> | case 1: /* latin 2 */
> | if (((ks>=XK_Aogonek)&&(ks<=XK_Zabovedot)&&(ks!=XK_breve))||
> | ((ks>=XK_Racute)&&(ks<=XK_Tcedilla))) {
> | rtrn|= _XkbKSUpper;
> | }
> 
> So I think it's covered already?

Shit, imagine I'm actually literate.  You're right. :)

Cheers,
Daniel
___
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


Re: [PATCH 2/3] xkb: Fix case checks for Latin 2.

2011-06-06 Thread Cyril Brulebois
Hello Daniel,

Daniel Stone  (06/06/2011):
> On Sun, Jun 05, 2011 at 03:27:08AM +0200, Cyril Brulebois wrote:
> > Those ones were getting _XkbKSLower for no reasons:
> >   XK_ogonek: U+02DB OGONEK
> >   XK_doubleacute: U+02DD DOUBLE ACUTE ACCENT
> 
> I think this one also needs a check to exclude XK_breve.

keysymdef.h has:
| #define XK_Aogonek   0x01a1  /* U+0104 LATIN CAPITAL 
LETTER A WITH OGONEK */
| #define XK_breve 0x01a2  /* U+02D8 BREVE */
| #define XK_Lstroke   0x01a3  /* U+0141 LATIN CAPITAL 
LETTER L WITH STROKE */

xkbfmisc.c has:
|   case 1: /* latin 2 */
|   if (((ks>=XK_Aogonek)&&(ks<=XK_Zabovedot)&&(ks!=XK_breve))||
|   ((ks>=XK_Racute)&&(ks<=XK_Tcedilla))) {
|   rtrn|= _XkbKSUpper;
|   }

So I think it's covered already?

> Otherwise, for all three patches:
> Reviewed-by: Daniel Stone 

Thanks for reviewing.

Mraw,
KiBi.


signature.asc
Description: Digital signature
___
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

Re: [PATCH xproto] Fix sorting by codepoint in Latin 2.

2011-06-06 Thread Cyril Brulebois
Alan Coopersmith  (04/06/2011):
> On 06/ 4/11 06:26 PM, Cyril Brulebois wrote:
> > Sort performed by calling “sort -k 3” on the part between #ifdef
> > XK_LATIN8 and #endif.
> > 
> > Signed-off-by: Cyril Brulebois 
> > ---
> >  keysymdef.h |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/keysymdef.h b/keysymdef.h
> > index 1a07d83..73b5533 100644
> > --- a/keysymdef.h
> > +++ b/keysymdef.h
> > @@ -782,9 +782,9 @@ SOFTWARE.
> >  #define XK_nacute0x01f1  /* U+0144 LATIN SMALL 
> > LETTER N WITH ACUTE */
> >  #define XK_ncaron0x01f2  /* U+0148 LATIN SMALL 
> > LETTER N WITH CARON */
> >  #define XK_odoubleacute  0x01f5  /* U+0151 LATIN SMALL 
> > LETTER O WITH DOUBLE ACUTE */
> > -#define XK_udoubleacute  0x01fb  /* U+0171 LATIN SMALL 
> > LETTER U WITH DOUBLE ACUTE */
> >  #define XK_rcaron0x01f8  /* U+0159 LATIN SMALL 
> > LETTER R WITH CARON */
> >  #define XK_uring 0x01f9  /* U+016F LATIN SMALL 
> > LETTER U WITH RING ABOVE */
> > +#define XK_udoubleacute  0x01fb  /* U+0171 LATIN SMALL 
> > LETTER U WITH DOUBLE ACUTE */
> >  #define XK_tcedilla  0x01fe  /* U+0163 LATIN SMALL 
> > LETTER T WITH CEDILLA */
> >  #define XK_abovedot  0x01ff  /* U+02D9 DOT ABOVE */
> >  #endif /* XK_LATIN2 */
> 
> Reviewed-by: Alan Coopersmith 

Thanks, pushed:
   d8920f7..951295c  master -> master

Mraw,
KiBi.


signature.asc
Description: Digital signature
___
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

Re: [PATCH xserver 01/12] Support XI 2.1

2011-06-06 Thread Daniel Stone
On Thu, Jun 02, 2011 at 05:13:35PM +1000, Peter Hutterer wrote:
> Signed-off-by: Peter Hutterer 

For the server series, modulo my comments:
Reviewed-by: Daniel Stone 

Thanks!

Cheers,
Daniel
___
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


Re: Build fixes (was Anything more for xproto 7.0.22?)

2011-06-06 Thread Gaetan Nadon
On Sun, 2011-06-05 at 14:50 -0400, Gaetan Nadon wrote:

> On Sun, 2011-06-05 at 14:11 -0400, Gaetan Nadon wrote:
> 
> > On Sun, 2011-06-05 at 18:34 +0200, Julien Cristau wrote: 
> > 
> > > On Sun, Jun  5, 2011 at 11:22:31 -0400, Gaetan Nadon wrote:
> > > 
> > > > An alternative is to install the *.db files in their respective packages
> > > > and have the xorg-sgml-doctools masterdb point to the packages. Either
> > > 
> > > xorg-sgml-doctools doesn't know where the packages are installed, and
> > > shouldn't have to know.  Please don't introduce a dependency loop here.
> > > 
> > 
> > Correct, it does not know and should not know about where other
> > packages are installed (the value of their prefix). This is not what
> > I am proposing. There is a requirement that all packages be
> > installed under the prefix fir cross-referencing to work. The
> > xorg-sgml-doctools can make an assumption based on that requirement
> > and "assume" that packages were installed under the same prefix
> > value.
> > 
> > There is no guarantee they will be installed there, but there is no
> > harm in making that assumption because if they aren't installed
> > there, it won't work anyway (cross-referencing). That characteristic
> > is unique to docs. Even if the packages are independent and can each
> > be installed under different prefix, they must be installed under
> > the same prefix for docs to work properly. When you look at things
> > from the datarootdir/doc perspective, you must see a well formed
> > tree of documents and thier target dbs as if it came from a
> > monolithic tree. 
> > 
> > I am not insisting here, just explaining what this alternative is. I
> > am not sure yet it is a good alternative, challenges are welcome.
> > 
> > 
> > /usr/share/doc/libX11/libX11/libX11.html-installed by libX11 
> > package
> > /usr/share/sgml/X11/dbs/xorg.css-installed by 
> > xorg-sgml-doctools
> > /usr/share/sgml/X11/dbs/masterdb.html.xml   -instaled 
> > byxorg-sgml-doctools 
> > 
> > /usr/share/sgml/X11/dbs/libX11.html.db  -currently 
> > installed by libX11 package in sgml dir
> > /usr/share/doc/libX11/libX11/libX11.html.db -alternative: installed 
> > by libX11 package in docdir
> > 
> > 
> > The generated html has a relative reference to the libX11.html file.
> > If the referencing html is installed under a different prefix, the
> > referenced html will not be found.
> > 
> >  > class="olink">XNextRequest
> > 
> > That's why I think it is "safe" to assume that packages are
> > installed under the same prefix, because if they aren't, it won't
> > work anyway. Of course, "safe" here means that it won't break it any
> > more than it already is.
> > 
> 
> The whole discussion about prefix assumptions is a moot point. If the
> tree isn't the way it is expected, it won't work. How the doc tree got
> out of whack is irrelevant. Trying to summarize what this alternative
> is: instead of installing the target dbs in sgml, install them where
> the docs normally are. Instead of sgml reading from sgml/X11/dbs for
> target dbs, read them from the doc directory.
> 

Just to follow-up. So far, everything works fine when installing the
target dbs in their respective doc location. There is no issue with
distcheck and write permissions as there is no attempt to install in a
foreign directory. This solution works only because of the unique nature
of the document tree. I am preparing patches for review. Note that 21
packages are affected by this change and I'll need to bump doctools
version to 1.8 as well as their configure.ac and docbook.am file. 

The nice part is that the overall complexity is being reduced rather
than being augmented by a workaround without adding any restrictions or
fragile code.


> > > Cheers,
> > > Julien
> > 
> > 
> > 
> > ___
> > 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
> 
> 
> 
> ___
> 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




signature.asc
Description: This is a digitally signed message part
___
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

Re: [PATCH xserver 05/12] dix: split client list retrieval out of DeliverEventToClients

2011-06-06 Thread Daniel Stone
Hi,

On Thu, Jun 02, 2011 at 05:13:39PM +1000, Peter Hutterer wrote:
> +static int
> +GetClientsForDelivery(DeviceIntPtr dev, WindowPtr win,
> +  xEvent *events, Mask filter, InputClients **clients)
>  {
> -int attempt;
> -enum EventDeliveryState rc = EVENT_SKIP;
> -InputClients *other;
> +int rc = 0;
>  
>  [...]
>  
> +rc = 1;
> +out:
> +return rc;
> +}
> 
> [...]
> 
> +if (!GetClientsForDelivery(dev, win, events, filter, &other))
> +goto out;

This would probably be better off as Bool than int.

Cheers,
Daniel
___
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


Re: [PATCH v2 3/12] Xi: Add support for confine_to on XI 2.1 XIPassiveGrab requests.

2011-06-06 Thread Daniel Stone
Hi,

On Fri, Jun 03, 2011 at 12:45:01PM +1000, Peter Hutterer wrote:
> diff --git a/Xi/xigrabdev.h b/Xi/xigrabdev.h
> index 08309c9..2f74788 100644
> --- a/Xi/xigrabdev.h
> +++ b/Xi/xigrabdev.h
> @@ -38,4 +38,6 @@ int SProcXIUngrabDevice(ClientPtr client);
>  
>  void SRepXIGrabDevice(ClientPtr client, int size, xXIGrabDeviceReply * rep);
>  
> +Bool HasConfineTo(ClientPtr client);
> +
>  #endif /* XIGRABDEV_H */

If this is now non-static, a less ambiguous name would be nice.

> +/**
> + * The confine_to window in XI2.1 trails the request. It is the first 4
> + * bytes after the fixed request + the mask bytes + modifier bytes.
> + * Mask and modifiers are both in 4-byte units.
> + *
> + * @return A pointer to the 4 bytes that represent the confine_to on the
> + * wire.
> + */
> +static Window*
> +get_confine_to(const xXIPassiveGrabDeviceReq *req)
> +{
> +char *data = (char*)&req[1] + req->mask_len * 4 + req->num_modifiers * 4;
> +return  (Window*)data;
> +}

Disambiguating this name would be pretty keen as well, since we now have
two get_confine_tos.  Also, there's an extraneous space in the return
statement.

Cheers,
Daniel
___
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


Re: [PATCH 2/4] Remove the cacheing of the last scratch PixmapRec

2011-06-06 Thread Chris Wilson
On Sun, 05 Jun 2011 22:48:59 -0700, Keith Packard  wrote:
> On Mon,  6 Jun 2011 06:36:07 +0100, Chris Wilson  
> wrote:
> 
> > In order for the driver to be notified of when the resource backing the
> > scratch pixmap becomes no longer accessible, it needs to be called on
> > every FreeScratchPixmapHeader(). As we instead maybe cached the
> > PixmapRec (to avoid the free and malloc overhead), this notification
> > went astray, and the driver would fail to insert the correct barriers on
> > the backing resource. That resource would then be reused by the Xserver,
> > leading to rampant memory corruption as the GPU flushed it write caches
> > at some point in the future and overwriting random structures.
> 
> This looks like a good thing to fix, and I'd like to get something into
> 1.11 if possible, but I'd rather not change this much code post RC1. Is
> there a simpler fix you can think of for 1.11 while we pend the more
> invasive change for 1.12?

The critical notification that is missing is the one from
FreeScratchPixmapHeader(). Adding a call to screen->ModifyPixmapHeader
and listening for that in the ddx is sufficient for us to insert the
appropriate barriers.

I think using CreatePixmap(usage_hint=SCRATCH_HEADER) is a better
solution, both from the perspective of the ddx and the core server. As
there are no released drivers that depend upon that notification yet
(otherwise this issue would have been raised earlier), let's fix it in
a single step, at the beginning of the next cycle.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
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


Re: [PATCH xserver 02/12] Added processing of XI grabs with confine.

2011-06-06 Thread Daniel Stone
Hi,

On Thu, Jun 02, 2011 at 05:13:36PM +1000, Peter Hutterer wrote:
> +if(HasConfineTo(client) == TRUE)
> +memcpy((unsigned char *)&confine_to, (char*)&stuff[1] + 
> stuff->mask_len * 4, 4);

The first cast here is unnecessary.

Cheers,
Daniel
___
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


Re: [PATCH libXi 1/4] Add XI2 library-internal array offsets to XIint.h

2011-06-06 Thread Daniel Stone
Hi,

On Mon, Jun 06, 2011 at 10:19:10PM +1000, Peter Hutterer wrote:
> On 6/06/11 21:56 , Daniel Stone wrote:
> >On Thu, Jun 02, 2011 at 05:13:31PM +1000, Peter Hutterer wrote:
> >>These defines are currently defined in XI.h and XI2.h. Their only use is as
> >>offset into a library-internal array. Move them to XIint.h so they may be
> >>removed from the protocol headers with the next revision.
> >
> >Hmm, I'm kind of uneasy about having new protocol headers break builds
> >with old libraries. :\
> 
> I'm pretty sure we discussed this once before and the outcome was:
> the protocol headers can't be changed, the defines need to stay
> around.

Yep, sounds familiar.

> what we can do however is _add_ them to libXi and ifdef them out
> accordingly. this also means that the ifndef will never be hit, but
> it's still easier to read this way.
> 
> so there is no matching patch for inputproto for this change, we
> keep the defines around for backwards compatibility. the new
> additions however are only in libXi.

Hmm, well in that case I guess the commit message at least needs to be
fixed, but if we're (quite sensibly) not adding any new defines going
forward, and these will never be used as they're just duplicating
existing defines, I'm not really sure I see the point? Maybe just a
comment saying 'see XI.h for previous values' or similar?

Cheers,
Daniel
___
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


Re: [PATCH libXi 1/4] Add XI2 library-internal array offsets to XIint.h

2011-06-06 Thread Peter Hutterer

On 6/06/11 21:56 , Daniel Stone wrote:

On Thu, Jun 02, 2011 at 05:13:31PM +1000, Peter Hutterer wrote:

These defines are currently defined in XI.h and XI2.h. Their only use is as
offset into a library-internal array. Move them to XIint.h so they may be
removed from the protocol headers with the next revision.


Hmm, I'm kind of uneasy about having new protocol headers break builds
with old libraries. :\


I'm pretty sure we discussed this once before and the outcome was: the 
protocol headers can't be changed, the defines need to stay around.


what we can do however is _add_ them to libXi and ifdef them out 
accordingly. this also means that the ifndef will never be hit, but it's 
still easier to read this way.


so there is no matching patch for inputproto for this change, we keep 
the defines around for backwards compatibility. the new additions 
however are only in libXi.


Cheers,
  peter
___
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


Re: [PATCH libXi 4/4] Add XIGrabButtonWithConfine and XIGrabEnterWithConfine with XI 2.1

2011-06-06 Thread Daniel Stone
On Thu, Jun 02, 2011 at 05:13:34PM +1000, Peter Hutterer wrote:
> Same as the XI 2.0 calls but they take a confine_to window.
> Both the old call and the new call send the confine_to down the wire now
> (None for XIGrabButton/XIGrabEnter).
> 
> Signed-off-by: Peter Hutterer 

Other than my comments on #1 and #2, for the libXi series:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
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


Re: [PATCH inputproto] XI 2.1: Add XI2-specific defines for grab and property requests

2011-06-06 Thread Daniel Stone
On Fri, Jun 03, 2011 at 03:26:20PM +1000, Peter Hutterer wrote:
> XI 2.0 headers forced clients to mix XI2 specific constants with defines for
> core input. Most notable here are the grab code which required GrabModeAsync
> or GrabModeSync from core, but _not_ AnyModifier (XIAnymodifier !=
> AnyModifier). This is a hard-to-debug cause for bugs.
> 
> Add defines for grab modes, grab return codes and property modes as well as
> a define for the AnyPropertyType. These defines are identical to the ones
> defined in core but stop the use of input-related defines from either core
> or XI 1.x.

Neat.  For the series:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
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


Re: [PATCH libXi 3/4] Added XIGrabDeviceWithConfine.

2011-06-06 Thread Daniel Stone
On Thu, Jun 02, 2011 at 09:59:22AM -0700, Jeremy Huddleston wrote:
> > @@ -65,10 +65,15 @@ XIGrabDevice(Display* dpy, int deviceid, Window 
> > grab_window, Time time,
> > buff = calloc(1, len * 4);
> > memcpy(buff, mask->mask, mask->mask_len);
> > 
> > +len++; /* for the confine_to */
> > +
> > SetReqLen(req, len, len);
> > -Data(dpy, buff, len * 4);
> > +Data(dpy, buff, (len - 1) * 4);
> > free(buff);
> > 
> > +/* put the confine_to window at the end */
> > +Data32(dpy, &confine_to, 4);
> > +
> > if (_XReply(dpy, (xReply *)&reply, 0, xTrue) == 0)
> > reply.status = GrabSuccess;
> 
> This feels cleaner to me:
> 
> len = req->mask_len + 1;
> buff = calloc(1, req->mask_len * 4);
> memcpy(buff, mask->mask, mask->mask_len);
> 
> SetReqLen(req, len, len);
> Data(dpy, buff, req->mask_len * 4);
> free(buff);
> 
> Data32(dpy, &confine_to, 4);

Or to really be picky:
buff = calloc(req->mask_len, 4);

Cheers,
Daniel
___
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


Re: [PATCH libXi 1/4] Add XI2 library-internal array offsets to XIint.h

2011-06-06 Thread Daniel Stone
On Thu, Jun 02, 2011 at 05:13:31PM +1000, Peter Hutterer wrote:
> These defines are currently defined in XI.h and XI2.h. Their only use is as
> offset into a library-internal array. Move them to XIint.h so they may be
> removed from the protocol headers with the next revision.

Hmm, I'm kind of uneasy about having new protocol headers break builds
with old libraries. :\

Cheers,
Daniel
___
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


Re: [PATCH inputproto 4/4] inputproto 2.1

2011-06-06 Thread Daniel Stone
Hi,

On Thu, Jun 02, 2011 at 05:13:30PM +1000, Peter Hutterer wrote:
> And revert the warnings and hoops we made the user jump through. They'll
> come back for XI 2.2.
> 
> Revert "Put a #warning and #error in to avoid unsuspecting XI 2.1 users."
> 
> This reverts commit 811a643d3eadb4baf4d21f5fef7b4f0c17318167.
> 
> Revert "Require configure flag to build this proto version."
> 
> This reverts commit 875879114d1681f79bd6bcf7d5bdf49f841da1c9
> 
> Signed-off-by: Peter Hutterer 

Other than my comment on #2, which also applies for #3, consider the
whole series:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
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


Re: [PATCH inputproto 2/4] XI2.1: Add confine_to to XIGrabDevice.

2011-06-06 Thread Daniel Stone
Hi,

On Thu, Jun 02, 2011 at 05:13:28PM +1000, Peter Hutterer wrote:
> +If the client supports XI 2.1 and confine_to is not None and the device is a
> +master pointer device, the cursor will be confined to that window.

If the device is an SD, won't its motion also be confined? As far as I
can tell, we call ConfineCursorToWindow pretty unconditionally, and it
does look like it would get clipped ...

Cheers,
Daniel
___
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


Re: [PATCH 14/14] doc: Try to mention all parameters [FIXME].

2011-06-06 Thread Daniel Stone
Hi,

On Mon, Jun 06, 2011 at 02:25:12AM +0200, Cyril Brulebois wrote:
> The FIXME in the title is intentional, a hunk needs some intel:
> a description of the 3 last parameters to the
> InitStringFeedbackClassDeviceStruct() function.
> 
> Thanks already!

max_symbols: largest number of characters that can be displayed by the
 LED (or whatever) at one time
num_symbols_supported: size of symbols (see below)
symbols: list of KeySyms the LED (or whatever) is able to display

Assuming Jamey's comment gets fixed, then for the series:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
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


Re: [PATCH 2/3] xkb: Fix case checks for Latin 2.

2011-06-06 Thread Daniel Stone
Hi,

On Sun, Jun 05, 2011 at 03:27:08AM +0200, Cyril Brulebois wrote:
> Those ones were getting _XkbKSLower for no reasons:
>   XK_ogonek: U+02DB OGONEK
>   XK_doubleacute: U+02DD DOUBLE ACUTE ACCENT

I think this one also needs a check to exclude XK_breve.  Otherwise, for
all three patches:
Reviewed-by: Daniel Stone 

Thanks!

Cheers,
Daniel
___
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


Re: [PATCH 27/27] Input: Add smooth-scrolling support to GetPointerEvents

2011-06-06 Thread Daniel Stone
Hi,

On Sun, Jun 05, 2011 at 08:49:43PM +0200, Simon Thum wrote:
> On 06/03/2011 05:00 PM, Daniel Stone wrote:
> > For scroll wheel support, we used to send buttons 4/5 and 6/7 for
> > horizontal/vertical positive/negative scroll events.  For touchpads, we
> > really want more fine-grained scroll values.  GetPointerEvents now
> > accepts both old-school scroll button presses, and new-style scroll axis
> > events, while emitting both types of events to support both old and new
> > clients.
> 
> I guess apps that want the best support across server generations are
> supposed to know they can ignore legacy scroll by virtue of the
> POINTER_EMULATED flag? If yes, shouldn't the flag be always set on
> either the axis updates or the button events?

Yeah -- there's a missing inputproto part that I'll send out with the
second round of patches.  Here's the applicable text:
 PointerEmulated means that the event has been emulated from another
 XI 2.x event for legacy client support, and that this event should
 be ignored if the client listens for these events.  This flag will
 be set on scroll ButtonPress events (buttons 4, 5, 6 and 7) if a
 smooth-scrolling event on the Rel Vert Scroll or Rel Horiz Scroll
 axis events was also generated.

You'll note that the GetPointerEvents helper does indeed always set
POINTER_EMULATED on buttons 4, 5, 6 and 7.

> I further guess that the non-raw scroll events will still be adding up,
> is that intentional?

Crap, good catch.  Guess that's what you get for playing around with
smooth scrolling and a bunch of accel changes at the same time ... I'll
probably have to fix that by adding a new axis mode, I guess (relative
but not additive).

Cheers,
Daniel
___
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


Re: [PATCH 16/27] Input: Convert acceleration code to using ValuatorMask

2011-06-06 Thread Daniel Stone
Hi,

On Sun, Jun 05, 2011 at 05:04:59PM +0200, Simon Thum wrote:
> On 06/03/2011 04:59 PM, Daniel Stone wrote:
> > Instead of passing a set of int* to the acceleration code, pass it a
> > mask instead, which avoids an unfortunate loss of precision.
> > 
> > Signed-off-by: Daniel Stone 
> > ---
> >  dix/getevents.c |   41 +---
> >  dix/ptrveloc.c  |   82 
> > ---
> >  2 files changed, 49 insertions(+), 74 deletions(-)
> > 
> > diff --git a/dix/getevents.c b/dix/getevents.c
> > index b796000..2f8e137 100644
> > --- a/dix/getevents.c
> > +++ b/dix/getevents.c
> > @@ -727,17 +727,19 @@ moveAbsolute(DeviceIntPtr dev, int *x_out, int 
> > *y_out, ValuatorMask *mask)
> >  
> > [...]
> >  }
> 
> 
> The above seems to belong to another patch.

Whoops, good catch.

> Otherwise, consider this
> 
> Reviewed-by: Simon Thum 

Thanks!

Cheers,
Daniel
___
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