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

[Manuel Bachmann]

Hi Jonas, Daniel, Derek, Bill,

My apologizes for the late answer, I initiallty wanted to come back Monday
with a new test case, and missed some answers.

* Jonas, good point, I forgot to bump the protocol here. I really think it
fits into xdg-shell ; what are you thinking of when you are referring to an
extension ? An additional .xml ? If it can be done without breaking
anything, I would be happy to push it for review.

Regarding the core of the topic - and the usefulness of these requests - my
point is that  :
- it is useful to desktop software (messaging IRC clients, text editors,
browsers, etc..) ;
- embedded-oriented distributions, such as AGL, will definitely need it as
it is a common usecase for their Wayland backends.

 I wanted to showcase a heavy client such as Empathy implementing this, it
unfortunately got delayed, but I'll try to have it ASAP.

* Daniels, I totally agree with you that "raise()" is a bad choice of
wording, it offers false expectations, and we agreed with Jasper that
present() is a lot better (plus, it resembles a "present()" function
present in popular toolkits such as GTK+) ;

* Bill, Derek, about having a single request which always takes a serial
argument : I initially thought the same because it seems a lot cleaner,
but as no consensus was reached, and Wayland wants to position itself as a
stable protocol, I think we better have the double request model for now.

Thanks a lot for your time and patience.





Regards,

*Manuel Bachmann, Graphics Engineer www.iot.bzh  *


2015-10-02 4:07 GMT+02:00 Jonas Ådahl :

> On Thu, Apr 09, 2015 at 06:22:53PM +0200, 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().
> >
> > Signed-off-by: Manuel Bachmann 
> > ---
> >  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 @@
> >
> >  
> >
> > +
> > +  
> > + The surface has new content and would like the compositor
> > + to notify 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.
> > +  
> > +  
> > +
> > +
>
> This changes the protocol without bumping any versions. I'm also a bit
> skeptical to, at this point, doing non-essential changes to xdg_shell.
> Changing the experimental version is a huge pain because a version
> mismatch causes any mismatching client trying to use it to be
> terminated, without any way to discover whether it is compatible or not.
> Could we maybe add this as an extension to xdg_shell or something like
> that?
>
>
> Jonas
>
> >  
> >
> >  The close event is sent by the 

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

[Jonas Ådahl]

On Thu, Apr 09, 2015 at 06:22:53PM +0200, 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().
> 
> Signed-off-by: Manuel Bachmann 
> ---
>  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 @@
>
>  
>  
> +
> +  
> + The surface has new content and would like the compositor
> + to notify 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.
> +  
> +  
> +
> +

This changes the protocol without bumping any versions. I'm also a bit
skeptical to, at this point, doing non-essential changes to xdg_shell.
Changing the experimental version is a huge pain because a version
mismatch causes any mismatching client trying to use it to be
terminated, without any way to discover whether it is compatible or not.
Could we maybe add this as an extension to xdg_shell or something like
that?


Jonas

>  
>
>  The close event is sent by the compositor when the user
> -- 
> 1.8.3.1
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Daniel Stone]

On Wednesday, 30 September 2015, Derek Foreman 
wrote:

> On 30/09/15 12:01 PM, Bill Spitzak wrote:
> > My main concern is that programmers are going to search for the word
> > "raise" to figure out how to raise windows. They are not going to search
> > for "present" or "indicate_activity" or any other bogus word. I suspect
> > that if you don't name this "raise" then a function called "raise" will
> > be added as an extension very soon, and probably break all your careful
> > plans of allowing the compositor to do something other than raise.
>
> Well, X calls similar functionality "UrgencyHint"...  That's actually
> more descriptive than imperative.  The idea is that we let the
> compositor know the surface needs attention, not that we tell it what to
> do.
>
> Programmers shouldn't be trying to figure out how to raise a window
> anyway, they should be searching for words like notify.  But if we put
> the word "raise" prominently in the documentation, I guess google should
> sort it out for them.  ;)
>
> In any event, I dislike "present" less than I dislike "raise", so I'm
> happy enough with the patch as presented.  You do raise a good point
> though.
>
> I'm done.  Really.  Others can continue to fight about this if they so
> choose, I've had my say.


 +1 from me, with optional bike shed to request_attention or note_urgency,
or anything of their ilk. I don't like 'present' overlapping with the
meaning of showing buffers on screen. Urgency / requesting attention have
prior art in X11/ICCCM/EWMH. Anyone designing protocols is unlikely to be
unaware of this, will have it pointed out to them in review, and can grep
the docs for 'raise' anyway.

Pending resolution of zero-as-invalid-serial, subsequent flattening of
requests, documentation that the extension relies on this behaviour:
Reviewed-by: Daniel Stone 

(On the assumption that any serial passed will be from xdg-shell or
similar; input serials would require a wl_seat to be passed, but if you're
reacting to input, then you by definition have the user's attention.)

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Derek Foreman]

On 30/09/15 12:01 PM, Bill Spitzak wrote:
> 
> 
> On Tue, Sep 29, 2015 at 12:18 PM, Derek Foreman  > wrote:
> 
> 
> > 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.
> 
> 
> "raise" is a very likely possible result of this. There is a non-zero
> chance that the result of this message will be an action that a user and
> programmer will call "raise". Your complaint is like saying "we can't
> have this function be called 'set_color' because on a black and white
> screen it will produce a gray shade".

I think I actually did say something like that in a review of a patch
using FB_VISUAL_MONO10 to represent greyscale the other day, so I'm ok
with that.

> My main concern is that programmers are going to search for the word
> "raise" to figure out how to raise windows. They are not going to search
> for "present" or "indicate_activity" or any other bogus word. I suspect
> that if you don't name this "raise" then a function called "raise" will
> be added as an extension very soon, and probably break all your careful
> plans of allowing the compositor to do something other than raise.

Well, X calls similar functionality "UrgencyHint"...  That's actually
more descriptive than imperative.  The idea is that we let the
compositor know the surface needs attention, not that we tell it what to do.

Programmers shouldn't be trying to figure out how to raise a window
anyway, they should be searching for words like notify.  But if we put
the word "raise" prominently in the documentation, I guess google should
sort it out for them.  ;)

In any event, I dislike "present" less than I dislike "raise", so I'm
happy enough with the patch as presented.  You do raise a good point though.

I'm done.  Really.  Others can continue to fight about this if they so
choose, I've had my say.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Bill Spitzak]

On Wed, Sep 30, 2015 at 10:27 AM, Derek Foreman 
wrote:

>
> Well, X calls similar functionality "UrgencyHint"...  That's actually
> more descriptive than imperative.  The idea is that we let the
> compositor know the surface needs attention, not that we tell it what to
> do.
>

That was because the word "raise" was already used for a different API. The
hope is that Wayland can avoid such mistakes.


> Programmers shouldn't be trying to figure out how to raise a window
> anyway, they should be searching for words like notify.  But if we put
> the word "raise" prominently in the documentation, I guess google should
> sort it out for them.  ;)
>

That will probably work. I guess "present" is ok, though I am very very
worried that enough people with some clout in writing extensions will not
be convinced that this is what "raise" should do, and will add another api
to do "raise".
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Bill Spitzak]

On Tue, Sep 29, 2015 at 12:18 PM, Derek Foreman 
wrote:

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

"raise" is a very likely possible result of this. There is a non-zero
chance that the result of this message will be an action that a user and
programmer will call "raise". Your complaint is like saying "we can't have
this function be called 'set_color' because on a black and white screen it
will produce a gray shade".

My main concern is that programmers are going to search for the word
"raise" to figure out how to raise windows. They are not going to search
for "present" or "indicate_activity" or any other bogus word. I suspect
that if you don't name this "raise" then a function called "raise" will be
added as an extension very soon, and probably break all your careful plans
of allowing the compositor to do something other than raise.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Manuel Bachmann]

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  *


2015-09-28 22:41 GMT+02:00 Derek Foreman :

> 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 
> > ---
> >  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 @@
> >
> >  
> >
> > +
> > +  
> > + The surface has new content and would like the compositor
> > + to notify 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.
> > +  
> > +  
> > +
> > +
> >  
> >
> >  The close event is sent by the compositor when the user
> >
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Bill Spitzak]

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.

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.


On Tue, Sep 29, 2015 at 12:53 AM, Manuel Bachmann 
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  *
>
>
> 2015-09-28 22:41 GMT+02:00 Derek Foreman :
>
>> 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 
>> > ---
>> >  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 @@
>> >
>> >  
>> >
>> > +
>> > +  
>> > + The surface has new content and would like the compositor
>> > + to notify 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.
>> > + 

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

[Daniel Stone]

Hi,

On 29 September 2015 at 18:47, 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.

Yes, exactly. I think it'd be good to keep it for exactly this reason.

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

They'd also get pretty upset if xdg_surface_raise() didn't in fact
raise your surface, because compositors tend not to do that on
arbitrary client demands, but instead make their own decisions. (And
let's pleasee not go down that rathole again ...)

Cheers,
Daniel

> On Tue, Sep 29, 2015 at 12:53 AM, Manuel Bachmann 
> 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
>>
>>
>> 2015-09-28 22:41 GMT+02:00 Derek Foreman :
>>>
>>> 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 
>>> > ---
>>> >  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 @@
>>> >
>>> >  
>>> >
>>> > +
>>> > +  
>>> > + The surface has new content and would like the compositor
>>> > + to notify the user.
>>> > +  
>>> > +
>>> > +
>>> > +
>>> > +  
>>> > + The surface has new content and would like the compositor
>>> > + to notify the user.
>>> > +
>>> > + If a valid 

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

[Derek Foreman]

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
> > 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  /
> 
> 
> 2015-09-28 22:41 GMT+02:00 Derek Foreman  >:
> 
> 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
>  >
> > ---
> >  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,
> > +   

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

[Derek Foreman]

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 
> ---
>  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 @@
>
>  
>  
> +
> +  
> + The surface has new content and would like the compositor
> + to notify 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.
> +  
> +  
> +
> +
>  
>
>  The close event is sent by the compositor when the user
> 

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Bryce Harrington]

On Thu, Apr 09, 2015 at 06:22:53PM +0200, 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().
 
 Signed-off-by: Manuel Bachmann manuel.bachm...@open.eurogiciel.org

Reviewed-by: Bryce Harrington br...@osg.samsung.com
 ---
  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
 -- 
 1.8.3.1
 
 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel