From 6d0cca5c7f8f4c92a9d1c017e315c61d25d09860 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Tue, 9 Nov 2021 12:36:24 +0200 Subject: [PATCH] Move all dirty region scene repaint scheduling to Scene The Compositor contains nothing that can potentially get dirty and need repainting. As is, the advantages of this move aren't really noticeable, but it makes sense with multiple scenes. Backend parts are far from ideal, they can be improved later on as we progress with the scene redesign. --- .../integration/buffer_size_change_test.cpp | 8 +++--- .../integration/generic_scene_opengl_test.cpp | 3 ++- autotests/integration/scene_qpainter_test.cpp | 6 ++--- .../integration/xwaylandserver_crash_test.cpp | 2 +- .../xwaylandserver_restart_test.cpp | 2 +- src/backends/drm/drm_backend.cpp | 9 ++++--- src/backends/drm/drm_output.cpp | 5 ++-- .../fbdev/scene_qpainter_fb_backend.cpp | 3 ++- src/backends/wayland/wayland_backend.cpp | 5 ++-- .../x11/standalone/overlaywindow_x11.cpp | 11 +++++--- src/composite.cpp | 27 ++----------------- src/composite.h | 7 ----- src/effects.cpp | 12 ++++----- src/item.cpp | 3 ++- src/layers.cpp | 3 --- src/platform.cpp | 12 ++++----- src/plugins/screencast/screencastmanager.cpp | 2 +- src/scene.cpp | 26 +++++++++++++++++- src/scene.h | 5 ++++ src/toplevel.cpp | 8 +++--- src/workspace.cpp | 3 +-- 21 files changed, 83 insertions(+), 79 deletions(-) diff --git a/autotests/integration/buffer_size_change_test.cpp b/autotests/integration/buffer_size_change_test.cpp index 790246dc1a..a3af5934f9 100644 --- a/autotests/integration/buffer_size_change_test.cpp +++ b/autotests/integration/buffer_size_change_test.cpp @@ -56,7 +56,7 @@ void BufferSizeChangeTest::testShmBufferSizeChange() // add a first repaint QSignalSpy frameRenderedSpy(Compositor::self()->scene(), &Scene::frameRendered); QVERIFY(frameRenderedSpy.isValid()); - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); QVERIFY(frameRenderedSpy.wait()); // now change buffer size @@ -65,7 +65,7 @@ void BufferSizeChangeTest::testShmBufferSizeChange() QSignalSpy damagedSpy(client, &AbstractClient::damaged); QVERIFY(damagedSpy.isValid()); QVERIFY(damagedSpy.wait()); - KWin::Compositor::self()->addRepaintFull(); + KWin::Compositor::self()->scene()->addRepaintFull(); QVERIFY(frameRenderedSpy.wait()); } @@ -93,7 +93,7 @@ void BufferSizeChangeTest::testShmBufferSizeChangeOnSubSurface() // add a first repaint QSignalSpy frameRenderedSpy(Compositor::self()->scene(), &Scene::frameRendered); QVERIFY(frameRenderedSpy.isValid()); - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); QVERIFY(frameRenderedSpy.wait()); // change buffer size of sub surface @@ -105,7 +105,7 @@ void BufferSizeChangeTest::testShmBufferSizeChangeOnSubSurface() QVERIFY(damagedParentSpy.wait()); // add a second repaint - KWin::Compositor::self()->addRepaintFull(); + KWin::Compositor::self()->scene()->addRepaintFull(); QVERIFY(frameRenderedSpy.wait()); } diff --git a/autotests/integration/generic_scene_opengl_test.cpp b/autotests/integration/generic_scene_opengl_test.cpp index 78975ea3e6..bed92668c5 100644 --- a/autotests/integration/generic_scene_opengl_test.cpp +++ b/autotests/integration/generic_scene_opengl_test.cpp @@ -13,6 +13,7 @@ #include "cursor.h" #include "platform.h" #include "renderbackend.h" +#include "scene.h" #include "wayland_server.h" #include @@ -80,7 +81,7 @@ void GenericSceneOpenGLTest::testRestart() QCOMPARE(kwinApp()->platform()->selectedCompositor(), KWin::OpenGLCompositing); // trigger a repaint - KWin::Compositor::self()->addRepaintFull(); + KWin::Compositor::self()->scene()->addRepaintFull(); // and wait 100 msec to ensure it's rendered // TODO: introduce frameRendered signal in SceneOpenGL QTest::qWait(100); diff --git a/autotests/integration/scene_qpainter_test.cpp b/autotests/integration/scene_qpainter_test.cpp index d2ff495efa..d3a3c4bed8 100644 --- a/autotests/integration/scene_qpainter_test.cpp +++ b/autotests/integration/scene_qpainter_test.cpp @@ -87,7 +87,7 @@ void SceneQPainterTest::initTestCase() void SceneQPainterTest::testStartFrame() { // this test verifies that the initial rendering is correct - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); auto scene = Compositor::self()->scene(); QVERIFY(scene); QCOMPARE(kwinApp()->platform()->selectedCompositor(), QPainterCompositing); @@ -261,7 +261,7 @@ void SceneQPainterTest::testCompositorRestart() QVERIFY(scene); // this should directly trigger a frame - KWin::Compositor::self()->addRepaintFull(); + KWin::Compositor::self()->scene()->addRepaintFull(); QSignalSpy frameRenderedSpy(scene, &Scene::frameRendered); QVERIFY(frameRenderedSpy.isValid()); QVERIFY(frameRenderedSpy.wait()); @@ -360,7 +360,7 @@ void SceneQPainterTest::testX11Window() QVERIFY(scene); // this should directly trigger a frame - KWin::Compositor::self()->addRepaintFull(); + KWin::Compositor::self()->scene()->addRepaintFull(); QSignalSpy frameRenderedSpy(scene, &Scene::frameRendered); QVERIFY(frameRenderedSpy.isValid()); QVERIFY(frameRenderedSpy.wait()); diff --git a/autotests/integration/xwaylandserver_crash_test.cpp b/autotests/integration/xwaylandserver_crash_test.cpp index 0922a6936c..6bb30722b2 100644 --- a/autotests/integration/xwaylandserver_crash_test.cpp +++ b/autotests/integration/xwaylandserver_crash_test.cpp @@ -130,7 +130,7 @@ void XwaylandServerCrashTest::testCrash() QCOMPARE(kwinApp()->x11ScreenNumber(), -1); // Render a frame to ensure that the compositor doesn't crash. - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); QSignalSpy frameRenderedSpy(Compositor::self()->scene(), &Scene::frameRendered); QVERIFY(frameRenderedSpy.wait()); } diff --git a/autotests/integration/xwaylandserver_restart_test.cpp b/autotests/integration/xwaylandserver_restart_test.cpp index 47dc6194aa..286539a832 100644 --- a/autotests/integration/xwaylandserver_restart_test.cpp +++ b/autotests/integration/xwaylandserver_restart_test.cpp @@ -106,7 +106,7 @@ void XwaylandServerRestartTest::testRestart() QVERIFY(client->isDecorated()); // Render a frame to ensure that the compositor doesn't crash. - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); QSignalSpy frameRenderedSpy(Compositor::self()->scene(), &Scene::frameRendered); QVERIFY(frameRenderedSpy.wait()); diff --git a/src/backends/drm/drm_backend.cpp b/src/backends/drm/drm_backend.cpp index b36c974042..b3a2e479d2 100644 --- a/src/backends/drm/drm_backend.cpp +++ b/src/backends/drm/drm_backend.cpp @@ -18,6 +18,7 @@ #include "logging.h" #include "main.h" #include "renderloop.h" +#include "scene.h" #include "scene_qpainter_drm_backend.h" #include "session.h" #include "udev.h" @@ -147,8 +148,8 @@ void DrmBackend::reactivate() output->renderLoop()->uninhibit(); } - if (Compositor *compositor = Compositor::self()) { - compositor->addRepaintFull(); + if (Compositor::compositing()) { + Compositor::self()->scene()->addRepaintFull(); } // While the session had been inactive, an output could have been added or @@ -712,8 +713,8 @@ bool DrmBackend::applyOutputChanges(const WaylandOutputConfig &config) } }; updateCursor(); - if (auto compositor = Compositor::self()) { - compositor->addRepaintFull(); + if (Compositor::compositing()) { + Compositor::self()->scene()->addRepaintFull(); } return true; } diff --git a/src/backends/drm/drm_output.cpp b/src/backends/drm/drm_output.cpp index 101d5f5e01..ec8a25ba6d 100644 --- a/src/backends/drm/drm_output.cpp +++ b/src/backends/drm/drm_output.cpp @@ -19,6 +19,7 @@ #include "main.h" #include "renderloop.h" #include "renderloop_p.h" +#include "scene.h" #include "screens.h" #include "session.h" #include "waylandoutputconfig.h" @@ -267,8 +268,8 @@ bool DrmOutput::setDrmDpmsMode(DpmsMode mode) if (active) { m_renderLoop->uninhibit(); m_gpu->platform()->checkOutputsAreOn(); - if (Compositor *compositor = Compositor::self()) { - compositor->addRepaintFull(); + if (Compositor::compositing()) { + Compositor::self()->scene()->addRepaintFull(); } } else { m_renderLoop->inhibit(); diff --git a/src/backends/fbdev/scene_qpainter_fb_backend.cpp b/src/backends/fbdev/scene_qpainter_fb_backend.cpp index f76a9459b2..cea75df3cc 100644 --- a/src/backends/fbdev/scene_qpainter_fb_backend.cpp +++ b/src/backends/fbdev/scene_qpainter_fb_backend.cpp @@ -13,6 +13,7 @@ #include "main.h" #include "platform.h" #include "renderloop.h" +#include "scene.h" #include "screens.h" #include "session.h" #include "vsyncmonitor.h" @@ -50,7 +51,7 @@ void FramebufferQPainterBackend::reactivate() for (AbstractOutput *output : outputs) { output->renderLoop()->uninhibit(); } - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); } void FramebufferQPainterBackend::deactivate() diff --git a/src/backends/wayland/wayland_backend.cpp b/src/backends/wayland/wayland_backend.cpp index 4f0b7441e2..b795778a9e 100644 --- a/src/backends/wayland/wayland_backend.cpp +++ b/src/backends/wayland/wayland_backend.cpp @@ -27,6 +27,7 @@ #include "cursor.h" #include "input.h" #include "main.h" +#include "scene.h" #include "screens.h" #include "pointer_input.h" #include "wayland_server.h" @@ -200,7 +201,7 @@ void WaylandSubSurfaceCursor::move(const QPointF &globalPosition) // place the sub-surface relative to the output it is on and factor in the hotspot const auto relativePosition = globalPosition.toPoint() - Cursors::self()->currentCursor()->hotspot() - m_output->geometry().topLeft(); m_subSurface->setPosition(relativePosition); - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); } WaylandInputDevice::WaylandInputDevice(KWayland::Client::Keyboard *keyboard, WaylandSeat *seat) @@ -859,7 +860,7 @@ WaylandOutput *WaylandBackend::createOutput(const QPoint &position, const QSize connect(waylandOutput, &WaylandOutput::sizeChanged, this, [this, waylandOutput](const QSize &size) { Q_UNUSED(size) updateScreenSize(waylandOutput); - Compositor::self()->addRepaintFull(); + Compositor::self()->scene()->addRepaintFull(); }); connect(waylandOutput, &WaylandOutput::frameRendered, this, [waylandOutput]() { waylandOutput->resetRendered(); diff --git a/src/backends/x11/standalone/overlaywindow_x11.cpp b/src/backends/x11/standalone/overlaywindow_x11.cpp index 33207646c0..7917205f56 100644 --- a/src/backends/x11/standalone/overlaywindow_x11.cpp +++ b/src/backends/x11/standalone/overlaywindow_x11.cpp @@ -11,6 +11,7 @@ #include "kwinglobals.h" #include "composite.h" +#include "scene.h" #include "screens.h" #include "utils.h" #include "xcbutils.h" @@ -167,7 +168,7 @@ bool OverlayWindowX11::event(xcb_generic_event_t *event) const auto *expose = reinterpret_cast(event); if (expose->window == rootWindow() // root window needs repainting || (m_window != XCB_WINDOW_NONE && expose->window == m_window)) { // overlay needs repainting - Compositor::self()->addRepaint(expose->x, expose->y, expose->width, expose->height); + Compositor::self()->scene()->addRepaint(expose->x, expose->y, expose->width, expose->height); } } else if (eventType == XCB_VISIBILITY_NOTIFY) { const auto *visibility = reinterpret_cast(event); @@ -177,8 +178,12 @@ bool OverlayWindowX11::event(xcb_generic_event_t *event) auto compositor = Compositor::self(); if (!was_visible && m_visible) { // hack for #154825 - compositor->addRepaintFull(); - QTimer::singleShot(2000, compositor, &Compositor::addRepaintFull); + compositor->scene()->addRepaintFull(); + QTimer::singleShot(2000, compositor, [compositor]() { + if (compositor->compositing()) { + compositor->scene()->addRepaintFull(); + } + }); } compositor->scheduleRepaint(); } diff --git a/src/composite.cpp b/src/composite.cpp index 0c04a3d555..0a1c319219 100644 --- a/src/composite.cpp +++ b/src/composite.cpp @@ -353,6 +353,7 @@ void Compositor::startupWithWorkspace() Workspace::self()->markXStackingOrderAsDirty(); Q_ASSERT(m_scene); + m_scene->initialize(); const Platform *platform = kwinApp()->platform(); if (platform->isPerScreenRenderingEnabled()) { @@ -373,7 +374,6 @@ void Compositor::startupWithWorkspace() // Sets also the 'effects' pointer. kwinApp()->platform()->createEffectsHandler(this, m_scene); connect(Workspace::self(), &Workspace::deletedRemoved, m_scene, &Scene::removeToplevel); - connect(effects, &EffectsHandler::virtualScreenGeometryChanged, this, &Compositor::addRepaintFull); for (X11Client *c : Workspace::self()->clientList()) { c->setupCompositing(); @@ -399,7 +399,7 @@ void Compositor::startupWithWorkspace() } // Render at least once. - addRepaintFull(); + m_scene->addRepaintFull(); } void Compositor::registerRenderLoop(RenderLoop *renderLoop, AbstractOutput *output) @@ -565,7 +565,6 @@ void Compositor::deleteUnusedSupportProperties() void Compositor::configChanged() { reinitialize(); - addRepaintFull(); } void Compositor::reinitialize() @@ -582,28 +581,6 @@ void Compositor::reinitialize() } } -void Compositor::addRepaint(int x, int y, int width, int height) -{ - addRepaint(QRegion(x, y, width, height)); -} - -void Compositor::addRepaint(const QRect &rect) -{ - addRepaint(QRegion(rect)); -} - -void Compositor::addRepaint(const QRegion ®ion) -{ - if (m_scene) { - m_scene->addRepaint(region); - } -} - -void Compositor::addRepaintFull() -{ - addRepaint(screens()->geometry()); -} - void Compositor::handleFrameRequested(RenderLoop *renderLoop) { composite(renderLoop); diff --git a/src/composite.h b/src/composite.h index 8b0c92c238..25001e0747 100644 --- a/src/composite.h +++ b/src/composite.h @@ -41,13 +41,6 @@ public: ~Compositor() override; static Compositor *self(); - // when adding repaints caused by a window, you probably want to use - // either Toplevel::addRepaint() or Toplevel::addWorkspaceRepaint() - void addRepaint(const QRect& r); - void addRepaint(const QRegion& r); - void addRepaint(int x, int y, int w, int h); - void addRepaintFull(); - /** * Schedules a new repaint if no repaint is currently scheduled. */ diff --git a/src/effects.cpp b/src/effects.cpp index 691e966cbe..a12e1d61c3 100644 --- a/src/effects.cpp +++ b/src/effects.cpp @@ -1172,22 +1172,22 @@ EffectWindow* EffectsHandlerImpl::currentTabBoxWindow() const void EffectsHandlerImpl::addRepaintFull() { - m_compositor->addRepaintFull(); + m_compositor->scene()->addRepaintFull(); } void EffectsHandlerImpl::addRepaint(const QRect& r) { - m_compositor->addRepaint(r); + m_compositor->scene()->addRepaint(r); } void EffectsHandlerImpl::addRepaint(const QRegion& r) { - m_compositor->addRepaint(r); + m_compositor->scene()->addRepaint(r); } void EffectsHandlerImpl::addRepaint(int x, int y, int w, int h) { - m_compositor->addRepaint(x, y, w, h); + m_compositor->scene()->addRepaint(x, y, w, h); } EffectScreen *EffectsHandlerImpl::activeScreen() const @@ -1349,7 +1349,7 @@ QStringList EffectsHandlerImpl::listOfEffects() const bool EffectsHandlerImpl::loadEffect(const QString& name) { makeOpenGLContextCurrent(); - m_compositor->addRepaintFull(); + m_compositor->scene()->addRepaintFull(); return m_effectLoader->loadEffect(name); } @@ -1371,7 +1371,7 @@ void EffectsHandlerImpl::unloadEffect(const QString& name) effect_order.erase(it); effectsChanged(); - m_compositor->addRepaintFull(); + m_compositor->scene()->addRepaintFull(); } void EffectsHandlerImpl::destroyEffect(Effect *effect) diff --git a/src/item.cpp b/src/item.cpp index bfb5be4eff..627d8e4cb5 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -10,6 +10,7 @@ #include "main.h" #include "platform.h" #include "renderloop.h" +#include "scene.h" #include "screens.h" #include "utils.h" @@ -27,7 +28,7 @@ Item::~Item() setParentItem(nullptr); for (const auto &dirty : qAsConst(m_repaints)) { if (!dirty.isEmpty()) { - Compositor::self()->addRepaint(dirty); + Compositor::self()->scene()->addRepaint(dirty); } } } diff --git a/src/layers.cpp b/src/layers.cpp index 3eb90425e7..59a4eded42 100644 --- a/src/layers.cpp +++ b/src/layers.cpp @@ -114,9 +114,6 @@ void Workspace::updateStackingOrder(bool propagate_new_clients) } Q_EMIT stackingOrderChanged(); - if (m_compositor) { - m_compositor->addRepaintFull(); - } if (active_client) active_client->updateMouseGrab(); diff --git a/src/platform.cpp b/src/platform.cpp index aad545be93..6d9494517a 100644 --- a/src/platform.cpp +++ b/src/platform.cpp @@ -269,11 +269,10 @@ void Platform::setSoftwareCursorForced(bool forced) void Platform::triggerCursorRepaint() { - if (!Compositor::self()) { - return; + if (Compositor::compositing()) { + Compositor::self()->scene()->addRepaint(m_cursor.lastRenderedGeometry); + Compositor::self()->scene()->addRepaint(Cursors::self()->currentCursor()->geometry()); } - Compositor::self()->addRepaint(m_cursor.lastRenderedGeometry); - Compositor::self()->addRepaint(Cursors::self()->currentCursor()->geometry()); } void Platform::cursorRendered(const QRect &geometry) @@ -469,10 +468,9 @@ void Platform::processPinchGestureCancelled(quint32 time) void Platform::repaint(const QRect &rect) { - if (!Compositor::self()) { - return; + if (Compositor::compositing()) { + Compositor::self()->scene()->addRepaint(rect); } - Compositor::self()->addRepaint(rect); } void Platform::setReady(bool ready) diff --git a/src/plugins/screencast/screencastmanager.cpp b/src/plugins/screencast/screencastmanager.cpp index 57b3defb2c..e5f47ffb0a 100644 --- a/src/plugins/screencast/screencastmanager.cpp +++ b/src/plugins/screencast/screencastmanager.cpp @@ -151,7 +151,7 @@ void ScreencastManager::streamOutput(KWaylandServer::ScreencastStreamV1Interface stream->recordFrame(texture.data(), region); }; connect(stream, &PipeWireStream::startStreaming, waylandStream, [streamOutput, stream, bufferToStream] { - Compositor::self()->addRepaint(streamOutput->geometry()); + Compositor::self()->scene()->addRepaint(streamOutput->geometry()); streamOutput->recordingStarted(); connect(streamOutput, &AbstractWaylandOutput::outputChange, stream, bufferToStream); }); diff --git a/src/scene.cpp b/src/scene.cpp index 0bec29e4d8..d55b3612d2 100644 --- a/src/scene.cpp +++ b/src/scene.cpp @@ -61,6 +61,7 @@ #include "unmanaged.h" #include "waylandclient.h" #include "windowitem.h" +#include "workspace.h" #include "x11client.h" #include @@ -86,7 +87,6 @@ namespace KWin Scene::Scene(QObject *parent) : QObject(parent) { - connect(kwinApp()->platform(), &Platform::outputDisabled, this, &Scene::removeRepaints); } Scene::~Scene() @@ -94,6 +94,30 @@ Scene::~Scene() Q_ASSERT(m_windows.isEmpty()); } +void Scene::initialize() +{ + connect(kwinApp()->platform(), &Platform::outputDisabled, this, &Scene::removeRepaints); + + connect(workspace(), &Workspace::geometryChanged, this, &Scene::addRepaintFull); + connect(workspace(), &Workspace::currentDesktopChanged, this, &Scene::addRepaintFull); + connect(workspace(), &Workspace::stackingOrderChanged, this, &Scene::addRepaintFull); +} + +void Scene::addRepaintFull() +{ + addRepaint(workspace()->geometry()); +} + +void Scene::addRepaint(int x, int y, int width, int height) +{ + addRepaint(QRegion(x, y, width, height)); +} + +void Scene::addRepaint(const QRect &rect) +{ + addRepaint(QRegion(rect)); +} + void Scene::addRepaint(const QRegion ®ion) { if (kwinApp()->platform()->isPerScreenRenderingEnabled()) { diff --git a/src/scene.h b/src/scene.h index c7216453a5..9764bc4447 100644 --- a/src/scene.h +++ b/src/scene.h @@ -53,10 +53,15 @@ public: class EffectFrame; class Window; + void initialize(); + /** * Schedules a repaint for the specified @a region. */ void addRepaint(const QRegion ®ion); + void addRepaint(const QRect &rect); + void addRepaint(int x, int y, int width, int height); + void addRepaintFull(); /** * Returns the repaints region for output with the specified @a output. diff --git a/src/toplevel.cpp b/src/toplevel.cpp index 970d4b480f..ad34ed8b6b 100644 --- a/src/toplevel.cpp +++ b/src/toplevel.cpp @@ -344,15 +344,15 @@ void Toplevel::addWorkspaceRepaint(int x, int y, int w, int h) void Toplevel::addWorkspaceRepaint(const QRect& r2) { - if (!Compositor::compositing()) - return; - Compositor::self()->addRepaint(r2); + if (Compositor::compositing()) { + Compositor::self()->scene()->addRepaint(r2); + } } void Toplevel::addWorkspaceRepaint(const QRegion ®ion) { if (Compositor::compositing()) { - Compositor::self()->addRepaint(region); + Compositor::self()->scene()->addRepaint(region); } } diff --git a/src/workspace.cpp b/src/workspace.cpp index b2e1f9e0b5..9e54d1fa0d 100644 --- a/src/workspace.cpp +++ b/src/workspace.cpp @@ -180,7 +180,6 @@ Workspace::Workspace() Q_ASSERT(kwinApp()->operationMode() == Application::OperationMode::OperationModeX11); m_compositor = X11Compositor::create(this); } - connect(this, &Workspace::currentDesktopChanged, m_compositor, &Compositor::addRepaintFull); connect(m_compositor, &QObject::destroyed, this, [this] { m_compositor = nullptr; }); auto decorationBridge = Decoration::DecorationBridge::create(this); @@ -1192,7 +1191,7 @@ void Workspace::updateCurrentActivity(const QString &new_activity) //if ( effects != NULL && old_desktop != 0 && old_desktop != new_desktop ) // static_cast( effects )->desktopChanged( old_desktop ); if (Compositor::compositing() && m_compositor) - m_compositor->addRepaintFull(); + m_compositor->scene()->addRepaintFull(); #else Q_UNUSED(new_activity) #endif