[X11] Force glXSwapBuffers to block with NVIDIA driver
ClosedPublic

Authored by ekurzinger on Mar 18 2019, 9:41 PM.

Details

Summary

The NVIDIA implementation of glXSwapBuffers will, by default, queue up
to two frames for presentation before blocking. KWin's compositor,
however, assumes that calls to glXSwapBuffers will always block until
the next vblank when rendering double buffered. This assumption isn't
valid, as glXSwapBuffers is specified as being an implicit glFlush,
not an implicit glFinish, and so it isn't required to block. When this
assumption is violated, KWin's frame timing logic will
break. Specifically, there will be extraneous calls to
setCompositeTimer with a waitTime of 0 after the non-blocking buffer
swaps, dramatically reducing desktop responsiveness. To remedy this,
a call to glXWaitGL was added by Thomas Luebking after glXSwapBuffers
in 2015 (see bug 346275, commit
8bea96d7018d02dff9462326ca9456f48e9fe9fb). That glXWaitGL call is
equivalent to a glFinish call in direct rendering, so it was a good
way to make glXSwapBuffers behave as though it implied a glFinish
call.

However, the NVIDIA driver will by default do a busy wait in glFinish,
for reduced latency. Therefore that change dramatically increased CPU
usage. GL_YIELD can be set to USLEEP (case insensitive) to change
the behavior and use usleep instead. When using the NVIDIA driver,
KWin will disable vsync entirely if
GL_YIELD isn't set to USLEEP
(case sensitive, a bug in KWin).

However, the NVIDIA driver supports another environment variable,
__GL_MaxFramesAllowed, which can be used to control how many frames
may be queued by glXSwapBuffers. If this is set to 1 the function
will always block until retrace, in line with KWin's expectations.
This allows the now-unnecessary call to glXWaitGL to be removed along
with the logic to conditionally disable vsync, providing a better
experience on NVIDIA hardware.

Test Plan

Run KWin using the X11 backend with the proprietary NVIDIA driver.
Ensure the TripleBuffer option is not set to true in Xorg's configuration file.
Ensure vsync has not been manually disabled (either in NVIDIA's or KWin's settings).

  • Moving windows should not exhibit significant lag
  • CPU usage should never be excessively high
  • No screen tearing should be observed

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ekurzinger created this revision.Mar 18 2019, 9:41 PM
Restricted Application added a subscriber: kwin. · View Herald TranscriptMar 18 2019, 9:41 PM
ekurzinger requested review of this revision.Mar 18 2019, 9:41 PM
zzag accepted this revision.Mar 19 2019, 9:20 AM
zzag added a subscriber: zzag.

I don't have any NVIDIA GPU to test, but code-wise this change makes sense.

Minor nitpick: please change "[X11]" to "[platforms/x11]" :-)

This revision is now accepted and ready to land.Mar 19 2019, 9:20 AM
davidedmundson accepted this revision.Mar 19 2019, 9:22 AM
davidedmundson added a subscriber: davidedmundson.

The rationale all makes sense.
Thanks for investigating this.

This revision was automatically updated to reflect the committed changes.

I guess you guys would consider this to be too risky for 5.15? (Although a lot of Nvidia owners - me included - are dying to get this fix).

Thanks a lot! I suggest to backport it to all supported versions.

zzag added a comment.Wed, Mar 27, 8:35 AM

I backported this patch to 5.12 and 5.15.