Lift output enablement into Platform

Summary:
This lifts the enablement code for outputs from the DRM backend to Platform
allowing other Wayland backends in the future to use this interface as well.

To do that we also create some helper functions on Platform level and have to
spill some KWayland classes into AbstractOutput what motivates a further split
of Platform into a Wayland child class like for AbstractOutput.

Test Plan: Disabled and enabled an output in DRM session.

Reviewers: #kwin

Subscribers: zzag, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D23545

abstract_output.cpp

@@ -73,6 +73,21 @@ AbstractOutput::~AbstractOutput()
 {
 }
 
+QByteArray AbstractOutput::uuid() const
+{
+    return QByteArray();
+}
+
+void AbstractOutput::setEnabled(bool enable)
+{
+    Q_UNUSED(enable)
+}
+
+void AbstractOutput::applyChanges(const KWayland::Server::OutputChangeSet *changeSet)
+{
+    Q_UNUSED(changeSet)
+}
+
 bool AbstractOutput::isInternal() const
 {
     return false;

abstract_output.h

@@ -27,6 +27,14 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <QSize>
 #include <QVector>
 
+namespace KWayland
+{
+namespace Server
+{
+class OutputChangeSet;
+}
+}
+
 namespace KWin
 {
 
@@ -100,6 +108,27 @@ public:
      */
     virtual QString name() const = 0;
 
+    /**
+     * Returns the identifying uuid of this output.
+     *
+     * Default implementation returns an empty byte array.
+     */
+    virtual QByteArray uuid() const;
+
+    /**
+     * Enable or disable the output.
+     *
+     * Default implementation does nothing
+     */
+    virtual void setEnabled(bool enable);
+
+    /**
+     * This sets the changes and tests them against the specific output.
+     *
+     * Default implementation does nothing
+     */
+    virtual void applyChanges(const KWayland::Server::OutputChangeSet *changeSet);
+
     /**
      * Returns geometry of this output in device independent pixels.
      */

abstract_wayland_output.cpp

@@ -50,6 +50,11 @@ QString AbstractWaylandOutput::name() const
                 m_waylandOutputDevice->model());
 }
 
+QByteArray AbstractWaylandOutput::uuid() const
+{
+    return m_waylandOutputDevice->uuid();
+}
+
 QRect AbstractWaylandOutput::geometry() const
 {
     return QRect(globalPos(), pixelSize() / scale());
@@ -112,31 +117,31 @@ void AbstractWaylandOutput::setScale(qreal scale)
     }
 }
 
-void AbstractWaylandOutput::setChanges(KWayland::Server::OutputChangeSet *changes)
+void AbstractWaylandOutput::applyChanges(const KWayland::Server::OutputChangeSet *changeSet)
 {
-    qCDebug(KWIN_CORE) << "Set changes in AbstractWaylandOutput.";
+    qCDebug(KWIN_CORE) << "Apply changes to the Wayland output.";
     bool emitModeChanged = false;
 
-    //enabledChanged is handled by plugin code
-    if (changes->modeChanged()) {
-        qCDebug(KWIN_CORE) << "Setting new mode:" << changes->mode();
-        m_waylandOutputDevice->setCurrentMode(changes->mode());
-        updateMode(changes->mode());
+    // Enablement changes are handled by platform.
+    if (changeSet->modeChanged()) {
+        qCDebug(KWIN_CORE) << "Setting new mode:" << changeSet->mode();
+        m_waylandOutputDevice->setCurrentMode(changeSet->mode());
+        updateMode(changeSet->mode());
         emitModeChanged = true;
     }
-    if (changes->transformChanged()) {
-        qCDebug(KWIN_CORE) << "Server setting transform: " << (int)(changes->transform());
-        transform(changes->transform());
+    if (changeSet->transformChanged()) {
+        qCDebug(KWIN_CORE) << "Server setting transform: " << (int)(changeSet->transform());
+        transform(changeSet->transform());
         emitModeChanged = true;
     }
-    if (changes->positionChanged()) {
-        qCDebug(KWIN_CORE) << "Server setting position: " << changes->position();
-        setGlobalPos(changes->position());
+    if (changeSet->positionChanged()) {
+        qCDebug(KWIN_CORE) << "Server setting position: " << changeSet->position();
+        setGlobalPos(changeSet->position());
         // may just work already!
     }
-    if (changes->scaleChanged()) {
-        qCDebug(KWIN_CORE) << "Setting scale:" << changes->scale();
-        setScale(changes->scaleF());
+    if (changeSet->scaleChanged()) {
+        qCDebug(KWIN_CORE) << "Setting scale:" << changeSet->scale();
+        setScale(changeSet->scaleF());
         emitModeChanged = true;
     }
 

abstract_wayland_output.h

@@ -60,6 +60,7 @@ public:
     ~AbstractWaylandOutput() override;
 
     QString name() const override;
+    QByteArray uuid() const override;
     bool isEnabled() const {
         return !m_waylandOutput.isNull();
     }
@@ -88,10 +89,7 @@ public:
     void setGlobalPos(const QPoint &pos);
     void setScale(qreal scale);
 
-    /**
-     * This sets the changes and tests them against the specific output.
-     */
-    void setChanges(KWayland::Server::OutputChangeSet *changeset);
+    void applyChanges(const KWayland::Server::OutputChangeSet *changeSet) override;
 
     QPointer<KWayland::Server::OutputInterface> waylandOutput() const {
         return m_waylandOutput;
@@ -103,7 +101,7 @@ public:
      * This differs from updateDpms as it also removes the wl_output.
      * The default is on.
      */
-    void setEnabled(bool enable);
+    void setEnabled(bool enable) override;
 
 Q_SIGNALS:
     void modeChanged();

platform.cpp

@@ -18,6 +18,8 @@ You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *********************************************************************/
 #include "platform.h"
+
+#include "abstract_output.h"
 #include <config-kwin.h>
 #include "composite.h"
 #include "cursor.h"
@@ -27,11 +29,13 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "outline.h"
 #include "pointer_input.h"
 #include "scene.h"
+#include "screens.h"
 #include "screenedge.h"
 #include "wayland_server.h"
 #include "colorcorrection/manager.h"
 
 #include <KWayland/Server/outputconfiguration_interface.h>
+#include <KWayland/Server/outputchangeset.h>
 
 namespace KWin
 {
@@ -121,11 +125,86 @@ void Platform::createPlatformCursor(QObject *parent)
     new InputRedirectionCursor(parent);
 }
 
-void Platform::configurationChangeRequested(KWayland::Server::OutputConfigurationInterface *config)
+void Platform::requestOutputsChange(KWayland::Server::OutputConfigurationInterface *config)
 {
-    qCWarning(KWIN_CORE) << "This backend does not support configuration changes.";
+    if (!m_supportsOutputChanges) {
+        qCWarning(KWIN_CORE) << "This backend does not support configuration changes.";
+        config->setFailed();
+        return;
+    }
 
-    config->setFailed();
+    using Enablement = KWayland::Server::OutputDeviceInterface::Enablement;
+
+    const auto changes = config->changes();
+    bool countChanged = false;
+
+    //process all non-disabling changes
+    for (auto it = changes.begin(); it != changes.end(); it++) {
+        const KWayland::Server::OutputChangeSet *changeset = it.value();
+
+        auto output = findOutput(it.key()->uuid());
+        if (!output) {
+            qCWarning(KWIN_CORE) << "Could NOT find output matching " << it.key()->uuid();
+            continue;
+        }
+
+        if (changeset->enabledChanged() &&
+                changeset->enabled() == Enablement::Enabled) {
+            output->setEnabled(true);
+            enableOutput(output, true);
+            countChanged = true;
+        }
+        output->applyChanges(changeset);
+    }
+
+    //process any disable requests
+    for (auto it = changes.begin(); it != changes.end(); it++) {
+        const KWayland::Server::OutputChangeSet *changeset = it.value();
+
+        if (changeset->enabledChanged() &&
+                changeset->enabled() == Enablement::Disabled) {
+            if (enabledOutputs().count() == 1) {
+                // TODO: check beforehand this condition and set failed otherwise
+                // TODO: instead create a dummy output?
+                qCWarning(KWIN_CORE) << "Not disabling final screen" << it.key()->uuid();
+                continue;
+            }
+            auto output = findOutput(it.key()->uuid());
+            if (!output) {
+                qCWarning(KWIN_CORE) << "Could NOT find output matching " << it.key()->uuid();
+                continue;
+            }
+            output->setEnabled(false);
+            enableOutput(output, false);
+            countChanged = true;
+        }
+    }
+
+    if (countChanged) {
+        emit screensQueried();
+    } else {
+        emit screens()->changed();
+    }
+    config->setApplied();
+}
+
+AbstractOutput *Platform::findOutput(const QByteArray &uuid)
+{
+    const auto outs = outputs();
+    auto it = std::find_if(outs.constBegin(), outs.constEnd(),
+        [uuid](AbstractOutput *output) {
+            return output->uuid() == uuid; }
+    );
+    if (it != outs.constEnd()) {
+        return *it;
+    }
+    return nullptr;
+}
+
+void Platform::enableOutput(AbstractOutput *output, bool enable)
+{
+    Q_UNUSED(output)
+    Q_UNUSED(enable)
 }
 
 void Platform::setSoftWareCursor(bool set)

platform.h

@@ -184,7 +184,7 @@ public:
      * Base implementation warns that the current backend does not implement this
      * functionality.
      */
-    virtual void configurationChangeRequested(KWayland::Server::OutputConfigurationInterface *config);
+    void requestOutputsChange(KWayland::Server::OutputConfigurationInterface *config);
 
     /**
      * Whether the Platform requires compositing for rendering.
@@ -427,6 +427,9 @@ public:
         return Outputs();
     }
 
+    AbstractOutput *findOutput(const QByteArray &uuid);
+    virtual void enableOutput(AbstractOutput *output, bool enable);
+
     /**
      * A string of information to include in kwin debug output
      * It should not be translated.
@@ -510,6 +513,13 @@ protected:
         m_supportsGammaControl = set;
     }
 
+    /**
+     * Whether the backend is supposed to change the configuration of outputs.
+     */
+    void supportsOutputChanges() {
+        m_supportsOutputChanges = true;
+    }
+
     /**
      * Actual platform specific way to hide the cursor.
      * Sub-classes need to implement if they support hiding the cursor.
@@ -554,6 +564,7 @@ private:
     int m_hideCursorCounter = 0;
     ColorCorrect::Manager *m_colorCorrect = nullptr;
     bool m_supportsGammaControl = false;
+    bool m_supportsOutputChanges = false;
     CompositingType m_selectedCompositor = NoCompositing;
 };
 

plugins/platforms/drm/drm_backend.cpp

@@ -40,7 +40,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #endif
 // KWayland
 #include <KWayland/Server/seat_interface.h>
-#include <KWayland/Server/outputconfiguration_interface.h>
 // KF5
 #include <KConfigGroup>
 #include <KCoreAddons>
@@ -83,6 +82,7 @@ DrmBackend::DrmBackend(QObject *parent)
     }
 #endif
     setSupportsGammaControl(true);
+    supportsOutputChanges();
 }
 
 DrmBackend::~DrmBackend()
@@ -562,54 +562,16 @@ QByteArray DrmBackend::generateOutputConfigurationUuid() const
     return hash.result().toHex().left(10);
 }
 
-void DrmBackend::configurationChangeRequested(KWayland::Server::OutputConfigurationInterface *config)
+void DrmBackend::enableOutput(AbstractOutput *output, bool enable)
 {
-    const auto changes = config->changes();
-    bool countChanged = false;
-
-    //process all non-disabling changes
-    for (auto it = changes.begin(); it != changes.end(); it++) {
-        KWayland::Server::OutputChangeSet *changeset = it.value();
-
-        auto drmoutput = findOutput(it.key()->uuid());
-        if (drmoutput == nullptr) {
-            qCWarning(KWIN_DRM) << "Could NOT find DrmOutput matching " << it.key()->uuid();
-            continue;
-        }
-        if (changeset->enabledChanged() && changeset->enabled() == KWayland::Server::OutputDeviceInterface::Enablement::Enabled) {
-            drmoutput->setEnabled(true);
-            m_enabledOutputs << drmoutput;
-            emit outputAdded(drmoutput);
-            countChanged = true;
-        }
-        drmoutput->setChanges(changeset);
-    }
-    //process any disable requests
-    for (auto it = changes.begin(); it != changes.end(); it++) {
-        KWayland::Server::OutputChangeSet *changeset = it.value();
-        if (changeset->enabledChanged() && changeset->enabled() == KWayland::Server::OutputDeviceInterface::Enablement::Disabled) {
-            if (m_enabledOutputs.count() == 1) {
-                qCWarning(KWIN_DRM) << "Not disabling final screen" << it.key()->uuid();
-                continue;
-            }
-            auto drmoutput = findOutput(it.key()->uuid());
-            if (drmoutput == nullptr) {
-                qCWarning(KWIN_DRM) << "Could NOT find DrmOutput matching " << it.key()->uuid();
-                continue;
-            }
-            drmoutput->setEnabled(false);
-            m_enabledOutputs.removeOne(drmoutput);
-            emit outputRemoved(drmoutput);
-            countChanged = true;
-        }
-    }
-
-    if (countChanged) {
-        emit screensQueried();
+    auto *drmOutput = static_cast<DrmOutput*>(output);
+    if (enable) {
+        m_enabledOutputs << drmOutput;
+        emit outputAdded(drmOutput);
     } else {
-        emit screens()->changed();
+        m_enabledOutputs.removeOne(drmOutput);
+        emit outputRemoved(drmOutput);
     }
-    config->setApplied();
 }
 
 DrmOutput *DrmBackend::findOutput(quint32 connector)
@@ -623,17 +585,6 @@ DrmOutput *DrmBackend::findOutput(quint32 connector)
     return nullptr;
 }
 
-DrmOutput *DrmBackend::findOutput(const QByteArray &uuid)
-{
-    auto it = std::find_if(m_outputs.constBegin(), m_outputs.constEnd(), [uuid] (DrmOutput *o) {
-        return o->m_uuid == uuid;
-    });
-    if (it != m_outputs.constEnd()) {
-        return *it;
-    }
-    return nullptr;
-}
-
 bool DrmBackend::present(DrmBuffer *buffer, DrmOutput *output)
 {
     if (!buffer || buffer->bufferId() == 0) {

plugins/platforms/drm/drm_backend.h

@@ -42,17 +42,6 @@ struct gbm_bo;
 struct gbm_device;
 struct gbm_surface;
 
-namespace KWayland
-{
-namespace Server
-{
-class OutputInterface;
-class OutputDeviceInterface;
-class OutputChangeSet;
-class OutputManagementInterface;
-}
-}
-
 namespace KWin
 {
 
@@ -75,7 +64,6 @@ public:
     explicit DrmBackend(QObject *parent = nullptr);
     ~DrmBackend() override;
 
-    void configurationChangeRequested(KWayland::Server::OutputConfigurationInterface *config) override;
     Screens *createScreens(QObject *parent = nullptr) override;
     QPainterBackend *createQPainterBackend() override;
     OpenGLBackend* createOpenGLBackend() override;
@@ -101,6 +89,8 @@ public:
         return m_enabledOutputs;
     }
 
+    void enableOutput(AbstractOutput *output, bool enable) override;
+
     QVector<DrmPlane*> planes() const {
         return m_planes;
     }
@@ -175,7 +165,6 @@ private:
     void writeOutputsConfiguration();
     QByteArray generateOutputConfigurationUuid() const;
     DrmOutput *findOutput(quint32 connector);
-    DrmOutput *findOutput(const QByteArray &uuid);
     QScopedPointer<Udev> m_udev;
     QScopedPointer<UdevMonitor> m_udevMonitor;
     int m_fd = -1;
@@ -203,7 +192,6 @@ private:
     QVector<DrmPlane*> m_planes;
     QVector<DrmPlane*> m_overlayPlanes;
     QScopedPointer<DpmsInputEventFilter> m_dpmsFilter;
-    KWayland::Server::OutputManagementInterface *m_outputManagement = nullptr;
     gbm_device *m_gbmDevice = nullptr;
 };
 

plugins/platforms/drm/drm_output.h

@@ -72,10 +72,6 @@ public:
         return m_dpmsModePending == DpmsMode::On;
     }
 
-    QByteArray uuid() const {
-        return m_uuid;
-    }
-
     const DrmCrtc *crtc() const {
         return m_crtc;
     }

wayland_server.cpp

@@ -453,7 +453,7 @@ bool WaylandServer::init(const QByteArray &socketName, InitalizationFlags flags)
     m_outputManagement = m_display->createOutputManagement(m_display);
     connect(m_outputManagement, &OutputManagementInterface::configurationChangeRequested,
             this, [this](KWayland::Server::OutputConfigurationInterface *config) {
-                kwinApp()->platform()->configurationChangeRequested(config);
+                kwinApp()->platform()->requestOutputsChange(config);
     });
     m_outputManagement->create();