[PATCH 0/5] Improve Present support in Xwayland with per window flips

[Michel Dänzer]

On 30/08/17 06:15 PM, Roman Gilg wrote:
> On Wed, Aug 30, 2017 at 4:42 AM, Michel Dänzer <michel at daenzer.net
> <mailto:michel at daenzer.net>> wrote:
>     On 30/08/17 12:24 AM, Roman Gilg wrote:
>     >
>     > Originating from the bug report
>     >
>     > https://bugs.freedesktop.org/show_bug.cgi?id=99702
>     <https://bugs.freedesktop.org/show_bug.cgi?id=99702>
>     >
>     > and my own observations with Xwayland misbehaving when outscanning on overlay planes, this patch series aims at improving Present support in Xwayland.
>     >
>     > For that it introduces an internal flip mode API to Present, with that it's possible to try other Pixmap flips than just for a whole screen like before. For Xwayland we add a flip mode per window, but for example in the future we could also try to add a mode for flips per CRTC. Anyway the idea is to clearly separate different flip modes with their own code paths.
>     >
>     > In Xwayland we flip per window, and also with the last patch in the series use sub-surfaces for that in order to flip on child windows. In my tests this was still somewhat fragile.
> 
>     I made some high level comments based on commit logs. I haven't reviewed
>     the patches in detail yet, because it seems difficult unless at least
>     some of them are split up:
> 
>     * Moving code without any functional changes should be in its own patch,
>       not intermixed with functional changes.
> 
> 
> What do you mean exactly? I put the code moving of present.c to
> present_execute.c and present_vblank.c in the separate patch 2. Or
> should it not be part of this patch set at all?

Patch 2 is fine, it's an example of how this should be done.

> Or do you mean the moving of code in patch 1 to present_scrmode.c?

Yes, that's what I mean.


>     * Only one logical change per patch.
> 
> Can you give an example? What do you mean by a logical change.

E.g. in patch 4:

"There is a small change to the window mode in Present as well, [...]"

that's a separate logical change and should be in a separate patch.


> I partitioned my changes such that they form functional units and such
> that every patch is self-contained, i.e. such that the Xserver can work
> even if the later ones won't get pushed.

That's important as well.


>     > * Mutter: Neverball framerate capped to 120 (probably Mutter doesn't release buffers faster)
> 
>     Was that with PresentOptionAsync (e.g. via "V-Sync" disabled in the game
>     settings, or e.g. vblank_mode=0)? Without PresentOptionAsync, the
>     intention is for the presentation frame rate to match the scanout
>     refresh rate.
> 
> PresentOptionAsync is always on in Xwayland. And I disabled V-Sync in
> game (set vblank_mode=0). Although that in game setting makes with my
> current patches no difference, since V-Sync regressed for now with this
> patch series and only works again when later some mechanism for queuing
> vblanks, for example by using the Presentation Time protocol, is added.

That's unfortunate. Is there a way to at least keep it working about as
well as it has been without too much effort, e.g. by synchronizing to
the compositor's refresh cycle? In practice, the vast majority of
clients will always present on the next MSC, I don't think we need to
worry about supporting later MSCs at this point.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer