<div dir="ltr"><span class="gmail-im" style="font-size:12.8px">On Wed, Aug 30, 2017 at 4:42 AM, Michel Dänzer <span dir="ltr"><<a href="mailto:michel@daenzer.net" target="_blank">michel@daenzer.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>Hi Roman,<br><span class="gmail-m_-5097065328263038873gmail-"><br><br>On 30/08/17 12:24 AM, Roman Gilg wrote:<br>><br>> Originating from the bug report<br>><br>> <a href="https://bugs.freedesktop.org/show_bug.cgi?id=99702" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/s<wbr>how_bug.cgi?id=99702</a><br>><br>> and my own observations with Xwayland misbehaving when outscanning on overlay planes, this patch series aims at improving Present support in Xwayland.<br>><br>> 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.<br>><br>> 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.<br><br></span>I made some high level comments based on commit logs. I haven't reviewed<br>the patches in detail yet, because it seems difficult unless at least<br>some of them are split up:<br><br>* Moving code without any functional changes should be in its own patch,<br>  not intermixed with functional changes.<br></blockquote><div><br></div></span><div style="font-size:12.8px">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? Or do you mean the moving of code in patch 1 to present_scrmode.c?</div><span class="gmail-im" style="font-size:12.8px"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">* Only one logical change per patch.</blockquote><div> </div></span><div style="font-size:12.8px">Can you give an example? What do you mean by a logical change. 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.</div><span class="gmail-im" style="font-size:12.8px"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_-5097065328263038873gmail-">> * Mutter: Neverball framerate capped to 120 (probably Mutter doesn't release buffers faster)<br><br></span>Was that with PresentOptionAsync (e.g. via "V-Sync" disabled in the game<br>settings, or e.g. vblank_mode=0)? Without PresentOptionAsync, the<br>intention is for the presentation frame rate to match the scanout<br>refresh rate.</blockquote><div> </div></span><div style="font-size:12.8px">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.</div></div>