On 25/11/15 12:25 PM, Jason Ekstrand wrote: > On Wed, Nov 25, 2015 at 10:15 AM, Derek Foreman <[email protected]> > wrote: >> On 13/11/15 12:21 PM, Jason Ekstrand wrote: >>> On Fri, Nov 13, 2015 at 3:18 AM, Giulio Camuffo <[email protected]> >>> 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.
Ugh. Ok. That's a bit tricky. I absolutely don't have any better ideas than your suggestion of setting wl_display's proxy_version to 0 and letting all the old-client-created objects show as version 0. Thanks for the excellent explanation, Derek >> 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 <[email protected]>: >>>>> From: Jason Ekstrand <[email protected]> >>>>> >>>>> 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 >>>>> [email protected] >>>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel >>> _______________________________________________ >>> wayland-devel mailing list >>> [email protected] >>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel >>> >> > _______________________________________________ > wayland-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
