D23881: Possible solution for scheduling repaints on NVIDIA

[Fredrik Höglund]

fredrik added a comment.


  In D23881#535167 <https://phabricator.kde.org/D23881#535167>, @romangg wrote:
  
  > In D23881#529634 <https://phabricator.kde.org/D23881#529634>, @ekurzinger wrote:
  >
  > > 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 https://phabricator.kde.org/D23105?
  >
  >
  > @fredrik: 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).
  
  
  Yeah, that's the idea. This patch is meant to be integrated with those patches; it doesn't make any sense on its own.
  
  > And what do you think of using https://www.khronos.org/registry/OpenGL/extensions/NV/GLX_NV_delay_before_swap.txt as suggested by Erik in D23105#525696 <https://phabricator.kde.org/D23105#525696> instead of blocking through the `__GL_MaxFramesAllowed` 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.
  
  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.

INLINE COMMENTS

> ekurzinger wrote in glxbackend.cpp:339
> Is this necessary? If the swap thread isn't rendering anything I don't think it actually needs a current context, right?

Yeah, I actually just assumed that glXSwapBuffers() needs a current context, but indeed the specification doesn't say that anywhere.

> ekurzinger wrote in glxbackend.cpp:786
> 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.
> 
> For instance, the glXSwapBuffers spec mentions glFinish for this purpose:
> 
> > 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.

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.

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D23881

To: fredrik, #kwin, romangg
Cc: 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20190921/32a0f947/attachment-0001.html>