[RFC PATCH wayland-protocols] presentation: New protocol for presenting surfaces to the user

Fri Oct 18 07:56:49 UTC 2019

On Fri, 18 Oct 2019 01:08:36 +0200
Carlos Garnacho <carlosg at gnome.org> wrote:

> Hi Pekka,
> 
> Thank you for your comments!

Hi Carlos,

I pretty much agreed with all your responses, just few things to
elaborate further below.

> 
> On Wed, Oct 16, 2019 at 11:31 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 
> > On Wed, 16 Oct 2019 10:54:08 +0200
> > Olivier Fourdan <ofourdan at redhat.com> wrote:
> >  
> > > Hey Carlos,
> > >
> > > On Wed, Oct 16, 2019 at 10:43 AM Carlos Garnacho <carlosg at gnome.org>  
> > wrote:  
> > > >
> > > > This protocol takes over 3 different, but interrelated aspects of
> > > > DE/client interaction:
> > > > - Startup notification: presenting feedback about applications
> > > >   being launched, either through the compositor or another client
> > > > - Focus stealing prevention: letting the compositor figure out
> > > >   whether immediately switching focus to a surface makes sense.
> > > > - Window raising/activation: allowing existing clients to request
> > > >   focus in a controlled manner.
> > > >
> > > > Signed-off-by: Carlos Garnacho <carlosg at gnome.org>  
> >
> > Hi Carlos,
> >
> > this seems well written and a good idea to me. More comments inline.
> >  
> > > > ---
> > > >  Makefile.am                                   |   1 +
> > > >  unstable/presentation/README                  |   5 +
> > > >  .../presentation/presentation-unstable-v1.xml | 175 ++++++++++++++++++
> > > >  3 files changed, 181 insertions(+)
> > > >  create mode 100644 unstable/presentation/README
> > > >  create mode 100644 unstable/presentation/presentation-unstable-v1.xml

...

> > > > +  <interface name="zwp_presentation_manager" version="1">
> > > > +    <request name="create_presenter" since="1">
> > > > +      <description summary="create a new presenter">
> > > > +       Creates a new presenter.
> > > > +
> > > > +       The surface argument is the toplevel that initiated the  
> > presentation  
> > > > +       request through user input. Compositors may want to place the  
> > presented  
> > > > +       surface relative to the presenter.
> > > > +
> > > > +       Compositors that desire to implement focus stealing prevention
> > > > +       may use this request to find out the request time.  
> >
> > The serial argument should be explained here. It might be hard,
> > because we have been really poor at explaining the serials.
> >  
> 
> Will try :).
> 
> 
> >
> > Do we not also need to specify the wl_seat for which the input serial
> > is associated to?
> >  
> 
> Hm, right.
> 
> 
> >  
> > > > +      </description>
> > > > +      <arg name="id" type="new_id" interface="zwp_presenter"/>
> > > > +      <arg name="surface" type="object" interface="wl_surface"/>  
> >
> > If you use wl_surface as the type here, what about when the role is not
> > a toplevel? Should that result in a protocol error?
> >  
> 
> That sounds like a good idea.
> 
> 
> >
> > What defines a "toplevel"?
> >  
> 
> My guess is that xdg_toplevel / wl_shell_surface are the targets.

Right. We might want a sentence deferring the definition of "toplevel"
to other extensions and mention xdg_toplevel as an example - unless the
interface starts using xdg_toplevel specifically.

I would not care to add support for wl_shell stuff in new extensions.

> >  
> > > > +      <arg name="serial" type="uint" summary="the serial from the  
> > input event"/>  
> > > > +    </request>
> > > > +

...

> > > > +  <interface name="zwp_presenter" version="1">
> > > > +    <description summary="context for presenting a surface">
> > > > +      The wp_presenter object allows clients to get the necessary  
> > context to  
> > > > +      request that another surface or client should be presented to  
> > the user.  
> > > > +    </description>
> > > > +
> > > > +    <request name="set_app_id">
> > > > +      <description summary="sets the app ID of the launched  
> > application">  
> > > > +       Sets the app ID of the application to be launched, the  
> > compositor  
> > > > +       may use this information to look up other miscellaneous  
> > information  
> > > > +       about it (eg. translatable name, icon, …).
> > > > +
> > > > +       Clients do not need to set an app ID for presentation requests
> > > > +       meant to map surfaces owned by the same client.  
> >
> > "Do not need" or "must not"?
> >  
> 
> It's perhaps a good idea to stick to the latter.

Btw. if this mechanism is intended for cross-application activation
only, then the text about own surfaces would need to be dropped. I'm
referring to your PDF opening example.

> >
> > Is it possible to send an app ID that is obviously invalid and should
> > result in a protocol error?
> >  
> 
> That wording has been taken straight from xdg-shell, both should probably
> be kept in sync. Same restrictions apply, too :).

Ok. I was just wondering if it needs a protocol error defined.
Following xdg-shell is good I suppose.

> > > > +    </request>
> > > > +
> > > > +    <request name="destroy" type="destructor">
> > > > +      <description summary="destroy zwp_presenter">
> > > > +       Destroys this zwp_presenter object.
> > > > +      </description>
> > > > +    </request>
> > > > +
> > > > +    <event name="token" since="1">
> > > > +      <description summary="token for the presentation request">
> > > > +       Notifies of an unique presentation token (eg. UUIDs) to be  
> > used for the  
> > > > +       application about to be launched.  
> >
> > When is this event sent?
> >  
> 
> Right after the presenter is created. I've got a bit of a sore point here,
> since obtaining presenter+token requires a roundtrip on the client.

The roundtrip is necessary, since the compositor is creating the token
to ensure it is unique enough.

The spec should say when this event is sent, I didn't find that.

> > > > +
> > > > +       In order to guarantee interoperation with the XDG Startup  
> > Notification  
> > > > +       spec, this launch_id is recommended to be transmitted to the  
> > launched  
> > > > +       application through the DESKTOP_STARTUP_ID environment  
> > variable. The  
> > > > +       launch ID may be passed to a running client through other  
> > means not  
> > > > +       observed in this protocol.
> > > > +      </description>
> > > > +      <arg name="token" type="string"/>
> > > > +    </event>
> > > > +
> > > > +    <event name="cancelled" since="1">
> > > > +      <description summary="the presenter has expired">
> > > > +       Notifies that the compositor is no longer watching this  
> > launched  
> > > > +       application. This may indicate failure (eg. launchee crashed)  
> > or  
> > > > +       may simply be the result of the launchee not replying properly
> > > > +       (eg. does not implement this protocol).  
> >
> > Ok. Should there be also a request to cancel this from the client side
> > in case the client does the launching and sees that e.g. exec() failed?
> > Or does the destructor request imply that?
> >  
> 
> Yup, the idea was making it implicit on destroy.

Cool, make sure to define that in the spec.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20191018/f891c588/attachment.sig>