From 32e9de1e23b6d3c71e0d3afead5489453095647b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 27 Aug 2014 18:24:43 +0200 Subject: [PATCH] Refactor the swap completion interface Move the buffer-swap-pending state from the compositing backends to the Compositor class. The Compositor is the only class that needs to access the state, and this way it to do it without calling through a chain of virtual functions. This commit adds two new functions to Compositor; aboutToSwapBuffers() and bufferSwapComplete(). The backends call these functions to set and reset the buffer-swap-pending state. This commit also renames a number of functions and variables to make their meaning clear. The act of promoting the contents of the back buffer to become the contents of the front buffer is referred to as posting the buffer, presenting the buffer, or swapping the buffers; rendering the buffer is what paintScreen() does. --- composite.cpp | 36 +++++++++++++++++++++++++++--------- composite.h | 22 ++++++++++++++-------- egl_wayland_backend.cpp | 17 +++-------------- egl_wayland_backend.h | 3 --- scene.h | 8 -------- scene_opengl.cpp | 5 ----- scene_opengl.h | 13 ------------- scene_qpainter.cpp | 23 +++-------------------- scene_qpainter.h | 17 ----------------- scene_xrender.cpp | 24 +++++------------------- scene_xrender.h | 14 +------------- wayland_client/surface.cpp | 2 +- wayland_client/surface.h | 2 +- 13 files changed, 55 insertions(+), 131 deletions(-) diff --git a/composite.cpp b/composite.cpp index 2946f01e1f..de828dc4e2 100644 --- a/composite.cpp +++ b/composite.cpp @@ -88,7 +88,8 @@ Compositor::Compositor(QObject* workspace) , m_finishing(false) , m_timeSinceLastVBlank(0) , m_scene(NULL) - , m_waitingForFrameRendered(false) + , m_bufferSwapPending(false) + , m_composeAtSwapCompletion(false) { qRegisterMetaType("Compositor::SuspendReason"); connect(&unredirectTimer, SIGNAL(timeout()), SLOT(delayedCheckUnredirect())); @@ -559,22 +560,35 @@ void Compositor::timerEvent(QTimerEvent *te) QObject::timerEvent(te); } -void Compositor::lastFrameRendered() +void Compositor::aboutToSwapBuffers() { - if (!m_waitingForFrameRendered) { - return; + assert(!m_bufferSwapPending); + + m_bufferSwapPending = true; +} + +void Compositor::bufferSwapComplete() +{ + assert(m_bufferSwapPending); + m_bufferSwapPending = false; + + if (m_composeAtSwapCompletion) { + m_composeAtSwapCompletion = false; + performCompositing(); } - m_waitingForFrameRendered = false; - performCompositing(); } void Compositor::performCompositing() { if (m_scene->usesOverlayWindow() && !isOverlayWindowVisible()) return; // nothing is visible anyway - if (!m_scene->isLastFrameRendered()) { - m_waitingForFrameRendered = true; - return; // frame wouldn't make it on the screen + + // If a buffer swap is still pending, we return to the event loop and + // continue processing events until the swap has completed. + if (m_bufferSwapPending) { + m_composeAtSwapCompletion = true; + compositeTimer.stop(); + return; } // Create a list of all windows in the stacking order @@ -675,6 +689,10 @@ void Compositor::setCompositeTimer() if (!hasScene()) // should not really happen, but there may be e.g. some damage events still pending return; + // Don't start the timer if we're waiting for a swap event + if (m_bufferSwapPending && m_composeAtSwapCompletion) + return; + uint waitTime = 1; if (m_scene->blocksForRetrace()) { diff --git a/composite.h b/composite.h index b4aa8d2e54..571eaba783 100644 --- a/composite.h +++ b/composite.h @@ -90,13 +90,6 @@ public: * Set's the Scene's Overlay X Window visibility to @p visible. **/ void setOverlayWindowVisibility(bool visible); - /** - * @brief Hook for the Scene to notify about a that the last frame got rendered. - * - * In case the Compositor was waiting for the frame being rendered, the next rendering process - * is triggered. - */ - void lastFrameRendered(); Scene *scene() { return m_scene; @@ -178,6 +171,18 @@ public Q_SLOTS: void updateCompositeBlocking(); void updateCompositeBlocking(KWin::Client* c); + /** + * Notifies the compositor that SwapBuffers() is about to be called. + * Rendering of the next frame will be deferred until bufferSwapComplete() + * is called. + */ + void aboutToSwapBuffers(); + + /** + * Notifies the compositor that a pending buffer swap has completed. + */ + void bufferSwapComplete(); + Q_SIGNALS: void compositingToggled(bool active); @@ -229,7 +234,8 @@ private: bool m_starting; // start() sets this variable while starting qint64 m_timeSinceLastVBlank; Scene *m_scene; - bool m_waitingForFrameRendered; + bool m_bufferSwapPending; + bool m_composeAtSwapCompletion; KWIN_SINGLETON_VARIABLE(Compositor, s_compositor) }; diff --git a/egl_wayland_backend.cpp b/egl_wayland_backend.cpp index f86815f6d8..a8a45da12d 100644 --- a/egl_wayland_backend.cpp +++ b/egl_wayland_backend.cpp @@ -41,7 +41,6 @@ EglWaylandBackend::EglWaylandBackend() , m_bufferAge(0) , m_wayland(Wayland::WaylandBackend::self()) , m_overlay(NULL) - , m_lastFrameRendered(true) { if (!m_wayland) { setFailed("Wayland Backend has not been created"); @@ -195,7 +194,7 @@ bool EglWaylandBackend::initRenderingContext() const QSize &size = m_wayland->shellSurfaceSize(); Wayland::Surface *s = m_wayland->surface(); - connect(s, &Wayland::Surface::frameRendered, this, &EglWaylandBackend::lastFrameRendered); + connect(s, &Wayland::Surface::framePresented, Compositor::self(), &Compositor::bufferSwapComplete); m_overlay = wl_egl_window_create(*s, size.width(), size.height()); if (!m_overlay) { qCritical() << "Creating Wayland Egl window failed"; @@ -264,8 +263,9 @@ bool EglWaylandBackend::initBufferConfigs() void EglWaylandBackend::present() { - m_lastFrameRendered = false; m_wayland->surface()->setupFrameCallback(); + Compositor::self()->aboutToSwapBuffers(); + if (supportsBufferAge()) { eglSwapBuffers(m_display, m_surface); eglQuerySurface(m_display, m_surface, EGL_BUFFER_AGE_EXT, &m_bufferAge); @@ -368,17 +368,6 @@ void EglWaylandBackend::overlaySizeChanged(const QSize &size) wl_egl_window_resize(m_overlay, size.width(), size.height(), 0, 0); } -bool EglWaylandBackend::isLastFrameRendered() const -{ - return m_lastFrameRendered; -} - -void EglWaylandBackend::lastFrameRendered() -{ - m_lastFrameRendered = true; - Compositor::self()->lastFrameRendered(); -} - bool EglWaylandBackend::usesOverlayWindow() const { return false; diff --git a/egl_wayland_backend.h b/egl_wayland_backend.h index 4c7ab6babe..3b8c370037 100644 --- a/egl_wayland_backend.h +++ b/egl_wayland_backend.h @@ -66,9 +66,7 @@ public: virtual void endRenderingFrame(const QRegion &renderedRegion, const QRegion &damagedRegion); virtual bool makeCurrent() override; virtual void doneCurrent() override; - virtual bool isLastFrameRendered() const override; Xcb::Shm *shm(); - void lastFrameRendered(); virtual bool usesOverlayWindow() const override; protected: @@ -91,7 +89,6 @@ private: Wayland::WaylandBackend *m_wayland; wl_egl_window *m_overlay; QScopedPointer m_shm; - bool m_lastFrameRendered; bool m_havePlatformBase; friend class EglWaylandTexture; }; diff --git a/scene.h b/scene.h index 34e527aa82..433515a57e 100644 --- a/scene.h +++ b/scene.h @@ -125,14 +125,6 @@ public: * Whether the Scene uses an X11 overlay window to perform compositing. */ virtual bool usesOverlayWindow() const = 0; - /** - * @brief Allows the Compositor to delay the rendering of the next frame until the last one - * has been rendered. This is mostly interesting in case that the system compositor is not able - * to keep up with KWin's frame rate. - * - * @return bool @c true if the next frame should be rendered, @c false otherwise - */ - virtual bool isLastFrameRendered() const = 0; virtual void triggerFence(); diff --git a/scene_opengl.cpp b/scene_opengl.cpp index f7cdbe9fcf..bacc980ca9 100644 --- a/scene_opengl.cpp +++ b/scene_opengl.cpp @@ -343,11 +343,6 @@ QRegion OpenGLBackend::accumulatedDamageHistory(int bufferAge) const return region; } -bool OpenGLBackend::isLastFrameRendered() const -{ - return true; -} - OverlayWindow* OpenGLBackend::overlayWindow() { return NULL; diff --git a/scene_opengl.h b/scene_opengl.h index 38cf2e4e21..276fddc675 100644 --- a/scene_opengl.h +++ b/scene_opengl.h @@ -59,7 +59,6 @@ public: virtual bool syncsToVBlank() const; virtual bool makeOpenGLContextCurrent() override; virtual void doneOpenGLContextCurrent() override; - virtual bool isLastFrameRendered() const override; virtual void triggerFence() override; void insertWait(); @@ -397,13 +396,6 @@ public: virtual void endRenderingFrame(const QRegion &damage, const QRegion &damagedRegion) = 0; virtual bool makeCurrent() = 0; virtual void doneCurrent() = 0; - /** - * @brief Backend specific code to determine whether the last frame got rendered. - * - * Default implementation always returns @c true. That is it's always assumed that the last - * frame got rendered. If a backend needs more control it needs to implement this method. - */ - virtual bool isLastFrameRendered() const; virtual bool usesOverlayWindow() const = 0; /** * @brief Compositor is going into idle mode, flushes any pending paints. @@ -594,11 +586,6 @@ inline bool SceneOpenGL::hasPendingFlush() const return m_backend->hasPendingFlush(); } -inline bool SceneOpenGL::isLastFrameRendered() const -{ - return m_backend->isLastFrameRendered(); -} - inline bool SceneOpenGL::usesOverlayWindow() const { return m_backend->usesOverlayWindow(); diff --git a/scene_qpainter.cpp b/scene_qpainter.cpp index fcbb2e878c..bc73b6ffa4 100644 --- a/scene_qpainter.cpp +++ b/scene_qpainter.cpp @@ -53,11 +53,6 @@ QPainterBackend::~QPainterBackend() { } -bool QPainterBackend::isLastFrameRendered() const -{ - return true; -} - OverlayWindow* QPainterBackend::overlayWindow() { return NULL; @@ -85,7 +80,6 @@ void QPainterBackend::setFailed(const QString &reason) WaylandQPainterBackend::WaylandQPainterBackend() : QPainterBackend() - , m_lastFrameRendered(true) , m_needsFullRepaint(true) , m_backBuffer(QImage(QSize(), QImage::Format_ARGB32_Premultiplied)) , m_buffer(NULL) @@ -93,8 +87,8 @@ WaylandQPainterBackend::WaylandQPainterBackend() connect(Wayland::WaylandBackend::self()->shmPool(), SIGNAL(poolResized()), SLOT(remapBuffer())); connect(Wayland::WaylandBackend::self(), &Wayland::WaylandBackend::shellSurfaceSizeChanged, this, &WaylandQPainterBackend::screenGeometryChanged); - connect(Wayland::WaylandBackend::self()->surface(), &Wayland::Surface::frameRendered, - this, &WaylandQPainterBackend::lastFrameRendered); + connect(Wayland::WaylandBackend::self()->surface(), &Wayland::Surface::framePresented, + Compositor::self(), &Compositor::bufferSwapComplete); } WaylandQPainterBackend::~WaylandQPainterBackend() @@ -104,11 +98,6 @@ WaylandQPainterBackend::~WaylandQPainterBackend() } } -bool WaylandQPainterBackend::isLastFrameRendered() const -{ - return m_lastFrameRendered; -} - bool WaylandQPainterBackend::usesOverlayWindow() const { return false; @@ -120,7 +109,7 @@ void WaylandQPainterBackend::present(int mask, const QRegion &damage) if (m_backBuffer.isNull()) { return; } - m_lastFrameRendered = false; + Compositor::self()->aboutToSwapBuffers(); m_needsFullRepaint = false; Wayland::Surface *s = Wayland::WaylandBackend::self()->surface(); s->attachBuffer(m_buffer->buffer()); @@ -128,12 +117,6 @@ void WaylandQPainterBackend::present(int mask, const QRegion &damage) s->commit(); } -void WaylandQPainterBackend::lastFrameRendered() -{ - m_lastFrameRendered = true; - Compositor::self()->lastFrameRendered(); -} - void WaylandQPainterBackend::screenGeometryChanged(const QSize &size) { Q_UNUSED(size) diff --git a/scene_qpainter.h b/scene_qpainter.h index 5846eba97e..569b47a258 100644 --- a/scene_qpainter.h +++ b/scene_qpainter.h @@ -61,13 +61,6 @@ public: * @param size The new screen size */ virtual void screenGeometryChanged(const QSize &size); - /** - * @brief Backend specific code to determine whether the last frame got rendered. - * - * Default implementation always returns @c true. That is it's always assumed that the last - * frame got rendered. If a backend needs more control it needs to implement this method. - */ - virtual bool isLastFrameRendered() const; /** * @brief Whether the creation of the Backend failed. * @@ -112,16 +105,13 @@ public: virtual void present(int mask, const QRegion& damage) override; virtual bool usesOverlayWindow() const override; - virtual bool isLastFrameRendered() const override; virtual void screenGeometryChanged(const QSize &size) override; virtual QImage *buffer() override; virtual void prepareRenderingFrame() override; virtual bool needsFullRepaint() const override; - void lastFrameRendered(); private Q_SLOTS: void remapBuffer(); private: - bool m_lastFrameRendered; bool m_needsFullRepaint; QImage m_backBuffer; Wayland::Buffer *m_buffer; @@ -134,7 +124,6 @@ class SceneQPainter : public Scene public: virtual ~SceneQPainter(); - virtual bool isLastFrameRendered() const override; virtual bool usesOverlayWindow() const override; virtual OverlayWindow* overlayWindow() override; virtual qint64 paint(QRegion damage, ToplevelList windows) override; @@ -239,12 +228,6 @@ OverlayWindow* SceneQPainter::overlayWindow() return m_backend->overlayWindow(); } -inline -bool SceneQPainter::isLastFrameRendered() const -{ - return m_backend->isLastFrameRendered(); -} - inline QPainter* SceneQPainter::painter() { diff --git a/scene_xrender.cpp b/scene_xrender.cpp index cb057f9691..4a1462943b 100644 --- a/scene_xrender.cpp +++ b/scene_xrender.cpp @@ -108,10 +108,6 @@ void XRenderBackend::screenGeometryChanged(const QSize &size) Q_UNUSED(size) } -bool XRenderBackend::isLastFrameRendered() const -{ - return true; -} //**************************************** // X11XRenderBackend @@ -226,7 +222,6 @@ bool X11XRenderBackend::usesOverlayWindow() const WaylandXRenderBackend::WaylandXRenderBackend() : m_shm(new Xcb::Shm) - , m_lastFrameRendered(true) , m_format(0) { if (!m_shm->isValid()) { @@ -238,8 +233,8 @@ WaylandXRenderBackend::WaylandXRenderBackend() init(); connect(Wayland::WaylandBackend::self(), &Wayland::WaylandBackend::shellSurfaceSizeChanged, this, &WaylandXRenderBackend::createBuffer); - connect(Wayland::WaylandBackend::self()->surface(), &Wayland::Surface::frameRendered, - this, &WaylandXRenderBackend::lastFrameRendered); + connect(Wayland::WaylandBackend::self()->surface(), &Wayland::Surface::framePresented, + Compositor::self(), &Compositor::bufferSwapComplete); } WaylandXRenderBackend::~WaylandXRenderBackend() @@ -294,24 +289,15 @@ void WaylandXRenderBackend::present(int mask, const QRegion &damage) qDebug() << "Did not get a buffer"; return; } - m_lastFrameRendered = false; + + Compositor::self()->aboutToSwapBuffers(); + Wayland::Surface *s = wl->surface(); s->attachBuffer(buffer); s->damage(damage); s->commit(); } -bool WaylandXRenderBackend::isLastFrameRendered() const -{ - return m_lastFrameRendered; -} - -void WaylandXRenderBackend::lastFrameRendered() -{ - m_lastFrameRendered = true; - Compositor::self()->lastFrameRendered(); -} - bool WaylandXRenderBackend::usesOverlayWindow() const { return false; diff --git a/scene_xrender.h b/scene_xrender.h index 42e965f86f..1a983d0fe0 100644 --- a/scene_xrender.h +++ b/scene_xrender.h @@ -73,13 +73,6 @@ public: * @param size The new screen size */ virtual void screenGeometryChanged(const QSize &size); - /** - * @brief Backend specific code to determine whether the last frame got rendered. - * - * Default implementation always returns @c true. That is it's always assumed that the last - * frame got rendered. If a backend needs more control it needs to implement this method. - */ - virtual bool isLastFrameRendered() const; /** * @brief The compositing buffer hold by this backend. * @@ -159,14 +152,12 @@ public: WaylandXRenderBackend(); virtual ~WaylandXRenderBackend(); virtual void present(int mask, const QRegion &damage); - virtual bool isLastFrameRendered() const; virtual bool usesOverlayWindow() const; - void lastFrameRendered(); + private: void createBuffer(); void init(); QScopedPointer m_shm; - bool m_lastFrameRendered; xcb_render_pictformat_t m_format; }; #endif @@ -193,9 +184,6 @@ public: virtual bool usesOverlayWindow() const { return m_backend->usesOverlayWindow(); } - virtual bool isLastFrameRendered() const { - return m_backend->isLastFrameRendered(); - } static SceneXrender *createScene(); protected: diff --git a/wayland_client/surface.cpp b/wayland_client/surface.cpp index f897d24f14..a7093e5ca8 100644 --- a/wayland_client/surface.cpp +++ b/wayland_client/surface.cpp @@ -81,7 +81,7 @@ void Surface::frameCallback(void *data, wl_callback *callback, uint32_t time) void Surface::handleFrameCallback() { m_frameCallbackInstalled = false; - frameRendered(); + emit framePresented(); } const struct wl_callback_listener Surface::s_listener = { diff --git a/wayland_client/surface.h b/wayland_client/surface.h index fffdb331f5..78364cd1e1 100644 --- a/wayland_client/surface.h +++ b/wayland_client/surface.h @@ -71,7 +71,7 @@ public: static Surface *get(wl_surface *native); Q_SIGNALS: - void frameRendered(); + void framePresented(); void sizeChanged(const QSize&); private: