Semantics of wl_subcompositor::get_subsurface

[Pekka Paalanen]

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.

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?

***

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?

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?

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.

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.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160321/d2b0967d/attachment.sig>