From ad892ce3a65efa3e802249835a9f9bbf06581311 Mon Sep 17 00:00:00 2001 From: Roman Gilg Date: Tue, 27 Aug 2019 21:36:18 +0200 Subject: [PATCH] [platforms/x11] Remove triple buffering detection Summary: It is not clear what the advantage of triple buffering is for KWin. An X11 compositor is meant to swap buffers once every monitor cycle. For that triple buffering is not necessary. The functionality is not maintained, does not reliably work as displayed by the existence of an environment variable to force some behavior, pollutes our code and every compositing-related problem that might be mitigated with triple buffering should find a simpler and more fitting solution with other means. There is one caveat which is if we shall block for retrace. We set it currently according to the result of the swap profiler and in the most common case with double buffering it is set to true. But on Nvidia systems this might be actual the wrong behavior. Instead of trying to work around this ignore the issue for now and move the overall architecture to something less complex by presenting after paint how we do it in the Wayland DRM backend and with double buffering on GLX (although this is at the moment also borken because we actually present then twice). Test Plan: kwin_x11 tested on i915. Reviewers: #kwin, zzag Reviewed By: #kwin, zzag Subscribers: zzag, fredrik, kwin Tags: #kwin Maniphest Tasks: T11071 Differential Revision: https://phabricator.kde.org/D23504 --- platformsupport/scenes/opengl/CMakeLists.txt | 1 - platformsupport/scenes/opengl/backend.h | 3 +- .../scenes/opengl/swap_profiler.cpp | 57 ------------------- platformsupport/scenes/opengl/swap_profiler.h | 53 ----------------- .../platforms/x11/common/eglonxbackend.cpp | 49 +--------------- plugins/platforms/x11/common/eglonxbackend.h | 2 - .../platforms/x11/standalone/glxbackend.cpp | 34 ----------- plugins/platforms/x11/standalone/glxbackend.h | 2 - 8 files changed, 3 insertions(+), 198 deletions(-) delete mode 100644 platformsupport/scenes/opengl/swap_profiler.cpp delete mode 100644 platformsupport/scenes/opengl/swap_profiler.h diff --git a/platformsupport/scenes/opengl/CMakeLists.txt b/platformsupport/scenes/opengl/CMakeLists.txt index 7f3fd8f46b..01efc02335 100644 --- a/platformsupport/scenes/opengl/CMakeLists.txt +++ b/platformsupport/scenes/opengl/CMakeLists.txt @@ -2,7 +2,6 @@ set(SCENE_OPENGL_BACKEND_SRCS abstract_egl_backend.cpp backend.cpp egl_dmabuf.cpp - swap_profiler.cpp texture.cpp ) diff --git a/platformsupport/scenes/opengl/backend.h b/platformsupport/scenes/opengl/backend.h index c311d21b9a..2879f68e6e 100644 --- a/platformsupport/scenes/opengl/backend.h +++ b/platformsupport/scenes/opengl/backend.h @@ -137,7 +137,8 @@ public: /** * @brief Whether VSync blocks execution until the screen is in the retrace * - * Case for waitVideoSync and non triple buffering buffer swaps + * Case for waitVideoSync and non triple buffering buffer swaps (triple buffering support + * has been removed). * */ bool blocksForRetrace() const { diff --git a/platformsupport/scenes/opengl/swap_profiler.cpp b/platformsupport/scenes/opengl/swap_profiler.cpp deleted file mode 100644 index 619a2ae656..0000000000 --- a/platformsupport/scenes/opengl/swap_profiler.cpp +++ /dev/null @@ -1,57 +0,0 @@ -/******************************************************************** - KWin - the KDE window manager - This file is part of the KDE project. - -Copyright (C) 2006 Lubos Lunak -Copyright (C) 2009, 2010, 2011 Martin Gräßlin - -This program is free software; you can redistribute it and/or modify -it under the terms of the GNU General Public License as published by -the Free Software Foundation; either version 2 of the License, or -(at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. - -You should have received a copy of the GNU General Public License -along with this program. If not, see . -*********************************************************************/ -#include "swap_profiler.h" -#include - -namespace KWin -{ - -SwapProfiler::SwapProfiler() -{ - init(); -} - -void SwapProfiler::init() -{ - m_time = 2 * 1000*1000; // we start with a long time mean of 2ms ... - m_counter = 0; -} - -void SwapProfiler::begin() -{ - m_timer.start(); -} - -char SwapProfiler::end() -{ - // .. and blend in actual values. - // this way we prevent extremes from killing our long time mean - m_time = (10*m_time + m_timer.nsecsElapsed())/11; - if (++m_counter > 500) { - const bool blocks = m_time > 1000 * 1000; // 1ms, i get ~250µs and ~7ms w/o triple buffering... - qCDebug(KWIN_OPENGL) << "Triple buffering detection:" << QString(blocks ? QStringLiteral("NOT available") : QStringLiteral("Available")) << - " - Mean block time:" << m_time/(1000.0*1000.0) << "ms"; - return blocks ? 'd' : 't'; - } - return 0; -} - -} diff --git a/platformsupport/scenes/opengl/swap_profiler.h b/platformsupport/scenes/opengl/swap_profiler.h deleted file mode 100644 index cbc4d16369..0000000000 --- a/platformsupport/scenes/opengl/swap_profiler.h +++ /dev/null @@ -1,53 +0,0 @@ -/******************************************************************** - KWin - the KDE window manager - This file is part of the KDE project. - -Copyright (C) 2006 Lubos Lunak -Copyright (C) 2009, 2010, 2011 Martin Gräßlin - -This program is free software; you can redistribute it and/or modify -it under the terms of the GNU General Public License as published by -the Free Software Foundation; either version 2 of the License, or -(at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. - -You should have received a copy of the GNU General Public License -along with this program. If not, see . -*********************************************************************/ -#ifndef KWIN_SCENE_OPENGL_SWAP_PROFILER_H -#define KWIN_SCENE_OPENGL_SWAP_PROFILER_H - -#include -#include - -namespace KWin -{ - -/** - * @short Profiler to detect whether we have triple buffering - * The strategy is to start setBlocksForRetrace(false) but assume blocking and have the system prove that assumption wrong - */ -class KWIN_EXPORT SwapProfiler -{ -public: - SwapProfiler(); - void init(); - void begin(); - /** - * @return char being 'd' for double, 't' for triple (or more - but non-blocking) buffering and - * 0 (NOT '0') otherwise, so you can act on "if (char result = SwapProfiler::end()) { fooBar(); } - */ - char end(); -private: - QElapsedTimer m_timer; - qint64 m_time; - int m_counter; -}; - -} - -#endif diff --git a/plugins/platforms/x11/common/eglonxbackend.cpp b/plugins/platforms/x11/common/eglonxbackend.cpp index 0bb109d382..baa9c06ac2 100644 --- a/plugins/platforms/x11/common/eglonxbackend.cpp +++ b/plugins/platforms/x11/common/eglonxbackend.cpp @@ -73,9 +73,6 @@ EglOnXBackend::EglOnXBackend(xcb_connection_t *connection, Display *display, xcb setIsDirectRendering(true); } -static bool gs_tripleBufferUndetected = true; -static bool gs_tripleBufferNeedsDetection = false; - EglOnXBackend::~EglOnXBackend() { if (isFailed() && m_overlayWindow) { @@ -83,9 +80,6 @@ EglOnXBackend::~EglOnXBackend() } cleanup(); - gs_tripleBufferUndetected = true; - gs_tripleBufferNeedsDetection = false; - if (m_overlayWindow) { if (overlayWindow()->window()) { overlayWindow()->destroy(); @@ -128,9 +122,7 @@ void EglOnXBackend::init() } setSyncsToVBlank(false); - setBlocksForRetrace(false); - gs_tripleBufferNeedsDetection = false; - m_swapProfiler.init(); + setBlocksForRetrace(true); if (surfaceHasSubPost) { qCDebug(KWIN_CORE) << "EGL implementation and surface support eglPostSubBufferNV, let's use it"; @@ -142,12 +134,6 @@ void EglOnXBackend::init() if (eglSwapInterval(eglDisplay(), 1)) { qCDebug(KWIN_CORE) << "Enabled v-sync"; setSyncsToVBlank(true); - const QByteArray tripleBuffer = qgetenv("KWIN_TRIPLE_BUFFER"); - if (!tripleBuffer.isEmpty()) { - setBlocksForRetrace(qstrcmp(tripleBuffer, "0") == 0); - gs_tripleBufferUndetected = false; - } - gs_tripleBufferNeedsDetection = gs_tripleBufferUndetected; } } else { qCWarning(KWIN_CORE) << "Cannot enable v-sync as max. swap interval is" << val; @@ -343,32 +329,8 @@ void EglOnXBackend::presentSurface(EGLSurface surface, const QRegion &damage, co const bool fullRepaint = supportsBufferAge() || (damage == screenGeometry); if (fullRepaint || !surfaceHasSubPost) { - if (gs_tripleBufferNeedsDetection) { - eglWaitGL(); - m_swapProfiler.begin(); - } // the entire screen changed, or we cannot do partial updates (which implies we enabled surface preservation) eglSwapBuffers(eglDisplay(), surface); - if (gs_tripleBufferNeedsDetection) { - eglWaitGL(); - if (char result = m_swapProfiler.end()) { - gs_tripleBufferUndetected = gs_tripleBufferNeedsDetection = false; - if (result == 'd' && GLPlatform::instance()->driver() == Driver_NVidia) { - // TODO this is a workaround, we should get __GL_YIELD set before libGL checks it - if (qstrcmp(qgetenv("__GL_YIELD"), "USLEEP")) { - options->setGlPreferBufferSwap(0); - eglSwapInterval(eglDisplay(), 0); - result = 0; // hint proper behavior - qCWarning(KWIN_CORE) << "\nIt seems you are using the nvidia driver without triple buffering\n" - "You must export __GL_YIELD=\"USLEEP\" to prevent large CPU overhead on synced swaps\n" - "Preferably, enable the TripleBuffer Option in the xorg.conf Device\n" - "For this reason, the tearing prevention has been disabled.\n" - "See https://bugs.kde.org/show_bug.cgi?id=322060\n"; - } - } - setBlocksForRetrace(result == 'd'); - } - } if (supportsBufferAge()) { eglQuerySurface(eglDisplay(), surface, EGL_BUFFER_AGE_EXT, &m_bufferAge); } @@ -399,15 +361,6 @@ QRegion EglOnXBackend::prepareRenderingFrame() { QRegion repaint; - if (gs_tripleBufferNeedsDetection) { - // the composite timer floors the repaint frequency. This can pollute our triple buffering - // detection because the glXSwapBuffers call for the new frame has to wait until the pending - // one scanned out. - // So we compensate for that by waiting an extra milisecond to give the driver the chance to - // fllush the buffer queue - usleep(1000); - } - present(); if (supportsBufferAge()) diff --git a/plugins/platforms/x11/common/eglonxbackend.h b/plugins/platforms/x11/common/eglonxbackend.h index 0af43eb0f3..02ab1a5527 100644 --- a/plugins/platforms/x11/common/eglonxbackend.h +++ b/plugins/platforms/x11/common/eglonxbackend.h @@ -20,7 +20,6 @@ along with this program. If not, see . #ifndef KWIN_EGL_ON_X_BACKEND_H #define KWIN_EGL_ON_X_BACKEND_H #include "abstract_egl_backend.h" -#include "swap_profiler.h" #include @@ -82,7 +81,6 @@ private: xcb_window_t m_renderingWindow = XCB_WINDOW_NONE; bool m_havePlatformBase = false; bool m_x11TextureFromPixmapSupported = true; - SwapProfiler m_swapProfiler; friend class EglTexture; }; diff --git a/plugins/platforms/x11/standalone/glxbackend.cpp b/plugins/platforms/x11/standalone/glxbackend.cpp index 65ed613b83..a019ff817b 100644 --- a/plugins/platforms/x11/standalone/glxbackend.cpp +++ b/plugins/platforms/x11/standalone/glxbackend.cpp @@ -127,9 +127,6 @@ GlxBackend::GlxBackend(Display *display) QOpenGLContext::supportsThreadedOpenGL(); } -static bool gs_tripleBufferUndetected = true; -static bool gs_tripleBufferNeedsDetection = false; - GlxBackend::~GlxBackend() { if (isFailed()) { @@ -141,9 +138,6 @@ GlxBackend::~GlxBackend() doneCurrent(); EffectQuickView::setShareContext(nullptr); - gs_tripleBufferUndetected = true; - gs_tripleBufferNeedsDetection = false; - if (ctx) glXDestroyContext(display(), ctx); @@ -242,19 +236,11 @@ void GlxBackend::init() setSyncsToVBlank(false); setBlocksForRetrace(false); haveWaitSync = false; - gs_tripleBufferNeedsDetection = false; - m_swapProfiler.init(); const bool wantSync = options->glPreferBufferSwap() != Options::NoSwapEncourage; if (wantSync && glXIsDirect(display(), ctx)) { if (haveSwapInterval) { // glXSwapInterval is preferred being more reliable setSwapInterval(1); setSyncsToVBlank(true); - const QByteArray tripleBuffer = qgetenv("KWIN_TRIPLE_BUFFER"); - if (!tripleBuffer.isEmpty()) { - setBlocksForRetrace(qstrcmp(tripleBuffer, "0") == 0); - gs_tripleBufferUndetected = false; - } - gs_tripleBufferNeedsDetection = gs_tripleBufferUndetected; } else if (hasExtension(QByteArrayLiteral("GLX_SGI_video_sync"))) { unsigned int sync; if (glXGetVideoSyncSGI(&sync) == 0 && glXWaitVideoSyncSGI(1, 0, &sync) == 0) { @@ -736,18 +722,7 @@ void GlxBackend::present() Compositor::self()->aboutToSwapBuffers(); if (haveSwapInterval) { - if (gs_tripleBufferNeedsDetection) { - glXWaitGL(); - m_swapProfiler.begin(); - } glXSwapBuffers(display(), glxWindow); - if (gs_tripleBufferNeedsDetection) { - glXWaitGL(); - if (char result = m_swapProfiler.end()) { - gs_tripleBufferUndetected = gs_tripleBufferNeedsDetection = false; - setBlocksForRetrace(result == 'd'); - } - } } else { waitSync(); glXSwapBuffers(display(), glxWindow); @@ -798,15 +773,6 @@ QRegion GlxBackend::prepareRenderingFrame() { QRegion repaint; - if (gs_tripleBufferNeedsDetection) { - // the composite timer floors the repaint frequency. This can pollute our triple buffering - // detection because the glXSwapBuffers call for the new frame has to wait until the pending - // one scanned out. - // So we compensate for that by waiting an extra milisecond to give the driver the chance to - // fllush the buffer queue - usleep(1000); - } - present(); if (supportsBufferAge()) diff --git a/plugins/platforms/x11/standalone/glxbackend.h b/plugins/platforms/x11/standalone/glxbackend.h index f34ef103a8..33914337fb 100644 --- a/plugins/platforms/x11/standalone/glxbackend.h +++ b/plugins/platforms/x11/standalone/glxbackend.h @@ -21,7 +21,6 @@ along with this program. If not, see . #define KWIN_GLX_BACKEND_H #include "backend.h" #include "texture.h" -#include "swap_profiler.h" #include "x11eventfilter.h" #include @@ -119,7 +118,6 @@ private: bool haveSwapInterval = false; bool haveWaitSync = false; Display *m_x11Display; - SwapProfiler m_swapProfiler; friend class GlxTexture; };