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

[Scott Moreau]

Hi Jason,

On Wed, Mar 20, 2013 at 7:56 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> Scott et. al,
> I'm not going to try and answer everything because a lot has happened
> on this topic and I think we're on the same page on most of the
> technical details.
>
>
>>> 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.
>
> Ok, I think we're on the same page: The compositor gives the request
> to the client -> client tells the compositor what windows need
> changing -> compositor performs changes.

Yes that's 1). Or 2) the client acts on its own and requests a state
change (client->compositor) 3) A surface_data request is received to
ask the shell to perform a state change on a particular surface_data
object (shell_client->compositor->client)

One important thing to note here is that client != surface. In fact,
clients can have multiple surfaces. We might need to keep this in mind
for things like closing single surfaces demonstrated here
https://github.com/soreau/wayland/commit/65f8a3f5f683c3a91913a26496cc373633f01896

This allows us to tell a client that 'the user has indicated that they
would like one of your surfaces to be closed (this one)'. By way of
contrast, the current code kills the entire client and all of its
surfaces. Unless I am not understanding correctly, we don't have a way
to tell a client to kill only one (or more) of its surfaces with the
current protocol. It might be a good idea to write a test client that
simply has two surfaces from the same client to exercise these cases
(unless there isn't one already). We've been testing with
google-chrome mostly.

>
> As far as frame callbacks go, like pq said, that's pretty much
> orthogonal.  I think we should just let the compositor implementation
> take care of that.

Yes I meant to comment on this, acknowledging that I understand and
agree with Paalanen's point. Hope you get well soon pq.


> The point here is that none of this should get
> implemented by the client removing the buffer from the surface in
> order to unmap it.  That's unacceptable.  Instead, it should happen in
> terms of modes.

I don't know what you mean by this really. Two questions at least:
1) What do you mean by modes exactly?
2) What would you do instead of placing it from the drawn list to a
dedicated minimized surface list?


>
> There is one more question that I think needs to be answered.  And
> that is: do we handle things in terms of set/unset or in terms of
> set_maximized, set_fullscreen, set_minimized, and set_normal (probably
> want a better name for that one).  Really, which of those two we do
> doesn't matter that much because the toolkit can force it either way.
> It's mostly a matter of who tracks the state and handles it.  I think
> I like simply setting the state instead of keeping track of set/unset
> better, but I'm open to discussion on that.

I have this all working in gh next. The only thing left to consider
that I can think of is: Do we want to support 'unmaximizing or
unminimizing (or unfullscreening) a surface retains stacking state'.
So basically if there is a bottom-level surface and you state change
it then toggle it back, do we always want it on top no matter what? Or
do we want to optionally support retaining stacking order on state
restore (setting back to 'normal'). If we want to support this
feature, then a new un* request is required for each state set
request. I move that we do support this feature and I'm working on
this in gh next. One other question, do we want to support fullscreen
from a source other than the client? For instance we could have
fullscreen as a selection in the drop down menu? I guess maximize and
minimize are expected features, fullscreen is optional. For this, we'd
need (un)fullscreen events.  Hm, I wonder if there's a way to have the
client tell us what states it actually supports so we can correctly
represent this in the panel taskbar menus..

A note on 'request ($state_type) and request (un$state_type)' vs
'request ($state_type, 1) and request ($state_type, 0)':

After looking at the code in a working state, it's far clearer to have
explicit $state and un$state events/request because there are a lot of
paths in the code we have to run through to make this all syn up and
work out properly. Using a state variable to simplify the protocol
will likely complicate the description in the protocol and complicate
the code.

>From your post ago, I wanted to make another related comment so I will
insert it here:

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

An interesting case is that of xwm toolkit(?) where the decorations
are actually CSD from the eyes of the compositor but compositor side
from xclients POV. So for cases like chrome, you can toggle CSD there
and yea. That's coming up erm, next.


> 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. So to recap:

1) Client is ultimately in control of its state. A state request is
assumed 'on'. Restoring will call either toplevel if that is always
the assumption, or the un$state request counterpart to support
stacking order preservation.
2) Compositor and client are responsible for their own state tracking
of each surface
3) One case to keep in mind is clients that have multiple surfaces

Now I'd like to give a stripped down version of what the full featured
proposed protocol would look like:

# - Already in the current wl_shell_surface protocol (also in gh next,
which rebases to master)
n - In gh next
+ - Required to support stack-preservation-upon-state-restoration
= - Required to support fullscreen-function-from-taskbar-menu

 n    request - minimize
  +   request - unminimize
#     request - maximize
  +   request - unmaximize
#     request - fullscreen
  +   request - unfullscreen

n      event - minimize
n      event - unminimize
n      event - maximize
n      event - unmaximize
=      event - fullscreen
=      event - unfullscreen

More questions about protocol versioning:

1) If we always add protocol to the end, it will likely be incoherent,
unmatched and not very easy to read. I know wl_shell interface is
disposable but for the sake of clarity how wayland protocol versioning
system, I'd like to know what the expected convention is.
2) I noticed that changing the version requires no changes client
side. How is this supposed to work?
3) The protocol semantics were recently changed. When semantics are
changed of existing protocol does this not constitute an interface
version bump?


>
> Assuming we can get the above question answered, I think I'd like to
> see a version 2 with the following changes:
>
> 1. Clear and precise documentation.  If we're going to require a
> client response to a minimize event, it needs to be well-documented.
> 2. Events for each of: request_maximize, request_minimize, and
> request_close (The client should be able to handle fullscreen itself).
>  I do prefer the request_x because it makes it more clear that the
> compositor is requesting the client to minimize, not simply telling it
> that it already is.
> 3. Some way for the client to unset maximized, minimized etc.
> 4. An enum and a request telling the compositor which of these
> operations is supported.  This way the compositor can hide the buttons
> or whatever.  The client can always ignore the request_x events, but
> this provides a nicer UI.
>
> Thanks,
> --Jason Ekstrand