protocol: make get_subsurface double-buffered

The existing specification was not explicitly clear on when
wl_subcompositor.get_subsurface request actually adds the sub-surface to
the parent in the compositor's scenegraph. The implicit assumption was
that this happens immediately, but it was not written anywhere.

If it happens immediately, the client doing things in a wrong order may
cause a glitch on screen. Particularly, if the wl_surface B that is
going to be a sub-surface for wl_surface A (the parent) already has a
buffer committed, and the parent surface is mapped, then get_subsurface
will (may?) cause wl_surface B to become mapped immediately. That leaves
no time to set up the sub-surface z-order or position before mapping,
hence there can be a visible glitch.

The way to avoid that, given that the parent surface is mapped, is to
not commit a buffer to wl_surface B until all the sub-surface setup is
done.

However, doing the sub-surface setup always requires a wl_surface.commit
on the parent surface unless the defaults happen to be correct.

To make setting up a subsurface slightly easier by removing one
possibility for a glitch, this patch amends the specification to require
a wl_surface.commit on the parent surface for get_subsurface to
complete. The sub-surface cannot become mapped before a parent commit.

This change may break existing clients that relied on the glitchy
sequence to not need a parent surface commit to map the sub-surface.
However, presumably all uses would at least issue a
wl_subsurface.set_position, which requires a parent surface commit to
apply. That would guarantee that there is a parent surface commit after
get_subsurface, and so reduces the chances of breaking anything.

In other cases, this change may simply remove a possibility for the
glitch.

This patch also adds a note about changing wl_surface.commit behaviour
on wl_subcompositor.get_subsurface. (That could be a separate patch.)

The behaviour of wl_subsurface.destroy remains as specified, even though
it is now slightly asymmetrical to get_subsurface. This is emphasized by
adding the word "immediately". The effects of destruction were already
explicitly documented, as is the way to achieve synchronized unmapping,
so changing destruction behaviour would likely be more disruptive, and
also open up more corner cases (what would happen between destroy and
unmapping?).

Bug: https://phabricator.freedesktop.org/T7358
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Reviewed-by: Martin Gräßlin <mgraesslin@kde.org>

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 77d991e..b5662e0 100644 (file)
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -2577,6 +2577,14 @@
        The to-be sub-surface must not already have another role, and it
        must not have an existing wl_subsurface object. Otherwise a protocol
        error is raised.
+
+       Adding sub-surfaces to a parent is a double-buffered operation on the
+       parent (see wl_surface.commit). The effect of adding a sub-surface
+       becomes visible on the next time the state of the parent surface is
+       applied.
+
+       This request modifies the behaviour of wl_surface.commit request on
+       the sub-surface, see the documentation on wl_subsurface interface.
       </description>
       <arg name="id" type="new_id" interface="wl_subsurface"
           summary="the new sub-surface object ID"/>
@@ -2646,7 +2654,7 @@
        that was turned into a sub-surface with a
        wl_subcompositor.get_subsurface request. The wl_surface's association
        to the parent is deleted, and the wl_surface loses its role as
-       a sub-surface. The wl_surface is unmapped.
+       a sub-surface. The wl_surface is unmapped immediately.
       </description>
     </request>