From f3aaede3823c9962ea53d9ba7d323f00c37ea967 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Sat, 25 Nov 2023 19:19:23 +0100 Subject: [PATCH] backends/drm: properly handle neither CTM and gamma being supported Instead of hardcoding only NVidia, try to use CTM and GAMMA_LUT before falling back to the shader path. This way it also works on other GPUs that lack color management hardware, and only falls back to the color management path on older NVidia drivers. This commit also ensures that the color management hardware is set properly after toggling color management on and off again, and simplifies ColorDevice to only deal with rgb factors instead of always calculating luts. This should improve performance of night color animations on hardware where CTMs are supported CCBUG: 453701 --- src/backends/drm/drm_gpu.cpp | 6 - src/backends/drm/drm_gpu.h | 2 - src/backends/drm/drm_output.cpp | 69 +++---- src/backends/drm/drm_output.h | 3 +- src/backends/drm/drm_pipeline.cpp | 14 ++ src/backends/drm/drm_pipeline.h | 2 + .../x11/standalone/x11_standalone_output.cpp | 9 +- .../x11/standalone/x11_standalone_output.h | 2 +- src/colors/colordevice.cpp | 168 +++--------------- src/core/colortransformation.cpp | 27 +++ src/core/colortransformation.h | 2 + 11 files changed, 115 insertions(+), 189 deletions(-) diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp index b569f2ecd5..bc6a3d8818 100644 --- a/src/backends/drm/drm_gpu.cpp +++ b/src/backends/drm/drm_gpu.cpp @@ -78,7 +78,6 @@ DrmGpu::DrmGpu(DrmBackend *backend, const QString &devNode, int fd, dev_t device // find out what driver this kms device is using DrmUniquePtr version(drmGetVersion(fd)); - m_isNVidia = strstr(version->name, "nvidia-drm"); m_isI915 = strstr(version->name, "i915"); m_isVirtualMachine = strstr(version->name, "virtio") || strstr(version->name, "qxl") || strstr(version->name, "vmwgfx") || strstr(version->name, "vboxvideo"); @@ -691,11 +690,6 @@ bool DrmGpu::asyncPageflipSupported() const return m_asyncPageflipSupported; } -bool DrmGpu::isNVidia() const -{ - return m_isNVidia; -} - bool DrmGpu::isI915() const { return m_isI915; diff --git a/src/backends/drm/drm_gpu.h b/src/backends/drm/drm_gpu.h index 0204a49f64..8a83375479 100644 --- a/src/backends/drm/drm_gpu.h +++ b/src/backends/drm/drm_gpu.h @@ -24,7 +24,6 @@ struct gbm_device; namespace KWin { - class DrmOutput; class DrmObject; class DrmCrtc; @@ -78,7 +77,6 @@ public: bool atomicModeSetting() const; bool addFB2ModifiersSupported() const; bool asyncPageflipSupported() const; - bool isNVidia() const; bool isI915() const; gbm_device *gbmDevice() const; EglDisplay *eglDisplay() const; diff --git a/src/backends/drm/drm_output.cpp b/src/backends/drm/drm_output.cpp index 8a982051de..a3725276df 100644 --- a/src/backends/drm/drm_output.cpp +++ b/src/backends/drm/drm_output.cpp @@ -386,6 +386,11 @@ void DrmOutput::applyQueuedChanges(const std::shared_ptr &props m_renderLoop->setRefreshRate(refreshRate()); m_renderLoop->scheduleRepaint(); + if (!next.wideColorGamut && !next.highDynamicRange && !m_pipeline->iccProfile()) { + // re-set the CTM and/or gamma lut + doSetChannelFactors(m_channelFactors); + } + Q_EMIT changed(); } @@ -404,50 +409,54 @@ DrmOutputLayer *DrmOutput::cursorLayer() const return m_pipeline->cursorLayer(); } -bool DrmOutput::setGammaRamp(const std::shared_ptr &transformation) -{ - if (!m_pipeline->activePending() || needsColormanagement()) { - return false; - } - m_pipeline->setGammaRamp(transformation); - m_pipeline->setCTM(QMatrix3x3()); - if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test) == DrmPipeline::Error::None) { - m_pipeline->applyPendingChanges(); - m_renderLoop->scheduleRepaint(); - return true; - } else { - m_pipeline->revertPendingChanges(); - return false; - } -} - bool DrmOutput::setChannelFactors(const QVector3D &rgb) { - if (m_channelFactors == rgb) { + return m_channelFactors == rgb || doSetChannelFactors(rgb); +} + +bool DrmOutput::doSetChannelFactors(const QVector3D &rgb) +{ + m_renderLoop->scheduleRepaint(); + m_channelFactors = rgb; + if (m_state.wideColorGamut || m_state.highDynamicRange || m_state.iccProfile) { + // the shader "fallback" is always active return true; } - m_channelFactors = rgb; - if (!needsColormanagement()) { - if (!m_pipeline->activePending()) { - return false; - } + if (!m_pipeline->activePending()) { + return false; + } + if (m_pipeline->hasCTM()) { QMatrix3x3 ctm; ctm(0, 0) = rgb.x(); ctm(1, 1) = rgb.y(); ctm(2, 2) = rgb.z(); m_pipeline->setCTM(ctm); + m_pipeline->setGammaRamp(nullptr); if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test) == DrmPipeline::Error::None) { m_pipeline->applyPendingChanges(); - m_renderLoop->scheduleRepaint(); + m_channelFactorsNeedShaderFallback = false; return true; } else { - m_pipeline->revertPendingChanges(); - return false; + m_pipeline->setCTM(QMatrix3x3()); + m_pipeline->applyPendingChanges(); } - } else { - m_renderLoop->scheduleRepaint(); - return true; } + if (m_pipeline->hasGammaRamp()) { + auto lut = ColorTransformation::createScalingTransform(rgb); + if (lut) { + m_pipeline->setGammaRamp(std::move(lut)); + if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test) == DrmPipeline::Error::None) { + m_pipeline->applyPendingChanges(); + m_channelFactorsNeedShaderFallback = false; + return true; + } else { + m_pipeline->setGammaRamp(nullptr); + m_pipeline->applyPendingChanges(); + } + } + } + m_channelFactorsNeedShaderFallback = m_channelFactors != QVector3D{1, 1, 1}; + return true; } QVector3D DrmOutput::channelFactors() const @@ -457,7 +466,7 @@ QVector3D DrmOutput::channelFactors() const bool DrmOutput::needsColormanagement() const { - return m_state.wideColorGamut || m_state.highDynamicRange || m_state.iccProfile || m_gpu->isNVidia(); + return m_state.wideColorGamut || m_state.highDynamicRange || m_state.iccProfile || m_channelFactorsNeedShaderFallback; } } diff --git a/src/backends/drm/drm_output.h b/src/backends/drm/drm_output.h index 6aad737948..e5347f4f4b 100644 --- a/src/backends/drm/drm_output.h +++ b/src/backends/drm/drm_output.h @@ -58,7 +58,6 @@ public: void leased(DrmLease *lease); void leaseEnded(); - bool setGammaRamp(const std::shared_ptr &transformation) override; bool setChannelFactors(const QVector3D &rgb) override; QVector3D channelFactors() const; bool needsColormanagement() const; @@ -66,6 +65,7 @@ public: private: bool setDrmDpmsMode(DpmsMode mode); void setDpmsMode(DpmsMode mode) override; + bool doSetChannelFactors(const QVector3D &rgb); QList> getModes() const; @@ -76,6 +76,7 @@ private: DrmLease *m_lease = nullptr; QVector3D m_channelFactors = {1, 1, 1}; + bool m_channelFactorsNeedShaderFallback = false; }; } diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp index b891bed406..9fffa3c6c3 100644 --- a/src/backends/drm/drm_pipeline.cpp +++ b/src/backends/drm/drm_pipeline.cpp @@ -474,6 +474,20 @@ QMap> DrmPipeline::cursorFormats() const } } +bool DrmPipeline::hasCTM() const +{ + return m_pending.crtc && m_pending.crtc->ctm.isValid(); +} + +bool DrmPipeline::hasGammaRamp() const +{ + if (gpu()->atomicModeSetting()) { + return m_pending.crtc && m_pending.crtc->gammaLut.isValid(); + } else { + return m_pending.crtc && m_pending.crtc->gammaRampSize() > 0; + } +} + bool DrmPipeline::pruneModifier() { const DmaBufAttributes *dmabufAttributes = m_primaryLayer->currentBuffer() ? m_primaryLayer->currentBuffer()->buffer()->dmabufAttributes() : nullptr; diff --git a/src/backends/drm/drm_pipeline.h b/src/backends/drm/drm_pipeline.h index 34326a8dc7..65a284110d 100644 --- a/src/backends/drm/drm_pipeline.h +++ b/src/backends/drm/drm_pipeline.h @@ -95,6 +95,8 @@ public: QMap> formats() const; QMap> cursorFormats() const; + bool hasCTM() const; + bool hasGammaRamp() const; bool pruneModifier(); void setOutput(DrmOutput *output); diff --git a/src/backends/x11/standalone/x11_standalone_output.cpp b/src/backends/x11/standalone/x11_standalone_output.cpp index 4b8f26e16d..862a257711 100644 --- a/src/backends/x11/standalone/x11_standalone_output.cpp +++ b/src/backends/x11/standalone/x11_standalone_output.cpp @@ -8,6 +8,7 @@ */ #include "x11_standalone_output.h" #include "core/colorlut.h" +#include "core/colortransformation.h" #include "main.h" #include "x11_standalone_backend.h" @@ -40,12 +41,16 @@ void X11Output::setXineramaNumber(int number) m_xineramaNumber = number; } -bool X11Output::setGammaRamp(const std::shared_ptr &transformation) +bool X11Output::setChannelFactors(const QVector3D &rgb) { if (m_crtc == XCB_NONE) { return true; } - ColorLUT lut(transformation, m_gammaRampSize); + auto transformation = ColorTransformation::createScalingTransform(rgb); + if (!transformation) { + return false; + } + ColorLUT lut(std::move(transformation), m_gammaRampSize); xcb_randr_set_crtc_gamma(kwinApp()->x11Connection(), m_crtc, lut.size(), lut.red(), lut.green(), lut.blue()); return true; } diff --git a/src/backends/x11/standalone/x11_standalone_output.h b/src/backends/x11/standalone/x11_standalone_output.h index e702bd2b19..5628502c78 100644 --- a/src/backends/x11/standalone/x11_standalone_output.h +++ b/src/backends/x11/standalone/x11_standalone_output.h @@ -39,7 +39,7 @@ public: int xineramaNumber() const; void setXineramaNumber(int number); - bool setGammaRamp(const std::shared_ptr &transformation) override; + bool setChannelFactors(const QVector3D &rgb) override; private: void setCrtc(xcb_randr_crtc_t crtc); diff --git a/src/colors/colordevice.cpp b/src/colors/colordevice.cpp index 77d66c506f..a5c9835729 100644 --- a/src/colors/colordevice.cpp +++ b/src/colors/colordevice.cpp @@ -19,40 +19,17 @@ namespace KWin { -struct CmsDeleter -{ - void operator()(cmsToneCurve *toneCurve) - { - if (toneCurve) { - cmsFreeToneCurve(toneCurve); - } - } -}; -using UniqueToneCurvePtr = std::unique_ptr; - class ColorDevicePrivate { public: - enum DirtyToneCurveBit { - DirtyTemperatureToneCurve = 0x1, - DirtyBrightnessToneCurve = 0x2, - }; - Q_DECLARE_FLAGS(DirtyToneCurves, DirtyToneCurveBit) - - void rebuildPipeline(); - - void updateTemperatureToneCurves(); - void updateBrightnessToneCurves(); + void recalculateFactors(); Output *output; - DirtyToneCurves dirtyCurves; QTimer *updateTimer; uint brightness = 100; uint temperature = 6500; - std::unique_ptr temperatureStage; QVector3D temperatureFactors = QVector3D(1, 1, 1); - std::unique_ptr brightnessStage; QVector3D brightnessFactors = QVector3D(1, 1, 1); std::shared_ptr transformation; @@ -60,133 +37,34 @@ public: QVector3D simpleTransformation = QVector3D(1, 1, 1); }; -void ColorDevicePrivate::rebuildPipeline() -{ - if (dirtyCurves & DirtyBrightnessToneCurve) { - updateBrightnessToneCurves(); - } - if (dirtyCurves & DirtyTemperatureToneCurve) { - updateTemperatureToneCurves(); - } - dirtyCurves = DirtyToneCurves(); - - std::vector> stages; - if (brightnessStage) { - if (auto s = brightnessStage->dup()) { - stages.push_back(std::move(s)); - } else { - return; - } - } - if (temperatureStage) { - if (auto s = temperatureStage->dup()) { - stages.push_back(std::move(s)); - } else { - return; - } - } - - const auto tmp = std::make_shared(std::move(stages)); - if (tmp->valid()) { - transformation = tmp; - simpleTransformation = brightnessFactors * temperatureFactors; - } -} - static qreal interpolate(qreal a, qreal b, qreal blendFactor) { return (1 - blendFactor) * a + blendFactor * b; } -void ColorDevicePrivate::updateTemperatureToneCurves() +void ColorDevicePrivate::recalculateFactors() { - temperatureStage.reset(); - - if (temperature == 6500) { - return; - } - - // Note that cmsWhitePointFromTemp() returns a slightly green-ish white point. - const int blackBodyColorIndex = ((temperature - 1000) / 100) * 3; - const qreal blendFactor = (temperature % 100) / 100.0; - - const qreal xWhitePoint = interpolate(blackbodyColor[blackBodyColorIndex + 0], - blackbodyColor[blackBodyColorIndex + 3], - blendFactor); - const qreal yWhitePoint = interpolate(blackbodyColor[blackBodyColorIndex + 1], - blackbodyColor[blackBodyColorIndex + 4], - blendFactor); - const qreal zWhitePoint = interpolate(blackbodyColor[blackBodyColorIndex + 2], - blackbodyColor[blackBodyColorIndex + 5], - blendFactor); - - temperatureFactors = QVector3D(xWhitePoint, yWhitePoint, zWhitePoint); - - const double redCurveParams[] = {1.0, xWhitePoint, 0.0}; - const double greenCurveParams[] = {1.0, yWhitePoint, 0.0}; - const double blueCurveParams[] = {1.0, zWhitePoint, 0.0}; - - UniqueToneCurvePtr redCurve(cmsBuildParametricToneCurve(nullptr, 2, redCurveParams)); - if (!redCurve) { - qCWarning(KWIN_CORE) << "Failed to build the temperature tone curve for the red channel"; - return; - } - UniqueToneCurvePtr greenCurve(cmsBuildParametricToneCurve(nullptr, 2, greenCurveParams)); - if (!greenCurve) { - qCWarning(KWIN_CORE) << "Failed to build the temperature tone curve for the green channel"; - return; - } - UniqueToneCurvePtr blueCurve(cmsBuildParametricToneCurve(nullptr, 2, blueCurveParams)); - if (!blueCurve) { - qCWarning(KWIN_CORE) << "Failed to build the temperature tone curve for the blue channel"; - return; - } - - // The ownership of the tone curves will be moved to the pipeline stage. - cmsToneCurve *toneCurves[] = {redCurve.release(), greenCurve.release(), blueCurve.release()}; - - temperatureStage = std::make_unique(cmsStageAllocToneCurves(nullptr, 3, toneCurves)); - if (!temperatureStage) { - qCWarning(KWIN_CORE) << "Failed to create the color temperature pipeline stage"; - } -} - -void ColorDevicePrivate::updateBrightnessToneCurves() -{ - brightnessStage.reset(); - - if (brightness == 100) { - return; - } - - const double curveParams[] = {1.0, brightness / 100.0, 0.0}; brightnessFactors = QVector3D(brightness / 100.0, brightness / 100.0, brightness / 100.0); - UniqueToneCurvePtr redCurve(cmsBuildParametricToneCurve(nullptr, 2, curveParams)); - if (!redCurve) { - qCWarning(KWIN_CORE) << "Failed to build the brightness tone curve for the red channel"; - return; - } + if (temperature == 6500) { + temperatureFactors = QVector3D(1, 1, 1); + } else { + // Note that cmsWhitePointFromTemp() returns a slightly green-ish white point. + const int blackBodyColorIndex = ((temperature - 1000) / 100) * 3; + const qreal blendFactor = (temperature % 100) / 100.0; - UniqueToneCurvePtr greenCurve(cmsBuildParametricToneCurve(nullptr, 2, curveParams)); - if (!greenCurve) { - qCWarning(KWIN_CORE) << "Failed to build the brightness tone curve for the green channel"; - return; - } - - UniqueToneCurvePtr blueCurve(cmsBuildParametricToneCurve(nullptr, 2, curveParams)); - if (!blueCurve) { - qCWarning(KWIN_CORE) << "Failed to build the brightness tone curve for the blue channel"; - return; - } - - // The ownership of the tone curves will be moved to the pipeline stage. - cmsToneCurve *toneCurves[] = {redCurve.release(), greenCurve.release(), blueCurve.release()}; - - brightnessStage = std::make_unique(cmsStageAllocToneCurves(nullptr, 3, toneCurves)); - if (!brightnessStage) { - qCWarning(KWIN_CORE) << "Failed to create the color brightness pipeline stage"; + const qreal xWhitePoint = interpolate(blackbodyColor[blackBodyColorIndex + 0], + blackbodyColor[blackBodyColorIndex + 3], + blendFactor); + const qreal yWhitePoint = interpolate(blackbodyColor[blackBodyColorIndex + 1], + blackbodyColor[blackBodyColorIndex + 4], + blendFactor); + const qreal zWhitePoint = interpolate(blackbodyColor[blackBodyColorIndex + 2], + blackbodyColor[blackBodyColorIndex + 5], + blendFactor); + temperatureFactors = QVector3D(xWhitePoint, yWhitePoint, zWhitePoint); } + simpleTransformation = brightnessFactors * temperatureFactors; } ColorDevice::ColorDevice(Output *output, QObject *parent) @@ -230,7 +108,6 @@ void ColorDevice::setBrightness(uint brightness) return; } d->brightness = brightness; - d->dirtyCurves |= ColorDevicePrivate::DirtyBrightnessToneCurve; scheduleUpdate(); Q_EMIT brightnessChanged(); } @@ -250,17 +127,14 @@ void ColorDevice::setTemperature(uint temperature) return; } d->temperature = temperature; - d->dirtyCurves |= ColorDevicePrivate::DirtyTemperatureToneCurve; scheduleUpdate(); Q_EMIT temperatureChanged(); } void ColorDevice::update() { - d->rebuildPipeline(); - if (!d->output->setGammaRamp(d->transformation)) { - d->output->setChannelFactors(d->simpleTransformation); - } + d->recalculateFactors(); + d->output->setChannelFactors(d->simpleTransformation); } void ColorDevice::scheduleUpdate() diff --git a/src/core/colortransformation.cpp b/src/core/colortransformation.cpp index 173e708361..c6d063e05b 100644 --- a/src/core/colortransformation.cpp +++ b/src/core/colortransformation.cpp @@ -77,4 +77,31 @@ QVector3D ColorTransformation::transform(QVector3D in) const cmsPipelineEvalFloat(&in[0], &ret[0], m_pipeline); return ret; } + +std::unique_ptr ColorTransformation::createScalingTransform(const QVector3D &scale) +{ + std::array curveParams = {1.0, scale.x(), 0.0}; + auto r = cmsBuildParametricToneCurve(nullptr, 2, curveParams.data()); + curveParams = {1.0, scale.y(), 0.0}; + auto g = cmsBuildParametricToneCurve(nullptr, 2, curveParams.data()); + curveParams = {1.0, scale.z(), 0.0}; + auto b = cmsBuildParametricToneCurve(nullptr, 2, curveParams.data()); + if (!r || !g || !b) { + qCWarning(KWIN_CORE) << "Failed to build tone curves"; + return nullptr; + } + const std::array curves = {r, g, b}; + const auto stage = cmsStageAllocToneCurves(nullptr, 3, curves.data()); + if (!stage) { + qCWarning(KWIN_CORE) << "Failed to allocate tone curves"; + return nullptr; + } + std::vector> stages; + stages.push_back(std::make_unique(stage)); + auto transform = std::make_unique(std::move(stages)); + if (!transform->valid()) { + return nullptr; + } + return transform; +} } diff --git a/src/core/colortransformation.h b/src/core/colortransformation.h index 31d986134e..f63a6f60f1 100644 --- a/src/core/colortransformation.h +++ b/src/core/colortransformation.h @@ -36,6 +36,8 @@ public: std::tuple transform(uint16_t r, uint16_t g, uint16_t b) const; QVector3D transform(QVector3D in) const; + static std::unique_ptr createScalingTransform(const QVector3D &scale); + private: cmsPipeline *const m_pipeline; std::vector> m_stages;