backends/drm: manage drm objects with std::unique_ptr

Also makes the connector detection code a bit more readable

src/backends/drm/drm_gpu.cpp

@@ -133,9 +133,9 @@ DrmGpu::~DrmGpu()
     if (m_eglDisplay != EGL_NO_DISPLAY) {
         eglTerminate(m_eglDisplay);
     }
-    qDeleteAll(m_crtcs);
+    m_crtcs.clear();
-    qDeleteAll(m_connectors);
+    m_connectors.clear();
-    qDeleteAll(m_planes);
+    m_planes.clear();
     delete m_socketNotifier;
     if (m_gbmDevice) {
         gbm_device_destroy(m_gbmDevice);
@@ -167,42 +167,40 @@ void DrmGpu::initDrmResources()
             // create the plane objects
             for (unsigned int i = 0; i < planeResources->count_planes; ++i) {
                 DrmUniquePtr<drmModePlane> kplane(drmModeGetPlane(m_fd, planeResources->planes[i]));
-                DrmPlane *p = new DrmPlane(this, kplane->plane_id);
+                auto plane = std::make_unique<DrmPlane>(this, kplane->plane_id);
-                if (p->init()) {
+                if (plane->init()) {
-                    m_planes << p;
+                    m_allObjects << plane.get();
-                    m_allObjects << p;
+                    m_planes.push_back(std::move(plane));
-                } else {
-                    delete p;
                 }
             }
-            if (m_planes.isEmpty()) {
+            if (m_planes.empty()) {
                 qCWarning(KWIN_DRM) << "Failed to create any plane. Falling back to legacy mode on GPU " << m_devNode;
             }
         } else {
             qCWarning(KWIN_DRM) << "Failed to get plane resources. Falling back to legacy mode on GPU " << m_devNode;
         }
     }
-    m_atomicModeSetting = !m_planes.isEmpty();
+    m_atomicModeSetting = !m_planes.empty();
 
     DrmUniquePtr<drmModeRes> resources(drmModeGetResources(m_fd));
     if (!resources) {
         qCCritical(KWIN_DRM) << "drmModeGetResources for getting CRTCs failed on GPU" << m_devNode;
         return;
     }
-    auto planes = m_planes;
+    QVector<DrmPlane *> assignedPlanes;
     for (int i = 0; i < resources->count_crtcs; ++i) {
         uint32_t crtcId = resources->crtcs[i];
         DrmPlane *primary = nullptr;
         DrmPlane *cursor = nullptr;
-        for (const auto &plane : qAsConst(planes)) {
+        for (const auto &plane : m_planes) {
-            if (plane->isCrtcSupported(i)) {
+            if (plane->isCrtcSupported(i) && !assignedPlanes.contains(plane.get())) {
                 if (plane->type() == DrmPlane::TypeIndex::Primary) {
                     if (!primary || primary->getProp(DrmPlane::PropertyIndex::CrtcId)->pending() == crtcId) {
-                        primary = plane;
+                        primary = plane.get();
                     }
                 } else if (plane->type() == DrmPlane::TypeIndex::Cursor) {
                     if (!cursor || cursor->getProp(DrmPlane::PropertyIndex::CrtcId)->pending() == crtcId) {
-                        cursor = plane;
+                        cursor = plane.get();
                     }
                 }
             }
@@ -211,14 +209,14 @@ void DrmGpu::initDrmResources()
             qCWarning(KWIN_DRM) << "Could not find a suitable primary plane for crtc" << resources->crtcs[i];
             continue;
         }
-        planes.removeOne(primary);
+        assignedPlanes.push_back(primary);
-        auto c = new DrmCrtc(this, crtcId, i, primary, cursor);
+        assignedPlanes.push_back(cursor);
-        if (!c->init()) {
+        auto crtc = std::make_unique<DrmCrtc>(this, crtcId, i, primary, cursor);
-            delete c;
+        if (!crtc->init()) {
             continue;
         }
-        m_crtcs << c;
+        m_allObjects << crtc.get();
-        m_allObjects << c;
+        m_crtcs.push_back(std::move(crtc));
     }
 }
 
@@ -250,65 +248,61 @@ bool DrmGpu::updateOutputs()
     }
 
     // check for added and removed connectors
+    QVector<DrmConnector *> existing;
     QVector<DrmOutput *> addedOutputs;
-    QVector<DrmConnector *> removedConnectors = m_connectors;
     for (int i = 0; i < resources->count_connectors; ++i) {
         const uint32_t currentConnector = resources->connectors[i];
-        auto it = std::find_if(m_connectors.constBegin(), m_connectors.constEnd(), [currentConnector](DrmConnector *c) {
+        const auto it = std::find_if(m_connectors.begin(), m_connectors.end(), [currentConnector](const auto &connector) {
-            return c->id() == currentConnector;
+            return connector->id() == currentConnector;
         });
-        DrmConnector *conn = it == m_connectors.constEnd() ? nullptr : *it;
+        if (it == m_connectors.end()) {
-        if (!conn) {
+            auto conn = std::make_unique<DrmConnector>(this, currentConnector);
-            conn = new DrmConnector(this, currentConnector);
             if (!conn->init()) {
-                delete conn;
                 continue;
             }
-            m_connectors << conn;
+            existing.push_back(conn.get());
-            m_allObjects << conn;
+            m_allObjects.push_back(conn.get());
+            m_connectors.push_back(std::move(conn));
         } else {
-            conn->updateProperties();
+            (*it)->updateProperties();
-            removedConnectors.removeOne(conn);
+            existing.push_back(it->get());
         }
-        auto output = findOutput(conn->id());
+    }
-        auto leaseOutput = findLeaseOutput(conn->id());
+    for (auto it = m_connectors.begin(); it != m_connectors.end();) {
-        if (conn->isConnected()) {
+        DrmConnector *conn = it->get();
-            if (!output && !leaseOutput) {
+        const auto output = findOutput(conn->id());
-                qCDebug(KWIN_DRM, "New %soutput on GPU %s: %s", conn->isNonDesktop() ? "non-desktop " : "", qPrintable(m_devNode), qPrintable(conn->modelName()));
+        const auto leaseOutput = findLeaseOutput(conn->id());
-                const auto pipeline = conn->pipeline();
+        const bool stillExists = existing.contains(conn);
-                m_pipelines << pipeline;
+        if (!stillExists || !conn->isConnected()) {
-                if (conn->isNonDesktop()) {
-                    auto leaseOutput = new DrmLeaseOutput(pipeline, m_leaseDevice);
-                    m_leaseOutputs << leaseOutput;
-                } else {
-                    auto output = new DrmOutput(pipeline);
-                    m_drmOutputs << output;
-                    m_outputs << output;
-                    addedOutputs << output;
-                    Q_EMIT outputAdded(output);
-                }
-                pipeline->setLayers(m_platform->renderBackend()->createPrimaryLayer(pipeline), m_platform->renderBackend()->createCursorLayer(pipeline));
-                pipeline->setActive(!conn->isNonDesktop());
-                pipeline->applyPendingChanges();
-            }
-        } else {
-            conn->disable();
             if (output) {
                 removeOutput(output);
             } else if (leaseOutput) {
                 removeLeaseOutput(leaseOutput);
             }
+            conn->disable();
+        } else if (!output && !leaseOutput) {
+            qCDebug(KWIN_DRM, "New %soutput on GPU %s: %s", conn->isNonDesktop() ? "non-desktop " : "", qPrintable(m_devNode), qPrintable(conn->modelName()));
+            const auto pipeline = conn->pipeline();
+            m_pipelines << pipeline;
+            if (conn->isNonDesktop()) {
+                auto leaseOutput = new DrmLeaseOutput(pipeline, m_leaseDevice);
+                m_leaseOutputs << leaseOutput;
+            } else {
+                auto output = new DrmOutput(pipeline);
+                m_drmOutputs << output;
+                m_outputs << output;
+                addedOutputs << output;
+                Q_EMIT outputAdded(output);
+            }
+            pipeline->setLayers(m_platform->renderBackend()->createPrimaryLayer(pipeline), m_platform->renderBackend()->createCursorLayer(pipeline));
+            pipeline->setActive(!conn->isNonDesktop());
+            pipeline->applyPendingChanges();
         }
-    }
+        if (stillExists) {
-    for (const auto &connector : qAsConst(removedConnectors)) {
+            it++;
-        if (auto output = findOutput(connector->id())) {
+        } else {
-            removeOutput(output);
+            it = m_connectors.erase(it);
-        } else if (auto leaseOutput = findLeaseOutput(connector->id())) {
-            removeLeaseOutput(leaseOutput);
         }
-        m_connectors.removeOne(connector);
-        m_allObjects.removeOne(connector);
-        delete connector;
     }
 
     // update crtc properties
@@ -333,7 +327,11 @@ bool DrmGpu::updateOutputs()
             pipeline->revertPendingChanges();
         }
         for (const auto &output : qAsConst(addedOutputs)) {
-            m_connectors.removeOne(output->connector());
+            const auto it = std::find_if(m_connectors.begin(), m_connectors.end(), [output](const auto &conn) {
+                return conn.get() == output->connector();
+            });
+            Q_ASSERT(it != m_connectors.end());
+            m_connectors.erase(it);
             removeOutput(output);
         }
         QTimer::singleShot(50, m_platform, &DrmBackend::updateOutputs);
@@ -415,17 +413,22 @@ DrmPipeline::Error DrmGpu::checkCrtcAssignment(QVector<DrmConnector *> connector
 DrmPipeline::Error DrmGpu::testPendingConfiguration()
 {
     QVector<DrmConnector *> connectors;
-    for (const auto &conn : qAsConst(m_connectors)) {
+    QVector<DrmCrtc *> crtcs;
-        if (conn->isConnected()) {
+    // only change resources that aren't currently leased away
-            connectors << conn;
+    for (const auto &conn : m_connectors) {
+        bool isLeased = std::any_of(m_leaseOutputs.cbegin(), m_leaseOutputs.cend(), [&conn](const auto output) {
+            return output->lease() && output->pipeline()->connector() == conn.get();
+        });
+        if (!isLeased) {
+            connectors.push_back(conn.get());
         }
     }
-    auto crtcs = m_crtcs;
+    for (const auto &crtc : m_crtcs) {
-    // don't touch resources that are leased
+        bool isLeased = std::any_of(m_leaseOutputs.cbegin(), m_leaseOutputs.cend(), [&crtc](const auto output) {
-    for (const auto &output : qAsConst(m_leaseOutputs)) {
+            return output->lease() && output->pipeline()->crtc() == crtc.get();
-        if (output->lease()) {
+        });
-            connectors.removeOne(output->pipeline()->connector());
+        if (!isLeased) {
-            crtcs.removeOne(output->pipeline()->crtc());
+            crtcs.push_back(crtc.get());
         }
     }
     if (m_atomicModeSetting) {

src/backends/drm/drm_gpu.h

@@ -121,9 +121,9 @@ private:
     EGLDisplay m_eglDisplay = EGL_NO_DISPLAY;
     DrmBackend *const m_platform;
 
-    QVector<DrmPlane *> m_planes;
+    std::vector<std::unique_ptr<DrmPlane>> m_planes;
-    QVector<DrmCrtc *> m_crtcs;
+    std::vector<std::unique_ptr<DrmCrtc>> m_crtcs;
-    QVector<DrmConnector *> m_connectors;
+    std::vector<std::unique_ptr<DrmConnector>> m_connectors;
     QVector<DrmObject *> m_allObjects;
     QVector<DrmPipeline *> m_pipelines;