On Fri, 26 Jan 2018 18:47:59 +0200 Alexandros Frantzis <alexandros.frant...@collabora.com> wrote:
> The current test client code waits for all wl_seat globals to arrive
> before checking them and deciding which one is the test seat global to
> use for the input object. This method doesn't support dynamic addition
> of the test seat global (i.e., after client start-up), which will be
> needed in upcoming commits.
>
> This commit changes the code to check for the test seat and set up the
> input object while handling the wl_seat information events.
>
> Signed-off-by: Alexandros Frantzis <alexandros.frant...@collabora.com>
> ---
> tests/weston-test-client-helper.c | 78
> ++++++++++++++++++++++-----------------
> tests/weston-test-client-helper.h | 1 +
> 2 files changed, 46 insertions(+), 33 deletions(-)
Hi,
I feel this patch might be doing more than what it says on the tin:
- move input_update_devices() call from client_set_input() into the
event handlers
- remove the check for all seats must have a name
I suppose justifying these in the commit message would be enough in
this case.
> diff --git a/tests/weston-test-client-helper.c
> b/tests/weston-test-client-helper.c
> index 6e0a5246..854978d0 100644
> --- a/tests/weston-test-client-helper.c
> +++ b/tests/weston-test-client-helper.c
> @@ -538,6 +538,14 @@ static const struct weston_test_listener test_listener =
> {
> test_handle_capture_screenshot_done,
> };
>
> +static void
> +input_destroy(struct input *inp)
> +{
> + wl_list_remove(&inp->link);
> + wl_seat_destroy(inp->wl_seat);
> + free(inp);
> +}
> +
> static void
> input_update_devices(struct input *input)
> {
> @@ -598,22 +606,56 @@ seat_handle_capabilities(void *data, struct wl_seat
> *seat,
>
> /* we will create/update the devices only with the right (test) seat.
> * If we haven't discovered which seat is the test seat, just
> - * store capabilities and bail out */
> - if (input->seat_name && strcmp(input->seat_name, "test-seat") == 0)
> + * store capabilities and bail out. Note that we don't need to
> + * check the name contents here, since only the test seat input will
> + * have its name set */
> + if (input->seat_name) {
> input_update_devices(input);
In fact, this bit of old code was not actually necessary, because
client_set_input() would have done the right thing anyway. Your patch
fixes this "imbalance" by making both event handlers call
input_update_devices().
> + input->client->input = input;
> + }
> +
>
> fprintf(stderr, "test-client: got seat %p capabilities: %x\n",
> input, caps);
> }
>
> +static struct input *
> +client_find_input_with_seat_name(struct client *client, const char *name)
> +{
This function seems unnecessary, because...
> + struct input *input;
> +
> + wl_list_for_each(input, &client->inputs, link) {
> + if (input->seat_name && strcmp(input->seat_name, name) == 0)
> + return input;
> + }
> +
> + return NULL;
> +}
> +
> static void
> seat_handle_name(void *data, struct wl_seat *seat, const char *name)
> {
> struct input *input = data;
>
> + /* We don't care about seats other than the test seat */
> + if (strcmp(name, "test-seat") != 0) {
> + input_destroy(input);
> + return;
> + }
> +
> + assert(!client_find_input_with_seat_name(input->client, name) &&
> + "Multiple test seats detected!");
..this test could as well be done...
> +
> input->seat_name = strdup(name);
> assert(input->seat_name && "No memory");
>
> + /* We assume that the test seat always has some capabilities,
> + * so caps == 0 just means that we haven't received them yet */
We don't actually need this assumption, because whether the caps really
are zero or are not yet handled, the result should be exactly the same.
We can uncoditionally set client->input here, and remove the setting
from the capabilities handler. input_update_devices() also already does
the right thing regardless whether capabilities have been received yet.
> + if (input->caps != 0) {
> + input_update_devices(input);
> + input->client->input = input;
..as an assert on this assignment that client->input must be NULL
before we set it.
Thanks,
pq
> + }
> +
> fprintf(stderr, "test-client: got seat %p name: \'%s\'\n",
> input, name);
> }
> @@ -705,6 +747,7 @@ handle_global(void *data, struct wl_registry *registry,
> &wl_compositor_interface, version);
> } else if (strcmp(interface, "wl_seat") == 0) {
> input = xzalloc(sizeof *input);
> + input->client = client;
> input->wl_seat =
> wl_registry_bind(registry, id,
> &wl_seat_interface, version);
> @@ -809,34 +852,6 @@ log_handler(const char *fmt, va_list args)
> vfprintf(stderr, fmt, args);
> }
>
> -static void
> -input_destroy(struct input *inp)
> -{
> - wl_list_remove(&inp->link);
> - wl_seat_destroy(inp->wl_seat);
> - free(inp);
> -}
> -
> -/* find the test-seat and set it in client.
> - * Destroy other inputs */
> -static void
> -client_set_input(struct client *cl)
> -{
> - struct input *inp, *inptmp;
> - wl_list_for_each_safe(inp, inptmp, &cl->inputs, link) {
> - assert(inp->seat_name && "BUG: input with no name");
> - if (strcmp(inp->seat_name, "test-seat") == 0) {
> - cl->input = inp;
> - input_update_devices(inp);
> - } else {
> - input_destroy(inp);
> - }
> - }
> -
> - /* we keep only one input */
> - assert(wl_list_length(&cl->inputs) == 1);
> -}
> -
> struct client *
> create_client(void)
> {
> @@ -862,9 +877,6 @@ create_client(void)
> * events */
> client_roundtrip(client);
>
> - /* find the right input for us */
> - client_set_input(client);
> -
> /* must have WL_SHM_FORMAT_ARGB32 */
> assert(client->has_argb);
>
> diff --git a/tests/weston-test-client-helper.h
> b/tests/weston-test-client-helper.h
> index 09a5df4a..f16356e5 100644
> --- a/tests/weston-test-client-helper.h
> +++ b/tests/weston-test-client-helper.h
> @@ -74,6 +74,7 @@ struct test {
> };
>
> struct input {
> + struct client *client;
> struct wl_seat *wl_seat;
> struct pointer *pointer;
> struct keyboard *keyboard;
pgpFhC4nmbAto.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
