Port gamma ramp code to generic color transformations

Instead of creating a gammaramp object with a fixed size, make the color
device create a color transformation object that can be used to construct
arbitrary LUTs. This is needed in order to support tiled displays well
and is useful for further color management work.

src/CMakeLists.txt

@@ -34,6 +34,7 @@ target_sources(kwin PRIVATE
     client_machine.cpp
     colordevice.cpp
     colormanager.cpp
+    colors.cpp
     composite.cpp
     cursor.cpp
     cursordelegate_opengl.cpp

src/backends/drm/drm_output.cpp

@@ -330,25 +330,6 @@ bool DrmOutput::present()
     return false;
 }
 
-int DrmOutput::gammaRampSize() const
-{
-    return m_pipeline->pending.crtc ? m_pipeline->pending.crtc->gammaRampSize() : 256;
-}
-
-bool DrmOutput::setGammaRamp(const GammaRamp &gamma)
-{
-    m_pipeline->pending.gamma = QSharedPointer<DrmGammaRamp>::create(m_gpu, gamma);
-    if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test)) {
-        m_pipeline->applyPendingChanges();
-        m_renderLoop->scheduleRepaint();
-        return true;
-    } else {
-        qCWarning(KWIN_DRM) << "Applying gamma ramp failed on output" << this;
-        m_pipeline->revertPendingChanges();
-        return false;
-    }
-}
-
 DrmConnector *DrmOutput::connector() const
 {
     return m_connector;
@@ -428,4 +409,14 @@ DrmOutputLayer *DrmOutput::outputLayer() const
     return m_pipeline->pending.layer.data();
 }
 
+void DrmOutput::setColorTransformation(const QSharedPointer<ColorTransformation> &transformation)
+{
+    m_pipeline->pending.colorTransformation = transformation;
+    if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test)) {
+        m_pipeline->applyPendingChanges();
+        m_renderLoop->scheduleRepaint();
+    } else {
+        m_pipeline->revertPendingChanges();
+    }
+}
 }

src/backends/drm/drm_output.h

@@ -9,10 +9,10 @@
 #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"
-#include "drm_pointer.h"
 
 #include <QObject>
 #include <QPoint>
@@ -57,6 +57,8 @@ public:
 
     bool usesSoftwareCursor() const override;
 
+    void setColorTransformation(const QSharedPointer<ColorTransformation> &transformation) override;
+
 private:
     void initOutputDevice();
 
@@ -66,8 +68,6 @@ private:
 
     QVector<Output::Mode> getModes() const;
 
-    int gammaRampSize() const override;
-    bool setGammaRamp(const GammaRamp &gamma) override;
     void updateCursor();
     void moveCursor();
 

src/backends/drm/drm_pipeline.cpp

@@ -175,6 +175,10 @@ bool DrmPipeline::populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags)
     }
     if (pending.crtc) {
         pending.crtc->setPending(DrmCrtc::PropertyIndex::VrrEnabled, pending.syncMode == RenderLoopPrivate::SyncMode::Adaptive);
+        const auto currentTransformation = pending.gamma ? pending.gamma->lut().transformation() : nullptr;
+        if (pending.colorTransformation != currentTransformation) {
+            pending.gamma = QSharedPointer<DrmGammaRamp>::create(pending.crtc, pending.colorTransformation);
+        }
         pending.crtc->setPending(DrmCrtc::PropertyIndex::Gamma_LUT, pending.gamma ? pending.gamma->blobId() : 0);
         const auto modeSize = pending.mode->size();
         const auto buffer = pending.layer->currentBuffer().data();
@@ -471,18 +475,18 @@ DrmCrtc *DrmPipeline::currentCrtc() const
     return m_current.crtc;
 }
 
-DrmGammaRamp::DrmGammaRamp(DrmGpu *gpu, const GammaRamp &lut)
+DrmGammaRamp::DrmGammaRamp(DrmCrtc *crtc, const QSharedPointer<ColorTransformation> &transformation)
-    : m_gpu(gpu)
+    : m_gpu(crtc->gpu())
-    , m_lut(lut)
+    , m_lut(transformation, crtc->gammaRampSize())
 {
-    if (gpu->atomicModeSetting()) {
+    if (crtc->gpu()->atomicModeSetting()) {
-        QVector<drm_color_lut> atomicLut(lut.size());
+        QVector<drm_color_lut> atomicLut(m_lut.size());
-        for (uint32_t i = 0; i < lut.size(); i++) {
+        for (uint32_t i = 0; i < m_lut.size(); i++) {
-            atomicLut[i].red = lut.red()[i];
+            atomicLut[i].red = m_lut.red()[i];
-            atomicLut[i].green = lut.green()[i];
+            atomicLut[i].green = m_lut.green()[i];
-            atomicLut[i].blue = lut.blue()[i];
+            atomicLut[i].blue = m_lut.blue()[i];
         }
-        if (drmModeCreatePropertyBlob(gpu->fd(), atomicLut.data(), sizeof(drm_color_lut) * lut.size(), &m_blobId) != 0) {
+        if (drmModeCreatePropertyBlob(crtc->gpu()->fd(), atomicLut.data(), sizeof(drm_color_lut) * m_lut.size(), &m_blobId) != 0) {
             qCWarning(KWIN_DRM) << "Failed to create gamma blob!" << strerror(errno);
         }
     }
@@ -500,24 +504,9 @@ uint32_t DrmGammaRamp::blobId() const
     return m_blobId;
 }
 
-uint32_t DrmGammaRamp::size() const
+const ColorLUT &DrmGammaRamp::lut() const
 {
-    return m_lut.size();
+    return m_lut;
-}
-
-uint16_t *DrmGammaRamp::red() const
-{
-    return const_cast<uint16_t *>(m_lut.red());
-}
-
-uint16_t *DrmGammaRamp::green() const
-{
-    return const_cast<uint16_t *>(m_lut.green());
-}
-
-uint16_t *DrmGammaRamp::blue() const
-{
-    return const_cast<uint16_t *>(m_lut.blue());
 }
 
 void DrmPipeline::printFlags(uint32_t flags)

src/backends/drm/drm_pipeline.h

@@ -17,6 +17,7 @@
 #include <chrono>
 #include <xf86drmMode.h>
 
+#include "colors.h"
 #include "drm_object_plane.h"
 #include "output.h"
 #include "renderloop_p.h"
@@ -36,18 +37,15 @@ class DrmPipelineLayer;
 class DrmGammaRamp
 {
 public:
-    DrmGammaRamp(DrmGpu *gpu, const GammaRamp &lut);
+    DrmGammaRamp(DrmCrtc *crtc, const QSharedPointer<ColorTransformation> &transformation);
     ~DrmGammaRamp();
 
-    uint32_t size() const;
+    const ColorLUT &lut() const;
-    uint16_t *red() const;
-    uint16_t *green() const;
-    uint16_t *blue() const;
     uint32_t blobId() const;
 
 private:
     DrmGpu *m_gpu;
-    const GammaRamp m_lut;
+    const ColorLUT m_lut;
     uint32_t m_blobId = 0;
 };
 
@@ -104,6 +102,7 @@ public:
         uint32_t overscan = 0;
         Output::RgbRange rgbRange = Output::RgbRange::Automatic;
         RenderLoopPrivate::SyncMode syncMode = RenderLoopPrivate::SyncMode::Fixed;
+        QSharedPointer<ColorTransformation> colorTransformation;
         QSharedPointer<DrmGammaRamp> gamma;
 
         QSharedPointer<DrmPipelineLayer> layer;

src/backends/drm/drm_pipeline_legacy.cpp

@@ -105,7 +105,7 @@ bool DrmPipeline::applyPendingChangesLegacy()
         if (needsModeset() && !legacyModeset()) {
             return false;
         }
-        if (pending.gamma && drmModeCrtcSetGamma(gpu()->fd(), pending.crtc->id(), pending.gamma->size(), pending.gamma->red(), pending.gamma->green(), pending.gamma->blue()) != 0) {
+        if (pending.gamma && drmModeCrtcSetGamma(gpu()->fd(), pending.crtc->id(), pending.gamma->lut().size(), pending.gamma->lut().red(), pending.gamma->lut().green(), pending.gamma->lut().blue()) != 0) {
             qCWarning(KWIN_DRM) << "Setting gamma failed!" << strerror(errno);
             return false;
         }

src/backends/drm/drm_virtual_output.cpp

@@ -77,17 +77,6 @@ void DrmVirtualOutput::updateEnablement(bool enable)
     m_gpu->platform()->enableOutput(this, enable);
 }
 
-int DrmVirtualOutput::gammaRampSize() const
-{
-    return 200;
-}
-
-bool DrmVirtualOutput::setGammaRamp(const GammaRamp &gamma)
-{
-    Q_UNUSED(gamma);
-    return true;
-}
-
 DrmOutputLayer *DrmVirtualOutput::outputLayer() const
 {
     return m_layer.data();

src/backends/drm/drm_virtual_output.h

@@ -30,8 +30,6 @@ public:
     ~DrmVirtualOutput() override;
 
     bool present() override;
-    int gammaRampSize() const override;
-    bool setGammaRamp(const GammaRamp &gamma) override;
     DrmOutputLayer *outputLayer() const override;
     void recreateSurface();
 

src/backends/virtual/virtual_output.h

@@ -32,19 +32,7 @@ public:
     SoftwareVsyncMonitor *vsyncMonitor() const;
 
     void init(const QPoint &logicalPosition, const QSize &pixelSize);
-
     void setGeometry(const QRect &geo);
-
-    int gammaRampSize() const override
-    {
-        return m_gammaSize;
-    }
-    bool setGammaRamp(const GammaRamp &gamma) override
-    {
-        Q_UNUSED(gamma);
-        return m_gammaResult;
-    }
-
     void updateEnablement(bool enable) override;
 
 private:

src/backends/x11/standalone/x11_output.cpp

@@ -7,6 +7,7 @@
     SPDX-License-Identifier: GPL-2.0-or-later
 */
 #include "x11_output.h"
+#include "colors.h"
 #include "main.h"
 
 namespace KWin
@@ -38,21 +39,13 @@ void X11Output::setXineramaNumber(int number)
     m_xineramaNumber = number;
 }
 
-int X11Output::gammaRampSize() const
+void X11Output::setColorTransformation(const QSharedPointer<ColorTransformation> &transformation)
-{
-    return m_gammaRampSize;
-}
-
-bool X11Output::setGammaRamp(const GammaRamp &gamma)
 {
     if (m_crtc == XCB_NONE) {
-        return false;
+        return;
     }
-
+    ColorLUT lut(transformation, m_gammaRampSize);
-    xcb_randr_set_crtc_gamma(kwinApp()->x11Connection(), m_crtc, gamma.size(), gamma.red(),
+    xcb_randr_set_crtc_gamma(kwinApp()->x11Connection(), m_crtc, lut.size(), lut.red(), lut.green(), lut.blue());
-                             gamma.green(), gamma.blue());
-
-    return true;
 }
 
 void X11Output::setCrtc(xcb_randr_crtc_t crtc)

src/backends/x11/standalone/x11_output.h

@@ -38,8 +38,7 @@ public:
     int xineramaNumber() const;
     void setXineramaNumber(int number);
 
-    int gammaRampSize() const override;
+    void setColorTransformation(const QSharedPointer<ColorTransformation> &transformation) override;
-    bool setGammaRamp(const GammaRamp &gamma) override;
 
     void setPhysicalSize(const QSize &size);
     void setMode(const QSize &size, int refreshRate);

src/colordevice.cpp

@@ -5,6 +5,7 @@
 */
 
 #include "colordevice.h"
+#include "colors.h"
 #include "output.h"
 #include "utils/common.h"
 
@@ -23,17 +24,6 @@ struct CmsDeleter;
 template<typename T>
 using CmsScopedPointer = QScopedPointer<T, CmsDeleter<T>>;
 
-template<>
-struct CmsDeleter<cmsPipeline>
-{
-    static inline void cleanup(cmsPipeline *pipeline)
-    {
-        if (pipeline) {
-            cmsPipelineFree(pipeline);
-        }
-    }
-};
-
 template<>
 struct CmsDeleter<cmsStage>
 {
@@ -67,7 +57,6 @@ public:
     Q_DECLARE_FLAGS(DirtyToneCurves, DirtyToneCurveBit)
 
     void rebuildPipeline();
-    void unlinkPipeline();
 
     void updateTemperatureToneCurves();
     void updateBrightnessToneCurves();
@@ -84,17 +73,17 @@ public:
     CmsScopedPointer<cmsStage> brightnessStage;
     CmsScopedPointer<cmsStage> calibrationStage;
 
-    CmsScopedPointer<cmsPipeline> pipeline;
+    QSharedPointer<ColorTransformation> transformation;
 };
 
 void ColorDevicePrivate::rebuildPipeline()
 {
+    cmsPipeline *pipeline = cmsPipelineAlloc(nullptr, 3, 3);
     if (!pipeline) {
-        pipeline.reset(cmsPipelineAlloc(nullptr, 3, 3));
+        qCWarning(KWIN_CORE) << "Failed to allocate cmsPipeline!";
+        return;
     }
 
-    unlinkPipeline();
-
     if (dirtyCurves & DirtyCalibrationToneCurve) {
         updateCalibrationToneCurves();
     }
@@ -108,32 +97,21 @@ void ColorDevicePrivate::rebuildPipeline()
     dirtyCurves = DirtyToneCurves();
 
     if (calibrationStage) {
-        if (!cmsPipelineInsertStage(pipeline.data(), cmsAT_END, calibrationStage.data())) {
+        if (!cmsPipelineInsertStage(pipeline, cmsAT_END, calibrationStage.data())) {
             qCWarning(KWIN_CORE) << "Failed to insert the color calibration pipeline stage";
         }
     }
     if (temperatureStage) {
-        if (!cmsPipelineInsertStage(pipeline.data(), cmsAT_END, temperatureStage.data())) {
+        if (!cmsPipelineInsertStage(pipeline, cmsAT_END, temperatureStage.data())) {
             qCWarning(KWIN_CORE) << "Failed to insert the  color temperature pipeline stage";
         }
     }
     if (brightnessStage) {
-        if (!cmsPipelineInsertStage(pipeline.data(), cmsAT_END, brightnessStage.data())) {
+        if (!cmsPipelineInsertStage(pipeline, cmsAT_END, brightnessStage.data())) {
             qCWarning(KWIN_CORE) << "Failed to insert the color brightness pipeline stage";
         }
     }
-}
+    transformation = QSharedPointer<ColorTransformation>::create(pipeline);
-
-void ColorDevicePrivate::unlinkPipeline()
-{
-    while (true) {
-        cmsStage *last = nullptr;
-        cmsPipelineUnlinkStage(pipeline.data(), cmsAT_END, &last);
-
-        if (!last) {
-            break;
-        }
-    }
 }
 
 static qreal interpolate(qreal a, qreal b, qreal blendFactor)
@@ -277,9 +255,6 @@ ColorDevice::ColorDevice(Output *output, QObject *parent)
 
 ColorDevice::~ColorDevice()
 {
-    if (d->pipeline) {
-        d->unlinkPipeline();
-    }
 }
 
 Output *ColorDevice::output() const
@@ -346,28 +321,7 @@ void ColorDevice::setProfile(const QString &profile)
 void ColorDevice::update()
 {
     d->rebuildPipeline();
-
+    d->output->setColorTransformation(d->transformation);
-    GammaRamp gammaRamp(d->output->gammaRampSize());
-    uint16_t *redChannel = gammaRamp.red();
-    uint16_t *greenChannel = gammaRamp.green();
-    uint16_t *blueChannel = gammaRamp.blue();
-
-    for (uint32_t i = 0; i < gammaRamp.size(); ++i) {
-        // ensure 64 bit calculation to prevent overflows
-        const uint16_t index = (static_cast<uint64_t>(i) * 0xffff) / (gammaRamp.size() - 1);
-
-        const uint16_t in[3] = {index, index, index};
-        uint16_t out[3] = {0};
-        cmsPipelineEval16(in, out, d->pipeline.data());
-
-        redChannel[i] = out[0];
-        greenChannel[i] = out[1];
-        blueChannel[i] = out[2];
-    }
-
-    if (!d->output->setGammaRamp(gammaRamp)) {
-        qCWarning(KWIN_CORE) << "Failed to update gamma ramp for output" << d->output;
-    }
 }
 
 void ColorDevice::scheduleUpdate()

src/colors.cpp

@@ -0,0 +1,73 @@
+/*
+    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 "colors.h"
+
+#include <lcms2.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)
+{
+    m_data.fill(0, 3 * size);
+    for (uint64_t i = 0; i < size; i++) {
+        const uint16_t index = (i * 0xFFFF) / size;
+        std::tie(m_data[i], m_data[size + i], m_data[size * 2 + i]) = transformation->transform(index, index, index);
+    }
+}
+
+uint16_t *ColorLUT::red() const
+{
+    return const_cast<uint16_t *>(m_data.constData());
+}
+
+uint16_t *ColorLUT::green() const
+{
+    return const_cast<uint16_t *>(m_data.constData() + size());
+}
+
+uint16_t *ColorLUT::blue() const
+{
+    return const_cast<uint16_t *>(m_data.constData() + 2 * size());
+}
+
+size_t ColorLUT::size() const
+{
+    return m_data.size() / 3;
+}
+
+QSharedPointer<ColorTransformation> ColorLUT::transformation() const
+{
+    return m_transformation;
+}
+
+}

src/colors.h

@@ -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
+*/
+#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 KWIN_EXPORT ColorLUT
+{
+public:
+    ColorLUT(const QSharedPointer<ColorTransformation> &transformation, size_t size);
+
+    uint16_t *red() const;
+    uint16_t *green() const;
+    uint16_t *blue() const;
+    size_t size() const;
+    QSharedPointer<ColorTransformation> transformation() const;
+
+private:
+    QVector<uint16_t> m_data;
+    const QSharedPointer<ColorTransformation> m_transformation;
+};
+
+}

src/output.cpp

@@ -17,47 +17,6 @@
 namespace KWin
 {
 
-GammaRamp::GammaRamp(uint32_t size)
-    : m_table(3 * size)
-    , m_size(size)
-{
-}
-
-uint32_t GammaRamp::size() const
-{
-    return m_size;
-}
-
-uint16_t *GammaRamp::red()
-{
-    return m_table.data();
-}
-
-const uint16_t *GammaRamp::red() const
-{
-    return m_table.data();
-}
-
-uint16_t *GammaRamp::green()
-{
-    return m_table.data() + m_size;
-}
-
-const uint16_t *GammaRamp::green() const
-{
-    return m_table.data() + m_size;
-}
-
-uint16_t *GammaRamp::blue()
-{
-    return m_table.data() + 2 * m_size;
-}
-
-const uint16_t *GammaRamp::blue() const
-{
-    return m_table.data() + 2 * m_size;
-}
-
 QDebug operator<<(QDebug debug, const Output *output)
 {
     QDebugStateSaver saver(debug);
@@ -128,17 +87,6 @@ bool Output::isInternal() const
     return m_internal;
 }
 
-int Output::gammaRampSize() const
-{
-    return 0;
-}
-
-bool Output::setGammaRamp(const GammaRamp &gamma)
-{
-    Q_UNUSED(gamma);
-    return false;
-}
-
 void Output::inhibitDirectScanout()
 {
     m_directScanoutCount++;
@@ -477,4 +425,9 @@ void Output::setPhysicalSizeInternal(const QSize &size)
     m_physicalSize = size;
 }
 
+void Output::setColorTransformation(const QSharedPointer<ColorTransformation> &transformation)
+{
+    Q_UNUSED(transformation);
+}
+
 } // namespace KWin

src/output.h

@@ -27,60 +27,7 @@ namespace KWin
 class EffectScreenImpl;
 class RenderLoop;
 class OutputConfiguration;
-
+class ColorTransformation;
-class KWIN_EXPORT GammaRamp
-{
-public:
-    GammaRamp(uint32_t size);
-
-    /**
-     * Returns the size of the gamma ramp.
-     */
-    uint32_t size() const;
-
-    /**
-     * Returns pointer to the first red component in the gamma ramp.
-     *
-     * The returned pointer can be used for altering the red component
-     * in the gamma ramp.
-     */
-    uint16_t *red();
-
-    /**
-     * Returns pointer to the first red component in the gamma ramp.
-     */
-    const uint16_t *red() const;
-
-    /**
-     * Returns pointer to the first green component in the gamma ramp.
-     *
-     * The returned pointer can be used for altering the green component
-     * in the gamma ramp.
-     */
-    uint16_t *green();
-
-    /**
-     * Returns pointer to the first green component in the gamma ramp.
-     */
-    const uint16_t *green() const;
-
-    /**
-     * Returns pointer to the first blue component in the gamma ramp.
-     *
-     * The returned pointer can be used for altering the blue component
-     * in the gamma ramp.
-     */
-    uint16_t *blue();
-
-    /**
-     * Returns pointer to the first blue component in the gamma ramp.
-     */
-    const uint16_t *blue() const;
-
-private:
-    QVector<uint16_t> m_table;
-    uint32_t m_size;
-};
 
 /**
  * Generic output representation.
@@ -207,20 +154,6 @@ public:
      */
     QSize physicalSize() const;
 
-    /**
-     * Returns the size of the gamma lookup table.
-     *
-     * Default implementation returns 0.
-     */
-    virtual int gammaRampSize() const;
-
-    /**
-     * Sets the gamma ramp of this output.
-     *
-     * Returns @c true if the gamma ramp was successfully set.
-     */
-    virtual bool setGammaRamp(const GammaRamp &gamma);
-
     /** Returns the resolution of the output.  */
     QSize pixelSize() const;
     QSize modeSize() const;
@@ -303,6 +236,8 @@ public:
 
     bool isPlaceholder() const;
 
+    virtual void setColorTransformation(const QSharedPointer<ColorTransformation> &transformation);
+
 Q_SIGNALS:
     /**
      * This signal is emitted when the geometry of this output has changed.