From 0d381846f1cc8f8f3385483e05fbaba369e560d7 Mon Sep 17 00:00:00 2001 From: Vlad Zagorodniy Date: Mon, 17 Jun 2019 12:07:19 +0300 Subject: [PATCH] Backport Night Color feature to X11 Summary: The color correction manager doesn't make any specific assumptions about underlying platform, e.g. whether it's x11, etc. The platform just has to be capable of setting gamma ramps. Given that, there are no any significant technical blockers for making this feature work on x. Reviewers: #kwin, davidedmundson, romangg Reviewed By: #kwin, davidedmundson, romangg Subscribers: romangg, neobrain, GB_2, filipf, davidedmundson, ngraham, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D21345 --- abstract_output.cpp | 47 ++++++++++++-- abstract_output.h | 61 +++++++++++++++++-- colorcorrection/gammaramp.h | 53 ---------------- colorcorrection/manager.cpp | 18 +++--- plugins/platforms/drm/drm_object_crtc.cpp | 13 ++-- plugins/platforms/drm/drm_object_crtc.h | 11 ++-- plugins/platforms/drm/drm_output.cpp | 6 +- plugins/platforms/drm/drm_output.h | 4 +- plugins/platforms/virtual/virtual_output.h | 4 +- .../platforms/x11/standalone/x11_output.cpp | 27 ++++++++ plugins/platforms/x11/standalone/x11_output.h | 16 +++++ .../platforms/x11/standalone/x11_platform.cpp | 14 ++++- 12 files changed, 184 insertions(+), 90 deletions(-) delete mode 100644 colorcorrection/gammaramp.h diff --git a/abstract_output.cpp b/abstract_output.cpp index 1536697c90..94d828285e 100644 --- a/abstract_output.cpp +++ b/abstract_output.cpp @@ -17,16 +17,53 @@ GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see . *********************************************************************/ + #include "abstract_output.h" -// KF5 -#include - -#include - 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; +} + AbstractOutput::AbstractOutput(QObject *parent) : QObject(parent) { diff --git a/abstract_output.h b/abstract_output.h index 63ae4ba1dd..151febc7ef 100644 --- a/abstract_output.h +++ b/abstract_output.h @@ -25,13 +25,64 @@ along with this program. If not, see . #include #include #include +#include namespace KWin { -namespace ColorCorrect { -struct GammaRamp; -} +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 m_table; + uint32_t m_size; +}; /** * Generic output representation in a Wayland session @@ -64,10 +115,10 @@ public: return Qt::PrimaryOrientation; } - virtual int getGammaRampSize() const { + virtual int gammaRampSize() const { return 0; } - virtual bool setGammaRamp(const ColorCorrect::GammaRamp &gamma) { + virtual bool setGammaRamp(const GammaRamp &gamma) { Q_UNUSED(gamma); return false; } diff --git a/colorcorrection/gammaramp.h b/colorcorrection/gammaramp.h deleted file mode 100644 index b215af168a..0000000000 --- a/colorcorrection/gammaramp.h +++ /dev/null @@ -1,53 +0,0 @@ -/******************************************************************** - KWin - the KDE window manager - This file is part of the KDE project. - -Copyright 2017 Roman Gilg - -This program is free software; you can redistribute it and/or modify -it under the terms of the GNU General Public License as published by -the Free Software Foundation; either version 2 of the License, or -(at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. - -You should have received a copy of the GNU General Public License -along with this program. If not, see . -*********************************************************************/ -#ifndef KWIN_GAMMARAMP_H -#define KWIN_GAMMARAMP_H - -namespace KWin -{ - -namespace ColorCorrect -{ - -struct GammaRamp { - GammaRamp(int _size) { - size = _size; - red = new uint16_t[3 * _size]; - green = red + _size; - blue = green + _size; - } - ~GammaRamp() { - delete[] red; - red = green = blue = nullptr; - } - - uint32_t size = 0; - uint16_t *red = nullptr; - uint16_t *green = nullptr; - uint16_t *blue = nullptr; - -private: - Q_DISABLE_COPY(GammaRamp) -}; - -} -} - -#endif // KWIN_GAMMARAMP_H diff --git a/colorcorrection/manager.cpp b/colorcorrection/manager.cpp index 0cd03693ab..1822651910 100644 --- a/colorcorrection/manager.cpp +++ b/colorcorrection/manager.cpp @@ -20,7 +20,6 @@ along with this program. If not, see . #include "manager.h" #include "colorcorrectdbusinterface.h" #include "suncalc.h" -#include "gammaramp.h" #include #include @@ -513,20 +512,23 @@ void Manager::commitGammaRamps(int temperature) const auto outs = kwinApp()->platform()->outputs(); for (auto *o : outs) { - int rampsize = o->getGammaRampSize(); + int rampsize = o->gammaRampSize(); GammaRamp ramp(rampsize); /* * The gamma calculation below is based on the Redshift app: * https://github.com/jonls/redshift */ + uint16_t *red = ramp.red(); + uint16_t *green = ramp.green(); + uint16_t *blue = ramp.blue(); // linear default state for (int i = 0; i < rampsize; i++) { uint16_t value = (double)i / rampsize * (UINT16_MAX + 1); - ramp.red[i] = value; - ramp.green[i] = value; - ramp.blue[i] = value; + red[i] = value; + green[i] = value; + blue[i] = value; } // approximate white point @@ -538,9 +540,9 @@ void Manager::commitGammaRamps(int temperature) whitePoint[2] = (1. - alpha) * blackbodyColor[bbCIndex + 2] + alpha * blackbodyColor[bbCIndex + 5]; for (int i = 0; i < rampsize; i++) { - ramp.red[i] = (double)ramp.red[i] / (UINT16_MAX+1) * whitePoint[0] * (UINT16_MAX+1); - ramp.green[i] = (double)ramp.green[i] / (UINT16_MAX+1) * whitePoint[1] * (UINT16_MAX+1); - ramp.blue[i] = (double)ramp.blue[i] / (UINT16_MAX+1) * whitePoint[2] * (UINT16_MAX+1); + red[i] = qreal(red[i]) / (UINT16_MAX+1) * whitePoint[0] * (UINT16_MAX+1); + green[i] = qreal(green[i]) / (UINT16_MAX+1) * whitePoint[1] * (UINT16_MAX+1); + blue[i] = qreal(blue[i]) / (UINT16_MAX+1) * whitePoint[2] * (UINT16_MAX+1); } if (o->setGammaRamp(ramp)) { diff --git a/plugins/platforms/drm/drm_object_crtc.cpp b/plugins/platforms/drm/drm_object_crtc.cpp index 245734fd0a..34ae31349f 100644 --- a/plugins/platforms/drm/drm_object_crtc.cpp +++ b/plugins/platforms/drm/drm_object_crtc.cpp @@ -23,7 +23,6 @@ along with this program. If not, see . #include "drm_buffer.h" #include "drm_pointer.h" #include "logging.h" -#include namespace KWin { @@ -114,9 +113,15 @@ bool DrmCrtc::blank() return false; } -bool DrmCrtc::setGammaRamp(const ColorCorrect::GammaRamp &gamma) { - bool isError = drmModeCrtcSetGamma(m_backend->fd(), m_id, gamma.size, - gamma.red, gamma.green, gamma.blue); +bool DrmCrtc::setGammaRamp(const GammaRamp &gamma) +{ + uint16_t *red = const_cast(gamma.red()); + uint16_t *green = const_cast(gamma.green()); + uint16_t *blue = const_cast(gamma.blue()); + + const bool isError = drmModeCrtcSetGamma(m_backend->fd(), m_id, + gamma.size(), red, green, blue); + return !isError; } diff --git a/plugins/platforms/drm/drm_object_crtc.h b/plugins/platforms/drm/drm_object_crtc.h index c5f77e0e1f..1f6fe11692 100644 --- a/plugins/platforms/drm/drm_object_crtc.h +++ b/plugins/platforms/drm/drm_object_crtc.h @@ -25,13 +25,10 @@ along with this program. If not, see . namespace KWin { -namespace ColorCorrect { -struct GammaRamp; -} - class DrmBackend; class DrmBuffer; class DrmDumbBuffer; +class GammaRamp; class DrmCrtc : public DrmObject { @@ -47,7 +44,7 @@ public: Active, Count }; - + bool initProps(); int resIndex() const { @@ -67,10 +64,10 @@ public: void flipBuffer(); bool blank(); - int getGammaRampSize() const { + int gammaRampSize() const { return m_gammaRampSize; } - bool setGammaRamp(const ColorCorrect::GammaRamp &gamma); + bool setGammaRamp(const GammaRamp &gamma); private: int m_resIndex; diff --git a/plugins/platforms/drm/drm_output.cpp b/plugins/platforms/drm/drm_output.cpp index 1bce5d3a57..1b72c09921 100644 --- a/plugins/platforms/drm/drm_output.cpp +++ b/plugins/platforms/drm/drm_output.cpp @@ -1201,12 +1201,12 @@ void DrmOutput::automaticRotation() emit screens()->changed(); } -int DrmOutput::getGammaRampSize() const +int DrmOutput::gammaRampSize() const { - return m_crtc->getGammaRampSize(); + return m_crtc->gammaRampSize(); } -bool DrmOutput::setGammaRamp(const ColorCorrect::GammaRamp &gamma) +bool DrmOutput::setGammaRamp(const GammaRamp &gamma) { return m_crtc->setGammaRamp(gamma); } diff --git a/plugins/platforms/drm/drm_output.h b/plugins/platforms/drm/drm_output.h index c2199e677f..c84f169271 100644 --- a/plugins/platforms/drm/drm_output.h +++ b/plugins/platforms/drm/drm_output.h @@ -133,8 +133,8 @@ private: void transform(KWayland::Server::OutputDeviceInterface::Transform transform) override; void automaticRotation(); - int getGammaRampSize() const override; - bool setGammaRamp(const ColorCorrect::GammaRamp &gamma) override; + int gammaRampSize() const override; + bool setGammaRamp(const GammaRamp &gamma) override; QMatrix4x4 matrixDisplay(const QSize &s) const; DrmBackend *m_backend; diff --git a/plugins/platforms/virtual/virtual_output.h b/plugins/platforms/virtual/virtual_output.h index 4569c7dd07..5ddb9beaf6 100644 --- a/plugins/platforms/virtual/virtual_output.h +++ b/plugins/platforms/virtual/virtual_output.h @@ -41,10 +41,10 @@ public: void setGeometry(const QRect &geo); - int getGammaRampSize() const override { + int gammaRampSize() const override { return m_gammaSize; } - bool setGammaRamp(const ColorCorrect::GammaRamp &gamma) override { + bool setGammaRamp(const GammaRamp &gamma) override { Q_UNUSED(gamma); return m_gammaResult; } diff --git a/plugins/platforms/x11/standalone/x11_output.cpp b/plugins/platforms/x11/standalone/x11_output.cpp index 9a1e30a08c..d86aa1bb79 100644 --- a/plugins/platforms/x11/standalone/x11_output.cpp +++ b/plugins/platforms/x11/standalone/x11_output.cpp @@ -61,4 +61,31 @@ void X11Output::setRefreshRate(int set) m_refreshRate = set; } +int X11Output::gammaRampSize() const +{ + return m_gammaRampSize; +} + +bool X11Output::setGammaRamp(const GammaRamp &gamma) +{ + if (m_crtc == XCB_NONE) { + return false; + } + + xcb_randr_set_crtc_gamma(connection(), m_crtc, gamma.size(), gamma.red(), + gamma.green(), gamma.blue()); + + return true; +} + +void X11Output::setCrtc(xcb_randr_crtc_t crtc) +{ + m_crtc = crtc; +} + +void X11Output::setGammaRampSize(int size) +{ + m_gammaRampSize = size; +} + } diff --git a/plugins/platforms/x11/standalone/x11_output.h b/plugins/platforms/x11/standalone/x11_output.h index d5f094058f..944c123aa1 100644 --- a/plugins/platforms/x11/standalone/x11_output.h +++ b/plugins/platforms/x11/standalone/x11_output.h @@ -26,6 +26,8 @@ along with this program. If not, see . #include #include +#include + namespace KWin { @@ -35,6 +37,7 @@ namespace KWin class KWIN_EXPORT X11Output : public AbstractOutput { Q_OBJECT + public: explicit X11Output(QObject *parent = nullptr); virtual ~X11Output() = default; @@ -53,10 +56,23 @@ public: int refreshRate() const override; void setRefreshRate(int set); + /** + * The size of gamma lookup table. + **/ + int gammaRampSize() const override; + bool setGammaRamp(const GammaRamp &gamma) override; + private: + void setCrtc(xcb_randr_crtc_t crtc); + void setGammaRampSize(int size); + + xcb_randr_crtc_t m_crtc = XCB_NONE; QString m_name; QRect m_geometry; + int m_gammaRampSize; int m_refreshRate; + + friend class X11StandalonePlatform; }; } diff --git a/plugins/platforms/x11/standalone/x11_platform.cpp b/plugins/platforms/x11/standalone/x11_platform.cpp index 215af1ca24..2ad2a3f0b5 100644 --- a/plugins/platforms/x11/standalone/x11_platform.cpp +++ b/plugins/platforms/x11/standalone/x11_platform.cpp @@ -81,6 +81,8 @@ X11StandalonePlatform::X11StandalonePlatform(QObject *parent) } } ); + + setSupportsGammaControl(true); } X11StandalonePlatform::~X11StandalonePlatform() @@ -456,6 +458,7 @@ void X11StandalonePlatform::doUpdateOutputs() { auto fallback = [this]() { auto *o = new X11Output(this); + o->setGammaRampSize(0); o->setRefreshRate(-1.0f); o->setName(QStringLiteral("Xinerama")); m_outputs << o; @@ -514,14 +517,23 @@ void X11StandalonePlatform::doUpdateOutputs() const QRect geo = info.rect(); if (geo.isValid()) { + xcb_randr_crtc_t crtc = crtcs[i]; + + // TODO: Perhaps the output has to save the inherited gamma ramp and + // restore it during tear down. Currently neither standalone x11 nor + // drm platform do this. + Xcb::RandR::CrtcGamma gamma(crtc); + auto *o = new X11Output(this); + o->setCrtc(crtc); + o->setGammaRampSize(gamma.isNull() ? 0 : gamma->size); o->setGeometry(geo); o->setRefreshRate(refreshRate); QString name; for (int j = 0; j < info->num_outputs; ++j) { Xcb::RandR::OutputInfo outputInfo(outputInfos.at(j)); - if (crtcs[i] == outputInfo->crtc) { + if (crtc == outputInfo->crtc) { name = outputInfo.name(); break; }