From bcf64af49bd939b68bbc1cce76b6b88432e6c3e2 Mon Sep 17 00:00:00 2001 From: Roman Gilg Date: Thu, 9 Jan 2020 18:18:22 +0100 Subject: [PATCH] Revert "Remove vsync detection and configurability" This reverts commit b3a19f9e5b779bb42276a2210813e6b4dd322c8c. See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html --- composite.cpp | 18 +++++----- kcmkwin/kwincompositing/compositing.cpp | 24 +++++++------ kcmkwin/kwincompositing/compositing.ui | 5 +++ options.cpp | 4 +-- options.h | 2 +- platformsupport/scenes/opengl/backend.cpp | 3 +- platformsupport/scenes/opengl/backend.h | 24 +++++++++++++ plugins/platforms/drm/egl_gbm_backend.cpp | 2 +- plugins/platforms/drm/egl_stream_backend.cpp | 1 + .../hwcomposer/egl_hwcomposer_backend.cpp | 1 + .../platforms/x11/common/eglonxbackend.cpp | 22 +++++++----- .../platforms/x11/standalone/glxbackend.cpp | 36 ++++++++++++++----- plugins/platforms/x11/standalone/glxbackend.h | 2 ++ plugins/scenes/opengl/scene_opengl.cpp | 5 +++ plugins/scenes/opengl/scene_opengl.h | 1 + scene.cpp | 5 +++ scene.h | 1 + 17 files changed, 116 insertions(+), 40 deletions(-) diff --git a/composite.cpp b/composite.cpp index 51fb0c667f..0243688cd0 100644 --- a/composite.cpp +++ b/composite.cpp @@ -334,12 +334,14 @@ void Compositor::startupWithWorkspace() setupX11Support(); fpsInterval = options->maxFpsInterval(); - const auto rate = currentRefreshRate(); - Q_ASSERT(rate != 0); // There is a fallback in options.cpp, so why check at all? - - // If we do vsync, set the fps to the next multiple of the vblank rate. - vBlankInterval = milliToNano(1000) / currentRefreshRate(); - fpsInterval = qMax((fpsInterval / vBlankInterval) * vBlankInterval, vBlankInterval); + if (m_scene->syncsToVBlank()) { + // If we do vsync, set the fps to the next multiple of the vblank rate. + vBlankInterval = milliToNano(1000) / currentRefreshRate(); + fpsInterval = qMax((fpsInterval / vBlankInterval) * vBlankInterval, vBlankInterval); + } else { + // No vsync - DO NOT set "0", would cause div-by-zero segfaults. + vBlankInterval = milliToNano(1); + } // Sets also the 'effects' pointer. kwinApp()->platform()->createEffectsHandler(this, m_scene); @@ -717,7 +719,7 @@ void Compositor::performCompositing() // is called the next time. If there would be nothing pending, it will not restart the timer and // scheduleRepaint() would restart it again somewhen later, called from functions that // would again add something pending. - if (m_bufferSwapPending) { + if (m_bufferSwapPending && m_scene->syncsToVBlank()) { m_composeAtSwapCompletion = true; } else { scheduleRepaint(); @@ -816,7 +818,7 @@ void Compositor::setCompositeTimer() waitTime = 1; } } - /* else if (m_timeSinceLastVBlank - fpsInterval < (vBlankInterval<<1)) { + /* else if (m_scene->syncsToVBlank() && 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 diff --git a/kcmkwin/kwincompositing/compositing.cpp b/kcmkwin/kwincompositing/compositing.cpp index eb63d977a9..b43f7cea20 100644 --- a/kcmkwin/kwincompositing/compositing.cpp +++ b/kcmkwin/kwincompositing/compositing.cpp @@ -81,14 +81,16 @@ void Compositing::reset() auto swapStrategy = [&kwinConfig]() { const QString glSwapStrategyValue = kwinConfig.readEntry("GLPreferBufferSwap", "a"); - if (glSwapStrategyValue == "a") { + if (glSwapStrategyValue == "n") { return 0; - } else if (glSwapStrategyValue == "e") { + } else if (glSwapStrategyValue == "a") { return 1; - } else if (glSwapStrategyValue == "p") { + } else if (glSwapStrategyValue == "e") { return 2; - } else if (glSwapStrategyValue == "c") { + } else if (glSwapStrategyValue == "p") { return 3; + } else if (glSwapStrategyValue == "c") { + return 4; } return 0; }; @@ -284,13 +286,15 @@ void Compositing::save() } auto swapStrategy = [this] { switch (glSwapStrategy()) { - case 1: - return QStringLiteral("e"); - case 2: - return QStringLiteral("p"); - case 3: - return QStringLiteral("c"); case 0: + return QStringLiteral("n"); + case 2: + return QStringLiteral("e"); + case 3: + return QStringLiteral("p"); + case 4: + return QStringLiteral("c"); + case 1: default: return QStringLiteral("a"); } diff --git a/kcmkwin/kwincompositing/compositing.ui b/kcmkwin/kwincompositing/compositing.ui index 45e82fe70b..ec30fc32fb 100644 --- a/kcmkwin/kwincompositing/compositing.ui +++ b/kcmkwin/kwincompositing/compositing.ui @@ -165,6 +165,11 @@ Alternatively, you might want to use the XRender backend instead. + + + Never + + Automatic diff --git a/options.cpp b/options.cpp index aa30eb6b0f..63f922cf6d 100644 --- a/options.cpp +++ b/options.cpp @@ -918,12 +918,12 @@ void Options::reloadCompositingSettings(bool force) } setGLCoreProfile(config.readEntry("GLCore", Options::defaultGLCoreProfile())); - char c = 'a'; + char c = 0; const QString s = config.readEntry("GLPreferBufferSwap", QString(Options::defaultGlPreferBufferSwap())); if (!s.isEmpty()) c = s.at(0).toLatin1(); if (c != 'a' && c != 'c' && c != 'p' && c != 'e') - c = 'a'; + c = 0; setGlPreferBufferSwap(c); m_xrenderSmoothScale = config.readEntry("XRenderSmoothScale", false); diff --git a/options.h b/options.h index 8f28f1df84..ee31cfea90 100644 --- a/options.h +++ b/options.h @@ -570,7 +570,7 @@ public: return m_glPlatformInterface; } - enum GlSwapStrategy { CopyFrontBuffer = 'c', PaintFullScreen = 'p', ExtendDamage = 'e', AutoSwapStrategy = 'a' }; + enum GlSwapStrategy { NoSwapEncourage = 0, CopyFrontBuffer = 'c', PaintFullScreen = 'p', ExtendDamage = 'e', AutoSwapStrategy = 'a' }; GlSwapStrategy glPreferBufferSwap() const { return m_glPreferBufferSwap; } diff --git a/platformsupport/scenes/opengl/backend.cpp b/platformsupport/scenes/opengl/backend.cpp index 18fc7e61b6..45b6fbd30a 100644 --- a/platformsupport/scenes/opengl/backend.cpp +++ b/platformsupport/scenes/opengl/backend.cpp @@ -30,7 +30,8 @@ namespace KWin { OpenGLBackend::OpenGLBackend() - : m_blocksForRetrace(false) + : m_syncsToVBlank(false) + , m_blocksForRetrace(false) , m_directRendering(false) , m_haveBufferAge(false) , m_failed(false) diff --git a/platformsupport/scenes/opengl/backend.h b/platformsupport/scenes/opengl/backend.h index 6e405b663f..2879f68e6e 100644 --- a/platformsupport/scenes/opengl/backend.h +++ b/platformsupport/scenes/opengl/backend.h @@ -124,6 +124,16 @@ public: bool isFailed() const { return m_failed; } + /** + * @brief Whether the Backend provides VSync. + * + * Currently only the GLX backend can provide VSync. + * + * @return bool @c true if VSync support is available, @c false otherwise + */ + bool syncsToVBlank() const { + return m_syncsToVBlank; + } /** * @brief Whether VSync blocks execution until the screen is in the retrace * @@ -202,6 +212,16 @@ protected: * @param reason The reason why the initialization failed. */ void setFailed(const QString &reason); + /** + * @brief Sets whether the backend provides VSync. + * + * Should be called by the concrete subclass once it is determined whether VSync is supported. + * If the subclass does not call this method, the backend defaults to @c false. + * @param enabled @c true if VSync support available, @c false otherwise. + */ + void setSyncsToVBlank(bool enabled) { + m_syncsToVBlank = enabled; + } /** * @brief Sets whether the VSync iplementation blocks * @@ -264,6 +284,10 @@ protected: } private: + /** + * @brief Whether VSync is available and used, defaults to @c false. + */ + bool m_syncsToVBlank; /** * @brief Whether present() will block execution until the next vertical retrace @c false. */ diff --git a/plugins/platforms/drm/egl_gbm_backend.cpp b/plugins/platforms/drm/egl_gbm_backend.cpp index cd21c748ba..2da6f7a226 100644 --- a/plugins/platforms/drm/egl_gbm_backend.cpp +++ b/plugins/platforms/drm/egl_gbm_backend.cpp @@ -42,7 +42,7 @@ EglGbmBackend::EglGbmBackend(DrmBackend *drmBackend) { // Egl is always direct rendering. setIsDirectRendering(true); - + setSyncsToVBlank(true); connect(m_backend, &DrmBackend::outputAdded, this, &EglGbmBackend::createOutput); connect(m_backend, &DrmBackend::outputRemoved, this, &EglGbmBackend::removeOutput); } diff --git a/plugins/platforms/drm/egl_stream_backend.cpp b/plugins/platforms/drm/egl_stream_backend.cpp index ce5741d959..374ab42bbb 100644 --- a/plugins/platforms/drm/egl_stream_backend.cpp +++ b/plugins/platforms/drm/egl_stream_backend.cpp @@ -86,6 +86,7 @@ EglStreamBackend::EglStreamBackend(DrmBackend *b) : AbstractEglBackend(), m_backend(b) { setIsDirectRendering(true); + setSyncsToVBlank(true); connect(m_backend, &DrmBackend::outputAdded, this, &EglStreamBackend::createOutput); connect(m_backend, &DrmBackend::outputRemoved, this, [this] (DrmOutput *output) { diff --git a/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp b/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp index 7bf525959d..e102b8c75c 100644 --- a/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp +++ b/plugins/platforms/hwcomposer/egl_hwcomposer_backend.cpp @@ -30,6 +30,7 @@ EglHwcomposerBackend::EglHwcomposerBackend(HwcomposerBackend *backend) { // EGL is always direct rendering setIsDirectRendering(true); + setSyncsToVBlank(true); } EglHwcomposerBackend::~EglHwcomposerBackend() diff --git a/plugins/platforms/x11/common/eglonxbackend.cpp b/plugins/platforms/x11/common/eglonxbackend.cpp index f4b8fe8cdb..baa9c06ac2 100644 --- a/plugins/platforms/x11/common/eglonxbackend.cpp +++ b/plugins/platforms/x11/common/eglonxbackend.cpp @@ -121,21 +121,27 @@ void EglOnXBackend::init() } } + setSyncsToVBlank(false); setBlocksForRetrace(true); if (surfaceHasSubPost) { qCDebug(KWIN_CORE) << "EGL implementation and surface support eglPostSubBufferNV, let's use it"; - // check if swap interval 1 is supported - EGLint val; - eglGetConfigAttrib(eglDisplay(), config(), EGL_MAX_SWAP_INTERVAL, &val); - if (val >= 1) { - if (eglSwapInterval(eglDisplay(), 1)) { - qCDebug(KWIN_CORE) << "Enabled v-sync"; + if (options->glPreferBufferSwap() != Options::NoSwapEncourage) { + // check if swap interval 1 is supported + EGLint val; + eglGetConfigAttrib(eglDisplay(), config(), EGL_MAX_SWAP_INTERVAL, &val); + if (val >= 1) { + if (eglSwapInterval(eglDisplay(), 1)) { + qCDebug(KWIN_CORE) << "Enabled v-sync"; + setSyncsToVBlank(true); + } + } else { + qCWarning(KWIN_CORE) << "Cannot enable v-sync as max. swap interval is" << val; } } else { - qCWarning(KWIN_CORE) << "Cannot enable v-sync as max. swap interval is" << val; + // disable v-sync + eglSwapInterval(eglDisplay(), 0); } - } else { /* In the GLX backend, we fall back to using glCopyPixels if we have no extension providing support for partial screen updates. * However, that does not work in EGL - glCopyPixels with glDrawBuffer(GL_FRONT); does nothing. diff --git a/plugins/platforms/x11/standalone/glxbackend.cpp b/plugins/platforms/x11/standalone/glxbackend.cpp index 453f345ac9..e632109543 100644 --- a/plugins/platforms/x11/standalone/glxbackend.cpp +++ b/plugins/platforms/x11/standalone/glxbackend.cpp @@ -113,6 +113,7 @@ GlxBackend::GlxBackend(Display *display) , glxWindow(None) , ctx(nullptr) , m_bufferAge(0) + , haveSwapInterval(false) , m_x11Display(display) { // Ensures calls to glXSwapBuffers will always block until the next @@ -220,6 +221,8 @@ void GlxBackend::init() glXSelectEvent(display(), glxWindow, GLX_BUFFER_SWAP_COMPLETE_INTEL_MASK); } + haveSwapInterval = m_haveMESASwapControl || m_haveEXTSwapControl; + setSupportsBufferAge(false); if (hasExtension(QByteArrayLiteral("GLX_EXT_buffer_age"))) { @@ -229,16 +232,20 @@ void GlxBackend::init() setSupportsBufferAge(true); } + setSyncsToVBlank(false); setBlocksForRetrace(true); - - if (m_haveEXTSwapControl) { - glXSwapIntervalEXT(display(), glxWindow, 1); - } else if (m_haveMESASwapControl) { - glXSwapIntervalMESA(1); + const bool wantSync = options->glPreferBufferSwap() != Options::NoSwapEncourage; + if (wantSync && glXIsDirect(display(), ctx)) { + if (haveSwapInterval) { // glXSwapInterval is preferred being more reliable + setSwapInterval(1); + setSyncsToVBlank(true); + } else { + qCWarning(KWIN_X11STANDALONE) << "NO VSYNC! glSwapInterval is not supported"; + } } else { - qCWarning(KWIN_X11STANDALONE) << "NO VSYNC! glSwapInterval is not supported"; + // disable v-sync (if possible) + setSwapInterval(0); } - if (glPlatform->isVirtualBox()) { // VirtualBox does not support glxQueryDrawable // this should actually be in kwinglutils_funcs, but QueryDrawable seems not to be provided by an extension @@ -665,6 +672,14 @@ FBConfigInfo *GlxBackend::infoForVisual(xcb_visualid_t visual) return info; } +void GlxBackend::setSwapInterval(int interval) +{ + if (m_haveEXTSwapControl) + glXSwapIntervalEXT(display(), glxWindow, interval); + else if (m_haveMESASwapControl) + glXSwapIntervalMESA(interval); +} + void GlxBackend::present() { if (lastDamage().isEmpty()) @@ -678,8 +693,11 @@ void GlxBackend::present() if (m_haveINTELSwapEvent) Compositor::self()->aboutToSwapBuffers(); - glXSwapBuffers(display(), glxWindow); - + if (haveSwapInterval) { + glXSwapBuffers(display(), glxWindow); + } else { + glXSwapBuffers(display(), glxWindow); + } if (supportsBufferAge()) { glXQueryDrawable(display(), glxWindow, GLX_BACK_BUFFER_AGE_EXT, (GLuint *) &m_bufferAge); } diff --git a/plugins/platforms/x11/standalone/glxbackend.h b/plugins/platforms/x11/standalone/glxbackend.h index d72dda8658..ec55ac49b1 100644 --- a/plugins/platforms/x11/standalone/glxbackend.h +++ b/plugins/platforms/x11/standalone/glxbackend.h @@ -89,6 +89,7 @@ private: bool initRenderingContext(); bool initFbConfig(); void initVisualDepthHashTable(); + void setSwapInterval(int interval); Display *display() const { return m_x11Display; } @@ -112,6 +113,7 @@ private: bool m_haveMESASwapControl = false; bool m_haveEXTSwapControl = false; bool m_haveINTELSwapEvent = false; + bool haveSwapInterval = false; Display *m_x11Display; friend class GlxTexture; }; diff --git a/plugins/scenes/opengl/scene_opengl.cpp b/plugins/scenes/opengl/scene_opengl.cpp index 4f7478ea59..3b5d2432d0 100644 --- a/plugins/scenes/opengl/scene_opengl.cpp +++ b/plugins/scenes/opengl/scene_opengl.cpp @@ -492,6 +492,11 @@ OverlayWindow *SceneOpenGL::overlayWindow() const return m_backend->overlayWindow(); } +bool SceneOpenGL::syncsToVBlank() const +{ + return m_backend->syncsToVBlank(); +} + bool SceneOpenGL::blocksForRetrace() const { return m_backend->blocksForRetrace(); diff --git a/plugins/scenes/opengl/scene_opengl.h b/plugins/scenes/opengl/scene_opengl.h index 1b5be8bbe4..e1a4036319 100644 --- a/plugins/scenes/opengl/scene_opengl.h +++ b/plugins/scenes/opengl/scene_opengl.h @@ -53,6 +53,7 @@ public: OverlayWindow *overlayWindow() const override; bool usesOverlayWindow() const override; bool blocksForRetrace() const override; + bool syncsToVBlank() const override; bool makeOpenGLContextCurrent() override; void doneOpenGLContextCurrent() override; Decoration::Renderer *createDecorationRenderer(Decoration::DecoratedClientImpl *impl) override; diff --git a/scene.cpp b/scene.cpp index a1115dc280..f8facec29d 100644 --- a/scene.cpp +++ b/scene.cpp @@ -626,6 +626,11 @@ bool Scene::blocksForRetrace() const return false; } +bool Scene::syncsToVBlank() const +{ + return false; +} + void Scene::screenGeometryChanged(const QSize &size) { if (!overlayWindow()) { diff --git a/scene.h b/scene.h index 82e0379fb4..cb18317327 100644 --- a/scene.h +++ b/scene.h @@ -146,6 +146,7 @@ public: // there's nothing to paint (adjust time_diff later) virtual void idle(); virtual bool blocksForRetrace() const; + virtual bool syncsToVBlank() const; virtual OverlayWindow* overlayWindow() const = 0; virtual bool makeOpenGLContextCurrent();