color management: refactor and move to its own directory

The pipeline stages are also now properly managed, which should prevent
use-after-free errors.

BUG: 453026

CMakeLists.txt

@@ -417,6 +417,7 @@ include_directories(BEFORE
     ${CMAKE_CURRENT_SOURCE_DIR}/src/effects
     ${CMAKE_CURRENT_SOURCE_DIR}/src/tabbox
     ${CMAKE_CURRENT_SOURCE_DIR}/src/platformsupport
+    ${CMAKE_CURRENT_SOURCE_DIR}/src/colors
 )
 
 if (KF5DocTools_FOUND)

src/CMakeLists.txt

@@ -31,9 +31,11 @@ target_sources(kwin PRIVATE
     appmenu.cpp
     atoms.cpp
     client_machine.cpp
-    colordevice.cpp
-    colormanager.cpp
-    colors.cpp
+    colors/colordevice.cpp
+    colors/colorlut.cpp
+    colors/colormanager.cpp
+    colors/colorpipelinestage.cpp
+    colors/colortransformation.cpp
     composite.cpp
     cursor.cpp
     cursordelegate_opengl.cpp

src/backends/drm/drm_output.h

@@ -9,7 +9,6 @@
 #ifndef KWIN_DRM_OUTPUT_H
 #define KWIN_DRM_OUTPUT_H
 
-#include "colors.h"
 #include "drm_abstract_output.h"
 #include "drm_object.h"
 #include "drm_object_plane.h"

src/backends/drm/drm_pipeline.h

@@ -17,7 +17,7 @@
 #include <chrono>
 #include <xf86drmMode.h>
 
-#include "colors.h"
+#include "colorlut.h"
 #include "drm_object_plane.h"
 #include "output.h"
 #include "renderloop_p.h"

src/backends/x11/standalone/x11_output.cpp

@@ -7,7 +7,7 @@
     SPDX-License-Identifier: GPL-2.0-or-later
 */
 #include "x11_output.h"
-#include "colors.h"
+#include "colorlut.h"
 #include "main.h"
 
 namespace KWin

src/colors/colordevice.cpp

@@ -5,7 +5,8 @@
 */
 
 #include "colordevice.h"
-#include "colors.h"
+#include "colorpipelinestage.h"
+#include "colortransformation.h"
 #include "output.h"
 #include "utils/common.h"
 
@@ -24,17 +25,6 @@ struct CmsDeleter;
 template<typename T>
 using CmsScopedPointer = QScopedPointer<T, CmsDeleter<T>>;
 
-template<>
-struct CmsDeleter<cmsStage>
-{
-    static inline void cleanup(cmsStage *stage)
-    {
-        if (stage) {
-            cmsStageFree(stage);
-        }
-    }
-};
-
 template<>
 struct CmsDeleter<cmsToneCurve>
 {
@@ -69,21 +59,15 @@ public:
     uint brightness = 100;
     uint temperature = 6500;
 
-    CmsScopedPointer<cmsStage> temperatureStage;
-    CmsScopedPointer<cmsStage> brightnessStage;
-    CmsScopedPointer<cmsStage> calibrationStage;
+    QSharedPointer<ColorPipelineStage> temperatureStage;
+    QSharedPointer<ColorPipelineStage> brightnessStage;
+    QSharedPointer<ColorPipelineStage> calibrationStage;
 
     QSharedPointer<ColorTransformation> transformation;
 };
 
 void ColorDevicePrivate::rebuildPipeline()
 {
-    cmsPipeline *pipeline = cmsPipelineAlloc(nullptr, 3, 3);
-    if (!pipeline) {
-        qCWarning(KWIN_CORE) << "Failed to allocate cmsPipeline!";
-        return;
-    }
-
     if (dirtyCurves & DirtyCalibrationToneCurve) {
         updateCalibrationToneCurves();
     }
@@ -93,26 +77,13 @@ void ColorDevicePrivate::rebuildPipeline()
     if (dirtyCurves & DirtyTemperatureToneCurve) {
         updateTemperatureToneCurves();
     }
-
     dirtyCurves = DirtyToneCurves();
 
-    if (calibrationStage) {
-        if (!cmsPipelineInsertStage(pipeline, cmsAT_END, calibrationStage.data())) {
-            qCWarning(KWIN_CORE) << "Failed to insert the color calibration pipeline stage";
+    const auto tmp = QSharedPointer<ColorTransformation>::create(QVector{calibrationStage, brightnessStage, temperatureStage});
+    if (tmp->valid()) {
+        transformation = tmp;
     }
 }
-    if (temperatureStage) {
-        if (!cmsPipelineInsertStage(pipeline, cmsAT_END, temperatureStage.data())) {
-            qCWarning(KWIN_CORE) << "Failed to insert the  color temperature pipeline stage";
-        }
-    }
-    if (brightnessStage) {
-        if (!cmsPipelineInsertStage(pipeline, cmsAT_END, brightnessStage.data())) {
-            qCWarning(KWIN_CORE) << "Failed to insert the color brightness pipeline stage";
-        }
-    }
-    transformation = QSharedPointer<ColorTransformation>::create(pipeline);
-}
 
 static qreal interpolate(qreal a, qreal b, qreal blendFactor)
 {
@@ -164,7 +135,7 @@ void ColorDevicePrivate::updateTemperatureToneCurves()
     // The ownership of the tone curves will be moved to the pipeline stage.
     cmsToneCurve *toneCurves[] = {redCurve.take(), greenCurve.take(), blueCurve.take()};
 
-    temperatureStage.reset(cmsStageAllocToneCurves(nullptr, 3, toneCurves));
+    temperatureStage = QSharedPointer<ColorPipelineStage>::create(cmsStageAllocToneCurves(nullptr, 3, toneCurves));
     if (!temperatureStage) {
         qCWarning(KWIN_CORE) << "Failed to create the color temperature pipeline stage";
     }
@@ -201,7 +172,7 @@ void ColorDevicePrivate::updateBrightnessToneCurves()
     // The ownership of the tone curves will be moved to the pipeline stage.
     cmsToneCurve *toneCurves[] = {redCurve.take(), greenCurve.take(), blueCurve.take()};
 
-    brightnessStage.reset(cmsStageAllocToneCurves(nullptr, 3, toneCurves));
+    brightnessStage = QSharedPointer<ColorPipelineStage>::create(cmsStageAllocToneCurves(nullptr, 3, toneCurves));
     if (!brightnessStage) {
         qCWarning(KWIN_CORE) << "Failed to create the color brightness pipeline stage";
     }
@@ -231,11 +202,7 @@ void ColorDevicePrivate::updateCalibrationToneCurves()
             cmsDupToneCurve(vcgt[1]),
             cmsDupToneCurve(vcgt[2]),
         };
-
-        calibrationStage.reset(cmsStageAllocToneCurves(nullptr, 3, toneCurves));
-        if (!calibrationStage) {
-            qCWarning(KWIN_CORE) << "Failed to create the color calibration pipeline stage";
-        }
+        calibrationStage = QSharedPointer<ColorPipelineStage>::create(cmsStageAllocToneCurves(nullptr, 3, toneCurves));
     }
 
     cmsCloseProfile(handle);

src/colors/colorlut.cpp

@@ -6,35 +6,13 @@
 
     SPDX-License-Identifier: GPL-2.0-or-later
 */
-#include "colors.h"
+#include "colorlut.h"
 
-#include <lcms2.h>
+#include "colortransformation.h"
 
 namespace KWin
 {
 
-ColorTransformation::ColorTransformation(cmsPipeline *pipeline)
-    : m_pipeline(pipeline)
-{
-}
-
-ColorTransformation::~ColorTransformation()
-{
-    cmsStage *last = nullptr;
-    do {
-        cmsPipelineUnlinkStage(m_pipeline, cmsAT_END, &last);
-    } while (last);
-    cmsPipelineFree(m_pipeline);
-}
-
-std::tuple<uint16_t, uint16_t, uint16_t> ColorTransformation::transform(uint16_t r, uint16_t g, uint16_t b) const
-{
-    const uint16_t in[3] = {r, g, b};
-    uint16_t out[3] = {0, 0, 0};
-    cmsPipelineEval16(in, out, m_pipeline);
-    return {out[0], out[1], out[2]};
-}
-
 ColorLUT::ColorLUT(const QSharedPointer<ColorTransformation> &transformation, size_t size)
     : m_transformation(transformation)
 {

src/colors/colorlut.h

@@ -9,26 +9,13 @@
 #pragma once
 
 #include <QSharedPointer>
-#include <QVector>
 
 #include "kwin_export.h"
 
-typedef struct _cmsPipeline_struct cmsPipeline;
-
 namespace KWin
 {
 
-class KWIN_EXPORT ColorTransformation
-{
-public:
-    ColorTransformation(cmsPipeline *pipeline);
-    ~ColorTransformation();
-
-    std::tuple<uint16_t, uint16_t, uint16_t> transform(uint16_t r, uint16_t g, uint16_t b) const;
-
-private:
-    cmsPipeline *const m_pipeline;
-};
+class ColorTransformation;
 
 class KWIN_EXPORT ColorLUT
 {

src/colors/colorpipelinestage.cpp

@@ -0,0 +1,49 @@
+/*
+    KWin - the KDE window manager
+    This file is part of the KDE project.
+
+    SPDX-FileCopyrightText: 2022 Xaver Hugl <xaver.hugl@gmail.com>
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+#include "colorpipelinestage.h"
+
+#include <lcms2.h>
+
+#include "utils/common.h"
+
+namespace KWin
+{
+
+ColorPipelineStage::ColorPipelineStage(cmsStage *stage)
+    : m_stage(stage)
+{
+}
+
+ColorPipelineStage::~ColorPipelineStage()
+{
+    if (m_stage) {
+        cmsStageFree(m_stage);
+    }
+}
+
+QSharedPointer<ColorPipelineStage> ColorPipelineStage::dup() const
+{
+    if (m_stage) {
+        auto dup = cmsStageDup(m_stage);
+        if (dup) {
+            return QSharedPointer<ColorPipelineStage>::create(dup);
+        } else {
+            qCWarning(KWIN_CORE) << "Failed to duplicate cmsStage!";
+        }
+    }
+    return nullptr;
+    ;
+}
+
+cmsStage *ColorPipelineStage::stage() const
+{
+    return m_stage;
+}
+
+}

src/colors/colorpipelinestage.h

@@ -0,0 +1,33 @@
+/*
+    KWin - the KDE window manager
+    This file is part of the KDE project.
+
+    SPDX-FileCopyrightText: 2022 Xaver Hugl <xaver.hugl@gmail.com>
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+#pragma once
+
+#include "kwin_export.h"
+
+#include <QSharedPointer>
+
+typedef struct _cmsStage_struct cmsStage;
+
+namespace KWin
+{
+
+class KWIN_EXPORT ColorPipelineStage
+{
+public:
+    ColorPipelineStage(cmsStage *stage);
+    ~ColorPipelineStage();
+
+    QSharedPointer<ColorPipelineStage> dup() const;
+    cmsStage *stage() const;
+
+private:
+    cmsStage *const m_stage;
+};
+
+}

src/colors/colortransformation.cpp

@@ -0,0 +1,61 @@
+
+#include "colortransformation.h"
+
+#include <lcms2.h>
+
+#include "colorpipelinestage.h"
+#include "utils/common.h"
+
+namespace KWin
+{
+
+ColorTransformation::ColorTransformation(const QVector<QSharedPointer<ColorPipelineStage>> &stages)
+    : m_pipeline(cmsPipelineAlloc(nullptr, 3, 3))
+{
+    if (!m_pipeline) {
+        qCWarning(KWIN_CORE) << "Failed to allocate cmsPipeline!";
+        m_valid = false;
+        return;
+    }
+    for (const auto &stage : stages) {
+        if (stage) {
+            const auto dup = stage->dup();
+            if (!dup) {
+                m_valid = false;
+                return;
+            }
+            m_stages << dup;
+            if (!cmsPipelineInsertStage(m_pipeline, cmsAT_END, dup->stage())) {
+                qCWarning(KWIN_CORE) << "Failed to insert cmsPipeline stage!";
+                m_valid = false;
+                return;
+            }
+        }
+    }
+}
+
+ColorTransformation::~ColorTransformation()
+{
+    if (m_pipeline) {
+        cmsStage *last = nullptr;
+        do {
+            cmsPipelineUnlinkStage(m_pipeline, cmsAT_END, &last);
+        } while (last);
+        cmsPipelineFree(m_pipeline);
+    }
+}
+
+bool ColorTransformation::valid() const
+{
+    return m_valid;
+}
+
+std::tuple<uint16_t, uint16_t, uint16_t> ColorTransformation::transform(uint16_t r, uint16_t g, uint16_t b) const
+{
+    const uint16_t in[3] = {r, g, b};
+    uint16_t out[3] = {0, 0, 0};
+    cmsPipelineEval16(in, out, m_pipeline);
+    return {out[0], out[1], out[2]};
+}
+
+}

src/colors/colortransformation.h

@@ -0,0 +1,41 @@
+/*
+    KWin - the KDE window manager
+    This file is part of the KDE project.
+
+    SPDX-FileCopyrightText: 2022 Xaver Hugl <xaver.hugl@gmail.com>
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+#pragma once
+
+#include <QSharedPointer>
+#include <QVector>
+#include <stdint.h>
+#include <tuple>
+
+#include "kwin_export.h"
+
+typedef struct _cmsPipeline_struct cmsPipeline;
+
+namespace KWin
+{
+
+class ColorPipelineStage;
+
+class KWIN_EXPORT ColorTransformation
+{
+public:
+    ColorTransformation(const QVector<QSharedPointer<ColorPipelineStage>> &stages);
+    ~ColorTransformation();
+
+    bool valid() const;
+
+    std::tuple<uint16_t, uint16_t, uint16_t> transform(uint16_t r, uint16_t g, uint16_t b) const;
+
+private:
+    cmsPipeline *const m_pipeline;
+    QVector<QSharedPointer<ColorPipelineStage>> m_stages;
+    bool m_valid = true;
+};
+
+}