[PATCH wayland v3] protocol: Add minimize/maximize protocol

[Scott Moreau]

Hi Jason,

First I'd like to thank you for your attention to this matter. I have
been looking for someone to address this series so I can bounce ideas
around and discuss this. There are many interesting possibilities
here. You hit many points dead-on. Comments below.

On Mon, Mar 18, 2013 at 1:10 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> Scott,
> Allow me to be a bit more specific as to what I am thinking.  In
> general, I like the requests and events in the protocol, I just think
> things should be set out more clearly.

Yes, I agree.

> The documentation given in the
> protocol isn't terribly specific as to how someone is supposed to
> implement it.

Yes, I have been waiting for someone to review this series so that we
can openly discuss it and finally, agree on what events, requests
should be used but possibly more importantly, their descriptions that
will be the protocol documentation.

> From reading your code, it seems like you simply have
> this magic "minimized" state that you're trying to keep in sync.
> Unfortunately, it's not terribly clear.

Yes, it is not clear I agree. It is a working proof-of-concept but the
way it should be implemented is not outlined in the protocol. I fought
a bit with this at first, trying to come up with descriptions,
especially for minimize. Instead of wasting time on trying to come up
with anything, I just put the protocol in place with generic
descriptions. I know that this is an important topic and the
descriptions should be as correct as possible, so compositors and
clients know exactly what to do.

>
> Here is how I think I would have such a protocol work. (Perhaps this
> is what you intend, but, like I said, it's kind of hard to tell.)
>
> If the minimize event comes from compositor-side:
> 1. The compositor sends a "request_minimize" event to the client (or
> just "minimize", but I like the other a little better)
> 2. The client responds with "set_minimized" events on any number of
> top-level surfaces.
> 3. The compositor minimizes the top-level surfaces as requested by the client
>
> If the minimize event originates client-side:
> 1. The client sends calls "set_minimized" on any number of top-level surfaces
> 2. The compositor minimizes the top-level surfaces as requested by the client

Yes, I completely agree. This is the same semantics of the current
maximize implementation.

>
> In this way the client remains in control of what windows get
> minimized etc.  I think it also makes it more explicit as to when a
> window is/isn't max/minimized.  I also like this addition because it
> allows for the possibility of server-side decorations.  Not that those
> are going to be a for-sure thing, but allowing the compositor to relay
> these kinds of events to the client is very useful.

Yep.

> Along these
> lines, a "request_close" event is probably not a bad idea.

Giulio Camuffo had the exact same idea. He wrote some patches and it
is now in gh next wayland/weston/gtk/qtwayland repos. This allows us
to close individual surfaces from a common client and not just blindly
destroy the entire client. This allows us to have the close function
in the window list to work per-surface and not per-client as it was.

> That said,
> I think things should be client-controlled not compositor-controlled
> as it leads to fewer edge-cases.

Yes, this is the way it works for maximize and there is no reason why
it shouldn't work the same for minimize.

>
> As far as use-cases go, I think there are a number of things to think about:
> 1. A window that does not want to be maximized.  What if the window
> wants to have a fixed size?  Do you simply set it as maximized and let
> it fail the configure event?  In that case the compositor thinks it's
> maximized but, in reality, it's not.

The client is in control, as you detailed how it should work with
minimize. So you ask the client to maximize and if it doesn't send the
maximize request, it never gets maximized. We can't possibly
force-maximize a surface without client support lest we get garbage
buffers. So, this is how it already works and a non-issue.

>
> 2. A window that, when you minimize it actually minimizes other
> windows as well (dialogues for example).  This is where Bill's example
> falls.  If you want it more concrete, think about the original
> multi-window layout of GIMP.  I don't want the dialogues visible
> whenever at least one image window is visible and invisible when none
> of the image windows are visible.  This could probably be handled in a
> compositor-dictated system, but it will look strange if the image
> window gets minimized and then, because of lag, the dialogue gets
> removed a few seconds later.

You kinda lost me here but the client can already send a minimize
request per surface. Now that you mention it, we probably should have
an unminimize request as well. This way we can do what (if I
understand correctly) you're talking about here. This would also let
clients unminimize select surfaces in response to a 'tray icon' right
click menu function for example.

>
> 3. A window that, when you maximize it, responds by maximizing
> something else.  Consider a video player.  It may be using a
> subsurface to display its video and then, when you press the maximize
> button it actually detaches the video surface and sets it to
> fullscreen.  Again, in a compositor-dominated environment this would
> really confuse the compositor.

I am thinking this is another non-issue. Clients are in control of
sending fullscreen or maximize events for their surfaces and setting
them back (toplevel).

>
> Hopefully this is concrete enough to demonstrate my point.

Yes and again, I would like to thank you for taking the time out to
address this. I now have a couple of other outstanding cases I would
like to introduce to the discussion. These are cases to consider when
thinking about what descriptions should be used for the minimize
protocol documentation.

1) We want live minimized surface previews. This means that we want to
be able to display a minimized surface in several cases and have the
client still rendering frames of whatever it draws. For example, a
hover action on the panel's window list item could pop-up a scaled
down preview of the surface. Another use case is for 'scale' (compiz)
or 'expose' (osx) effect. We want to be able to view minimized surface
in this mode as well. There are plenty of other use cases as well. The
question is, how should the semantics work here? Should the client
become unminimized for this case? Or should the client do nothing but
just always keep rendering and attaching frames even in the minimized
case?

2) Minimized surfaces do not receive frame events. The current
implementation removes the surface from the compositor render list of
surfaces to draw. This means that a) the surface has no assigned
output and b) does not get sent frame events while minimized. I
thought about this and ended up with a separate minimized surface list
that contains the list of currently minimized surfaces. Should output
repaint send frame events to minimized surfaces as well?

3) Currently, most all wayland clients depend on frame events to drive
their rendering. If the clients are not sent these frame events, the
client rendering will stall. This causes problems for video players
reliant on the frame event. Should we expect all clients to render not
depending on the frame events? It is not clear from the frame event
protocol description. if this is the case, we should add it to the
protocol description and change the clients accordingly. As I
understand, eglSwapBuffers() automatically blocks on each call to sync
properly. So even if you called it at the 'wrong' time (i.e. outside
of the frame event handler) it would still sync in the egl case. I'm
not sure about how shm would work here.


Thanks,

Scott

>
> On Sun, Mar 17, 2013 at 9:02 PM, Scott Moreau <oreaus at gmail.com> wrote:
>>>> I am not really sure what you are talking about but I'm also not sure I have
>>>> time for it. The fact is that this is only a basic implementation to
>>>> exercise the new protocol. If you would like to contribute code, the policy
>>>> is that patches are welcome. A working implementation of what you think is
>>>> better might also help to illustrate your points better.
>>>
>>> That's not really a good answer when we're talking about the core
>>> protocol.

Yes you are right. Unfortunately, I typically disregard Bill's
opinions because they are not well organized, often over-thought and
he never submits any code at all. I don't really have time for it.

>>  If this is so experimental, it should be added as a weston
>>> extension, not to the wayland core.  Before we start adding things to
>>> the wayland core protocol, we need to get it right.
>>
>> This series provides a complete example implementation in weston.
>> Maximize and fullscreen are part of wl_shell_interface. It wouldn't
>> make sense to put it anywhere else, than the same place as these
>> existing events/requests.
>
> Yes, it is an implementation.  However, I think there are some
> corner-cases and things that need to be thought about.  Part of my
> point above is that you can't blow off the people who review your code
> with "I don't have time" and then expect Kristian to apply your
> patches.

I never expect my patches are pushed upstream to the official repos
without proper review. It is the review I'm looking for, so that
problems can be addressed and fixed, for eventual potential inclusion
and use for all. Unfortunately, since Kristian has finally announced
that he is not interested in any patches with DE-like goals from
anyone other than DE developers. The DE part here is the window list
and surface_data protocol. However, the surface_data protocol is just
local weston protocol. The minimize/maximize items are things that
should be in core protocol regardless if they provide DE-like
functionality or not.

> We need to be careful with the core protocol and make sure
> we get it right.

Yes, I fully agree. And *again* I want to thank for your time and
attention to this. I have been *looking* for a reviewer, though I was
expecting it to be krh. I think your ideas are right on target.

> There is a big limit as to how much of the
> "implement it now and fix it later" we can do with the core protocol.

Yes, you are right. This is why I want to have a working
implementation in weston, as it should serve as the official reference
implementation for all.

> As soon as we have the 1.1 release, we (and the rest of the Linux
> community) are stuck with it regardless of whether or not it's a good
> implementation.

Indubitably.

>
> I hope that's more clear,

It does, all I wanted was some attention from someone with experience.

Note to Bill Spitzac: I find your posts to be often frivolous and
incoherent. I don't mean to be rude here but I have tried to consider
many of your points and you often go on long tangents about some
problem that doesn't exist in reality or a highly isolated use case.
Many times I see your postings as a frivolous spec in a ml paragraph
of how you think something should work. My point here is, if you want
something to work a certain way, then write the code. This way, people
can review and comment on your code (not your random comments) or you
can just use it for yourself. I would like to ask that if you do
respond, please put in the time and effort to make your thoughts
coherent and in the scope of the wayland reference compositor.

> --Jason Ekstrand

- Scott Moreau