[PATCH wayland] protocol: make get_subsurface double-buffered

Tue May 10 14:17:05 UTC 2016

On May 10, 2016, at 3:19 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 
> On Mon, 9 May 2016 07:58:32 -0500
> Yong Bakos <junk at humanoriented.com> wrote:
> 
>> Hi Pekka,
>> Two minor nits in the description, noted inline below.
>> 
>> yong
>> 
>> On May 9, 2016, at 6:45 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>> 
>>> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>>> 
>>> 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
>>> Cc: Martin Gräßlin <mgraesslin at kde.org>
>>> Cc: Jonas Ådahl <jadahl at gmail.com>
>>> Cc: Chris Michael <cpmichael at comcast.net>
>>> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>>> ---
>>> protocol/wayland.xml | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>>> index 92e3f43..1546b46 100644
>>> --- a/protocol/wayland.xml
>>> +++ b/protocol/wayland.xml
>>> @@ -2487,6 +2487,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
>> 
>> visible the next time
>> 
> 
> Yes.
> 
>>> +	applied.
>>> +
>>> +	This request modifies the behaviour of wl_surface.commit request on
>> 
>> requests
> 
> "request" is used as a noun here and there is only one, so I think it's
> correct as is.
> 
> Or which one did you mean? I think the latter is also singular, but I
> was thinking in interface terms, not as "from now on commits will
> behave differently" which would be true too, thinking in runtime terms.
> 
> Alternative suggestions?

Ah, I should have been more clear. I was suggesting either:

This request modifies the behaviour of wl_surface.commit requests on
the sub-surface
-or-
This request modifies the behaviour of a wl_surface.commit request on
the sub-surface

yong

> 
> Thanks,
> pq
> 
>>> +	the sub-surface, see the documentation on wl_subsurface interface.
>>>      </description>
>>> 
>>>      <arg name="id" type="new_id" interface="wl_subsurface"
>>> @@ -2557,7 +2565,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>
>>> 
>>> --
>>> 2.7.3
>>> 
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160510/2f3e5254/attachment.sig>