<table><tr><td style="">fredrik added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D23881">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D23881#535167" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D23881#535167</a>, <a href="https://phabricator.kde.org/p/romangg/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@romangg</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D23881#529634" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D23881#529634</a>, <a href="https://phabricator.kde.org/p/ekurzinger/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@ekurzinger</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>Also, apart from the above two comments, any thoughts to how this relates to Roman's pending re-work of a lot of the GLX code <a href="https://phabricator.kde.org/D23105" class="remarkup-link" target="_blank" rel="noreferrer">https://phabricator.kde.org/D23105</a>?</p></div>
</blockquote>
<p><a href="https://phabricator.kde.org/p/fredrik/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@fredrik</a>: I would be interested in this as well. This change allows to schedule buffer swaps like we do with swap events at some point before the vblank and then get an event through second thread when the thread is unblocked again i.e. when the swap has been completed, right? This should also work with my rework patches only providing a single path with an event after swap (or a fallback timer if such an event is not available on the hardware).</p></div>
</blockquote>
<p>Yeah, that's the idea. This patch is meant to be integrated with those patches; it doesn't make any sense on its own.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>And what do you think of using <a href="https://www.khronos.org/registry/OpenGL/extensions/NV/GLX_NV_delay_before_swap.txt" class="remarkup-link" target="_blank" rel="noreferrer">https://www.khronos.org/registry/OpenGL/extensions/NV/GLX_NV_delay_before_swap.txt</a> as suggested by Erik in <a href="https://phabricator.kde.org/D23105#525696" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D23105#525696</a> instead of blocking through the <tt style="background: #ebebeb; font-size: 13px;">__GL_MaxFramesAllowed</tt> equals 1 setting? Your approach with the thread maps to the model "swap -> wait for event -> (delay for smaller latency ->) swap -> wait for event -> ..." used by the mesa drivers pretty well though.</p></blockquote>
<p>I actually started on a DelayBeforeSwapTimer that calls glXDelayBeforeSwapNV() in a separate thread. The idea being that Compositor would use that timer instead of QBasicTimer when the driver supports the extension. But as I was writing that code I realized that that approach would end up being more complex than simply doing the buffer swap in the other thread.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23881#inline-135007">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ekurzinger</span> wrote in <span style="color: #4b4d51; font-weight: bold;">glxbackend.cpp:339</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Is this necessary? If the swap thread isn't rendering anything I don't think it actually needs a current context, right?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yeah, I actually just assumed that glXSwapBuffers() needs a current context, but indeed the specification doesn't say that anywhere.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23881#inline-135009">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ekurzinger</span> wrote in <span style="color: #4b4d51; font-weight: bold;">glxbackend.cpp:786</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I'm not 100% sure this is sufficient to ensure all rendering is complete before the swap. It only guarantees that any commands have been submitted to the GPU, but not necessarily that they've all finished executing. Furthermore, glXSwapBuffers should cause an implicit glFlush anyway.</p>
<p style="padding: 0; margin: 8px;">For instance, the glXSwapBuffers spec mentions glFinish for this purpose:</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">All GLX rendering contexts share the same notion of which are front buffers and which are back buffers. One consequence is that when multiple clients are rendering to the same double-buffered window, all of them should finish rendering before one of them issues the command to swap buffers. The clients are responsible for implementing this synchronization. Typically this is accomplished by executing glFinish and then using a semaphore in shared memory to rendezvous before swapping.</p></blockquote></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yeah, I was actually thinking that we may need to pass a fence to the other thread to ensure that. glXSwapBuffers() only flushes the context current to the thread where it is called, so that's why I added the explicit flush.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D23881">https://phabricator.kde.org/D23881</a></div></div><br /><div><strong>To: </strong>fredrik, KWin, romangg<br /><strong>Cc: </strong>ekurzinger, kwin, LeGast00n, The-Feren-OS-Dev, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, apol, mart<br /></div>