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

Thu Oct 17 23:08:36 UTC 2019

Hi Pekka,

Thank you for your comments!

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
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index d2c89a8..bd0e52d 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -24,6 +24,7 @@ unstable_protocols =
>                           \
> > >         unstable/xdg-decoration/xdg-decoration-unstable-v1.xml  \
> > >
>  unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> \
> > >         unstable/primary-selection/primary-selection-unstable-v1.xml
>           \
> > > +       unstable/presentation/presentation-unstable-v1.xml
>           \
> > >         $(NULL)
> > >
> > >  stable_protocols =
>          \
> > > diff --git a/unstable/presentation/README
> b/unstable/presentation/README
> > > new file mode 100644
> > > index 0000000..5a77e97
> > > --- /dev/null
> > > +++ b/unstable/presentation/README
> > > @@ -0,0 +1,5 @@
> > > +Presentation protocol
> > > +
> > > +Maintainers:
> > > +Carlos Garnacho <carlosg at gnome.org>
> > > +
> > > diff --git a/unstable/presentation/presentation-unstable-v1.xml
> b/unstable/presentation/presentation-unstable-v1.xml
> > > new file mode 100644
> > > index 0000000..84bf961
> > > --- /dev/null
> > > +++ b/unstable/presentation/presentation-unstable-v1.xml
> > > @@ -0,0 +1,175 @@
> > > +<?xml version="1.0" encoding="UTF-8"?>
> > > +<protocol name="presentation_v1">
> > > +  <copyright>
> > > +    Copyright 2018-2019 © Red Hat, Inc.
> > > +
> > > +    Permission is hereby granted, free of charge, to any person
> > > +    obtaining a copy of this software and associated documentation
> files
> > > +    (the "Software"), to deal in the Software without restriction,
> > > +    including without limitation the rights to use, copy, modify,
> merge,
> > > +    publish, distribute, sublicense, and/or sell copies of the
> Software,
> > > +    and to permit persons to whom the Software is furnished to do so,
> > > +    subject to the following conditions:
> > > +
> > > +    The above copyright notice and this permission notice (including
> the
> > > +    next paragraph) shall be included in all copies or substantial
> > > +    portions of the Software.
> > > +
> > > +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > +    EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > +    MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > > +    NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> HOLDERS
> > > +    BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > > +    ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > > +    CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > > +    SOFTWARE.
> > > +  </copyright>
> > > +
> > > +  <description summary="Presentation request protocol">
> > > +    This description provides a high-level overview of the interplay
> between
> > > +    the interfaces defined this protocol. For details, see the
> protocol
> > > +    specification.
>
> This is the protocol specification, the whole file.
>

Uh, got that bit of the wording from zwp_tablet, removed locally :).

>
> > > +
> > > +    The finality of the protocol is defining a compositor context for
> > > +    surfaces requesting to be presented. Presentation requests are
> usually
> > > +    initiated by an existing surface, and acknowledged by a surface
> being
> > > +    mapped. By having both ends talk with the compositor through this
> protocol,
> > > +    the compositor has the information to implement different
> commonplace
> > > +    policies:
> > > +
> > > +    - Startup notification: The compositor may track wp_presenter
> interfaces
> > > +      being created from the launcher side, and replied upon on the
> launchee
> > > +      side. Compositors may also perform the application launcher role
> > > +      themselves.
>
> Ok, I see that.
>
> > > +
> > > +    - Focus stealing prevention: The compositor may know whether
> there is
> > > +      recent user input that happened after the presentation request,
> and
> > > +      ensure no disruptions happen.
>
> This might need elaboration on how this is supposed to work.
>

Indeed, I will describe how it could work.

>
> > > +
> > > +    - Window raising/activation: The presented surface may be another
> previously
> > > +      existing one, which might require bringing it to focus.
>
> Why do a roundtrip and play with tokens, when there could be a simple
> request with an input serial and a surface to be raised/activated?
>

This was made so to cover the activation of foreign surfaces (eg. opening a
pdf in a file browser, with the pdf view being a single instance app
already opened in the background or other workspace).

In the gnome/gtk world, the main instance of the application would
typically receive the necessary environment via DBus from within
GtkApplication, so it may be that the requestor and the presented surface
are from different clients.

I will add a blurb in this regard.

>
> > > +
> > > +    A client that wishes to present another surface (of its own or
> from another
> > > +    client) will call wp_presentation_manager.create_presenter to
> create a
> > > +    presentation request. Compositors may use this object to track
> the source of
> > > +    the request in order to apply its policies when mapping the
> surface or
> > > +    bringing an existing one to focus.
> > > +
> > > +    In the presented surface side, the request token will be notified
> upon
> > > +    through the wp_presentation_manager.acknowledge request. The
> method to pass
> > > +    the token across clients is considered an implementation detail,
> and is
> > > +    explicitly not observed here.
> > > +
> > > +    Upon a successfully acknowledged presentation token, the client
> will receive
> > > +    a wp_presenter.done event. In the rare cases that launching an
> application
> > > +    would fail, the compositor may emit a wp_presenter.cancelled event
> > > +    after a reasonable timeout.
> > > +  </description>
> > > +
> > > +  <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.

>
> > > +      <arg name="serial" type="uint" summary="the serial from the
> input event"/>
> > > +    </request>
> > > +
> > > +    <request name="acknowledge" since="1">
> > > +      <description summary="acknowledges a presentation token">
> > > +       Acknowledges that the calling client handled the presentation
> token.
> > > +
> > > +       Applications will typically receive this through the
> DESKTOP_STARTUP_ID
> > > +       environment variable as set by its launcher, and should unset
> the
> > > +       environment variable right after this request, in order to
> avoid
> > > +       propagating it to child processes.
> > > +
> > > +       Compositors will ignore unknown presentation tokens.
> > > +
> > > +       Presentation tokens may be transferred across clients through
> means not
> > > +       described in this protocol.
> > > +      </description>
> > > +      <arg name="token" type="string"/>
> > > +      <arg name="surface" type="object" interface="wl_surface"/>
>
> Is this required to be a toplevel as well?
>

I think so, yes.

>
> Does the ack always need a surface? Could something be launched into
> background (e.g. systray, whatever, might not even be a wl_surface)?
>

As replied to Dorota, if we use usecases for it, sure, I just didn't see
any.

>
> > > +    </request>
> > > +
> > > +    <request name="destroy" type="destructor">
> > > +      <description summary="release the memory of the presentation
> manager object">
> > > +       Destroy the wp_presentation_manager object. Objects created
> from this
> > > +       object are unaffected and should be destroyed separately.
> > > +      </description>
> > > +    </request>
> > > +  </interface>
> > > +
> > > +  <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.

>
> > > +
> > > +       As a best practice, it is suggested to select app
> > > +       ID's that match the basename of the application's .desktop
> file.
> > > +       For example, "org.freedesktop.FooViewer" where the .desktop
> file is
> > > +       "org.freedesktop.FooViewer.desktop".
> > > +
> > > +       See the desktop-entry specification [0] for more details on
> > > +       application identifiers and how they relate to .desktop files.
> > > +
> > > +       [0] http://standards.freedesktop.org/desktop-entry-spec/
> > > +      </description>
>
> Missing arguments?
>

Doh!

>
> 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 :).

>
> > > +    </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.

>
> > > +
> > > +       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.

>
> > > +      </description>
> > > +    </event>
> > > +
> > > +    <event name="done" since="1">
> > > +      <description summary="the presentation was performed">
> > > +       Notifies that the launched application successfully called
> > > +       zwp_presentation_manager.acknowledge.
> > > +      </description>
> > > +    </event>
> > > +  </interface>
> > > +</protocol>
> > > --
> > > 2.23.0
> >
> > The name itself, “presentation” might be confusing considering we
> > already have a “presentation-time” protocol.
>
> Yeah. Also, should this aim into the xdg namespace and use xdg_toplevel
> instead of wl_surface?
>

That's probably a good idea :).

Cheers,
  Carlos
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20191018/c935d4e3/attachment-0001.html>