<div dir="ltr"><div dir="ltr">Hi Pekka,</div><div dir="ltr"><br></div><div>Thank you for your comments!</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 16, 2019 at 11:31 AM Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">On Wed, 16 Oct 2019 10:54:08 +0200<br>
Olivier Fourdan <<a href="mailto:ofourdan@redhat.com" target="_blank">ofourdan@redhat.com</a>> wrote:<br>
<br>
> Hey Carlos,<br>
> <br>
> On Wed, Oct 16, 2019 at 10:43 AM Carlos Garnacho <<a href="mailto:carlosg@gnome.org" target="_blank">carlosg@gnome.org</a>> wrote:<br>
> ><br>
> > This protocol takes over 3 different, but interrelated aspects of<br>
> > DE/client interaction:<br>
> > - Startup notification: presenting feedback about applications<br>
> >   being launched, either through the compositor or another client<br>
> > - Focus stealing prevention: letting the compositor figure out<br>
> >   whether immediately switching focus to a surface makes sense.<br>
> > - Window raising/activation: allowing existing clients to request<br>
> >   focus in a controlled manner.<br>
> ><br>
> > Signed-off-by: Carlos Garnacho <<a href="mailto:carlosg@gnome.org" target="_blank">carlosg@gnome.org</a>><br>
<br>
Hi Carlos,<br>
<br>
this seems well written and a good idea to me. More comments inline.<br>
<br>
> > ---<br>
> >  Makefile.am                                   |   1 +<br>
> >  unstable/presentation/README                  |   5 +<br>
> >  .../presentation/presentation-unstable-v1.xml | 175 ++++++++++++++++++<br>
> >  3 files changed, 181 insertions(+)<br>
> >  create mode 100644 unstable/presentation/README<br>
> >  create mode 100644 unstable/presentation/presentation-unstable-v1.xml<br>
> ><br>
> > diff --git a/Makefile.am b/Makefile.am<br>
> > index d2c89a8..bd0e52d 100644<br>
> > --- a/Makefile.am<br>
> > +++ b/Makefile.am<br>
> > @@ -24,6 +24,7 @@ unstable_protocols =                                                          \<br>
> >         unstable/xdg-decoration/xdg-decoration-unstable-v1.xml  \<br>
> >         unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \<br>
> >         unstable/primary-selection/primary-selection-unstable-v1.xml            \<br>
> > +       unstable/presentation/presentation-unstable-v1.xml                      \<br>
> >         $(NULL)<br>
> ><br>
> >  stable_protocols =                                                             \<br>
> > diff --git a/unstable/presentation/README b/unstable/presentation/README<br>
> > new file mode 100644<br>
> > index 0000000..5a77e97<br>
> > --- /dev/null<br>
> > +++ b/unstable/presentation/README<br>
> > @@ -0,0 +1,5 @@<br>
> > +Presentation protocol<br>
> > +<br>
> > +Maintainers:<br>
> > +Carlos Garnacho <<a href="mailto:carlosg@gnome.org" target="_blank">carlosg@gnome.org</a>><br>
> > +<br>
> > diff --git a/unstable/presentation/presentation-unstable-v1.xml b/unstable/presentation/presentation-unstable-v1.xml<br>
> > new file mode 100644<br>
> > index 0000000..84bf961<br>
> > --- /dev/null<br>
> > +++ b/unstable/presentation/presentation-unstable-v1.xml<br>
> > @@ -0,0 +1,175 @@<br>
> > +<?xml version="1.0" encoding="UTF-8"?><br>
> > +<protocol name="presentation_v1"><br>
> > +  <copyright><br>
> > +    Copyright 2018-2019 © Red Hat, Inc.<br>
> > +<br>
> > +    Permission is hereby granted, free of charge, to any person<br>
> > +    obtaining a copy of this software and associated documentation files<br>
> > +    (the "Software"), to deal in the Software without restriction,<br>
> > +    including without limitation the rights to use, copy, modify, merge,<br>
> > +    publish, distribute, sublicense, and/or sell copies of the Software,<br>
> > +    and to permit persons to whom the Software is furnished to do so,<br>
> > +    subject to the following conditions:<br>
> > +<br>
> > +    The above copyright notice and this permission notice (including the<br>
> > +    next paragraph) shall be included in all copies or substantial<br>
> > +    portions of the Software.<br>
> > +<br>
> > +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,<br>
> > +    EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF<br>
> > +    MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND<br>
> > +    NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS<br>
> > +    BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN<br>
> > +    ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN<br>
> > +    CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE<br>
> > +    SOFTWARE.<br>
> > +  </copyright><br>
> > +<br>
> > +  <description summary="Presentation request protocol"><br>
> > +    This description provides a high-level overview of the interplay between<br>
> > +    the interfaces defined this protocol. For details, see the protocol<br>
> > +    specification.<br>
<br>
This is the protocol specification, the whole file.<br></blockquote><div><br></div><div>Uh, got that bit of the wording from zwp_tablet, removed locally :).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +<br>
> > +    The finality of the protocol is defining a compositor context for<br>
> > +    surfaces requesting to be presented. Presentation requests are usually<br>
> > +    initiated by an existing surface, and acknowledged by a surface being<br>
> > +    mapped. By having both ends talk with the compositor through this protocol,<br>
> > +    the compositor has the information to implement different commonplace<br>
> > +    policies:<br>
> > +<br>
> > +    - Startup notification: The compositor may track wp_presenter interfaces<br>
> > +      being created from the launcher side, and replied upon on the launchee<br>
> > +      side. Compositors may also perform the application launcher role<br>
> > +      themselves.<br>
<br>
Ok, I see that.<br>
<br>
> > +<br>
> > +    - Focus stealing prevention: The compositor may know whether there is<br>
> > +      recent user input that happened after the presentation request, and<br>
> > +      ensure no disruptions happen.<br>
<br>
This might need elaboration on how this is supposed to work.<br></blockquote><div><br></div><div>Indeed, I will describe how it could work.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +<br>
> > +    - Window raising/activation: The presented surface may be another previously<br>
> > +      existing one, which might require bringing it to focus.<br>
<br>
Why do a roundtrip and play with tokens, when there could be a simple<br>
request with an input serial and a surface to be raised/activated?<br></blockquote><div><br></div><div>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).</div><div><br></div><div>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.</div><div><br></div><div>I will add a blurb in this regard.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +<br>
> > +    A client that wishes to present another surface (of its own or from another<br>
> > +    client) will call wp_presentation_manager.create_presenter to create a<br>
> > +    presentation request. Compositors may use this object to track the source of<br>
> > +    the request in order to apply its policies when mapping the surface or<br>
> > +    bringing an existing one to focus.<br>
> > +<br>
> > +    In the presented surface side, the request token will be notified upon<br>
> > +    through the wp_presentation_manager.acknowledge request. The method to pass<br>
> > +    the token across clients is considered an implementation detail, and is<br>
> > +    explicitly not observed here.<br>
> > +<br>
> > +    Upon a successfully acknowledged presentation token, the client will receive<br>
> > +    a wp_presenter.done event. In the rare cases that launching an application<br>
> > +    would fail, the compositor may emit a wp_presenter.cancelled event<br>
> > +    after a reasonable timeout.<br>
> > +  </description><br>
> > +<br>
> > +  <interface name="zwp_presentation_manager" version="1"><br>
> > +    <request name="create_presenter" since="1"><br>
> > +      <description summary="create a new presenter"><br>
> > +       Creates a new presenter.<br>
> > +<br>
> > +       The surface argument is the toplevel that initiated the presentation<br>
> > +       request through user input. Compositors may want to place the presented<br>
> > +       surface relative to the presenter.<br>
> > +<br>
> > +       Compositors that desire to implement focus stealing prevention<br>
> > +       may use this request to find out the request time.<br>
<br>
The serial argument should be explained here. It might be hard,<br>
because we have been really poor at explaining the serials.<br></blockquote><div><br></div><div>Will try :).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
Do we not also need to specify the wl_seat for which the input serial<br>
is associated to?<br></blockquote><div><br></div><div>Hm, right.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +      </description><br>
> > +      <arg name="id" type="new_id" interface="zwp_presenter"/><br>
> > +      <arg name="surface" type="object" interface="wl_surface"/><br>
<br>
If you use wl_surface as the type here, what about when the role is not<br>
a toplevel? Should that result in a protocol error?<br></blockquote><div><br></div><div>That sounds like a good idea.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
What defines a "toplevel"?<br></blockquote><div><br></div><div>My guess is that xdg_toplevel / wl_shell_surface are the targets.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +      <arg name="serial" type="uint" summary="the serial from the input event"/><br>
> > +    </request><br>
> > +<br>
> > +    <request name="acknowledge" since="1"><br>
> > +      <description summary="acknowledges a presentation token"><br>
> > +       Acknowledges that the calling client handled the presentation token.<br>
> > +<br>
> > +       Applications will typically receive this through the DESKTOP_STARTUP_ID<br>
> > +       environment variable as set by its launcher, and should unset the<br>
> > +       environment variable right after this request, in order to avoid<br>
> > +       propagating it to child processes.<br>
> > +<br>
> > +       Compositors will ignore unknown presentation tokens.<br>
> > +<br>
> > +       Presentation tokens may be transferred across clients through means not<br>
> > +       described in this protocol.<br>
> > +      </description><br>
> > +      <arg name="token" type="string"/><br>
> > +      <arg name="surface" type="object" interface="wl_surface"/><br>
<br>
Is this required to be a toplevel as well?<br></blockquote><div><br></div><div>I think so, yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
Does the ack always need a surface? Could something be launched into<br>
background (e.g. systray, whatever, might not even be a wl_surface)?<br></blockquote><div><br></div><div>As replied to Dorota, if we use usecases for it, sure, I just didn't see any.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +    </request><br>
> > +<br>
> > +    <request name="destroy" type="destructor"><br>
> > +      <description summary="release the memory of the presentation manager object"><br>
> > +       Destroy the wp_presentation_manager object. Objects created from this<br>
> > +       object are unaffected and should be destroyed separately.<br>
> > +      </description><br>
> > +    </request><br>
> > +  </interface><br>
> > +<br>
> > +  <interface name="zwp_presenter" version="1"><br>
> > +    <description summary="context for presenting a surface"><br>
> > +      The wp_presenter object allows clients to get the necessary context to<br>
> > +      request that another surface or client should be presented to the user.<br>
> > +    </description><br>
> > +<br>
> > +    <request name="set_app_id"><br>
> > +      <description summary="sets the app ID of the launched application"><br>
> > +       Sets the app ID of the application to be launched, the compositor<br>
> > +       may use this information to look up other miscellaneous information<br>
> > +       about it (eg. translatable name, icon, …).<br>
> > +<br>
> > +       Clients do not need to set an app ID for presentation requests<br>
> > +       meant to map surfaces owned by the same client.<br>
<br>
"Do not need" or "must not"?<br></blockquote><div><br></div><div>It's perhaps a good idea to stick to the latter.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +<br>
> > +       As a best practice, it is suggested to select app<br>
> > +       ID's that match the basename of the application's .desktop file.<br>
> > +       For example, "org.freedesktop.FooViewer" where the .desktop file is<br>
> > +       "org.freedesktop.FooViewer.desktop".<br>
> > +<br>
> > +       See the desktop-entry specification [0] for more details on<br>
> > +       application identifiers and how they relate to .desktop files.<br>
> > +<br>
> > +       [0] <a href="http://standards.freedesktop.org/desktop-entry-spec/" rel="noreferrer" target="_blank">http://standards.freedesktop.org/desktop-entry-spec/</a><br>
> > +      </description><br>
<br>
Missing arguments?<br></blockquote><div><br></div><div>Doh!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
Is it possible to send an app ID that is obviously invalid and should<br>
result in a protocol error?<br></blockquote><div><br></div><div>That wording has been taken straight from xdg-shell, both should probably be kept in sync. Same restrictions apply, too :).</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +    </request><br>
> > +<br>
> > +    <request name="destroy" type="destructor"><br>
> > +      <description summary="destroy zwp_presenter"><br>
> > +       Destroys this zwp_presenter object.<br>
> > +      </description><br>
> > +    </request><br>
> > +<br>
> > +    <event name="token" since="1"><br>
> > +      <description summary="token for the presentation request"><br>
> > +       Notifies of an unique presentation token (eg. UUIDs) to be used for the<br>
> > +       application about to be launched.<br>
<br>
When is this event sent?<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +<br>
> > +       In order to guarantee interoperation with the XDG Startup Notification<br>
> > +       spec, this launch_id is recommended to be transmitted to the launched<br>
> > +       application through the DESKTOP_STARTUP_ID environment variable. The<br>
> > +       launch ID may be passed to a running client through other means not<br>
> > +       observed in this protocol.<br>
> > +      </description><br>
> > +      <arg name="token" type="string"/><br>
> > +    </event><br>
> > +<br>
> > +    <event name="cancelled" since="1"><br>
> > +      <description summary="the presenter has expired"><br>
> > +       Notifies that the compositor is no longer watching this launched<br>
> > +       application. This may indicate failure (eg. launchee crashed) or<br>
> > +       may simply be the result of the launchee not replying properly<br>
> > +       (eg. does not implement this protocol).<br>
<br>
Ok. Should there be also a request to cancel this from the client side<br>
in case the client does the launching and sees that e.g. exec() failed?<br>
Or does the destructor request imply that?<br></blockquote><div><br></div><div>Yup, the idea was making it implicit on destroy.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
> > +      </description><br>
> > +    </event><br>
> > +<br>
> > +    <event name="done" since="1"><br>
> > +      <description summary="the presentation was performed"><br>
> > +       Notifies that the launched application successfully called<br>
> > +       zwp_presentation_manager.acknowledge.<br>
> > +      </description><br>
> > +    </event><br>
> > +  </interface><br>
> > +</protocol><br>
> > --<br>
> > 2.23.0<br>
> <br>
> The name itself, “presentation” might be confusing considering we<br>
> already have a “presentation-time” protocol.<br>
<br>
Yeah. Also, should this aim into the xdg namespace and use xdg_toplevel<br>
instead of wl_surface?<br></blockquote><div><br></div><div>That's probably a good idea :).</div><div><br></div><div>Cheers,</div><div>  Carlos</div></div></div>