Re: [PULL] unreviewed patches

2013-05-07 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 moved there. I assumed your rev-by, I don't want to do another revision. let
 me know if it's not to your liking.

Reviewed-by: Keith Packard kei...@keithp.com

 yes, but tbh I do like the idea of extensions cleaning up after themselves
 instead of leaving it to some other piece of code. I can drop the patch
 though, I don't care either way tbh.

Thinking more about it this morning, I do believe it would be better to
not have the extensions duplicate the cleanup work. I think the
duplication makes the API for extensions less clear as I suspect we'll
probably end up with extensions doing it both ways, which is never good
when people are reading through the code.

But, I don't care that deeply about the issue, and the code is correct
in either case -- my comment was more about clarifying the interaction
between the two patches than an actual complaint.

So send whichever version you prefer along, both patches are

Reviewed-by: Keith Packard kei...@keithp.com

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


pgpa8UPJILmXr.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: [PULL] unreviewed patches

2013-05-06 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

   Xi: fix comment - XI2 grabs aren't keysym grabs

Reviewed-by: Keith Packard kei...@keithp.com

   dix: send the current axis value in DeviceChangedEvents (#62321)

Reviewed-by: Keith Packard kei...@keithp.com

   Xi: always return BadMatch for XTest devices ChangeDeviceControl 
 requests

(this appears to cover all of the DDX entry points to avoid passing
XTest devices to them)

Reviewed-by: Keith Packard kei...@keithp.com

   If neither HAL nor udev backends are enabled, warn the user

Reviewed-by: Keith Packard kei...@keithp.com

   xkb: free XkbRulesUsed and XkbRulesDflt on extension cleanup

I think this should be done in CloseDownDevices -- XkbDeleteRulesDflts
is already called there, adding a call to your new XkbDeleteRulesUsed
(which looks fine, btw) seems like the right plan.

   Delete callbacks when extension are closed
   dix: delete all callbacks before reset

Looks like the second patch makes the first unnecessary?

   dix: reset the registry before quitting

Reviewed-by: Keith Packard kei...@keithp.com

   os: support pnprintf length modifiers for integers

Reviewed-by: Keith Packard kei...@keithp.com

   os: support %% in pnprintf

Reviewed-by: Keith Packard kei...@keithp.com

   os: support %c in pnprintf

Reviewed-by: Keith Packard kei...@keithp.com

   os: complain about unsupported pnprintf directives

Reviewed-by: Keith Packard kei...@keithp.com

   os: Use ErrorFSigSafe from FatalError and it's friends

Reviewed-by: Keith Packard kei...@keithp.com

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


pgpVlFS_pnwl0.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: [PULL] unreviewed patches

2013-05-06 Thread Peter Hutterer
On Mon, May 06, 2013 at 11:26:10AM -0700, Keith Packard wrote:
 Peter Hutterer peter.hutte...@who-t.net writes:
 
Xi: fix comment - XI2 grabs aren't keysym grabs
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
dix: send the current axis value in DeviceChangedEvents (#62321)
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
Xi: always return BadMatch for XTest devices ChangeDeviceControl 
  requests
 
 (this appears to cover all of the DDX entry points to avoid passing
 XTest devices to them)
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
If neither HAL nor udev backends are enabled, warn the user
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
xkb: free XkbRulesUsed and XkbRulesDflt on extension cleanup
 
 I think this should be done in CloseDownDevices -- XkbDeleteRulesDflts
 is already called there, adding a call to your new XkbDeleteRulesUsed
 (which looks fine, btw) seems like the right plan.

moved there. I assumed your rev-by, I don't want to do another revision. let
me know if it's not to your liking.

 
Delete callbacks when extension are closed
dix: delete all callbacks before reset
 
 Looks like the second patch makes the first unnecessary?

yes, but tbh I do like the idea of extensions cleaning up after themselves
instead of leaving it to some other piece of code. I can drop the patch
though, I don't care either way tbh.

Cheers,
   Peter

dix: reset the registry before quitting
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
os: support pnprintf length modifiers for integers
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
os: support %% in pnprintf
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
os: support %c in pnprintf
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
os: complain about unsupported pnprintf directives
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
os: Use ErrorFSigSafe from FatalError and it's friends
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
 -- 
 keith.pack...@intel.com


___
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] unreviewed patches

2013-05-01 Thread Peter Hutterer
Last patch has only been on the list for 2 days, the others between a few
weeks and over a year.

The following changes since commit 7ab98bafc9a3426fd40f8ae693430491333ba4fc:

  Merge remote-tracking branch 'geertu/master' (2013-04-24 14:14:45 -0700)

are available in the git repository at:


  git://people.freedesktop.org/~whot/xserver unreviewed

for you to fetch changes up to 7b8aeecb1138b3a1faa2a1a68d55cb67f1c3f352:

  os: Use ErrorFSigSafe from FatalError and it's friends (2013-05-02 14:14:17 
+1000)


Peter Hutterer (13):
  Xi: fix comment - XI2 grabs aren't keysym grabs
  dix: send the current axis value in DeviceChangedEvents (#62321)
  Xi: always return BadMatch for XTest devices ChangeDeviceControl requests
  If neither HAL nor udev backends are enabled, warn the user
  xkb: free XkbRulesUsed and XkbRulesDflt on extension cleanup
  Delete callbacks when extension are closed
  dix: delete all callbacks before reset
  dix: reset the registry before quitting
  os: support pnprintf length modifiers for integers
  os: support %% in pnprintf
  os: support %c in pnprintf
  os: complain about unsupported pnprintf directives
  os: Use ErrorFSigSafe from FatalError and it's friends

 Xi/chgdctl.c  |   6 +++
 Xi/exevents.c |   3 +-
 Xi/extinit.c  |   2 +
 Xi/setdval.c  |   3 ++
 Xi/setmode.c  |   4 ++
 composite/compext.c   |   8 +++-
 configure.ac  |   8 
 dix/dixutils.c|   8 +++-
 dix/eventconvert.c|   4 +-
 dix/getevents.c   |   1 +
 dix/main.c|   4 ++
 dix/registry.c|  25 
 glx/glxdri.c  |   2 +-
 glx/glxdri2.c |   2 +-
 glx/glxext.c  |  18 
 include/callback.h|   1 +
 include/eventstr.h|   1 +
 include/registry.h|   1 +
 include/xkbsrv.h  |   5 ++-
 os/log.c  | 111 ++
 randr/randr.c |   7 +++-
 render/render.c   |   8 +++-
 test/signal-logging.c |  95 ++
 xkb/xkb.c |   9 +++-
 xkb/xkbInit.c |  15 +++
 25 files changed, 303 insertions(+), 48 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: [PULL] unreviewed patches Jan 2013

2013-02-07 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 Peter Hutterer (14):
   test/signal-logging: simplify tests using sprintf

Reviewed-by: Keith Packard kei...@keithp.com

   os: silently ignore length modifiers in pnprintf

Reviewed-by: Keith Packard kei...@keithp.com

   os: add support for %f to pnprintf

(this could use some improvements to clean up rounding, but meh)

Reviewed-by: Keith Packard kei...@keithp.com

   dix: fix ptraccel debugging printfs

There are some gratuitous format width changes in this patch which
may not give the same result with a 'real' printf. I'd revert those
given that you ignore length modifiers entirely at this point.

   dix: add some more info to a ptraccel debug msg

Reviewed-by: Keith Packard kei...@keithp.com

   dix: use BUG_RETURN_VAL for an error message

Reviewed-by: Keith Packard kei...@keithp.com

   dix: unify prefix for ptraccel debugging in DebugAccelF macro

Reviewed-by: Keith Packard kei...@keithp.com

   dix: pre-scale relative events from abs devices to desktop ratio 
 (#31636)

So many coordinate systems, so little time.

Reviewed-by: Keith Packard kei...@keithp.com

   Xext: pass the current screen to miProcessDeviceEvent() from xtest calls

Reviewed-by: Keith Packard kei...@keithp.com

   Xext: if a root window is given in XTestFakeInput, move to that

The comment before the call to scale_from_screen needs updating to say
/* valuators are in screen or desktop coords */

otherwise,

Reviewed-by: Keith Packard kei...@keithp.com

   dmx: don't include dmx-config.h from xdmxconfig (#37502)

Reviewed-by: Keith Packard kei...@keithp.com

   dix: when shutting down slave devices, shut down xtest devices last

Would be lovely to have comments before each loop labeling what devices
they expect to shut down. Otherwise:

Reviewed-by: Keith Packard kei...@keithp.com

   dix: support the transformation matrix for relative devices.

I suggest leaving the current transform called 'transform' and creating
a new transform called 'relative_transform' or some such that has the
correct values in it (i.e., with the translation components set to zero
so you don't have to copy each time you use it). That will preserve API
at least.

Also, this needs to bump the ABI version number.

   Merge branch 'ptraccel-fixes' into unreviewed

  Xext/xtest.c   |  12 ++-
  dix/devices.c  |   8 +-
  dix/getevents.c|  72 ++--
  dix/ptrveloc.c |  40 -
  hw/dmx/config/xdmxconfig.c |   3 -
  include/input.h|   1 +
  include/inputstr.h |   5 +-
  include/misc.h |   1 +
  os/log.c   |  19 -
  os/utils.c |  32 
  test/signal-logging.c  | 200 
 ++---
  11 files changed, 253 insertions(+), 140 deletions(-)

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


pgp_1Ewwi4QIX.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: [PULL] unreviewed patches Jan 2013

2013-02-07 Thread Peter Hutterer
On Thu, Feb 07, 2013 at 04:53:55PM -0800, Keith Packard wrote:
 Peter Hutterer peter.hutte...@who-t.net writes:
 
  Peter Hutterer (14):
test/signal-logging: simplify tests using sprintf
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
os: silently ignore length modifiers in pnprintf
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
os: add support for %f to pnprintf
 
 (this could use some improvements to clean up rounding, but meh)
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
dix: fix ptraccel debugging printfs
 
 There are some gratuitous format width changes in this patch which
 may not give the same result with a 'real' printf. I'd revert those
 given that you ignore length modifiers entirely at this point.

done, see patch.

 
dix: add some more info to a ptraccel debug msg
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
dix: use BUG_RETURN_VAL for an error message
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
dix: unify prefix for ptraccel debugging in DebugAccelF macro
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
dix: pre-scale relative events from abs devices to desktop ratio 
  (#31636)
 
 So many coordinate systems, so little time.
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
Xext: pass the current screen to miProcessDeviceEvent() from xtest 
  calls
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
Xext: if a root window is given in XTestFakeInput, move to that
 
 The comment before the call to scale_from_screen needs updating to say
 /* valuators are in screen or desktop coords */

done.

 
 otherwise,
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
dmx: don't include dmx-config.h from xdmxconfig (#37502)
 
 Reviewed-by: Keith Packard kei...@keithp.com
 
dix: when shutting down slave devices, shut down xtest devices last
 
 Would be lovely to have comments before each loop labeling what devices
 they expect to shut down. Otherwise:

done

 Reviewed-by: Keith Packard kei...@keithp.com
 
dix: support the transformation matrix for relative devices.
 
 I suggest leaving the current transform called 'transform' and creating
 a new transform called 'relative_transform' or some such that has the
 correct values in it (i.e., with the translation components set to zero
 so you don't have to copy each time you use it). That will preserve API
 at least.

fwiw, this isn't API, it's only used internally. did half of what you
suggested, rel matrix is stored without zero mangling now, but abs axis is
renamed, both for obviousness and the compiler telling me where I forgot the
update.

 Also, this needs to bump the ABI version number.

While we're there, how about:
http://patchwork.freedesktop.org/patch/12960/

because if that goes in too, I can skip the minor bump.

Cheers,
   Peter

 
Merge branch 'ptraccel-fixes' into unreviewed
 
   Xext/xtest.c   |  12 ++-
   dix/devices.c  |   8 +-
   dix/getevents.c|  72 ++--
   dix/ptrveloc.c |  40 -
   hw/dmx/config/xdmxconfig.c |   3 -
   include/input.h|   1 +
   include/inputstr.h |   5 +-
   include/misc.h |   1 +
   os/log.c   |  19 -
   os/utils.c |  32 
   test/signal-logging.c  | 200 
  ++---
   11 files changed, 253 insertions(+), 140 deletions(-)
 
 -- 
 keith.pack...@intel.com


___
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: [PULL] unreviewed patches Jan 2013

2013-02-07 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 fwiw, this isn't API, it's only used internally. did half of what you
 suggested, rel matrix is stored without zero mangling now, but abs axis is
 renamed, both for obviousness and the compiler telling me where I forgot the
 update.

Better than my idea anyways. The rest of the patch looked good, so I've
already send my Rb line. Thanks for the quick turn-around; I'll finish
up the RC tomorrow and plan on closing the non-critical bug window then.

 Also, this needs to bump the ABI version number.

 While we're there, how about:
 http://patchwork.freedesktop.org/patch/12960/

 because if that goes in too, I can skip the minor bump.

Yeah, I like that patch. Let's add it for 1.14. The less code we call at
AbortServer time, the better.

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


pgp2d4JfqXC6M.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

[PULL] unreviewed patches Jan 2013

2013-01-31 Thread Peter Hutterer
None of these got some review lovin'.

A bunch of general fixes, one new feature, supporting the transformation
matrix for relative devices. And sigsafe logging now supports %f.

The following changes since commit 7fe5e6dfa5c1e71d8b7540b28c1d508687a2fbee:

  protocol-versions: Bump minor version of XI (2013-01-23 19:05:31 -0800)

are available in the git repository at:

  git://people.freedesktop.org/~whot/xserver unreviewed

for you to fetch changes up to a47f207ca8cd467169fef5e7f44541c470f2295d:

  Merge branch 'ptraccel-fixes' into unreviewed (2013-02-01 10:13:53 +1000)



Peter Hutterer (14):
  test/signal-logging: simplify tests using sprintf
  os: silently ignore length modifiers in pnprintf
  os: add support for %f to pnprintf
  dix: fix ptraccel debugging printfs
  dix: add some more info to a ptraccel debug msg
  dix: use BUG_RETURN_VAL for an error message
  dix: unify prefix for ptraccel debugging in DebugAccelF macro
  dix: pre-scale relative events from abs devices to desktop ratio (#31636)
  Xext: pass the current screen to miProcessDeviceEvent() from xtest calls
  Xext: if a root window is given in XTestFakeInput, move to that
  dmx: don't include dmx-config.h from xdmxconfig (#37502)
  dix: when shutting down slave devices, shut down xtest devices last
  dix: support the transformation matrix for relative devices.
  Merge branch 'ptraccel-fixes' into unreviewed

 Xext/xtest.c   |  12 ++-
 dix/devices.c  |   8 +-
 dix/getevents.c|  72 ++--
 dix/ptrveloc.c |  40 -
 hw/dmx/config/xdmxconfig.c |   3 -
 include/input.h|   1 +
 include/inputstr.h |   5 +-
 include/misc.h |   1 +
 os/log.c   |  19 -
 os/utils.c |  32 
 test/signal-logging.c  | 200 ++---
 11 files changed, 253 insertions(+), 140 deletions(-)


pgpOCfKGVptY2.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: [PULL] unreviewed patches

2012-10-26 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 All these need review lovin'. 

 Pull request is on top of my for-keith branch
 from last Friday.
 http://lists.freedesktop.org/archives/xorg-devel/2012-October/034064.html

 The following changes since commit c5396ec05a5c6cab6608ba677f703c5227b1de13:

   xf86: Fix build against recent Linux kernel (2012-10-19 13:12:33 +1000)

 are available in the git repository at:

   git://people.freedesktop.org/~whot/xserver unreviewed

 for you to fetch changes up to c97467d449c30da7529bdcc68c3ed344828a6baa:

   dix: don't filter RawEvents if the grab window is not the root window 
 (#53897) (2012-10-25 15:54:02 +1000)

 
 Peter Hutterer (7):
   dix: don't allow disabling XTest devices

I'd say the error values for the two code paths should be the same. I'd
suggest that BadMatch might be a slightly better value, but I'm easy if
you prefer BadAccess.

Also, the comment in DeviceSetProperty should be updated to reflect that
the xtest devices are also not subject to disabling.

I did verify that these are the only relevant code paths.

Mostly-happy-with: Keith Packard kei...@keithp.com

   xkb: ProcesssPointerEvent must work on the VCP if it gets the VCP

Yeah, this looked right when I looked at it a while ago.

Reviewed-by: Keith Packard kei...@keithp.com

   Xi: set xChangeDeviceControlReply.status to Success by default

Reviewed-by: Keith Packard kei...@keithp.com

   Xi: don't deliver TouchEnd to a client waiting for TouchBegin (#55738)

Following the state changes in exevents.c was entertaining, but there
doesn't seem to be any way to get past LISTENER_AWAITING_BEGIN without
delivering a TouchBegin event. Therefore, this simple patch seems
correct to me.

Reviewed-by: Keith Packard kei...@keithp.com

   dix: fix zaphod screen scrossing (#54654)

Reviewed-by: Keith Packard kei...@keithp.com

   xfree86: remove unused variable sigstate

Reviewed-by: Keith Packard kei...@keithp.com

   dix: don't filter RawEvents if the grab window is not the root window 
 (#53897)

I'd have to say that 'FilterRawEvents' is a pretty miserable function;
the semantics of its return value are muddled due to how it gets
invoked, in particular, it knows that an event may have been delivered
through the grab and that it must, therefore, suppress an extra event in
that case. Ick.

I have the feeling that it should be using the return value from
DeliverGrabbedEvent to help decide whether to deliver another event,
something like:

if (deliveries  SameClient(grab, client))
don't deliver another event

Then, the question is 2.0 vs 2.1; for the 2.0 case a grab always
prevents delivery

else if (2.0  grab)
don't deliver an event

else
deliver an event

Just sticking these tests right inside DeliverRawEvent would keep all of
the conditions together, instead of having some split out to
FilterRawEvent. Keeping the Xi version check in a separate function
makes good sense though; that's really the only complexity in
FilterRawEvents at present.

(I'm hoping I got the semantics of raw events right; the protocol spec
seems a lot less ambiguous than the code does...)

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


pgp8CjGWHhpJ3.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

[PULL] unreviewed patches

2012-10-25 Thread Peter Hutterer
All these need review lovin'. 

Pull request is on top of my for-keith branch
from last Friday.
http://lists.freedesktop.org/archives/xorg-devel/2012-October/034064.html

The following changes since commit c5396ec05a5c6cab6608ba677f703c5227b1de13:

  xf86: Fix build against recent Linux kernel (2012-10-19 13:12:33 +1000)

are available in the git repository at:

  git://people.freedesktop.org/~whot/xserver unreviewed

for you to fetch changes up to c97467d449c30da7529bdcc68c3ed344828a6baa:

  dix: don't filter RawEvents if the grab window is not the root window 
(#53897) (2012-10-25 15:54:02 +1000)


Peter Hutterer (7):
  dix: don't allow disabling XTest devices
  xkb: ProcesssPointerEvent must work on the VCP if it gets the VCP
  Xi: set xChangeDeviceControlReply.status to Success by default
  Xi: don't deliver TouchEnd to a client waiting for TouchBegin (#55738)
  dix: fix zaphod screen scrossing (#54654)
  xfree86: remove unused variable sigstate
  dix: don't filter RawEvents if the grab window is not the root window 
(#53897)

 Xi/chgdctl.c   | 8 ++--
 Xi/exevents.c  | 5 +
 dix/devices.c  | 6 --
 dix/events.c   | 9 ++---
 dix/getevents.c| 5 +++--
 hw/xfree86/common/xf86Events.c | 2 +-
 xkb/xkbAccessX.c   | 2 +-
 7 files changed, 26 insertions(+), 11 deletions(-)


pgpNrzp8QFMEX.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