Semantics of wl_subcompositor::get_subsurface

Thu Mar 24 07:53:11 UTC 2016

On Monday, March 21, 2016 11:07:39 AM CET you wrote:
> On Mon, 21 Mar 2016 08:36:59 +0100
> 
> Martin Graesslin <mgraesslin at kde.org> wrote:
> > Hi Wayland-devs,
> > 
> > while implementing sub-surface support in KWin I run into a small problem
> > regarding the get_subsurface request:
> > 
> > When should the new subsurface be added to the parent surface?
> > 
> > My interpretation was that the subsurface will only be added after the
> > next
> > commit on the parent surface. As that would allow setting up the
> > sub-surface completely and also calls like place_above are
> > double-buffered. To me this looked like all child surface ordering
> > changes are double buffered.
> Hi Martin,
> 
> child surface ordering changes are indeed meant to be applied
> atomically by a commit on the parent.
> 
> And by "commit" on that sentence I mean "when the pending parent
> surface state is applied". I have struggled a lot with the language in
> the sub-surface spec.

:-)

> 
> > But testing with real world applications (systemsettings5 on QtWayland)
> > showed that the parent surface does not get committed after the
> > get_subsurface request and my implementation wasn't able to make the
> > subsurface show at all due to that. On the other hand on Weston it works.
> > 
> > So what's the expected way? Is the get_subsurface request double buffered?
> > If yes, could the documentation be extended about that?
> 
> This is probably a case I have overlooked in both documentation and
> Weston implementation. I likely have not considered that the to-be
> sub-surface could be already fully set up, i.e. contents committed
> and all.
> 
> Considering that a sub-surface is specified to start in synchronized
> mode, I think the following would make sense when the parent surface is
> already mapped and the to-be sub-surface has already contents
> committed. The sub-surface gets mapped when:
> - the client calls commit on the parent surface the next time, or
> - the client calls set_desync on/after which according to the
>   sub-surface rules the pending/cached surface state gets applied.

that would be when the parents state gets applied?

> 
> IOW, I would agree with your interpretation when the sub-surface stays
> in synchronized mode.
> 
> *** (a dead-end pondering)
> 
> However, I don't think we can simply say "get_subsurface is
> double-buffered and gets applied on the next parent surface commit"
> without some more thought, because there are alternative paths that
> avoid the need for parent surface commit:
> 
> 1. wl_surface A is already mapped, and effectively in desync mode
> 2. wl_surface B has content applied (proper attach+commit)
> 3. B is made a sub-surface of A (wl_subcompositor.get_subsurface)
> 4. B is set to desync mode (wl_subsurface.set_desync)
> 
> Since the parent surface is effectively in desync mode, set_desync on B
> applies cached state immediately. That allows B to be mapped (appear on
> screen) with two possibilities:
> - immediately at set_desync, or
> - on the next commit on B after set_desync.
> 
> Which one happens is IMO unspecified. set_desync spec talks about
> cached state, but as B has not seen commits while being a sub-surface
> (they happened earlier), it does not have cached state.
> 
> Would it make more sense to pick the next commit on B after set_desync?

I would say that making surface B a sub-surface of A affects the pending state 
of surface A. Thus I would say it can only get mapped once surface A's state 
is applied and the state of surface B just doesn't matter.
> 
> ***
> 
> Or, if we actually do specify that get_subsurface will be effective on
> the next parent surface commit, then there is no way to map the
> sub-surface without a parent commit. As I see adding sub-surfaces as an
> action on the parent surface, I'd be fine with this interpretation too.
> If the client really wants the sub-surface to appear on its own time,
> it can simply commit the parent surface straight away. Maybe this would
> be simpler?

yep, that's how I would see it.

> 
> Hmm, and there is the question of positioning the sub-surface.
> Regardless of modes, getting sub-surface position change applied always
> requires a commit on the parent surface. Z-ordering is the same too. If
> the defaults do not happen to be fine, the surfaces would glitch, if
> the sub-surface was allowed to become mapped without a parent commit.
> This was probably a major point you wanted to make, right?

yes in deed. Actually that was the point which made me aware of that something 
is amiss. As I implemented the z-ordering in a double buffered way and 
considered adding a sub-surface as changing the z-order of the sub-surfaces.

Especially it's not possible to add a new sub-surface as the bottom most 
surface in a glitch free way if the adding is applied immediately.

> 
> I am happy to admit that the Weston implementation can be wrong here. I
> have probably subconsciously assumed that one always sets up the
> surface role first, and content second. Some roles imply that already,
> though for sub-surfaces you can do either order.
> 
> After this pondering, I'm starting to think that always requiring a
> commit on the parent surface to have get_subsurface applied and appear
> on screen is the right thing to do.

sounds good to me :-)

> 
> Does anyone else have an opinion for or against making the parent
> commit always required?
> 
> I have filed https://phabricator.freedesktop.org/T7358 to keep track of
> the needed actions, and a Weston bug depending on the solution.

Cheers
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160324/c63fbeac/attachment.sig>