Re: [PATCH weston 1/2 v2] input: Emit events on all resources for a client

2013-09-23 Thread Neil Roberts
Kristian Høgsberg hoegsb...@gmail.com writes:

 Instead of this manual marker, you can add comments like the above
 below the three dashes that git inserts between the commit message and
 the actual patch. git am will take the text up until the --- marker as
 the commit message and discard everything from there on until the
 actual patch.

This isn't quite a manual marker. If you apply the patch using ‘git am
--scissors’ it will automatically cut out the message at the top. You
can also do ‘git config --global mailinfo.scissors yes’ to set that
automatically. I'm not sure why that's not the default. I think the
scissors are slightly nicer than putting a comment next to the diffstat
because you can put the message at the beginning of the email. However
I'm happy to follow whatever convention the list uses.

It looks like the ‘From’ tag got messed up when the patch was applied so
now it has given me credit for Rob's patch and I have now co-authored it
with myself. Git is hard!

Regards,
- Neil
-
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 1/2 v2] input: Emit events on all resources for a client

2013-09-21 Thread Kristian Høgsberg
On Thu, Sep 19, 2013 at 05:32:00PM +0100, Neil Roberts wrote:
 Here is a second version of Robert's patch with the following changes:
 
 I think the problem where clients would not immediately become active
 when starting was caused by it calling weston_*_set_focus instead of
 sending the enter event for the first keyboard or pointer resource.
 The set_focus functions don't do anything if the surface already has
 focus so the client was never getting the enter event. I've changed it
 so that it uses the same code path regardless of whether it is the
 first or a subsequent resource and now it just always explicitly sends
 the enter event.
 
 I've removed the focus listeners. We don't need them anymore because
 the resource will automatically remove itself from the list when it is
 destroyed and that will have the same effect as the old destroy
 callback which would just unset the focus resource pointer. This
 should remove the need for Robert's patch which changes the destroy
 listeners to listen to the client resource.
 
 When setting the pointer focus it will now send the keyboard modifiers
 regardless of whether the focused client has a pointer resource. This
 is not how it works in master and Rob's patch also changed it to do
 this but I'm not sure if that was intentional. This is important
 because otherwise if you get the pointer later than getting the
 keyboard then the modifiers might not be up-to-date.
 
 I've changed the move_resource function to just use wl_list_insert
 which should have the same effect but will be faster because it
 doesn't need to iterate the list.
 
 In a couple of places the patch made it so that it would increment the
 serial even if there are no resources for the pointer or keyboard.
 I've changed these so that it checks if the resource list is empty to
 make it retain the old behaviour. In particular this was happening in
 binding_key and default_grab_key.
 
 When getting the keyboard, it now sends the modifiers to the client
 when it either has the pointer focus or the keyboard focus, instead of
 only if it has the keyboard focus.
 
 I've split up long lines and moved some bits of code into common
 functions, for example to emit the modifiers event.
 
 I've brought back the find_resource_for_surface functions so that the
 set focus functions can avoid incrementing the serial if there is no
 pointer or keyboard resource for the client of that surface.
 
 I removed the duplicated checks for seat-touch_focus != surface in
 wl_touch_set_focus because this is ensured right at the start of the
 function so it is redundant to check it again.
 
 I fixed the spelling of ‘focussed‘.

This is great, I think you got it just right, all those changes make
sense.

 Regards,
 - Neil
 
 -- 8 --

Instead of this manual marker, you can add comments like the above
below the three dashes that git inserts between the commit message and
the actual patch.  git am will take the text up until the --- marker
as the commit message and discard everything from there on until the
actual patch.

Kristian

 From: Rob Bradford r...@linux.intel.com
 
 The Wayland protocol permits a client to request the pointer, keyboard
 and touch multiple times from the seat global. This is very useful in a
 component like Clutter-GTK where we are combining two libraries that use
 Wayland together.
 
 This change migrates the weston input handling code to emit the
 events for all the resources for the client by using the newly added
 wl_resource_for_each macro to iterate over the resources that are
 associated with the focused surface's client.
 
 We maintain a list of focused resources on the pointer and keyboard
 which is updated when the focus changes. However since we can have
 resources created after the focus has already been set we must add the
 resources to the right list and also update any state.
 
 Additionally when setting the pointer focus it will now send the
 keyboard modifiers regardless of whether the focused client has a
 pointer resource. This is important because otherwise if the client
 gets the pointer later than you getting the keyboard then the
 modifiers might not be up-to-date.
 
 Co-author: Neil Roberts n...@linux.intel.com
 ---

[ That is, text here (including the diffstat below) will be discarded by
  git am, which makes this a good place for informal, out-of-band text. ]

  src/bindings.c|  21 +--
  src/compositor.h  |   9 +-
  src/data-device.c |  16 ++-
  src/input.c   | 399 
 +++---
  src/shell.c   |  34 +++--
  5 files changed, 309 insertions(+), 170 deletions(-)
 
 diff --git a/src/bindings.c b/src/bindings.c
 index a871c26..f6ec9ea 100644
 --- a/src/bindings.c
 +++ b/src/bindings.c
 @@ -160,7 +160,6 @@ binding_key(struct weston_keyboard_grab *grab,
   struct weston_keyboard *keyboard = grab-keyboard;
   struct wl_display *display = keyboard-seat-compositor-wl_display;
  
 - resource = grab-keyboard-focus_resource;

[PATCH weston 1/2 v2] input: Emit events on all resources for a client

2013-09-19 Thread Neil Roberts
Here is a second version of Robert's patch with the following changes:

I think the problem where clients would not immediately become active
when starting was caused by it calling weston_*_set_focus instead of
sending the enter event for the first keyboard or pointer resource.
The set_focus functions don't do anything if the surface already has
focus so the client was never getting the enter event. I've changed it
so that it uses the same code path regardless of whether it is the
first or a subsequent resource and now it just always explicitly sends
the enter event.

I've removed the focus listeners. We don't need them anymore because
the resource will automatically remove itself from the list when it is
destroyed and that will have the same effect as the old destroy
callback which would just unset the focus resource pointer. This
should remove the need for Robert's patch which changes the destroy
listeners to listen to the client resource.

When setting the pointer focus it will now send the keyboard modifiers
regardless of whether the focused client has a pointer resource. This
is not how it works in master and Rob's patch also changed it to do
this but I'm not sure if that was intentional. This is important
because otherwise if you get the pointer later than getting the
keyboard then the modifiers might not be up-to-date.

I've changed the move_resource function to just use wl_list_insert
which should have the same effect but will be faster because it
doesn't need to iterate the list.

In a couple of places the patch made it so that it would increment the
serial even if there are no resources for the pointer or keyboard.
I've changed these so that it checks if the resource list is empty to
make it retain the old behaviour. In particular this was happening in
binding_key and default_grab_key.

When getting the keyboard, it now sends the modifiers to the client
when it either has the pointer focus or the keyboard focus, instead of
only if it has the keyboard focus.

I've split up long lines and moved some bits of code into common
functions, for example to emit the modifiers event.

I've brought back the find_resource_for_surface functions so that the
set focus functions can avoid incrementing the serial if there is no
pointer or keyboard resource for the client of that surface.

I removed the duplicated checks for seat-touch_focus != surface in
wl_touch_set_focus because this is ensured right at the start of the
function so it is redundant to check it again.

I fixed the spelling of ‘focussed‘.

Regards,
- Neil

-- 8 --

From: Rob Bradford r...@linux.intel.com

The Wayland protocol permits a client to request the pointer, keyboard
and touch multiple times from the seat global. This is very useful in a
component like Clutter-GTK where we are combining two libraries that use
Wayland together.

This change migrates the weston input handling code to emit the
events for all the resources for the client by using the newly added
wl_resource_for_each macro to iterate over the resources that are
associated with the focused surface's client.

We maintain a list of focused resources on the pointer and keyboard
which is updated when the focus changes. However since we can have
resources created after the focus has already been set we must add the
resources to the right list and also update any state.

Additionally when setting the pointer focus it will now send the
keyboard modifiers regardless of whether the focused client has a
pointer resource. This is important because otherwise if the client
gets the pointer later than you getting the keyboard then the
modifiers might not be up-to-date.

Co-author: Neil Roberts n...@linux.intel.com
---
 src/bindings.c|  21 +--
 src/compositor.h  |   9 +-
 src/data-device.c |  16 ++-
 src/input.c   | 399 +++---
 src/shell.c   |  34 +++--
 5 files changed, 309 insertions(+), 170 deletions(-)

diff --git a/src/bindings.c b/src/bindings.c
index a871c26..f6ec9ea 100644
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -160,7 +160,6 @@ binding_key(struct weston_keyboard_grab *grab,
struct weston_keyboard *keyboard = grab-keyboard;
struct wl_display *display = keyboard-seat-compositor-wl_display;
 
-   resource = grab-keyboard-focus_resource;
if (key == b-key) {
if (state == WL_KEYBOARD_KEY_STATE_RELEASED) {
weston_keyboard_end_grab(grab-keyboard);
@@ -168,9 +167,15 @@ binding_key(struct weston_keyboard_grab *grab,
keyboard-grab = keyboard-input_method_grab;
free(b);
}
-   } else if (resource) {
+   } else if (!wl_list_empty(keyboard-focus_resource_list)) {
serial = wl_display_next_serial(display);
-   wl_keyboard_send_key(resource, serial, time, key, state);
+   wl_resource_for_each(resource, keyboard-focus_resource_list) {
+