Commit 8d137290 authored by Roman Gilg's avatar Roman Gilg
Browse files

[platforms/x11] Never block on retrace, always present after paint

Summary:
Compositing in X11 was done time shifted, meaning that we paint first, then
wait one vblank interval length and present on prepareRenderingFrame the
previous paint result. This is supposed to make sure we don't miss the vblank
and in case of block till retrace be able to continue issuing commands and
only shortly before next vblank present.

This is counter-intuitiv, not how we do it on Wayland or even on MESA with X.
The reason seems to be that the GLX backend was in the beginning written
against Nvidia proprietary driver which needed this but nowadays even this
driver defaults to non-blocking behavior on buffer swap.

Therefore remove this legacy anomaly fully and directly present after paint.
We then wait one refresh cycle and in the future can optimize this by delaying
the paint and present till shortly before vsync.

Test Plan: kwin_x11 tested on i915 and Nvidia proprietary driver.

Reviewers: #kwin

Subscribers: zzag, alexeymin, kwin

Tags: #kwin

Maniphest Tasks: T11071

Differential Revision: https://phabricator.kde.org/D23514
parent b3a19f9e
......@@ -779,62 +779,7 @@ void Compositor::setCompositeTimer()
return;
}
uint waitTime = 1;
if (m_scene->blocksForRetrace()) {
// TODO: make vBlankTime dynamic?!
// It's required because glXWaitVideoSync will *likely* block a full frame if one enters
// a retrace pass which can last a variable amount of time, depending on the actual screen
// Now, my ooold 19" CRT can do such retrace so that 2ms are entirely sufficient,
// while another ooold 15" TFT requires about 6ms
qint64 padding = m_timeSinceLastVBlank;
if (padding > fpsInterval) {
// We're at low repaints or spent more time in painting than the user wanted to wait
// for that frame. Align to next vblank:
padding = vBlankInterval - (padding % vBlankInterval);
} else {
// Align to the next maxFps tick:
// "remaining time of the first vsync" + "time for the other vsyncs of the frame"
padding = ((vBlankInterval - padding % vBlankInterval) +
(fpsInterval / vBlankInterval - 1) * vBlankInterval);
}
if (padding < options->vBlankTime()) {
// We'll likely miss this frame so we add one:
waitTime = nanoToMilli(padding + vBlankInterval - options->vBlankTime());
} else {
waitTime = nanoToMilli(padding - options->vBlankTime());
}
}
else { // w/o blocking vsync we just jump to the next demanded tick
if (fpsInterval > m_timeSinceLastVBlank) {
waitTime = nanoToMilli(fpsInterval - m_timeSinceLastVBlank);
if (!waitTime) {
// Will ensure we don't block out the eventloop - the system's just not faster ...
waitTime = 1;
}
}
/* else if (m_timeSinceLastVBlank - fpsInterval < (vBlankInterval<<1)) {
// NOTICE - "for later" ------------------------------------------------------------------
// It can happen that we push two frames within one refresh cycle.
// Swapping will then block even with triple buffering when the GPU does not discard but
// queues frames
// now here's the mean part: if we take that as "OMG, we're late - next frame ASAP",
// there'll immediately be 2 frames in the pipe, swapping will block, we think we're
// late ... ewww
// so instead we pad to the clock again and add 2ms safety to ensure the pipe is really
// free
// NOTICE: obviously m_timeSinceLastVBlank can be too big because we're too slow as well
// So if this code was enabled, we'd needlessly half the framerate once more (15 instead of 30)
waitTime = nanoToMilli(vBlankInterval - (m_timeSinceLastVBlank - fpsInterval)%vBlankInterval) + 2;
}*/
else {
// "0" would be sufficient here, but the compositor isn't the WMs only task.
waitTime = 1;
}
}
uint waitTime = 1000 / refreshRate();
// Force 4fps minimum:
compositeTimer.start(qMin(waitTime, 250u), this);
}
......@@ -863,6 +808,11 @@ void WaylandCompositor::start()
return;
}
// TODO: This is problematic on Wayland. We should get the highest refresh rate
// and not the one of the first output. Also on hotplug reevaluate.
// On X11 do it also like this?
m_refreshRate = KWin::currentRefreshRate();
if (Workspace::self()) {
startupWithWorkspace();
} else {
......@@ -873,10 +823,7 @@ void WaylandCompositor::start()
int WaylandCompositor::refreshRate() const
{
// TODO: This makes no sense on Wayland. First step would be to atleast
// set the refresh rate to the highest available one. Second step
// would be to not use a uniform value at all but per screen.
return KWin::currentRefreshRate();
return m_refreshRate;
}
X11Compositor::X11Compositor(QObject *parent)
......
......@@ -187,6 +187,8 @@ protected:
private:
explicit WaylandCompositor(QObject *parent);
int m_refreshRate;
};
class KWIN_EXPORT X11Compositor : public Compositor
......
......@@ -30,8 +30,7 @@ namespace KWin
{
OpenGLBackend::OpenGLBackend()
: m_blocksForRetrace(false)
, m_directRendering(false)
: m_directRendering(false)
, m_haveBufferAge(false)
, m_failed(false)
{
......
......@@ -124,16 +124,6 @@ public:
bool isFailed() const {
return m_failed;
}
/**
* @brief Whether VSync blocks execution until the screen is in the retrace
*
* Case for waitVideoSync and non triple buffering buffer swaps (triple buffering support
* has been removed).
*
*/
bool blocksForRetrace() const {
return m_blocksForRetrace;
}
/**
* @brief Whether the backend uses direct rendering.
*
......@@ -202,16 +192,6 @@ protected:
* @param reason The reason why the initialization failed.
*/
void setFailed(const QString &reason);
/**
* @brief Sets whether the VSync iplementation blocks
*
* Should be called by the concrete subclass once it is determined how VSync works.
* If the subclass does not call this method, the backend defaults to @c false.
* @param enabled @c true if VSync blocks, @c false otherwise.
*/
void setBlocksForRetrace(bool enabled) {
m_blocksForRetrace = enabled;
}
/**
* @brief Sets whether the OpenGL context is direct.
*
......@@ -264,10 +244,6 @@ protected:
}
private:
/**
* @brief Whether present() will block execution until the next vertical retrace @c false.
*/
bool m_blocksForRetrace;
/**
* @brief Whether direct rendering is used, defaults to @c false.
*/
......
......@@ -121,7 +121,6 @@ void EglOnXBackend::init()
}
}
setBlocksForRetrace(true);
if (surfaceHasSubPost) {
qCDebug(KWIN_CORE) << "EGL implementation and surface support eglPostSubBufferNV, let's use it";
......@@ -355,8 +354,6 @@ QRegion EglOnXBackend::prepareRenderingFrame()
{
QRegion repaint;
present();
if (supportsBufferAge())
repaint = accumulatedDamageHistory(m_bufferAge);
......@@ -386,16 +383,7 @@ void EglOnXBackend::endRenderingFrame(const QRegion &renderedRegion, const QRegi
}
setLastDamage(renderedRegion);
if (!blocksForRetrace()) {
// This also sets lastDamage to empty which prevents the frame from
// being posted again when prepareRenderingFrame() is called.
present();
} else {
// Make sure that the GPU begins processing the command stream
// now and not the next time prepareRenderingFrame() is called.
glFlush();
}
present();
if (m_overlayWindow && overlayWindow()->window()) // show the window only after the first pass,
overlayWindow()->show(); // since that pass may take long
......
......@@ -115,11 +115,6 @@ GlxBackend::GlxBackend(Display *display)
, m_bufferAge(0)
, m_x11Display(display)
{
// Ensures calls to glXSwapBuffers will always block until the next
// retrace when using the proprietary NVIDIA driver. This must be
// set before libGL.so is loaded.
setenv("__GL_MaxFramesAllowed", "1", true);
// Force initialization of GLX integration in the Qt's xcb backend
// to make it call XESetWireToEvent callbacks, which is required
// by Mesa when using DRI2.
......@@ -229,8 +224,6 @@ void GlxBackend::init()
setSupportsBufferAge(true);
}
setBlocksForRetrace(true);
if (m_haveEXTSwapControl) {
glXSwapIntervalEXT(display(), glxWindow, 1);
} else if (m_haveMESASwapControl) {
......@@ -726,13 +719,10 @@ QRegion GlxBackend::prepareRenderingFrame()
{
QRegion repaint;
present();
if (supportsBufferAge())
repaint = accumulatedDamageHistory(m_bufferAge);
startRenderTimer();
glXWaitX();
return repaint;
}
......@@ -757,16 +747,7 @@ void GlxBackend::endRenderingFrame(const QRegion &renderedRegion, const QRegion
}
setLastDamage(renderedRegion);
if (!blocksForRetrace()) {
// This also sets lastDamage to empty which prevents the frame from
// being posted again when prepareRenderingFrame() is called.
present();
} else {
// Make sure that the GPU begins processing the command stream
// now and not the next time prepareRenderingFrame() is called.
glFlush();
}
present();
if (overlayWindow()->window()) // show the window only after the first pass,
overlayWindow()->show(); // since that pass may take long
......
......@@ -525,11 +525,6 @@ OverlayWindow *SceneOpenGL::overlayWindow() const
return m_backend->overlayWindow();
}
bool SceneOpenGL::blocksForRetrace() const
{
return m_backend->blocksForRetrace();
}
void SceneOpenGL::idle()
{
m_backend->idle();
......
......@@ -53,7 +53,6 @@ public:
void screenGeometryChanged(const QSize &size) override;
OverlayWindow *overlayWindow() const override;
bool usesOverlayWindow() const override;
bool blocksForRetrace() const override;
bool makeOpenGLContextCurrent() override;
void doneOpenGLContextCurrent() override;
Decoration::Renderer *createDecorationRenderer(Decoration::DecoratedClientImpl *impl) override;
......
......@@ -627,11 +627,6 @@ void Scene::extendPaintRegion(QRegion &region, bool opaqueFullscreen)
Q_UNUSED(opaqueFullscreen);
}
bool Scene::blocksForRetrace() const
{
return false;
}
void Scene::screenGeometryChanged(const QSize &size)
{
if (!overlayWindow()) {
......
......@@ -145,7 +145,6 @@ public:
enum ImageFilterType { ImageFilterFast, ImageFilterGood };
// there's nothing to paint (adjust time_diff later)
virtual void idle();
virtual bool blocksForRetrace() const;
virtual OverlayWindow* overlayWindow() const = 0;
virtual bool makeOpenGLContextCurrent();
......
......@@ -1609,11 +1609,6 @@ QString Workspace::supportInformation() const
}
support.append(QStringLiteral("OpenGL 2 Shaders are used\n"));
support.append(QStringLiteral("Painting blocks for vertical retrace: "));
if (m_compositor->scene()->blocksForRetrace())
support.append(QStringLiteral(" yes\n"));
else
support.append(QStringLiteral(" no\n"));
break;
}
case XRenderCompositing:
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment