[PATCH weston 1/6] xdg-shell: define the present/present_from_event() requests

Derek Foreman derekf at osg.samsung.com
Tue Sep 29 12:18:17 PDT 2015


On 29/09/15 12:47 PM, Bill Spitzak wrote:
> Another important point is that the compositor can check what *type* of
> event was used for the raise. It could accept press/release and ignore
> move events, for instance. Keyboard events could be ignored if the
> client no longer has keyboard focus, while clicks are ignored if the
> client no longer has mouse focus.
> 
> I would really like to suggest two modifications:
> 
> 1. Merge these two requests into one that always takes an event. Make a
> way of saying "no event" as the event serial. The Wayland api is getting
> really bloated with unnecessary duplicate functions and it would be nice
> to make an effort to keep it small in really obvious cases like this.

I really want to agree with you on this one, but at this point I think
that ship has sailed.

afaict, serials are unsigned ints, starting with 0, and they just wrap
around naturally.

I don't think we can change that now.

I guess we could pass another int as a boolean to indicate the serial's
validity - I'm not sure that's less confusing than two functions.

We could always require a *valid* serial, but I'm not sure it's
reasonable to expect an app to have received anything with one before
it's allowed to request a raise...

> 2. Call the function "xdg_surface_raise". Because raising is exactly
> what the client expects. It does not mean that it *has* to raise the
> window. If you don't do this, lots of programmers are going to ask where
> the raise function is, let's stop that from being added as another call
> and get this right now into a single api.

mumble mumble "descriptive not prescriptive", raise is an imperative.

Then again "present" is too.  Not sure how far we want to bike shed this
one.  xdg_surface_indicate_activity?  heh.  getting a bit long to type.

Either way, we're expected to present something to the user (either an
indication or the surface).  We're not necessarily going to raise
anything.  So raise may be more confusing.


> On Tue, Sep 29, 2015 at 12:53 AM, Manuel Bachmann
> <manuel.bachmann at iot.bzh <mailto:manuel.bachmann at iot.bzh>> wrote:
> 
>     Hi Derek, and thanks a lot for your feedback,
> 
>     I think it makes sense if you consider the compositor can know if a
>     past input event could have happened "not long ago" or "a very long
>     time ago" (if the event happened not long ago, raise the window,
>     otherwise blink). It is really a matter of policy, but the idea was
>     to let the shell know some details on user interactions, so it can
>     decide what to do. What do you think of this ?
> 
>     PS : I  rebased the patches on top of Weston 1.9 *. I am also
>     preparing a demonstration using well-known applications with
>     external toolkits, but it takes some time.
> 
>     * : https://github.com/Tarnyko/weston-xdg_surface_present
> 
>     Regards,
>     /Manuel Bachmann, Graphics Engineer
>     www.iot.bzh <http://iot.bzh> /
> 
> 
>     2015-09-28 22:41 GMT+02:00 Derek Foreman <derekf at osg.samsung.com
>     <mailto:derekf at osg.samsung.com>>:
> 
>         On 09/04/15 11:22 AM, Manuel Bachmann wrote:
>         > xdg_surface_present() and xdg_surface_present_from_event()
>         > are new requests supposed to be called on an existing
>         > xdg_surface. They tell the compositor that the surface
>         > has new content which may be of interest to the user.
>         > The compositor may then choose to notify the user.
>         >
>         > xdg_surface_present_from_event() takes a serial coming
>         > from an input (wl_keyboard, wl_pointer, wl_touch) event as
>         > an argument. If the serial is valid and sufficiently recent,
>         > we can suppose the new content has been issued at the user's
>         > request ; the compositor may then choose to raise the
>         > surface directly. Otherwise, it just behaves like present().
> 
>         I'm having a hard time thinking of a use case for
>         xdg_surface_present_from_event()...  Picking an arbitrary motion
>         event
>         from the past seems odd - the compositor probably won't keep
>         track of
>         input serials in any way that makes this useful?
> 
>         > Signed-off-by: Manuel Bachmann
>         <manuel.bachmann at open.eurogiciel.org
>         <mailto:manuel.bachmann at open.eurogiciel.org>>
>         > ---
>         >  desktop-shell/shell.c  | 15 +++++++++++++++
>         >  protocol/xdg-shell.xml | 20 ++++++++++++++++++++
>         >  2 files changed, 35 insertions(+)
>         >
>         > diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
>         > index f7c928e..d1d3f3c 100644
>         > --- a/desktop-shell/shell.c
>         > +++ b/desktop-shell/shell.c
>         > @@ -3923,6 +3923,19 @@ xdg_surface_set_minimized(struct
>         wl_client *client,
>         >       set_minimized(shsurf->surface);
>         >  }
>         >
>         > +static void
>         > +xdg_surface_present(struct wl_client *client,
>         > +                 struct wl_resource *resource)
>         > +{
>         > +}
>         > +
>         > +static void
>         > +xdg_surface_present_from_event(struct wl_client *client,
>         > +                            struct wl_resource *resource,
>         > +                            uint32_t serial)
>         > +{
>         > +}
>         > +
>         >  static const struct xdg_surface_interface
>         xdg_surface_implementation = {
>         >       xdg_surface_destroy,
>         >       xdg_surface_set_parent,
>         > @@ -3938,6 +3951,8 @@ static const struct
>         xdg_surface_interface xdg_surface_implementation = {
>         >       xdg_surface_set_fullscreen,
>         >       xdg_surface_unset_fullscreen,
>         >       xdg_surface_set_minimized,
>         > +     xdg_surface_present,
>         > +     xdg_surface_present_from_event,
>         >  };
>         >
>         >  static void
>         > diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml
>         > index 68cf469..10f82c5 100644
>         > --- a/protocol/xdg-shell.xml
>         > +++ b/protocol/xdg-shell.xml
>         > @@ -396,6 +396,26 @@
>         >        </description>
>         >      </request>
>         >
>         > +    <request name="present">
>         > +      <description summary="the window wants attention from
>         the user">
>         > +     The surface has new content and would like the compositor
>         > +     to notify the user.
>         > +      </description>
>         > +    </request>
>         > +
>         > +    <request name="present_from_event">
>         > +      <description summary="the window needs attention from
>         the user">
>         > +     The surface has new content and would like the compositor
>         > +     to notify the user.
>         > +
>         > +     If a valid and sufficiently recent serial coming from an
>         > +     input (keyboard, pointer, touch) event is passed as an
>         > +     argument, the compositor may want to raise the surface.
>         > +     Otherwise, the request just behaves like the "present" one.
>         > +      </description>
>         > +      <arg name="serial" type="uint" summary="serial of an
>         input event"/>
>         > +    </request>
>         > +
>         >      <event name="close">
>         >        <description summary="surface wants to be closed">
>         >          The close event is sent by the compositor when the user
>         >
> 
>         _______________________________________________
>         wayland-devel mailing list
>         wayland-devel at lists.freedesktop.org
>         <mailto:wayland-devel at lists.freedesktop.org>
>         http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 
> 
> 
>     _______________________________________________
>     wayland-devel mailing list
>     wayland-devel at lists.freedesktop.org
>     <mailto:wayland-devel at lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 
> 



More information about the wayland-devel mailing list