[PATCH wayland] protocol: Add a surface_has_existing_role error

[Jasper St. Pierre]

I think it would be a pain to do that, and might not be clear for wl_seat
and wl_data_device. If you believe in it, all power to you.
On 22 Sep 2014 02:10, "Pekka Paalanen" <ppaalanen at gmail.com> wrote:

> On Mon, 25 Aug 2014 08:59:51 +0300
> Pekka Paalanen <ppaalanen at gmail.com> wrote:
>
> > On Fri, 22 Aug 2014 10:48:02 -0700
> > Jason Ekstrand <jason at jlekstrand.net> wrote:
> >
> > > As I mentioned on IRC, I don't really like adding and using an enum
> value
> > > without bumping protocol version.  However, the only time we ever use
> it is
> > > to kill the client so the worst thing that can happen is that the
> client
> > > doesn't report the error correctly.  I think I'm ok with it.
> >
> > Yeah, I don't see a problem in adding values to an error enum.
> >
> > However, I do wonder if this is logical.
> >
> > Let's say xdg_shell used this. A client issues request
> > xdg_shell at 33.get_xdg_surface(wl_surface at 20). There are quite likely
> > several other requests on wl_surface at 20, and since I assume
> > get_xdg_surface will be raising a protocol error, there will also be an
> > earlier request on some other interface already giving a role to
> > wl_surface at 20. And all these requests can be in the same burst.
> >
> > Using the below enum value, the compositor would send the error on
> > wl_surface at 20, but you don't know which interface call caused it.
> >
> > Using an error enum value in xdg_shell, the compositor would be sending
> > the error on xdg_shell at 33, but you don't know for which surface.
> >
> > One solution to both "don't know"s is to add the missing information to
> > the error message for humans to see.
> >
> > Both ways could work in practice, but this would be the first (legal)
> > case of sending the error on a different object than whose request
> > caused it to trigger.
> >
> > It may make sense in this one case, but if we do it here, we generally
> > allow it everywhere, which I believe can bring confusion on what errors
> > can be raised where. It will no longer be true, that the possible
> > errors are the ones defined in the interface containing the request,
> > plus wl_display errors. You will be also adding all errors on all
> > argument objects to the set, per request.
> >
> > That is why I am hesitant to commit this. I'm sure someone will find
> > a case where this would make something like writing language bindings
> > more difficult for him, even if it seems obscure or even silly to us.
> >
> > It's not like we share e.g. pixel format enums between, say, wl_shm and
> > wl_drm, even though they are the same AFAIK, and there is a
> > considerable amount of duplication. (Yes, that is a bit different case,
> > because there we would be breaking also the namespacing.)
> >
> >
> > Thanks,
> > pq
> >
> > > On Fri, Aug 22, 2014 at 10:37 AM, Jasper St. Pierre <
> jstpierre at mecheye.net>
> > > wrote:
> > >
> > > > This will be used by implementations to send out errors if clients
> try
> > > > to set roles on surfaces that already have roles.
> > > > ---
> > > >  protocol/wayland.xml | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > > index bb457bc..f6eb54d 100644
> > > > --- a/protocol/wayland.xml
> > > > +++ b/protocol/wayland.xml
> > > > @@ -983,6 +983,7 @@
> > > >        </description>
> > > >        <entry name="invalid_scale" value="0" summary="buffer scale
> value
> > > > is invalid"/>
> > > >        <entry name="invalid_transform" value="1" summary="buffer
> transform
> > > > value is invalid"/>
> > > > +      <entry name="surface_has_existing_role" value="2" summary="the
> > > > surface passed already has a role"/>
> > > >      </enum>
>
> Hi,
>
> does anyone still want to defend this patch, or should we add "existing
> role" error definitions to all interfaces assigning roles instead?
>
>
> Thanks,
> pq
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140922/51501901/attachment.html>