[PATCH 4/5] client: "_id" versions of constructors, alternative to proxy_wrapper

Giulio Camuffo giuliocamuffo at gmail.com
Mon Aug 8 09:28:01 UTC 2016


Hi,

What's the point of this now that we have the proxy wrapper approach?


Cheers,
Giulio


2016-05-17 8:18 GMT+02:00  <spitzak at gmail.com>:
> From: Bill Spitzak <spitzak at gmail.com>
>
> Scanner produces a wl_xyz_new() for each object, and for each constructor
> request it produces an "_id" version that takes one of these, instead of
> creating one. This allows the queue and listener to be set before creation of
> the object so events always go to the correct queue.
>
> Existing code using proxy_wrapper changed to use this.
>
> To implement this two new api's were added to wayland-client-core.h:
>
> wl_proxy_new(interface)
>
> wl_proxy_marshal_id(...): Same as wl_proxy_marshal except it handles 'n' arguments
> and assigns an id to them. wl_proxy_marshal could do this but it would add overhead
> to search for the 'n' to non-constructor requests.
>
> v2: Moved definition of wl_proxy back out of header file
> v3: _new api and named request _id.
>
> Signed-off-by: Bill Spitzak <spitzak at gmail.com>
> ---
>  src/scanner.c             | 59 ++++++++++++++++++++++++++++++++++++++
>  src/wayland-client-core.h |  6 ++++
>  src/wayland-client.c      | 72 ++++++++++++++++++++++++++++++++++++-----------
>  tests/queue-test.c        | 15 +++++-----
>  4 files changed, 127 insertions(+), 25 deletions(-)
>
> diff --git a/src/scanner.c b/src/scanner.c
> index 33271bc..fbe8193 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -1176,6 +1176,51 @@ emit_stubs(struct wl_list *message_list, struct interface *interface)
>                                ret->interface_name, ret->name);
>
>                 printf("}\n\n");
> +
> +               /* Produce the _id version of constructor calls.
> +                * FIXME: this is a cut + paste patch, it would be better
> +                * to reuse the above code and/or make the old constructor
> +                * be the special case.
> +                */
> +               if (!ret)
> +                       continue;
> +               if (!ret->interface_name)
> +                       continue; /* wl_registry_bind_id nyi */
> +               printf("/**\n"
> +                      " * @ingroup iface_%s\n", interface->name);
> +               printf(" *\n"
> +                      " * Same as %s_%s() except it uses an existing proxy created by\n"
> +                      " * %s_new().\n"
> +                      " */\n", interface->name, m->name, ret->interface_name);
> +               printf("static inline void\n");
> +
> +               printf("%s_%s_id(struct %s *%s",
> +                      interface->name, m->name,
> +                      interface->name, interface->name);
> +
> +               wl_list_for_each(a, &m->arg_list, link) {
> +                       printf(", ");
> +                       if (a->type != NEW_ID)
> +                               emit_type(a);
> +                       else
> +                               printf("struct %s *", ret->interface_name);
> +                       printf("%s", a->name);
> +               }
> +
> +               printf(")\n"
> +                      "{\n"
> +                      "\twl_proxy_marshal_id((struct wl_proxy *) %s,\n"
> +                      "\t\t\t %s_%s",
> +                      interface->name,
> +                      interface->uppercase_name,
> +                      m->uppercase_name);
> +
> +               wl_list_for_each(a, &m->arg_list, link) {
> +                       printf(", %s", a->name);
> +               }
> +               printf(");\n");
> +
> +               printf("}\n\n");
>         }
>  }
>
> @@ -1349,6 +1394,20 @@ emit_structs(struct wl_list *message_list, struct interface *interface, enum sid
>         printf("};\n\n");
>
>         if (side == CLIENT) {
> +           if (strcmp(interface->name, "wl_display"))
> +               printf("/**\n"
> +                  " * @ingroup iface_%s\n"
> +                  " *\n"
> +                  " * Create a %s and allocate an id.\n"
> +                  " */\n"
> +                  "static inline struct %s *\n"
> +                  "%s_new(void)\n"
> +                  "{\n"
> +                  "\treturn (struct %s *) wl_proxy_new(&%s_interface);\n"
> +                  "}\n\n",
> +                  interface->name, interface->name, interface->name,
> +                  interface->name, interface->name, interface->name);
> +
>             printf("/**\n"
>                    " * @ingroup iface_%s\n"
>                    " */\n", interface->name);
> diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h
> index 52bb553..8e48cab 100644
> --- a/src/wayland-client-core.h
> +++ b/src/wayland-client-core.h
> @@ -125,10 +125,16 @@ void
>  wl_proxy_marshal(struct wl_proxy *p, uint32_t opcode, ...);
>
>  void
> +wl_proxy_marshal_id(struct wl_proxy *proxy, uint32_t opcode, ...);
> +
> +void
>  wl_proxy_marshal_array(struct wl_proxy *p, uint32_t opcode,
>                        union wl_argument *args);
>
>  struct wl_proxy *
> +wl_proxy_new(const struct wl_interface *interface);
> +
> +struct wl_proxy *
>  wl_proxy_create(struct wl_proxy *factory,
>                 const struct wl_interface *interface);
>
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index f67678b..d539bf7 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -324,8 +324,17 @@ wl_display_create_queue(struct wl_display *display)
>         return queue;
>  }
>
> -static struct wl_proxy *
> -proxy_new(const struct wl_interface *interface)
> +/** Create a proxy object with a given interface
> + *
> + * \param interface Interface the proxy object should use
> + * \return A newly allocated proxy object or NULL on failure
> + *
> + * This function creates a new proxy object with the supplied interface.
> + * Caller should pass this to a constructor message to fill in the id
> + * and cause the matching object to be created by the server.
> + */
> +WL_EXPORT struct wl_proxy *
> +wl_proxy_new(const struct wl_interface *interface)
>  {
>         struct wl_proxy *proxy = zalloc(sizeof *proxy);
>         if (proxy) {
> @@ -358,7 +367,7 @@ WL_EXPORT struct wl_proxy *
>  wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
>  {
>         struct wl_display *display = factory->display;
> -       struct wl_proxy *proxy = proxy_new(interface);
> +       struct wl_proxy *proxy = wl_proxy_new(interface);
>         if (proxy == NULL)
>                 return NULL;
>
> @@ -379,7 +388,7 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
>                        uint32_t id, const struct wl_interface *interface)
>  {
>         struct wl_display *display = factory->display;
> -       struct wl_proxy *proxy = proxy_new(interface);
> +       struct wl_proxy *proxy = wl_proxy_new(interface);
>         if (proxy == NULL)
>                 return NULL;
>
> @@ -657,7 +666,7 @@ wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *factory,
>         struct wl_proxy *new_proxy = NULL;
>
>         if (interface) {
> -               new_proxy = proxy_new(interface);
> +               new_proxy = wl_proxy_new(interface);
>                 if (new_proxy == NULL)
>                         return NULL;
>                 new_proxy->display = factory->display;
> @@ -701,6 +710,42 @@ wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
>         proxy_marshal_array(proxy, opcode, args, NULL);
>  }
>
> +/** Same as wl_proxy_marshal() except this also searches for a new_id
> + * argument and allocates an id for the object.
> + *
> + * \param factory The proxy object
> + * \param opcode Opcode of the request to be sent
> + * \param ... Extra arguments for the given request
> + *
> + * \note This should not normally be used by non-generated code.
> + *
> + * \sa wl_proxy_new()
> + *
> + * \memberof wl_proxy
> + */
> +WL_EXPORT void
> +wl_proxy_marshal_id(struct wl_proxy *factory, uint32_t opcode, ...)
> +{
> +       union wl_argument args[WL_CLOSURE_MAX_ARGS];
> +       va_list ap;
> +       struct wl_proxy* new_proxy;
> +
> +       va_start(ap, opcode);
> +       wl_argument_from_va_list(factory->object.interface->methods[opcode].signature,
> +                                args, WL_CLOSURE_MAX_ARGS, ap);
> +       va_end(ap);
> +
> +       new_proxy = (struct wl_proxy *)(args[new_id_index(factory, opcode)].o);
> +       if (new_proxy->object.id)
> +               wl_abort("proxy already has an id");
> +       new_proxy->display = factory->display;
> +       if (new_proxy->queue == NULL)
> +               new_proxy->queue = factory->queue;
> +       new_proxy->version = factory->version;
> +
> +       proxy_marshal_array(factory, opcode, args, new_proxy);
> +}
> +
>  /** Prepare a request to be sent to the compositor
>   *
>   * \param factory The proxy object
> @@ -1104,24 +1149,17 @@ static const struct wl_callback_listener sync_listener = {
>  WL_EXPORT int
>  wl_display_roundtrip_queue(struct wl_display *display, struct wl_event_queue *queue)
>  {
> -       struct wl_display *display_wrapper;
>         struct wl_callback *callback;
>         int done, ret = 0;
>
>         done = 0;
> -
> -       display_wrapper = wl_proxy_create_wrapper(display);
> -       if (!display_wrapper)
> -               return -1;
> -
> -       wl_proxy_set_queue((struct wl_proxy *) display_wrapper, queue);
> -       callback = wl_display_sync(display_wrapper);
> -       wl_proxy_wrapper_destroy(display_wrapper);
> -
> +       callback = wl_callback_new();
>         if (callback == NULL)
>                 return -1;
> -
> +       wl_proxy_set_queue((struct wl_proxy *) callback, queue);
>         wl_callback_add_listener(callback, &sync_listener, &done);
> +       wl_display_sync_id(display, callback);
> +
>         while (!done && ret >= 0)
>                 ret = wl_display_dispatch_queue(display, queue);
>
> @@ -2034,7 +2072,7 @@ WL_EXPORT void *
>  wl_proxy_create_wrapper(void *factory)
>  {
>         struct wl_proxy *wrapped_proxy = factory;
> -       struct wl_proxy *wrapper = proxy_new(wrapped_proxy->object.interface);
> +       struct wl_proxy *wrapper = wl_proxy_new(wrapped_proxy->object.interface);
>         if (!wrapper)
>                 return NULL;
>
> diff --git a/tests/queue-test.c b/tests/queue-test.c
> index 932bc55..61a2903 100644
> --- a/tests/queue-test.c
> +++ b/tests/queue-test.c
> @@ -119,15 +119,17 @@ client_test_multiple_queues(void)
>         assert(queue);
>
>         state.done = false;
> -       callback1 = wl_display_sync(state.display);
> +       callback1 = wl_callback_new();
>         assert(callback1 != NULL);
>         wl_callback_add_listener(callback1, &sync_listener, &state);
>         wl_proxy_set_queue((struct wl_proxy *) callback1, queue);
> +       wl_display_sync_id(state.display, callback1);
>
> -       state.callback2 = wl_display_sync(state.display);
> +       state.callback2 = wl_callback_new();
>         assert(state.callback2 != NULL);
>         wl_callback_add_listener(state.callback2, &sync_listener, NULL);
>         wl_proxy_set_queue((struct wl_proxy *) state.callback2, queue);
> +       wl_display_sync_id(state.display, state.callback2);
>
>         wl_display_flush(state.display);
>
> @@ -209,7 +211,6 @@ client_test_queue_proxy_wrapper(void)
>  {
>         struct wl_event_queue *queue;
>         struct wl_display *display;
> -       struct wl_display *display_wrapper;
>         struct wl_callback *callback;
>         bool done = false;
>
> @@ -226,12 +227,10 @@ client_test_queue_proxy_wrapper(void)
>         queue = wl_display_create_queue(display);
>         assert(queue);
>
> -       display_wrapper = wl_proxy_create_wrapper(display);
> -       assert(display_wrapper);
> -       wl_proxy_set_queue((struct wl_proxy *) display_wrapper, queue);
> -       callback = wl_display_sync(display_wrapper);
> -       wl_proxy_wrapper_destroy(display_wrapper);
> +       callback = wl_callback_new();
>         assert(callback != NULL);
> +       wl_proxy_set_queue((struct wl_proxy *) callback, queue);
> +       wl_display_sync_id(display, callback);
>
>         /* Pretend we are now another thread and dispatch the dispatch the main
>          * queue while also knowing our callback is read and queued. */
> --
> 1.9.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list