wl_subsurface destruction, and design patterns (Re: [RFC Weston 00/10] Sub-surfaces v2)

[Pekka Paalanen]

On Fri, 22 Feb 2013 17:07:44 +0200
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> Open issues:
> 
> - What should wl_subsurface.destroy do? Nothing, or reset the
>   wl_surface's role? Currently it resets the role, since it was
>   easy to implement.
> 
...
> To do, bugs:
> 
> - destroying the wl_surface of a wl_subsurface will prevent
>   destroying the wl_subsurface, fix this, and adjust the
>   protocol description accordingly

I'll open up this problem a bit.

The nearest correspondence to wl_subsurface we have in the current
upstream protocol is wl_shell_surface. They are both defined as
additional interfaces to a wl_surface, and provide a wl_surface role.
Surface roles are always exclusive. A wl_surface can be at most one of
the following:
- a sub-surface
- a shell surface, e.g. a top-level window, tooltip, ...
- a pointer cursor
- a drag icon

Sub-surface and shell surface have a protocol interface associated with
them, cursor and drag icons do not.

In weston's code, a role is identified by what is plugged into struct
weston_surface::private and ::configure fields. In practice, a surface
without a role is never visible and does not take part in input
dispatching.

There are currently different kinds of implementations for removing a
role from a wl_surface. When a pointer cursor or a drag icon wl_surface
stops being used as such, it is returned to the no-role state, allowing
re-use for another purpose if the client wants to. However, I believe
the practice is to not re-use wl_surfaces, since they are cheap to
create and destroy protocol-wise.

wl_shell_surface role on the other hand is permanent. The role is
assigned by creating a wl_shell_surface object. Destroying the
wl_shell_surface object is a no-op, because it has no protocol for
destroy. Therefore there is no way to remove the role. Also, the
wl_shell_surface object is implicitly destroyed in the server, when the
wl_surface is destroyed. The specification requires wl_shell_surface to
be destroyed before the wl_surface, although I think doing it in the
opposite order does not directly lead to a protocol error. Doing it in
the specified order however prevents sending requests to a destroyed
object.

(I now believe that not defining protocol for wl_shell_surface.destroy
was a mistake, made by me.)

In sub-surfaces RFC v2, wl_subsurface object is implicitly destroyed
when the wl_surface object is destroyed. As wl_subsurface.destroy has
protocol defined, destroying the wl_surface first and the wl_subsurface
second will lead to an immediate protocol error, since the destroy
request is being sent to an already destroyed object. Therefore
wl_subsurface must be destroyed first. When wl_subsurface is destroyed,
the wl_surface returns to the no-role state. (Btw. this allows
reparenting of sub-surfaces.)

As you see, there is quite some inconsistency here.

When looking at what we have, cursor surfaces and drag icons are part
of the Wayland core protocol, and wl_subsurface will be a core
extension. Among these we can have consistency: the role can be
dismissed, and the wl_surface returns to the no-role state, ready to be
re-purposed if a client so wants.

wl_shell_surface is known to have design problems anyway, so we can
just punt it.

Finally, I think I'm going to lift the restriction that wl_subsurface
must always be destroyed before the wl_surface. If the wl_surface is
destroyed before the wl_subsurface, the wl_subsurface will become
"inert".

Inert is a term we will be employing with global Wayland
interfaces/objects. An inert protocol object exists, but ignores all
requests except destroy, and does not pass any events. It is only
waiting to be destroyed. With globals, this will be used when a global
is going away: all objects created from the global interface will
become inert. The purpose of inert objects is to avoid races. Before a
client gets to process the removal of a global interface, it may send
requests to objects created from it. These requests racing with the
destruction should not cause protocol errors but be silently ignored.

For the implementation in Weston, supporting inert objects means we
will better detach wl_resource from the corresponding weston data
structure. wl_resource will be a pointer than can be NULL, instead of
embedded. I think that will be nice and tidy, and we need to do that
with weston_surface one day, too, to support window closing animations
and server-only surfaces better.

It seems we have a Wayland protocol design pattern emerging here,
extending object's interface:
- a global for creating an extension (wl_subcompositor, wl_shell)
- an extension object (wl_subsurface, wl_shell_surface)
- destroy semantics (wl_surface.destroy makes the wl_subsurface inert)

Another design pattern is one-shot reply: wl_callback for
wl_display.sync and wl_surface.frame, and wl_probe_result for
wl_shell_surface.probe_area [1].

And the most generic design pattern, or rather a rule of thumb: always
define protocol for destroy. Only in very exceptional cases, like
one-shot reply objects, the destroy request will do more harm than good.

We also have Wayland protocol usage patterns:
- glitch-free maximization and fullscreening [2]
- glitch-free resizing of a window with sub-surfaces

I feel we should document these somewhere. :-)

Comments?


Thanks,
pq

[1] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007340.html
[2] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007650.html