Re: [PATCH libinput 3/3] Change the logging system to be per-context

2014-06-22 Thread Peter Hutterer 
On Wed, Jun 18, 2014 at 10:07:12AM +0200, Jonas Ådahl wrote:
 On Wed, Jun 18, 2014 at 05:52:04PM +1000, Peter Hutterer wrote:
  On Wed, Jun 18, 2014 at 09:24:03AM +0200, Jonas Ådahl wrote:
 The purpose of the struct was to provide an interface with the
 functionality that libinput would require to have to function without
 having to be root, and it doesn't feel logging function fits this
 purpose. It was already a set/get, wouldn't it fit better to just make
 them per context, while keeping the interface struct minimal?

yeah, fair enough. I arrived at this solution at a bit of a roundabout 
way
since I wanted to make the current udev_create work without changes and 
then
failed anyway. with the create_context function it's possible now to do 
the
following:

li = libinput_udev_create_context();
libinput_set_log_handler(li, ...);
libinput_set_log_priority(li, ...);
libinput_udev_set_set(li, seat);

That's what you're proposing, right?
   
   Yes, something like that. We could also possibly do what I think we
   discussed a while ago and default to suspended, i.e. _create_for_seat(),
   _set_(), _enable(), but either way really. The problem with _set_seat()
   though I guess is that it makes it look like we can actually switch seat
   of a context, which I don't know if we should support.
  
  well, we could do it as add/remove like in the path backend, I just don't
  know if there's a use-case for that?
 
 Not sure. A user could just create multiple contexts if libinput only
 would can handle one physical seat per context.
 
  
  for the current code I'll just add a check, right now it just overwrites it
  which is a bug.
  
  other name suggestions: assign_seat() or bind_seat()
 
 
 Any of these two would be best IMHO. If we'd want to support multiple
 physical backends some day, we could just add the unbind/unassign
 function, but for now, lets just support one. Maybe just fail if it's
 called twice instead of overwrite?

for the archives, i've done a s/set_seat/assign_seat/ on these functions,
and added the code to fail if the function is called once a seat has been
assigned.

Cheers,
   Peter

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH libinput 3/3] Change the logging system to be per-context

2014-06-18 Thread Jonas Ådahl 
On Wed, Jun 18, 2014 at 08:39:01AM +1000, Peter Hutterer wrote:
 On Tue, Jun 17, 2014 at 11:20:00PM +0200, Jonas Ådahl wrote:
  On Fri, Jun 13, 2014 at 12:48:33PM +1000, Peter Hutterer wrote:
   Rather than a single global logging function, make the logging dependent 
   on
   the individual context. This way we won't stomp on each other's feet in 
   the
   (admittedly unusual) case of having multiple libinput contexts.
   
   The log handler and the log priority is now a part of the libinput 
   interface.
   We can drop the various setters and getters, the caller owns the struct 
   anyway
   so we don't need functions to give it those values.
   
   The userdata argument to the log handler was dropped. The caller has a 
   ref to
   the libinput context now, any userdata can be attached to that context
   instead.
   
   There is no need for a default log function anymore. Any serious caller 
   should
   hook into it anyway, those that don't care can just use NULL.
   
   There is no default log priority anymore, a caller must set the desired
   priority in the interface.
   
   Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
   ---
   There's a side-effect to this that I'm not sure is intended. We don't copy
   the interface into libinput, we merely keep a reference. The caller is
   already able to change open_restricted/close_restricted at runtime, though
   we can't do this ourselves (it's const).
   
   Given that, I figured we can leave the log handler and priority up to the
   caller as well then, switching at runtime. That's the main reason for
   dropping the set/get priority calls. If that side effect wasn't intended,
   then we'll have rework a few things. Jonas?
  
  Not sure I like this change. The interface (function pointer struct) is
  intended to really be an constant interface where the caller never
  changes the function used. Of course it would be possible, given how its
  implemnted, but it was not intended.
 
 so should we copy the struct then? or trust callers to not do anything
 untoward?
 
  The purpose of the struct was to provide an interface with the
  functionality that libinput would require to have to function without
  having to be root, and it doesn't feel logging function fits this
  purpose. It was already a set/get, wouldn't it fit better to just make
  them per context, while keeping the interface struct minimal?
 
 yeah, fair enough. I arrived at this solution at a bit of a roundabout way
 since I wanted to make the current udev_create work without changes and then
 failed anyway. with the create_context function it's possible now to do the
 following:
 
 li = libinput_udev_create_context();
 libinput_set_log_handler(li, ...);
 libinput_set_log_priority(li, ...);
 libinput_udev_set_set(li, seat);
 
 That's what you're proposing, right?

Yes, something like that. We could also possibly do what I think we
discussed a while ago and default to suspended, i.e. _create_for_seat(),
_set_(), _enable(), but either way really. The problem with _set_seat()
though I guess is that it makes it look like we can actually switch seat
of a context, which I don't know if we should support.

Jonas

 
 Cheers,
Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH libinput 3/3] Change the logging system to be per-context

2014-06-18 Thread Peter Hutterer 
On Wed, Jun 18, 2014 at 09:24:03AM +0200, Jonas Ådahl wrote:
   The purpose of the struct was to provide an interface with the
   functionality that libinput would require to have to function without
   having to be root, and it doesn't feel logging function fits this
   purpose. It was already a set/get, wouldn't it fit better to just make
   them per context, while keeping the interface struct minimal?
  
  yeah, fair enough. I arrived at this solution at a bit of a roundabout way
  since I wanted to make the current udev_create work without changes and then
  failed anyway. with the create_context function it's possible now to do the
  following:
  
  li = libinput_udev_create_context();
  libinput_set_log_handler(li, ...);
  libinput_set_log_priority(li, ...);
  libinput_udev_set_set(li, seat);
  
  That's what you're proposing, right?
 
 Yes, something like that. We could also possibly do what I think we
 discussed a while ago and default to suspended, i.e. _create_for_seat(),
 _set_(), _enable(), but either way really. The problem with _set_seat()
 though I guess is that it makes it look like we can actually switch seat
 of a context, which I don't know if we should support.

well, we could do it as add/remove like in the path backend, I just don't
know if there's a use-case for that?

for the current code I'll just add a check, right now it just overwrites it
which is a bug.

other name suggestions: assign_seat() or bind_seat()

Cheers,
   Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH libinput 3/3] Change the logging system to be per-context

2014-06-17 Thread Jonas Ådahl 
On Fri, Jun 13, 2014 at 12:48:33PM +1000, Peter Hutterer wrote:
 Rather than a single global logging function, make the logging dependent on
 the individual context. This way we won't stomp on each other's feet in the
 (admittedly unusual) case of having multiple libinput contexts.
 
 The log handler and the log priority is now a part of the libinput interface.
 We can drop the various setters and getters, the caller owns the struct anyway
 so we don't need functions to give it those values.
 
 The userdata argument to the log handler was dropped. The caller has a ref to
 the libinput context now, any userdata can be attached to that context
 instead.
 
 There is no need for a default log function anymore. Any serious caller should
 hook into it anyway, those that don't care can just use NULL.
 
 There is no default log priority anymore, a caller must set the desired
 priority in the interface.
 
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
 There's a side-effect to this that I'm not sure is intended. We don't copy
 the interface into libinput, we merely keep a reference. The caller is
 already able to change open_restricted/close_restricted at runtime, though
 we can't do this ourselves (it's const).
 
 Given that, I figured we can leave the log handler and priority up to the
 caller as well then, switching at runtime. That's the main reason for
 dropping the set/get priority calls. If that side effect wasn't intended,
 then we'll have rework a few things. Jonas?

Not sure I like this change. The interface (function pointer struct) is
intended to really be an constant interface where the caller never
changes the function used. Of course it would be possible, given how its
implemnted, but it was not intended.

The purpose of the struct was to provide an interface with the
functionality that libinput would require to have to function without
having to be root, and it doesn't feel logging function fits this
purpose. It was already a set/get, wouldn't it fit better to just make
them per context, while keeping the interface struct minimal?

Jonas


 
  src/evdev-mt-touchpad-buttons.c |  15 +++--
  src/evdev-mt-touchpad-tap.c |  13 -
  src/evdev.c |  23 +---
  src/libinput-private.h  |  20 ---
  src/libinput.c  |  73 ++-
  src/libinput.h  |  99 +--
  src/path.c  |  20 +--
  src/timer.c |   4 +-
  src/udev-seat.c |  17 +++---
  test/litest.c   |   9 +--
  test/log.c  | 126 
 ++--
  test/misc.c |   2 +
  test/path.c |   2 +
  test/udev.c |   7 ++-
  tools/event-debug.c |  28 -
  15 files changed, 194 insertions(+), 264 deletions(-)
 
 diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
 index ce48ed0..b86f344 100644
 --- a/src/evdev-mt-touchpad-buttons.c
 +++ b/src/evdev-mt-touchpad-buttons.c
 @@ -452,6 +452,7 @@ tp_button_handle_event(struct tp_dispatch *tp,
  enum button_event event,
  uint64_t time)
  {
 + struct libinput *libinput = tp-device-base.seat-libinput;
   enum button_state current = t-button.state;
  
   switch(t-button.state) {
 @@ -485,7 +486,8 @@ tp_button_handle_event(struct tp_dispatch *tp,
   }
  
   if (current != t-button.state)
 - log_debug(button state: from %s, event %s to %s\n,
 + log_debug(libinput,
 +   button state: from %s, event %s to %s\n,
 button_state_to_str(current),
 button_event_to_str(event),
 button_state_to_str(t-button.state));
 @@ -538,11 +540,13 @@ tp_process_button(struct tp_dispatch *tp,
 const struct input_event *e,
 uint64_t time)
  {
 + struct libinput *libinput = tp-device-base.seat-libinput;
   uint32_t mask = 1  (e-code - BTN_LEFT);
  
   /* Ignore other buttons on clickpads */
   if (tp-buttons.is_clickpad  e-code != BTN_LEFT) {
 - log_bug_kernel(received %s button event on a clickpad\n,
 + log_bug_kernel(libinput,
 +received %s button event on a clickpad\n,
  libevdev_event_code_get_name(EV_KEY, e-code));
   return 0;
   }
 @@ -562,6 +566,7 @@ int
  tp_init_buttons(struct tp_dispatch *tp,
   struct evdev_device *device)
  {
 + struct libinput *libinput = tp-device-base.seat-libinput;
   struct tp_touch *t;
   int width, height;
   double diagonal;
 @@ -574,10 +579,12 @@ tp_init_buttons(struct tp_dispatch *tp,
   if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) ||
   libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT))

Re: [PATCH libinput 3/3] Change the logging system to be per-context

2014-06-17 Thread Peter Hutterer 
On Tue, Jun 17, 2014 at 11:20:00PM +0200, Jonas Ådahl wrote:
 On Fri, Jun 13, 2014 at 12:48:33PM +1000, Peter Hutterer wrote:
  Rather than a single global logging function, make the logging dependent on
  the individual context. This way we won't stomp on each other's feet in the
  (admittedly unusual) case of having multiple libinput contexts.
  
  The log handler and the log priority is now a part of the libinput 
  interface.
  We can drop the various setters and getters, the caller owns the struct 
  anyway
  so we don't need functions to give it those values.
  
  The userdata argument to the log handler was dropped. The caller has a ref 
  to
  the libinput context now, any userdata can be attached to that context
  instead.
  
  There is no need for a default log function anymore. Any serious caller 
  should
  hook into it anyway, those that don't care can just use NULL.
  
  There is no default log priority anymore, a caller must set the desired
  priority in the interface.
  
  Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
  ---
  There's a side-effect to this that I'm not sure is intended. We don't copy
  the interface into libinput, we merely keep a reference. The caller is
  already able to change open_restricted/close_restricted at runtime, though
  we can't do this ourselves (it's const).
  
  Given that, I figured we can leave the log handler and priority up to the
  caller as well then, switching at runtime. That's the main reason for
  dropping the set/get priority calls. If that side effect wasn't intended,
  then we'll have rework a few things. Jonas?
 
 Not sure I like this change. The interface (function pointer struct) is
 intended to really be an constant interface where the caller never
 changes the function used. Of course it would be possible, given how its
 implemnted, but it was not intended.

so should we copy the struct then? or trust callers to not do anything
untoward?

 The purpose of the struct was to provide an interface with the
 functionality that libinput would require to have to function without
 having to be root, and it doesn't feel logging function fits this
 purpose. It was already a set/get, wouldn't it fit better to just make
 them per context, while keeping the interface struct minimal?

yeah, fair enough. I arrived at this solution at a bit of a roundabout way
since I wanted to make the current udev_create work without changes and then
failed anyway. with the create_context function it's possible now to do the
following:

li = libinput_udev_create_context();
libinput_set_log_handler(li, ...);
libinput_set_log_priority(li, ...);
libinput_udev_set_set(li, seat);

That's what you're proposing, right?

Cheers,
   Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH libinput 3/3] Change the logging system to be per-context

2014-06-16 Thread Hans de Goede 
Hi,

On 06/13/2014 04:48 AM, Peter Hutterer wrote:
 Rather than a single global logging function, make the logging dependent on
 the individual context. This way we won't stomp on each other's feet in the
 (admittedly unusual) case of having multiple libinput contexts.
 
 The log handler and the log priority is now a part of the libinput interface.
 We can drop the various setters and getters, the caller owns the struct anyway
 so we don't need functions to give it those values.
 
 The userdata argument to the log handler was dropped. The caller has a ref to
 the libinput context now, any userdata can be attached to that context
 instead.
 
 There is no need for a default log function anymore. Any serious caller should
 hook into it anyway, those that don't care can just use NULL.
 
 There is no default log priority anymore, a caller must set the desired
 priority in the interface.
 
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
 There's a side-effect to this that I'm not sure is intended. We don't copy
 the interface into libinput, we merely keep a reference. The caller is
 already able to change open_restricted/close_restricted at runtime, though
 we can't do this ourselves (it's const).
 
 Given that, I figured we can leave the log handler and priority up to the
 caller as well then, switching at runtime. That's the main reason for
 dropping the set/get priority calls. If that side effect wasn't intended,
 then we'll have rework a few things. Jonas?
 
  src/evdev-mt-touchpad-buttons.c |  15 +++--
  src/evdev-mt-touchpad-tap.c |  13 -
  src/evdev.c |  23 +---
  src/libinput-private.h  |  20 ---
  src/libinput.c  |  73 ++-
  src/libinput.h  |  99 +--
  src/path.c  |  20 +--
  src/timer.c |   4 +-
  src/udev-seat.c |  17 +++---
  test/litest.c   |   9 +--
  test/log.c  | 126 
 ++--
  test/misc.c |   2 +
  test/path.c |   2 +
  test/udev.c |   7 ++-
  tools/event-debug.c |  28 -
  15 files changed, 194 insertions(+), 264 deletions(-)
 
 diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
 index ce48ed0..b86f344 100644
 --- a/src/evdev-mt-touchpad-buttons.c
 +++ b/src/evdev-mt-touchpad-buttons.c
 @@ -452,6 +452,7 @@ tp_button_handle_event(struct tp_dispatch *tp,
  enum button_event event,
  uint64_t time)
  {
 + struct libinput *libinput = tp-device-base.seat-libinput;
   enum button_state current = t-button.state;
  
   switch(t-button.state) {
 @@ -485,7 +486,8 @@ tp_button_handle_event(struct tp_dispatch *tp,
   }
  
   if (current != t-button.state)
 - log_debug(button state: from %s, event %s to %s\n,
 + log_debug(libinput,
 +   button state: from %s, event %s to %s\n,
 button_state_to_str(current),
 button_event_to_str(event),
 button_state_to_str(t-button.state));
 @@ -538,11 +540,13 @@ tp_process_button(struct tp_dispatch *tp,
 const struct input_event *e,
 uint64_t time)
  {
 + struct libinput *libinput = tp-device-base.seat-libinput;
   uint32_t mask = 1  (e-code - BTN_LEFT);
  
   /* Ignore other buttons on clickpads */
   if (tp-buttons.is_clickpad  e-code != BTN_LEFT) {
 - log_bug_kernel(received %s button event on a clickpad\n,
 + log_bug_kernel(libinput,
 +received %s button event on a clickpad\n,
  libevdev_event_code_get_name(EV_KEY, e-code));
   return 0;
   }
 @@ -562,6 +566,7 @@ int
  tp_init_buttons(struct tp_dispatch *tp,
   struct evdev_device *device)
  {
 + struct libinput *libinput = tp-device-base.seat-libinput;
   struct tp_touch *t;
   int width, height;
   double diagonal;
 @@ -574,10 +579,12 @@ tp_init_buttons(struct tp_dispatch *tp,
   if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) ||
   libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) {
   if (tp-buttons.is_clickpad)
 - log_bug_kernel(clickpad advertising right button\n);
 + log_bug_kernel(libinput,
 +clickpad advertising right button\n);
   } else {
   if (!tp-buttons.is_clickpad)
 - log_bug_kernel(non clickpad without right button?\n);
 + log_bug_kernel(libinput,
 +non clickpad without right button?\n);
   }
  
   width = abs(device-abs.max_x - device-abs.min_x);
 diff --git

Re: [PATCH libinput 3/3] Change the logging system to be per-context

2014-06-16 Thread Peter Hutterer 
On Mon, Jun 16, 2014 at 12:09:16PM +0200, Hans de Goede wrote:
[...]
 
 Your removing a bunch of exported symbols here, so you should also bump
 the soname to indicate ABI breakage.

Jonas - do you want the soname bump in this patch or do it as part of the
release? There may be other changes coming before the 0.4 release.

Cheers,
   Peter


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH libinput 3/3] Change the logging system to be per-context

2014-06-12 Thread Peter Hutterer 
Rather than a single global logging function, make the logging dependent on
the individual context. This way we won't stomp on each other's feet in the
(admittedly unusual) case of having multiple libinput contexts.

The log handler and the log priority is now a part of the libinput interface.
We can drop the various setters and getters, the caller owns the struct anyway
so we don't need functions to give it those values.

The userdata argument to the log handler was dropped. The caller has a ref to
the libinput context now, any userdata can be attached to that context
instead.

There is no need for a default log function anymore. Any serious caller should
hook into it anyway, those that don't care can just use NULL.

There is no default log priority anymore, a caller must set the desired
priority in the interface.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
There's a side-effect to this that I'm not sure is intended. We don't copy
the interface into libinput, we merely keep a reference. The caller is
already able to change open_restricted/close_restricted at runtime, though
we can't do this ourselves (it's const).

Given that, I figured we can leave the log handler and priority up to the
caller as well then, switching at runtime. That's the main reason for
dropping the set/get priority calls. If that side effect wasn't intended,
then we'll have rework a few things. Jonas?

 src/evdev-mt-touchpad-buttons.c |  15 +++--
 src/evdev-mt-touchpad-tap.c |  13 -
 src/evdev.c |  23 +---
 src/libinput-private.h  |  20 ---
 src/libinput.c  |  73 ++-
 src/libinput.h  |  99 +--
 src/path.c  |  20 +--
 src/timer.c |   4 +-
 src/udev-seat.c |  17 +++---
 test/litest.c   |   9 +--
 test/log.c  | 126 ++--
 test/misc.c |   2 +
 test/path.c |   2 +
 test/udev.c |   7 ++-
 tools/event-debug.c |  28 -
 15 files changed, 194 insertions(+), 264 deletions(-)

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index ce48ed0..b86f344 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -452,6 +452,7 @@ tp_button_handle_event(struct tp_dispatch *tp,
   enum button_event event,
   uint64_t time)
 {
+   struct libinput *libinput = tp-device-base.seat-libinput;
enum button_state current = t-button.state;
 
switch(t-button.state) {
@@ -485,7 +486,8 @@ tp_button_handle_event(struct tp_dispatch *tp,
}
 
if (current != t-button.state)
-   log_debug(button state: from %s, event %s to %s\n,
+   log_debug(libinput,
+ button state: from %s, event %s to %s\n,
  button_state_to_str(current),
  button_event_to_str(event),
  button_state_to_str(t-button.state));
@@ -538,11 +540,13 @@ tp_process_button(struct tp_dispatch *tp,
  const struct input_event *e,
  uint64_t time)
 {
+   struct libinput *libinput = tp-device-base.seat-libinput;
uint32_t mask = 1  (e-code - BTN_LEFT);
 
/* Ignore other buttons on clickpads */
if (tp-buttons.is_clickpad  e-code != BTN_LEFT) {
-   log_bug_kernel(received %s button event on a clickpad\n,
+   log_bug_kernel(libinput,
+  received %s button event on a clickpad\n,
   libevdev_event_code_get_name(EV_KEY, e-code));
return 0;
}
@@ -562,6 +566,7 @@ int
 tp_init_buttons(struct tp_dispatch *tp,
struct evdev_device *device)
 {
+   struct libinput *libinput = tp-device-base.seat-libinput;
struct tp_touch *t;
int width, height;
double diagonal;
@@ -574,10 +579,12 @@ tp_init_buttons(struct tp_dispatch *tp,
if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) ||
libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) {
if (tp-buttons.is_clickpad)
-   log_bug_kernel(clickpad advertising right button\n);
+   log_bug_kernel(libinput,
+  clickpad advertising right button\n);
} else {
if (!tp-buttons.is_clickpad)
-   log_bug_kernel(non clickpad without right button?\n);
+   log_bug_kernel(libinput,
+  non clickpad without right button?\n);
}
 
width = abs(device-abs.max_x - device-abs.min_x);
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 34bb0d0..2541218 100644