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

This reverts commit 8d13729031.

See: https://mail.kde.org/pipermail/kwin/2020-January/002999.html

composite.cpp

@@ -779,7 +779,62 @@ void Compositor::setCompositeTimer()
         return;
     }
 
-    uint waitTime = 1000 / refreshRate();
+    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;
+        }
+    }
     // Force 4fps minimum:
     compositeTimer.start(qMin(waitTime, 250u), this);
 }
@@ -808,11 +863,6 @@ 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 {
@@ -823,7 +873,10 @@ void WaylandCompositor::start()
 
 int WaylandCompositor::refreshRate() const
 {
-    return m_refreshRate;
+    // 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();
 }
 
 X11Compositor::X11Compositor(QObject *parent)

composite.h

@@ -187,8 +187,6 @@ protected:
 
 private:
     explicit WaylandCompositor(QObject *parent);
-
-    int m_refreshRate;
 };
 
 class KWIN_EXPORT X11Compositor : public Compositor

platformsupport/scenes/opengl/backend.cpp

@@ -30,7 +30,8 @@ namespace KWin
 {
 
 OpenGLBackend::OpenGLBackend()
-    : m_directRendering(false)
+    : m_blocksForRetrace(false)
+    , m_directRendering(false)
     , m_haveBufferAge(false)
     , m_failed(false)
 {

platformsupport/scenes/opengl/backend.h

@@ -124,6 +124,16 @@ 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.
      *
@@ -192,6 +202,16 @@ 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.
      *
@@ -244,6 +264,10 @@ 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.
      */

plugins/platforms/x11/common/eglonxbackend.cpp

@@ -121,6 +121,7 @@ void EglOnXBackend::init()
         }
     }
 
+    setBlocksForRetrace(true);
     if (surfaceHasSubPost) {
         qCDebug(KWIN_CORE) << "EGL implementation and surface support eglPostSubBufferNV, let's use it";
 
@@ -354,6 +355,8 @@ QRegion EglOnXBackend::prepareRenderingFrame()
 {
     QRegion repaint;
 
+    present();
+
     if (supportsBufferAge())
         repaint = accumulatedDamageHistory(m_bufferAge);
 
@@ -383,7 +386,16 @@ void EglOnXBackend::endRenderingFrame(const QRegion &renderedRegion, const QRegi
     }
 
     setLastDamage(renderedRegion);
-    present();
+
+    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();
+    }
 
     if (m_overlayWindow && overlayWindow()->window())  // show the window only after the first pass,
         overlayWindow()->show();   // since that pass may take long

plugins/platforms/x11/standalone/glxbackend.cpp

@@ -115,6 +115,11 @@ 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.
@@ -224,6 +229,8 @@ void GlxBackend::init()
             setSupportsBufferAge(true);
     }
 
+    setBlocksForRetrace(true);
+
     if (m_haveEXTSwapControl) {
         glXSwapIntervalEXT(display(), glxWindow, 1);
     } else if (m_haveMESASwapControl) {
@@ -719,10 +726,13 @@ QRegion GlxBackend::prepareRenderingFrame()
 {
     QRegion repaint;
 
+    present();
+
     if (supportsBufferAge())
         repaint = accumulatedDamageHistory(m_bufferAge);
 
     startRenderTimer();
+    glXWaitX();
 
     return repaint;
 }
@@ -747,7 +757,16 @@ void GlxBackend::endRenderingFrame(const QRegion &renderedRegion, const QRegion
     }
 
     setLastDamage(renderedRegion);
-    present();
+
+    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();
+    }
 
     if (overlayWindow()->window())  // show the window only after the first pass,
         overlayWindow()->show();   // since that pass may take long

plugins/scenes/opengl/scene_opengl.cpp

@@ -492,6 +492,11 @@ OverlayWindow *SceneOpenGL::overlayWindow() const
     return m_backend->overlayWindow();
 }
 
+bool SceneOpenGL::blocksForRetrace() const
+{
+    return m_backend->blocksForRetrace();
+}
+
 void SceneOpenGL::idle()
 {
     m_backend->idle();

plugins/scenes/opengl/scene_opengl.h

@@ -52,6 +52,7 @@ 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;

scene.cpp

@@ -621,6 +621,11 @@ void Scene::extendPaintRegion(QRegion &region, bool opaqueFullscreen)
     Q_UNUSED(opaqueFullscreen);
 }
 
+bool Scene::blocksForRetrace() const
+{
+    return false;
+}
+
 void Scene::screenGeometryChanged(const QSize &size)
 {
     if (!overlayWindow()) {

scene.h

@@ -145,6 +145,7 @@ 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();

workspace.cpp

@@ -1609,6 +1609,11 @@ 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: