On Thu, 24 Mar 2016 08:53:11 +0100 Martin Graesslin <[email protected]> wrote:
> On Monday, March 21, 2016 11:07:39 AM CET you wrote: > > On Mon, 21 Mar 2016 08:36:59 +0100 > > > > Martin Graesslin <[email protected]> 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? This is from off the top of my head, but if the parent surface is already acting non-synchronized itself, then set_desync would map the sub-surface immediately. However, please see below. If this gets too tricky, maybe we can just require parent state to be applied instead, to be more consistent in behaviour. > > 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. That could indeed be the way to make things more clear. > > > > *** > > > > 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. Cool. > > > > 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. 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. > > sounds good to me :-) Very good. Thanks, pq > > > > 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
pgpRnCMT1CfXG.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
