client: add wl_proxy_get_version function

Submitted by Rémi Denis-Courmont on Oct. 5, 2014, 9:02 a.m.

Details

Message ID 1412499739-24778-1-git-send-email-remi@remlab.net
State Rejected
Headers show

Not browsing as part of any series.

Commit Message

Rémi Denis-Courmont Oct. 5, 2014, 9:02 a.m.
In some scenarii, a component will obtain a reference to a proxy
object created by another component. It may then be necessary to
check the interface version of the proxy object at run-time to
determine if a certain interface request is supported by the proxy
object.

For instance, a media player GUI can pass a wl_surface to a video
playback library. If the video is rotated, the library will want to
use wl_surface.set_buffer_transform, which is only available in
version 2 of wl_surface.

Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
---
 src/wayland-client.c | 13 +++++++++++++
 src/wayland-client.h |  1 +
 2 files changed, 14 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..51216ae 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1721,6 +1721,19 @@  wl_proxy_get_class(struct wl_proxy *proxy)
 	return proxy->object.interface->name;
 }
 
+/** Get the interface version of a proxy object
+ *
+ * \param proxy The proxy object
+ * \return The interface version of the object associated with the proxy
+ *
+ * \memberof wl_proxy
+ */
+WL_EXPORT uint32_t
+wl_proxy_get_version(struct wl_proxy *proxy)
+{
+	return proxy->object.interface->version;
+}
+
 /** Assign a proxy to an event queue
  *
  * \param proxy The proxy object
diff --git a/src/wayland-client.h b/src/wayland-client.h
index 0801a81..60d87e6 100644
--- a/src/wayland-client.h
+++ b/src/wayland-client.h
@@ -146,6 +146,7 @@  void wl_proxy_set_user_data(struct wl_proxy *proxy, void *user_data);
 void *wl_proxy_get_user_data(struct wl_proxy *proxy);
 uint32_t wl_proxy_get_id(struct wl_proxy *proxy);
 const char *wl_proxy_get_class(struct wl_proxy *proxy);
+uint32_t wl_proxy_get_version(struct wl_proxy *proxy);
 void wl_proxy_set_queue(struct wl_proxy *proxy, struct wl_event_queue *queue);
 
 #include "wayland-client-protocol.h"

Comments

Nevermind, that gets the version number of the client protocol library, not 
the version actually instantiated for the object :-(
Remi,
While this would probably be nice, your approach isn't going to work.  The
version provided in wl_interface is the version that was compiled into
libwayland, not the version requested when the global was bound.  In order
to get this correct, it requires a bit more tracking.  I did implement this
a while ago but it never got merged.  You can find it here:

http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html

--Jason Ekstrand

On Sun, Oct 5, 2014 at 2:02 AM, Rémi Denis-Courmont <remi@remlab.net> wrote:

> In some scenarii, a component will obtain a reference to a proxy
> object created by another component. It may then be necessary to
> check the interface version of the proxy object at run-time to
> determine if a certain interface request is supported by the proxy
> object.
>
> For instance, a media player GUI can pass a wl_surface to a video
> playback library. If the video is rotated, the library will want to
> use wl_surface.set_buffer_transform, which is only available in
> version 2 of wl_surface.
>
> Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
> ---
>  src/wayland-client.c | 13 +++++++++++++
>  src/wayland-client.h |  1 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index b0f77b9..51216ae 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -1721,6 +1721,19 @@ wl_proxy_get_class(struct wl_proxy *proxy)
>         return proxy->object.interface->name;
>  }
>
> +/** Get the interface version of a proxy object
> + *
> + * \param proxy The proxy object
> + * \return The interface version of the object associated with the proxy
> + *
> + * \memberof wl_proxy
> + */
> +WL_EXPORT uint32_t
> +wl_proxy_get_version(struct wl_proxy *proxy)
> +{
> +       return proxy->object.interface->version;
> +}
> +
>  /** Assign a proxy to an event queue
>   *
>   * \param proxy The proxy object
> diff --git a/src/wayland-client.h b/src/wayland-client.h
> index 0801a81..60d87e6 100644
> --- a/src/wayland-client.h
> +++ b/src/wayland-client.h
> @@ -146,6 +146,7 @@ void wl_proxy_set_user_data(struct wl_proxy *proxy,
> void *user_data);
>  void *wl_proxy_get_user_data(struct wl_proxy *proxy);
>  uint32_t wl_proxy_get_id(struct wl_proxy *proxy);
>  const char *wl_proxy_get_class(struct wl_proxy *proxy);
> +uint32_t wl_proxy_get_version(struct wl_proxy *proxy);
>  void wl_proxy_set_queue(struct wl_proxy *proxy, struct wl_event_queue
> *queue);
>
>  #include "wayland-client-protocol.h"
> --
> 2.1.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
Hello,

Le 2014-10-06 03:29, Jason Ekstrand a écrit :
> Remi,
> While this would probably be nice, your approach isnt going to work. 

Yeah, I figured that much after I submitted the patch. And then I 
rediscovered that the client library does not actually know the version 
of the project objects, and wl_registry_bind() cannot be modified 
without breaking backward compatibility.

But now, I am even wondering whether this makes any sense. Assuming 
wl_proxy_get_version() gets implemented "somehow", what should the 
library do if the proxy version is *higher* than version the library 
knows of? Say a new version Y of an interface FOO adds a new request and 
a new event compared to older version X. If a FOO object is instantiated 
with version Y but the new request is never used and there is no event 
handler for the new event, will it still work as with version X? I guess 
not :-(

In my specific case, can the Wayland client treat a wl_surface version 
4 or higher as a wl_surface version 3? If not, I cannot rely on 
'wl_proxy_get_version(surface) >= 2'...

Also somewhat related to this question, what happens (or should happen) 
if a Wayland client binds a version supported by the display server, but 
not supported by the client's protocol bindings? That would happen if 
the client blindly copies the version from wl_registry.global to 
wl_registry.bind.

(...)
> 
> http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html
> [4]

I see, thanks.
Re-adding CCs and some more...

On Mon, 06 Oct 2014 09:17:41 +0300
Rémi Denis-Courmont <remi@remlab.net> wrote:

>     Hello,
> 
> Le 2014-10-06 03:29, Jason Ekstrand a écrit :
> > Remi,
> > While this would probably be nice, your approach isnt going to work. 
> 
> Yeah, I figured that much after I submitted the patch. And then I 
> rediscovered that the client library does not actually know the version 
> of the project objects, and wl_registry_bind() cannot be modified 
> without breaking backward compatibility.
> 
> But now, I am even wondering whether this makes any sense. Assuming 
> wl_proxy_get_version() gets implemented "somehow", what should the 
> library do if the proxy version is *higher* than version the library 
> knows of? Say a new version Y of an interface FOO adds a new request and 
> a new event compared to older version X. If a FOO object is instantiated 
> with version Y but the new request is never used and there is no event 
> handler for the new event, will it still work as with version X? I guess 
> not :-(

If there is no event handler set, the client will abort/crash if the
event is ever sent.

> In my specific case, can the Wayland client treat a wl_surface version 
> 4 or higher as a wl_surface version 3? If not, I cannot rely on 
> 'wl_proxy_get_version(surface) >= 2'...

Changing the semantics[5] of wl_surface.damage is a good example where
this would fail indeed. If a library only knows up to version 3, but
the app gives it a wl_surface of version 4, the damage coordinates
would be wrong.

Originally we had the idea, that all version bumps are backwards
compatible. Simply testing for version >= X would always work. The
wl_surface.damage change is not backwards compatible in that sense.

Looks to me like if we don't fix damage, then this versioning problem
would be solvable... :-(

> Also somewhat related to this question, what happens (or should happen) 
> if a Wayland client binds a version supported by the display server, but 
> not supported by the client's protocol bindings? That would happen if 
> the client blindly copies the version from wl_registry.global to 
> wl_registry.bind.

Such blind copy is a bug: a client will always need some code changes
to support new features added to an interface, so it always needs to
bind with MIN(advertized, my_supported). And my_supported cannot be
higher than the available XML definition during client build. But here
is a problem with client-side libraries: my_supported cannot be higher
than the supported version in any used library, if not all protocol
version bumps are backward compatible.

And even then, I can imagine a case where it would still fail: app
creates version 5 of a global, hands the global to a library that only
knows up to version 4. The library creates a new object with the
version 4 interface, automatically gets version *5* new object, the new
object delivers an event added in version 5, kaboom.

A compositor does not advertise a version that libwayland-server does
not support, but that includes the assumption that both server and
client use the same libwayland version... which might not be true.

And it's not really remedied by adding new protocols as XML-only into
libwayland, either. Such protocols would simply be built in to the
users, i.e. the client apps and libraries, which brings a problem if a
library is built with version lower than what the app uses.

Oh dear... :-/

I'm not sure there is any way around *bidirectional* explicit version
negotiation between an app the libraries it uses. Bidirectional means
that the library tells the max versions it can support and the app
needs to obey that, and any objects the app passes to the library needs
to pass the version, too. Well, the latter case might be able to be
automated via wl_proxy_get_version(), but not the former AFAICS.

> (...)
> > 
> > http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html
> > [4]
> 
> I see, thanks.
> 

Thanks,
pq


[5] https://bugs.freedesktop.org/show_bug.cgi?id=78190
On Mon, 6 Oct 2014 10:45:14 +0300
Pekka Paalanen <ppaalanen@gmail.com> wrote:

> Re-adding CCs and some more...
> 
> On Mon, 06 Oct 2014 09:17:41 +0300
> Rémi Denis-Courmont <remi@remlab.net> wrote:
> 
> >     Hello,
> > 
> > Le 2014-10-06 03:29, Jason Ekstrand a écrit :
> > > Remi,
> > > While this would probably be nice, your approach isnt going to work. 
> > 
> > Yeah, I figured that much after I submitted the patch. And then I 
> > rediscovered that the client library does not actually know the version 
> > of the project objects, and wl_registry_bind() cannot be modified 
> > without breaking backward compatibility.
> > 
> > But now, I am even wondering whether this makes any sense. Assuming 
> > wl_proxy_get_version() gets implemented "somehow", what should the 
> > library do if the proxy version is *higher* than version the library 
> > knows of? Say a new version Y of an interface FOO adds a new request and 
> > a new event compared to older version X. If a FOO object is instantiated 
> > with version Y but the new request is never used and there is no event 
> > handler for the new event, will it still work as with version X? I guess 
> > not :-(
> 
> If there is no event handler set, the client will abort/crash if the
> event is ever sent.
> 
> > In my specific case, can the Wayland client treat a wl_surface version 
> > 4 or higher as a wl_surface version 3? If not, I cannot rely on 
> > 'wl_proxy_get_version(surface) >= 2'...
> 
> Changing the semantics[5] of wl_surface.damage is a good example where
> this would fail indeed. If a library only knows up to version 3, but
> the app gives it a wl_surface of version 4, the damage coordinates
> would be wrong.
> 
> Originally we had the idea, that all version bumps are backwards
> compatible. Simply testing for version >= X would always work. The
> wl_surface.damage change is not backwards compatible in that sense.
> 
> Looks to me like if we don't fix damage, then this versioning problem
> would be solvable... :-(

Oh I'm being silly, we only need to fix damage in a backward compatible
way: add a new request, don't modify the behaviour of the old one.

Keeping backward compatibility wrt. version >= X checks seems like a
good idea and should reduce the problem set somewhat.

> [5] https://bugs.freedesktop.org/show_bug.cgi?id=78190


Thanks,
pq
Le 2014-10-06 11:26, Pekka Paalanen a écrit :
>> > 4 or higher as a wl_surface version 3? If not, I cannot rely on
>> > 'wl_proxy_get_version(surface) >= 2'...
>>
>> Changing the semantics[5] of wl_surface.damage is a good example 
>> where
>> this would fail indeed. If a library only knows up to version 3, but
>> the app gives it a wl_surface of version 4, the damage coordinates
>> would be wrong.
>>
>> Originally we had the idea, that all version bumps are backwards
>> compatible. Simply testing for version >= X would always work. The
>> wl_surface.damage change is not backwards compatible in that sense.
>>
>> Looks to me like if we don't fix damage, then this versioning 
>> problem
>> would be solvable... :-(
>
> Oh I'm being silly, we only need to fix damage in a backward 
> compatible
> way: add a new request, don't modify the behaviour of the old one.
>
> Keeping backward compatibility wrt. version >= X checks seems like a
> good idea and should reduce the problem set somewhat.

You could stick to adding new requests rather than redefine existing 
ones.

But what about events. AFAICT, they would still require either:
- never adding events in new versions,
  or
   - adding events that only fire after a new requests was sent,
    and
   - never copying the listener structure ever in the client library
     (to prevent out-of-bounds read).

Otherwise, it would still fail the code path setting the object 
listener targets an older interface version than the code path creating 
the object.
On Mon, 06 Oct 2014 11:43:56 +0300
Rémi Denis-Courmont <remi@remlab.net> wrote:

> Le 2014-10-06 11:26, Pekka Paalanen a écrit :
> >> > 4 or higher as a wl_surface version 3? If not, I cannot rely on
> >> > 'wl_proxy_get_version(surface) >= 2'...
> >>
> >> Changing the semantics[5] of wl_surface.damage is a good example 
> >> where
> >> this would fail indeed. If a library only knows up to version 3, but
> >> the app gives it a wl_surface of version 4, the damage coordinates
> >> would be wrong.
> >>
> >> Originally we had the idea, that all version bumps are backwards
> >> compatible. Simply testing for version >= X would always work. The
> >> wl_surface.damage change is not backwards compatible in that sense.
> >>
> >> Looks to me like if we don't fix damage, then this versioning 
> >> problem
> >> would be solvable... :-(
> >
> > Oh I'm being silly, we only need to fix damage in a backward 
> > compatible
> > way: add a new request, don't modify the behaviour of the old one.
> >
> > Keeping backward compatibility wrt. version >= X checks seems like a
> > good idea and should reduce the problem set somewhat.
> 
> You could stick to adding new requests rather than redefine existing 
> ones.
> 
> But what about events. AFAICT, they would still require either:
> - never adding events in new versions,
>   or
>    - adding events that only fire after a new requests was sent,
>     and
>    - never copying the listener structure ever in the client library
>      (to prevent out-of-bounds read).
> 
> Otherwise, it would still fail the code path setting the object 
> listener targets an older interface version than the code path creating 
> the object.

Is there any case where one would copy a listener struct?

Currently we rely on the server knowing the supported interface
version, so that it will never attempt to send an event the client
cannot handle. Therefore using old listener structs that lack entries
should be safe, when the version is negotiated properly.

The remaining problem here is communicating the correct supported
version to the server, isn't it?

Could this be solved (completely?) by saying that libraries that take
existing wl_proxy objects such that they
a) set up listeners, or
b) create new wl_proxies from it and set their listeners;
need to also offer API to tell which versions of the passed in
wl_proxies they support?

We somehow implement the interface version inheritance in
libwayland-client and offer wl_proxy_get_version().

A client library can use wl_proxy_get_version() to check the
effective interface version as necessary, and if it does a) or b), fail
if the effective version is greater than what they support. That
failure would be a bug in the application, because it didn't take
the library's supported version range into account.

Does this make sense?
Would this be a complete solution?
Any other ideas?

As a detail, this should work just fine for EGL which takes existing
wl_surface proxies. EGL does not set up any listeners, and it only
creates wl_callback objects through the passed-in wl_surface proxy,
which cannot cause a version mismatch.

It would be nice to be able to add events to interfaces, and we have
already done that, so forbidding it now is probably too much.

Requiring an event-opt-in request to be sent probably does not solve
anything either. It is quite equivalent to the effective interface
version communicated to the server, and app code can send it anyway
before giving the wl_proxy to a library.


Thanks,
pq
Le 2014-10-06 12:52, Pekka Paalanen a écrit :
> Could this be solved (completely?) by saying that libraries that take
> existing wl_proxy objects such that they
> a) set up listeners, or
> b) create new wl_proxies from it and set their listeners;
> need to also offer API to tell which versions of the passed in
> wl_proxies they support?

I think that works in principles. I am not sure how easy it would be to 
convince all the toolkit vendors (GTK, Qt...) to support that though. 
It's not so easy for the toolkit. Currently, I'd guess they just create 
a surface. Instead they would have to:
- add a Wayland-specific version parameter to the widget/whatever 
factory, then
- obtain desired surface version from the application,
- create a new registry object,
- bind a new compositor with the desired version,
- destroy the registry object,
- allocate the surface with the desired version,
- destroy the compositor object.
On Oct 6, 2014 12:45 AM, "Pekka Paalanen" <ppaalanen@gmail.com> wrote:
>
> Re-adding CCs and some more...
>
> On Mon, 06 Oct 2014 09:17:41 +0300
> Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> >     Hello,
> >
> > Le 2014-10-06 03:29, Jason Ekstrand a écrit :
> > > Remi,
> > > While this would probably be nice, your approach isnt going to work.
> >
> > Yeah, I figured that much after I submitted the patch. And then I
> > rediscovered that the client library does not actually know the version
> > of the project objects, and wl_registry_bind() cannot be modified
> > without breaking backward compatibility.
> >
> > But now, I am even wondering whether this makes any sense. Assuming
> > wl_proxy_get_version() gets implemented "somehow", what should the
> > library do if the proxy version is *higher* than version the library
> > knows of? Say a new version Y of an interface FOO adds a new request and
> > a new event compared to older version X. If a FOO object is instantiated
> > with version Y but the new request is never used and there is no event
> > handler for the new event, will it still work as with version X? I guess
> > not :-(
>
> If there is no event handler set, the client will abort/crash if the
> event is ever sent.
>
> > In my specific case, can the Wayland client treat a wl_surface version
> > 4 or higher as a wl_surface version 3? If not, I cannot rely on
> > 'wl_proxy_get_version(surface) >= 2'...

You have to do some sort of negotiation between The client and the
library.  If you have the get_version stuff you can enforce it somewhat by
giving the client an error if it goes too high.

>
> Changing the semantics[5] of wl_surface.damage is a good example where
> this would fail indeed. If a library only knows up to version 3, but
> the app gives it a wl_surface of version 4, the damage coordinates
> would be wrong.
>
> Originally we had the idea, that all version bumps are backwards
> compatible. Simply testing for version >= X would always work. The
> wl_surface.damage change is not backwards compatible in that sense.
>
> Looks to me like if we don't fix damage, then this versioning problem
> would be solvable... :-(

I'll respond to that on the bug.  I don't want to side-track this
conversation with damage.

>
> > Also somewhat related to this question, what happens (or should happen)
> > if a Wayland client binds a version supported by the display server, but
> > not supported by the client's protocol bindings? That would happen if
> > the client blindly copies the version from wl_registry.global to
> > wl_registry.bind.
>
> Such blind copy is a bug: a client will always need some code changes
> to support new features added to an interface, so it always needs to
> bind with MIN(advertized, my_supported). And my_supported cannot be
> higher than the available XML definition during client build. But here
> is a problem with client-side libraries: my_supported cannot be higher
> than the supported version in any used library, if not all protocol
> version bumps are backward compatible.
>
> And even then, I can imagine a case where it would still fail: app
> creates version 5 of a global, hands the global to a library that only
> knows up to version 4. The library creates a new object with the
> version 4 interface, automatically gets version *5* new object, the new
> object delivers an event added in version 5, kaboom.
>
> A compositor does not advertise a version that libwayland-server does
> not support, but that includes the assumption that both server and
> client use the same libwayland version... which might not be true.
>
> And it's not really remedied by adding new protocols as XML-only into
> libwayland, either. Such protocols would simply be built in to the
> users, i.e. the client apps and libraries, which brings a problem if a
> library is built with version lower than what the app uses.
>
> Oh dear... :-/
>
> I'm not sure there is any way around *bidirectional* explicit version
> negotiation between an app the libraries it uses. Bidirectional means
> that the library tells the max versions it can support and the app
> needs to obey that, and any objects the app passes to the library needs
> to pass the version, too. Well, the latter case might be able to be
> automated via wl_proxy_get_version(), but not the former AFAICS.

Yes, I don't think we can get around that.
--Jason

>
> > (...)
> > >
> > >
http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html
> > > [4]
> >
> > I see, thanks.
> >
>
> Thanks,
> pq
>
>
> [5] https://bugs.freedesktop.org/show_bug.cgi?id=78190
On Mon, 06 Oct 2014 13:09:05 +0300
Rémi Denis-Courmont <remi@remlab.net> wrote:

> Le 2014-10-06 12:52, Pekka Paalanen a écrit :
> > Could this be solved (completely?) by saying that libraries that take
> > existing wl_proxy objects such that they
> > a) set up listeners, or
> > b) create new wl_proxies from it and set their listeners;
> > need to also offer API to tell which versions of the passed in
> > wl_proxies they support?
> 
> I think that works in principles. I am not sure how easy it would be to 
> convince all the toolkit vendors (GTK, Qt...) to support that though. 
> It's not so easy for the toolkit. Currently, I'd guess they just create 
> a surface. Instead they would have to:
> - add a Wayland-specific version parameter to the widget/whatever 
> factory, then

Dealing out wl objects is Wayland-specific, anyway.

> - obtain desired surface version from the application,

Yes.

> - create a new registry object,

The toolkit already has one, no need for another.

> - bind a new compositor with the desired version,

Yes, if there is not one already.

> - destroy the registry object,

Not needed.

> - allocate the surface with the desired version,

Yes.

> - destroy the compositor object.

If it wants to.

Also, in addition to version, such API in toolkits should define also:
- if listener is already set or not, and
- which event queue the wl_proxy is on when delivered, and is it
  allowed to change that.

Well, I think the API needs to define everything about the object:
(when) will the toolkit touch it; if the toolkit sets the listeners how
it handles events; etc. I believe it is far from simple. It's not
limited only to the immediate properties, but also who is allowed to
set a role for the wl_surface, who may create associate objects like
wl_viewport, and so on.

Whatever will be using the "exported" wl_proxy will likely have its own
requirements on these things.

Going on a bit of a tangent here:

There is also a further complication with events like wl_surface.enter
that carry an object as an argument. To handle the argument of
wl_surface.enter, the client needs to have bound to the wl_output
global. Then the wl_output proxy comes in as an argument of the event,
and the code handling the event needs to detect if the proxy's
user_data is set by this code or some other code. If it's not set by
this code, then you don't know what it is, and cannot dereference it.
It is possible to bind to the same global multiple times, so each code
"module" can have its own if necessary, but everyone needs to be able
to detect their own user_data.

Input events may carry the targeted wl_surface as an argument. There it
is even more important to detect "our own user_data" in case some 3rd
party component creates its own wl_surfaces without telling the
toolkit. Unless the toolkit simply forbids that.


Thanks,
pq