Make Workspace process batched output updates

Currently the Workspace processes output updates as they occur, e.g.
when the drm backend scans connectors, the Workspace will handle
hotplugged outputs one by one or if an output configuration changes the
mode of several outputs, the workspace will process output layout
updates one by one instead of handling it in one pass. The main reason
for the current behavior is simplicity.

However, that can create issues because it's possible that the output
layout will be temporarily in degenerate state and features such as
sticking windows to their outputs will be broken.

In order to fix that, this change makes the Workspace process batched
output updates. There are several challenges - disconnected outputs have
to be alive when the outputsQueried signal is emitted, the workspace
needs to determine what outputs have been added or removed on its own.

autotests/integration/outputchanges_test.cpp

@@ -87,7 +87,7 @@ void OutputChangesTest::testWindowSticksToOutputAfterOutputIsDisabled()
         auto changeSet = config.changeSet(outputs[0]);
         changeSet->enabled = false;
     }
-    kwinApp()->platform()->applyOutputChanges(config);
+    workspace()->applyOutputConfiguration(config);
 
     // The window will be sent to the second output, which is at (1280, 0).
     QCOMPARE(window->frameGeometry(), QRect(1280 + 42, 0 + 67, 100, 50));
@@ -117,7 +117,7 @@ void OutputChangesTest::testWindowSticksToOutputAfterAnotherOutputIsDisabled()
         auto changeSet = config.changeSet(outputs[1]);
         changeSet->pos = QPoint(0, 0);
     }
-    kwinApp()->platform()->applyOutputChanges(config);
+    workspace()->applyOutputConfiguration(config);
 
     // The position of the window relative to its output should remain the same.
     QCOMPARE(window->frameGeometry(), QRect(42, 67, 100, 50));
@@ -143,7 +143,7 @@ void OutputChangesTest::testWindowSticksToOutputAfterOutputIsMoved()
         auto changeSet = config.changeSet(outputs[0]);
         changeSet->pos = QPoint(-10, 20);
     }
-    kwinApp()->platform()->applyOutputChanges(config);
+    workspace()->applyOutputConfiguration(config);
 
     // The position of the window relative to its output should remain the same.
     QCOMPARE(window->frameGeometry(), QRect(-10 + 42, 20 + 67, 100, 50));

autotests/integration/xdgshellwindow_rules_test.cpp

@@ -2910,14 +2910,14 @@ void TestXdgShellWindowRules::testScreenForce()
     OutputConfiguration config;
     auto changeSet = config.changeSet(outputs.at(1));
     changeSet->enabled = false;
-    kwinApp()->platform()->applyOutputChanges(config);
+    workspace()->applyOutputConfiguration(config);
 
     QVERIFY(!outputs.at(1)->isEnabled());
     QCOMPARE(m_window->output()->name(), outputs.at(0)->name());
 
     // Enable the output and check that the window is moved there again
     changeSet->enabled = true;
-    kwinApp()->platform()->applyOutputChanges(config);
+    workspace()->applyOutputConfiguration(config);
 
     QVERIFY(outputs.at(1)->isEnabled());
     QCOMPARE(m_window->output()->name(), outputs.at(1)->name());

src/backends/drm/drm_gpu.cpp

@@ -578,7 +578,7 @@ void DrmGpu::removeOutput(DrmOutput *output)
     output->pipeline()->setLayers(nullptr, nullptr);
     m_drmOutputs.removeOne(output);
     Q_EMIT outputRemoved(output);
-    delete output;
+    output->unref();
 }
 
 DrmBackend *DrmGpu::platform() const
@@ -603,7 +603,7 @@ void DrmGpu::removeVirtualOutput(DrmVirtualOutput *output)
 {
     if (m_virtualOutputs.removeOne(output)) {
         Q_EMIT outputRemoved(output);
-        delete output;
+        output->unref();
     }
 }
 

src/backends/virtual/virtual_backend.cpp

@@ -115,25 +115,12 @@ void VirtualBackend::setVirtualOutputs(int count, QVector<QRect> geometries, QVe
         output->updateEnabled(false);
         m_outputs.removeOne(output);
         Q_EMIT outputRemoved(output);
-        delete output;
+        output->unref();
     }
 
     Q_EMIT outputsQueried();
 }
 
-void VirtualBackend::removeOutput(Output *output)
-{
-    VirtualOutput *virtualOutput = static_cast<VirtualOutput *>(output);
-    virtualOutput->updateEnabled(false);
-
-    m_outputs.removeOne(virtualOutput);
-    Q_EMIT outputRemoved(virtualOutput);
-
-    delete virtualOutput;
-
-    Q_EMIT screensQueried();
-}
-
 QImage VirtualBackend::captureOutput(Output *output) const
 {
     if (auto backend = qobject_cast<VirtualQPainterBackend *>(Compositor::self()->backend())) {

src/backends/virtual/virtual_backend.h

@@ -53,7 +53,6 @@ public:
         return QVector<CompositingType>{OpenGLCompositing, QPainterCompositing};
     }
 
-    Q_INVOKABLE void removeOutput(Output *output);
     Q_INVOKABLE QImage captureOutput(Output *output) const;
 
 Q_SIGNALS:

src/backends/wayland/wayland_backend.cpp

@@ -978,7 +978,7 @@ void WaylandBackend::removeVirtualOutput(Output *output)
     if (waylandOutput && m_outputs.removeAll(waylandOutput)) {
         waylandOutput->updateEnabled(false);
         Q_EMIT outputRemoved(waylandOutput);
-        delete waylandOutput;
+        waylandOutput->unref();
     }
 }
 

src/backends/x11/standalone/x11_standalone_platform.cpp

@@ -606,7 +606,7 @@ void X11StandalonePlatform::doUpdateOutputs()
             nativeOutput->updateEnabled(false);
         }
         Q_EMIT outputRemoved(output);
-        delete output;
+        output->unref();
     }
 
     // Make sure that the position of an output in m_outputs matches its xinerama index, there

src/backends/x11/windowed/x11_windowed_backend.cpp

@@ -499,7 +499,7 @@ void X11WindowedBackend::handleClientMessage(xcb_client_message_event_t *event)
 
                 removedOutput->updateEnabled(false);
                 Q_EMIT outputRemoved(removedOutput);
-                delete removedOutput;
+                removedOutput->unref();
                 Q_EMIT outputsQueried();
             }
         }

src/core/output.cpp

@@ -69,6 +69,20 @@ Output::~Output()
 {
 }
 
+void Output::ref()
+{
+    m_refCount++;
+}
+
+void Output::unref()
+{
+    Q_ASSERT(m_refCount > 0);
+    m_refCount--;
+    if (m_refCount == 0) {
+        delete this;
+    }
+}
+
 QString Output::name() const
 {
     return m_information.name;

src/core/output.h

@@ -94,6 +94,9 @@ public:
     explicit Output(QObject *parent = nullptr);
     ~Output() override;
 
+    void ref();
+    void unref();
+
     /**
      * Maps the specified @a rect from the global coordinate system to the output-local coords.
      */
@@ -328,6 +331,7 @@ protected:
     Information m_information;
     QUuid m_uuid;
     int m_directScanoutCount = 0;
+    int m_refCount = 1;
     friend class EffectScreenImpl; // to access m_effectScreen
 };
 

src/screens.cpp

@@ -24,7 +24,7 @@ namespace KWin
 Screens::Screens()
     : m_maxScale(1.0)
 {
-    connect(kwinApp()->platform(), &Platform::outputsQueried, this, &Screens::changed);
+    connect(workspace(), &Workspace::outputsChanged, this, &Screens::changed);
 }
 
 void Screens::init()

src/wayland/outputmanagement_v2_interface.cpp

@@ -13,7 +13,6 @@
 #include "core/outputconfiguration.h"
 #include "core/platform.h"
 #include "main.h"
-#include "screens.h"
 #include "workspace.h"
 
 #include "qwayland-server-kde-output-management-v2.h"
@@ -227,14 +226,13 @@ void OutputConfigurationV2Interface::kde_output_configuration_v2_apply(Resource
         return;
     }
 
-    if (kwinApp()->platform()->applyOutputChanges(config)) {
+    if (workspace()->applyOutputConfiguration(config)) {
         if (primaryOutput.has_value()) {
             auto requestedPrimaryOutput = (*primaryOutput)->handle();
             if (requestedPrimaryOutput && requestedPrimaryOutput->isEnabled()) {
                 workspace()->setPrimaryOutput(requestedPrimaryOutput);
             }
         }
-        Q_EMIT workspace()->screens()->changed();
         send_applied();
     } else {
         qCDebug(KWIN_CORE) << "Applying config failed";

src/workspace.cpp

@@ -211,19 +211,8 @@ void Workspace::init()
     connect(options, &Options::separateScreenFocusChanged, m_focusChain.get(), &FocusChain::setSeparateScreenFocus);
     m_focusChain->setSeparateScreenFocus(options->isSeparateScreenFocus());
 
-    if (waylandServer()) {
-        updateOutputConfiguration();
-        connect(kwinApp()->platform(), &Platform::outputsQueried, this, &Workspace::updateOutputConfiguration);
-    }
-
-    Platform *platform = kwinApp()->platform();
-    connect(platform, &Platform::outputAdded, this, &Workspace::slotPlatformOutputAdded);
-    connect(platform, &Platform::outputRemoved, this, &Workspace::slotPlatformOutputRemoved);
-
-    const QVector<Output *> outputs = platform->outputs();
-    for (Output *output : outputs) {
-        slotPlatformOutputAdded(output);
-    }
+    slotPlatformOutputsQueried();
+    connect(kwinApp()->platform(), &Platform::outputsQueried, this, &Workspace::slotPlatformOutputsQueried);
 
     m_screens->init();
 
@@ -520,6 +509,13 @@ Workspace::~Workspace()
     m_placement.reset();
     delete m_windowKeysDialog;
 
+    if (m_placeholderOutput) {
+        m_placeholderOutput->unref();
+    }
+    for (Output *output : std::as_const(m_outputs)) {
+        output->unref();
+    }
+
     _self = nullptr;
 }
 
@@ -625,6 +621,15 @@ std::shared_ptr<OutputMode> parseMode(Output *output, const QJsonObject &modeInf
 }
 }
 
+bool Workspace::applyOutputConfiguration(const OutputConfiguration &config)
+{
+    if (!kwinApp()->platform()->applyOutputChanges(config)) {
+        return false;
+    }
+    updateOutputs();
+    return true;
+}
+
 void Workspace::updateOutputConfiguration()
 {
     // There's conflict between this code and setVirtualOutputs(), need to adjust the tests.
@@ -1409,87 +1414,67 @@ Output *Workspace::outputAt(const QPointF &pos) const
     return bestOutput;
 }
 
-void Workspace::slotPlatformOutputAdded(Output *output)
+void Workspace::slotPlatformOutputsQueried()
 {
-    if (output->isNonDesktop()) {
-        return;
+    if (waylandServer()) {
+        updateOutputConfiguration();
     }
+    updateOutputs();
+}
 
-    if (output->isEnabled()) {
-        addOutput(output);
-    }
+void Workspace::updateOutputs()
+{
+    const auto availableOutputs = kwinApp()->platform()->outputs();
+    const auto oldOutputs = m_outputs;
 
-    connect(output, &Output::enabledChanged, this, [this, output]() {
-        if (output->isEnabled()) {
-            addOutput(output);
-        } else {
-            removeOutput(output);
+    m_outputs.clear();
+    for (Output *output : availableOutputs) {
+        if (!output->isNonDesktop() && output->isEnabled()) {
+            m_outputs.append(output);
         }
-    });
-}
-
-void Workspace::slotPlatformOutputRemoved(Output *output)
-{
-    if (!output->isNonDesktop()) {
-        removeOutput(output);
-    }
-}
-
-void Workspace::addOutput(Output *output)
-{
-    if (!m_activeOutput) {
-        m_activeOutput = output;
-    }
-    if (!m_primaryOutput) {
-        setPrimaryOutput(output);
     }
 
-    m_outputs.append(output);
+    // The workspace requires at least one output connected.
+    if (m_outputs.isEmpty()) {
+        if (!m_placeholderOutput) {
+            m_placeholderOutput = new PlaceholderOutput(QSize(16535, 16535), 1);
+            m_placeholderFilter = std::make_unique<PlaceholderInputEventFilter>();
+            input()->prependInputEventFilter(m_placeholderFilter.get());
+        }
+        m_outputs.append(m_placeholderOutput);
+    } else {
+        if (m_placeholderOutput) {
+            m_placeholderOutput->unref();
+            m_placeholderOutput = nullptr;
+            m_placeholderFilter.reset();
+        }
+    }
+
+    if (!m_activeOutput || !m_outputs.contains(m_activeOutput)) {
+        setActiveOutput(m_outputs[0]);
+    }
+    if (!m_primaryOutput || !m_outputs.contains(m_primaryOutput)) {
+        setPrimaryOutput(m_outputs[0]);
+    }
 
-    connect(output, &Output::geometryChanged, this, &Workspace::desktopResized);
     desktopResized();
 
-    // Trigger a re-check of output-related rules on all windows
-    for (Window *window : qAsConst(m_allClients)) {
-        sendWindowToOutput(window, window->output());
+    const QSet<Output *> oldOutputsSet(oldOutputs.constBegin(), oldOutputs.constEnd());
+    const QSet<Output *> outputsSet(m_outputs.constBegin(), m_outputs.constEnd());
+
+    const auto added = outputsSet - oldOutputsSet;
+    for (Output *output : added) {
+        output->ref();
+        Q_EMIT outputAdded(output);
     }
 
-    Q_EMIT outputAdded(output);
-
-    if (m_placeholderOutput) {
-        m_outputs.removeOne(m_placeholderOutput.get());
-        Q_EMIT outputRemoved(m_placeholderOutput.get());
-        m_placeholderOutput.reset();
-        m_placeholderFilter.reset();
-    }
-}
-
-void Workspace::removeOutput(Output *output)
-{
-    if (!m_outputs.removeOne(output)) {
-        return;
-    }
-    if (m_outputs.empty()) {
-        // not all parts of KWin handle having no output yet. To prevent crashes, create a placeholder output
-        m_placeholderOutput = std::make_unique<PlaceholderOutput>(output->pixelSize(), output->scale());
-        m_outputs.append(m_placeholderOutput.get());
-        Q_EMIT outputAdded(m_placeholderOutput.get());
-        // also prevent accidental inputs while the user has no screen connected
-        m_placeholderFilter = std::make_unique<PlaceholderInputEventFilter>();
-        input()->prependInputEventFilter(m_placeholderFilter.get());
+    const auto removed = oldOutputsSet - outputsSet;
+    for (Output *output : removed) {
+        Q_EMIT outputRemoved(output);
+        output->unref();
     }
 
-    if (m_activeOutput == output) {
-        m_activeOutput = outputAt(output->geometry().center());
-    }
-    if (m_primaryOutput == output) {
-        setPrimaryOutput(m_outputs.constFirst());
-    }
-
-    disconnect(output, &Output::geometryChanged, this, &Workspace::desktopResized);
-    desktopResized();
-
-    Q_EMIT outputRemoved(output);
+    Q_EMIT outputsChanged();
 }
 
 void Workspace::slotDesktopAdded(VirtualDesktop *desktop)

src/workspace.h

@@ -78,6 +78,7 @@ class Activities;
 class PlaceholderInputEventFilter;
 class PlaceholderOutput;
 class Placement;
+class OutputConfiguration;
 
 class KWIN_EXPORT Workspace : public QObject
 {
@@ -464,6 +465,12 @@ public:
     Activities *activities() const;
 #endif
 
+    /**
+     * Apply the requested output configuration. Note that you must use this function
+     * instead of Platform::applyOutputChanges().
+     */
+    bool applyOutputConfiguration(const OutputConfiguration &config);
+
 public Q_SLOTS:
     void performWindowOperation(KWin::Window *window, Options::WindowOperation op);
     // Keybindings
@@ -543,8 +550,7 @@ private Q_SLOTS:
     void slotCurrentDesktopChangingCancelled();
     void slotDesktopAdded(VirtualDesktop *desktop);
     void slotDesktopRemoved(VirtualDesktop *desktop);
-    void slotPlatformOutputAdded(Output *output);
-    void slotPlatformOutputRemoved(Output *output);
+    void slotPlatformOutputsQueried();
 
 Q_SIGNALS:
     /**
@@ -574,6 +580,7 @@ Q_SIGNALS:
     void primaryOutputChanged();
     void outputAdded(KWin::Output *);
     void outputRemoved(KWin::Output *);
+    void outputsChanged();
     /**
      * This signal is emitted when the stacking order changed, i.e. a window is risen
      * or lowered
@@ -623,9 +630,6 @@ private:
     Unmanaged *createUnmanaged(xcb_window_t windowId);
     void addUnmanaged(Unmanaged *c);
 
-    void addOutput(Output *output);
-    void removeOutput(Output *output);
-
     void addWaylandWindow(Window *window);
     void removeWaylandWindow(Window *window);
 
@@ -639,6 +643,7 @@ private:
     QString getPlacementTrackerHash();
 
     void updateOutputConfiguration();
+    void updateOutputs();
 
     struct Constraint
     {
@@ -757,7 +762,7 @@ private:
 #endif
     std::unique_ptr<PlacementTracker> m_placementTracker;
 
-    std::unique_ptr<PlaceholderOutput> m_placeholderOutput;
+    PlaceholderOutput *m_placeholderOutput = nullptr;
     std::unique_ptr<PlaceholderInputEventFilter> m_placeholderFilter;
 
 private: