[RFC wayland] Track protocol object versions inside wl_proxy.
Jason Ekstrand
jason at jlekstrand.net
Wed Nov 25 10:25:42 PST 2015
On Wed, Nov 25, 2015 at 10:15 AM, Derek Foreman <derekf at osg.samsung.com> wrote:
> On 13/11/15 12:21 PM, Jason Ekstrand wrote:
>> On Fri, Nov 13, 2015 at 3:18 AM, Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>>> So do i understand correctly that if the app creating the
>>> wl_compositor was built against a libwayland without this patch the
>>> version returned in mesa by wl_proxy_get_version() for every proxy
>>> will be always 1?
>>
>> Yes, see also the second e-mail link below. I proposed to make
>> wl_display version 0 so that libraries can detect whether the client
>> was built against old or new libwayland.
>>
>> --Jason
>
> But the generated protocol headers won't have wl_surface_get_version()
> in that case, so build would fail?
>
> I'm finally trying to build a client that shows this failure mode, and
> it's starting to feel quite contrived...
The issue that arises is if you have a client built against an old
version of libwayland that uses a library built against a newer
version. Let's make this extremely concrete:
Suppose that we use this wl_surface_get_version() in mesa's EGL
implementation to determine whether or not we have a buffer_damage
request. We do that, merge it into mesa master, and start shipping it
in distros. Then some old client comes along that was built against
libwayland 1.3. It can run against libwayland 1.7 just fine because
everything's backwards compaible. It can also run against our
brand-new mesa because the GL and EGL apis are backwards compatible.
However, whatever wayland objects it passes in to EGL will be created
using the old api that doesn't have _versioned functions. The result
of this is that all objects appear to be version 1 because that's what
wl_display starts at.
This is ok for buffer_damage() because that's just adding a request.
However, if there's something more subtle that happens when an object
changes version such that treating a version 3 object as a version 1
object may not always be correct, then we have a problem. While, in
general, we should try to avoid these kinds of changes, they can
happen so we should let the user of wl_proxy_get_version() be able to
distinguish between version 1 and "I don't know".
Does that make more sense? It's quite subtle.
> Is there a reason we'd want to use wl_proxy_get_version() directly? I
> can't imagine having a proxy, needing to know its version, but not
> knowing exactly what it's a proxy for... Maybe I'm just not being
> imaginative enough though.
No, wrappers are fine.
> If we make foo_get_version()'s presence known through #defines then the
> absence of an appropriate foo_get_version() (ie: old header) could
> indicate we need to do whatever kind of fallback.
Sure. For that matter, the client can get that from a version #define
or from pkg-config. That's a standard "am I building with a recent
version" issue.
--Jason
> Thanks,
> Derek
>
>>> 2015-11-12 22:01 GMT+02:00 Derek Foreman <derekf at osg.samsung.com>:
>>>> From: Jason Ekstrand <jason at jlekstrand.net>
>>>>
>>>> This provides a standardized mechanism for tracking protocol object
>>>> versions in client code. The wl_display object is created with version 1.
>>>> Every time an object is created from within wl_registry_bind, it gets the
>>>> bound version. Every other time an object is created, it simply inherits
>>>> it's version from the parent object that created it.
>>>>
>>>> ---
>>>> Sooo, seems to finally fix the EGLSwapBuffersWithDamage problem, we also
>>>> need this patch. However, it's not complete.
>>>>
>>>> I've rebased it and updated it. Here's the original posting:
>>>> http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html
>>>>
>>>> Of special importance is:
>>>> http://lists.freedesktop.org/archives/wayland-devel/2014-April/014011.html
>>>>
>>>> And the proposed solution to the new backwards compatibility issue. If we
>>>> get a consensus on a way forward, I can pick this up and finish it off...
>>>>
>>>> src/scanner.c | 29 ++++++++----
>>>> src/wayland-client-core.h | 14 ++++++
>>>> src/wayland-client.c | 112 +++++++++++++++++++++++++++++++++++++++++++---
>>>> 3 files changed, 140 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/src/scanner.c b/src/scanner.c
>>>> index 8ecdd56..850e72a 100644
>>>> --- a/src/scanner.c
>>>> +++ b/src/scanner.c
>>>> @@ -889,6 +889,14 @@ emit_stubs(struct wl_list *message_list, struct interface *interface)
>>>> interface->name, interface->name, interface->name,
>>>> interface->name);
>>>>
>>>> + printf("static inline uint32_t\n"
>>>> + "%s_get_version(struct %s *%s)\n"
>>>> + "{\n"
>>>> + "\treturn wl_proxy_get_version((struct wl_proxy *) %s);\n"
>>>> + "}\n\n",
>>>> + interface->name, interface->name, interface->name,
>>>> + interface->name);
>>>> +
>>>> has_destructor = 0;
>>>> has_destroy = 0;
>>>> wl_list_for_each(m, message_list, link) {
>>>> @@ -960,20 +968,25 @@ emit_stubs(struct wl_list *message_list, struct interface *interface)
>>>>
>>>> printf(")\n"
>>>> "{\n");
>>>> - if (ret) {
>>>> + if (ret && ret->interface_name == NULL) {
>>>> printf("\tstruct wl_proxy *%s;\n\n"
>>>> - "\t%s = wl_proxy_marshal_constructor("
>>>> + "\t%s = wl_proxy_marshal_constructor_versioned("
>>>> "(struct wl_proxy *) %s,\n"
>>>> - "\t\t\t %s_%s, ",
>>>> + "\t\t\t %s_%s, interface, version",
>>>> ret->name, ret->name,
>>>> interface->name,
>>>> interface->uppercase_name,
>>>> m->uppercase_name);
>>>> -
>>>> - if (ret->interface_name == NULL)
>>>> - printf("interface");
>>>> - else
>>>> - printf("&%s_interface", ret->interface_name);
>>>> + } else if (ret) {
>>>> + printf("\tstruct wl_proxy *%s;\n\n"
>>>> + "\t%s = wl_proxy_marshal_constructor("
>>>> + "(struct wl_proxy *) %s,\n"
>>>> + "\t\t\t %s_%s, &%s_interface",
>>>> + ret->name, ret->name,
>>>> + interface->name,
>>>> + interface->uppercase_name,
>>>> + m->uppercase_name,
>>>> + ret->interface_name);
>>>> } else {
>>>> printf("\twl_proxy_marshal((struct wl_proxy *) %s,\n"
>>>> "\t\t\t %s_%s",
>>>> diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h
>>>> index 8b4b4b8..ed41773 100644
>>>> --- a/src/wayland-client-core.h
>>>> +++ b/src/wayland-client-core.h
>>>> @@ -138,10 +138,21 @@ wl_proxy_marshal_constructor(struct wl_proxy *proxy,
>>>> const struct wl_interface *interface,
>>>> ...);
>>>>
>>>> +struct wl_proxy *wl_proxy_marshal_constructor_versioned(struct wl_proxy *proxy,
>>>> + uint32_t opcode,
>>>> + const struct wl_interface *interface,
>>>> + uint32_t version,
>>>> + ...);
>>>> struct wl_proxy *
>>>> wl_proxy_marshal_array_constructor(struct wl_proxy *proxy,
>>>> uint32_t opcode, union wl_argument *args,
>>>> const struct wl_interface *interface);
>>>> +struct wl_proxy *
>>>> +wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy,
>>>> + uint32_t opcode,
>>>> + union wl_argument *args,
>>>> + const struct wl_interface *interface,
>>>> + uint32_t version);
>>>>
>>>> void
>>>> wl_proxy_destroy(struct wl_proxy *proxy);
>>>> @@ -165,6 +176,9 @@ void *
>>>> wl_proxy_get_user_data(struct wl_proxy *proxy);
>>>>
>>>> uint32_t
>>>> +wl_proxy_get_version(struct wl_proxy *proxy);
>>>> +
>>>> +uint32_t
>>>> wl_proxy_get_id(struct wl_proxy *proxy);
>>>>
>>>> const char *
>>>> diff --git a/src/wayland-client.c b/src/wayland-client.c
>>>> index b1c600f..3fa342f 100644
>>>> --- a/src/wayland-client.c
>>>> +++ b/src/wayland-client.c
>>>> @@ -62,6 +62,7 @@ struct wl_proxy {
>>>> int refcount;
>>>> void *user_data;
>>>> wl_dispatcher_func_t dispatcher;
>>>> + uint32_t version;
>>>> };
>>>>
>>>> struct wl_global {
>>>> @@ -326,7 +327,8 @@ wl_display_create_queue(struct wl_display *display)
>>>> }
>>>>
>>>> static struct wl_proxy *
>>>> -proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
>>>> +proxy_create(struct wl_proxy *factory, const struct wl_interface *interface,
>>>> + uint32_t version)
>>>> {
>>>> struct wl_proxy *proxy;
>>>> struct wl_display *display = factory->display;
>>>> @@ -341,6 +343,7 @@ proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
>>>> proxy->display = display;
>>>> proxy->queue = factory->queue;
>>>> proxy->refcount = 1;
>>>> + proxy->version = version;
>>>>
>>>> proxy->object.id = wl_map_insert_new(&display->objects, 0, proxy);
>>>>
>>>> @@ -373,7 +376,7 @@ wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
>>>> struct wl_proxy *proxy;
>>>>
>>>> pthread_mutex_lock(&display->mutex);
>>>> - proxy = proxy_create(factory, interface);
>>>> + proxy = proxy_create(factory, interface, factory->version);
>>>> pthread_mutex_unlock(&display->mutex);
>>>>
>>>> return proxy;
>>>> @@ -398,6 +401,7 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
>>>> proxy->display = display;
>>>> proxy->queue = factory->queue;
>>>> proxy->refcount = 1;
>>>> + proxy->version = factory->version;
>>>>
>>>> wl_map_insert_at(&display->objects, 0, id, proxy);
>>>>
>>>> @@ -529,7 +533,7 @@ wl_proxy_add_dispatcher(struct wl_proxy *proxy,
>>>> static struct wl_proxy *
>>>> create_outgoing_proxy(struct wl_proxy *proxy, const struct wl_message *message,
>>>> union wl_argument *args,
>>>> - const struct wl_interface *interface)
>>>> + const struct wl_interface *interface, uint32_t version)
>>>> {
>>>> int i, count;
>>>> const char *signature;
>>>> @@ -543,7 +547,7 @@ create_outgoing_proxy(struct wl_proxy *proxy, const struct wl_message *message,
>>>>
>>>> switch (arg.type) {
>>>> case 'n':
>>>> - new_proxy = proxy_create(proxy, interface);
>>>> + new_proxy = proxy_create(proxy, interface, version);
>>>> if (new_proxy == NULL)
>>>> return NULL;
>>>>
>>>> @@ -568,7 +572,8 @@ create_outgoing_proxy(struct wl_proxy *proxy, const struct wl_message *message,
>>>> *
>>>> * For new-id arguments, this function will allocate a new wl_proxy
>>>> * and send the ID to the server. The new wl_proxy will be returned
>>>> - * on success or NULL on errror with errno set accordingly.
>>>> + * on success or NULL on errror with errno set accordingly. The newly
>>>> + * created proxy will inherit their version from their parent.
>>>> *
>>>> * \note This is intended to be used by language bindings and not in
>>>> * non-generated code.
>>>> @@ -582,6 +587,43 @@ wl_proxy_marshal_array_constructor(struct wl_proxy *proxy,
>>>> uint32_t opcode, union wl_argument *args,
>>>> const struct wl_interface *interface)
>>>> {
>>>> + return wl_proxy_marshal_array_constructor_versioned(proxy, opcode,
>>>> + args, interface,
>>>> + proxy->version);
>>>> +}
>>>> +
>>>> +
>>>> +/** Prepare a request to be sent to the compositor
>>>> + *
>>>> + * \param proxy The proxy object
>>>> + * \param opcode Opcode of the request to be sent
>>>> + * \param args Extra arguments for the given request
>>>> + * \param interface The interface to use for the new proxy
>>>> + * \param version The protocol object version for the new proxy
>>>> + *
>>>> + * Translates the request given by opcode and the extra arguments into the
>>>> + * wire format and write it to the connection buffer. This version takes an
>>>> + * array of the union type wl_argument.
>>>> + *
>>>> + * For new-id arguments, this function will allocate a new wl_proxy
>>>> + * and send the ID to the server. The new wl_proxy will be returned
>>>> + * on success or NULL on errror with errno set accordingly. The newly
>>>> + * created proxy will have the version specified.
>>>> + *
>>>> + * \note This is intended to be used by language bindings and not in
>>>> + * non-generated code.
>>>> + *
>>>> + * \sa wl_proxy_marshal()
>>>> + *
>>>> + * \memberof wl_proxy
>>>> + */
>>>> +WL_EXPORT struct wl_proxy *
>>>> +wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy,
>>>> + uint32_t opcode,
>>>> + union wl_argument *args,
>>>> + const struct wl_interface *interface,
>>>> + uint32_t version)
>>>> +{
>>>> struct wl_closure *closure;
>>>> struct wl_proxy *new_proxy = NULL;
>>>> const struct wl_message *message;
>>>> @@ -591,7 +633,8 @@ wl_proxy_marshal_array_constructor(struct wl_proxy *proxy,
>>>> message = &proxy->object.interface->methods[opcode];
>>>> if (interface) {
>>>> new_proxy = create_outgoing_proxy(proxy, message,
>>>> - args, interface);
>>>> + args, interface,
>>>> + version);
>>>> if (new_proxy == NULL)
>>>> goto err_unlock;
>>>> }
>>>> @@ -663,7 +706,8 @@ wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
>>>> *
>>>> * For new-id arguments, this function will allocate a new wl_proxy
>>>> * and send the ID to the server. The new wl_proxy will be returned
>>>> - * on success or NULL on errror with errno set accordingly.
>>>> + * on success or NULL on errror with errno set accordingly. The newly
>>>> + * created proxy will inherit their version from their parent.
>>>> *
>>>> * \note This should not normally be used by non-generated code.
>>>> *
>>>> @@ -685,6 +729,46 @@ wl_proxy_marshal_constructor(struct wl_proxy *proxy, uint32_t opcode,
>>>> args, interface);
>>>> }
>>>>
>>>> +
>>>> +/** Prepare a request to be sent to the compositor
>>>> + *
>>>> + * \param proxy The proxy object
>>>> + * \param opcode Opcode of the request to be sent
>>>> + * \param interface The interface to use for the new proxy
>>>> + * \param version The protocol object version of the new proxy
>>>> + * \param ... Extra arguments for the given request
>>>> + * \return A new wl_proxy for the new_id argument or NULL on error
>>>> + *
>>>> + * Translates the request given by opcode and the extra arguments into the
>>>> + * wire format and write it to the connection buffer.
>>>> + *
>>>> + * For new-id arguments, this function will allocate a new wl_proxy
>>>> + * and send the ID to the server. The new wl_proxy will be returned
>>>> + * on success or NULL on errror with errno set accordingly. The newly
>>>> + * created proxy will have the version specified.
>>>> + *
>>>> + * \note This should not normally be used by non-generated code.
>>>> + *
>>>> + * \memberof wl_proxy
>>>> + */
>>>> +WL_EXPORT struct wl_proxy *
>>>> +wl_proxy_marshal_constructor_versioned(struct wl_proxy *proxy, uint32_t opcode,
>>>> + const struct wl_interface *interface,
>>>> + uint32_t version, ...)
>>>> +{
>>>> + union wl_argument args[WL_CLOSURE_MAX_ARGS];
>>>> + va_list ap;
>>>> +
>>>> + va_start(ap, version);
>>>> + wl_argument_from_va_list(proxy->object.interface->methods[opcode].signature,
>>>> + args, WL_CLOSURE_MAX_ARGS, ap);
>>>> + va_end(ap);
>>>> +
>>>> + return wl_proxy_marshal_array_constructor_versioned(proxy, opcode,
>>>> + args, interface,
>>>> + version);
>>>> +}
>>>> +
>>>> /** Prepare a request to be sent to the compositor
>>>> *
>>>> * \param proxy The proxy object
>>>> @@ -848,6 +932,7 @@ wl_display_connect_to_fd(int fd)
>>>> display->proxy.queue = &display->default_queue;
>>>> display->proxy.flags = 0;
>>>> display->proxy.refcount = 1;
>>>> + display->proxy.version = 1;
>>>>
>>>> display->connection = wl_connection_create(display->fd);
>>>> if (display->connection == NULL)
>>>> @@ -1839,6 +1924,19 @@ wl_proxy_get_user_data(struct wl_proxy *proxy)
>>>> return proxy->user_data;
>>>> }
>>>>
>>>> +/** Get the protocol object version of a proxy object
>>>> + *
>>>> + * \param proxy The proxy object
>>>> + * \return The protocol object version of the proxy
>>>> + *
>>>> + * \memberof wl_proxy
>>>> + */
>>>> +WL_EXPORT uint32_t
>>>> +wl_proxy_get_version(struct wl_proxy *proxy)
>>>> +{
>>>> + return proxy->version;
>>>> +}
>>>> +
>>>> /** Get the id of a proxy object
>>>> *
>>>> * \param proxy The proxy object
>>>> --
>>>> 2.6.2
>>>>
>>>> _______________________________________________
>>>> wayland-devel mailing list
>>>> wayland-devel at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>
More information about the wayland-devel
mailing list