From a8d2fe5c2e8af722f5c37254307bca4564a69251 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Mon, 8 Jan 2024 18:11:13 +0100 Subject: [PATCH] core: refactor colorimetry a bit and add a constructor That way the vectors can never be uninitialized, and the to/fromXYZ matrices can be cached --- src/backends/drm/drm_pipeline.cpp | 12 +-- src/backends/drm/icc_shader.cpp | 2 +- src/core/colorspace.cpp | 133 +++++++++++++++--------- src/core/colorspace.h | 32 ++++-- src/core/iccprofile.cpp | 6 +- src/opengl/glshader.cpp | 2 +- src/utils/edid.cpp | 21 ++-- src/utils/edid.h | 4 +- src/wayland/frog_colormanagement_v1.cpp | 8 +- 9 files changed, 132 insertions(+), 88 deletions(-) diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp index 5735134d74..1b74efd55b 100644 --- a/src/backends/drm/drm_pipeline.cpp +++ b/src/backends/drm/drm_pipeline.cpp @@ -318,7 +318,7 @@ bool DrmPipeline::prepareAtomicModeset(DrmAtomicCommit *commit) } else if (m_pending.colorDescription.transferFunction() != NamedTransferFunction::gamma22) { return false; } - if (m_pending.colorDescription.colorimetry().name == NamedColorimetry::BT2020) { + if (m_pending.colorDescription.colorimetry() == NamedColorimetry::BT2020) { if (!m_connector->colorspace.isValid() || !m_connector->colorspace.hasEnum(DrmConnector::Colorspace::BT2020_RGB)) { return false; } @@ -736,7 +736,7 @@ std::shared_ptr DrmPipeline::createHdrMetadata(NamedTransferFunction tr if (!m_connector->edid()->supportsPQ()) { return nullptr; } - const auto colorimetry = m_connector->edid()->colorimetry(); + const auto colorimetry = m_connector->edid()->colorimetry().value_or(Colorimetry::fromName(NamedColorimetry::BT709)); const auto to16Bit = [](float value) { return uint16_t(std::round(value / 0.00002)); }; @@ -754,11 +754,11 @@ std::shared_ptr DrmPipeline::createHdrMetadata(NamedTransferFunction tr .metadata_type = 0, // in 0.00002 nits .display_primaries = { - {to16Bit(colorimetry.red.x()), to16Bit(colorimetry.red.y())}, - {to16Bit(colorimetry.green.x()), to16Bit(colorimetry.green.y())}, - {to16Bit(colorimetry.blue.x()), to16Bit(colorimetry.blue.y())}, + {to16Bit(colorimetry.red().x()), to16Bit(colorimetry.red().y())}, + {to16Bit(colorimetry.green().x()), to16Bit(colorimetry.green().y())}, + {to16Bit(colorimetry.blue().x()), to16Bit(colorimetry.blue().y())}, }, - .white_point = {to16Bit(colorimetry.white.x()), to16Bit(colorimetry.white.y())}, + .white_point = {to16Bit(colorimetry.white().x()), to16Bit(colorimetry.white().y())}, // in nits .max_display_mastering_luminance = uint16_t(std::round(m_connector->edid()->desiredMaxFrameAverageLuminance().value_or(0))), // in 0.0001 nits diff --git a/src/backends/drm/icc_shader.cpp b/src/backends/drm/icc_shader.cpp index b33b533247..e8b00070fc 100644 --- a/src/backends/drm/icc_shader.cpp +++ b/src/backends/drm/icc_shader.cpp @@ -63,7 +63,7 @@ bool IccShader::setProfile(const std::shared_ptr &profile) std::unique_ptr C; std::unique_ptr A; if (const IccProfile::BToATagData *tag = profile->BtToATag()) { - matrix1 = Colorimetry::chromaticAdaptationMatrix(profile->colorimetry().white, D50) * profile->colorimetry().toXYZ(); + matrix1 = Colorimetry::chromaticAdaptationMatrix(profile->colorimetry().white(), D50) * profile->colorimetry().toXYZ(); if (tag->B) { const auto sample = [&tag](size_t x) { const float relativeX = x / double(lutSize - 1); diff --git a/src/core/colorspace.cpp b/src/core/colorspace.cpp index e0be6f0f33..f913771ca7 100644 --- a/src/core/colorspace.cpp +++ b/src/core/colorspace.cpp @@ -60,16 +60,6 @@ QVector2D Colorimetry::xyzToXY(QVector3D xyz) return QVector2D(xyz.x() / (xyz.x() + xyz.y() + xyz.z()), xyz.y() / (xyz.x() + xyz.y() + xyz.z())); } -QMatrix3x3 Colorimetry::toXYZ() const -{ - const auto r_xyz = xyToXYZ(red); - const auto g_xyz = xyToXYZ(green); - const auto b_xyz = xyToXYZ(blue); - const auto w_xyz = xyToXYZ(white); - const auto component_scale = inverse(matrixFromColumns(r_xyz, g_xyz, b_xyz)) * w_xyz; - return matrixFromColumns(r_xyz * component_scale.x(), g_xyz * component_scale.y(), b_xyz * component_scale.z()); -} - QMatrix3x3 Colorimetry::chromaticAdaptationMatrix(QVector2D sourceWhitepoint, QVector2D destinationWhitepoint) { static const QMatrix3x3 bradford = []() { @@ -98,6 +88,9 @@ QMatrix3x3 Colorimetry::chromaticAdaptationMatrix(QVector2D sourceWhitepoint, QV ret(2, 2) = 0.9684867; return ret; }(); + if (sourceWhitepoint == destinationWhitepoint) { + return QMatrix3x3{}; + } const QVector3D factors = (bradford * xyToXYZ(destinationWhitepoint)) / (bradford * xyToXYZ(sourceWhitepoint)); QMatrix3x3 adaptation{}; adaptation(0, 0) = factors.x(); @@ -106,51 +99,104 @@ QMatrix3x3 Colorimetry::chromaticAdaptationMatrix(QVector2D sourceWhitepoint, QV return inverseBradford * adaptation * bradford; } +QMatrix3x3 Colorimetry::calculateToXYZMatrix(QVector3D red, QVector3D green, QVector3D blue, QVector3D white) +{ + const auto component_scale = inverse(matrixFromColumns(red, green, blue)) * white; + return matrixFromColumns(red * component_scale.x(), green * component_scale.y(), blue * component_scale.z()); +} + +Colorimetry::Colorimetry(QVector2D red, QVector2D green, QVector2D blue, QVector2D white) + : m_red(red) + , m_green(green) + , m_blue(blue) + , m_white(white) + , m_toXYZ(calculateToXYZMatrix(xyToXYZ(red), xyToXYZ(green), xyToXYZ(blue), xyToXYZ(white))) + , m_fromXYZ(inverse(m_toXYZ)) +{ +} + +Colorimetry::Colorimetry(QVector3D red, QVector3D green, QVector3D blue, QVector3D white) + : m_red(xyzToXY(red)) + , m_green(xyzToXY(green)) + , m_blue(xyzToXY(blue)) + , m_white(xyzToXY(white)) + , m_toXYZ(calculateToXYZMatrix(red, green, blue, white)) + , m_fromXYZ(inverse(m_toXYZ)) +{ +} + +const QMatrix3x3 &Colorimetry::toXYZ() const +{ + return m_toXYZ; +} + +const QMatrix3x3 &Colorimetry::fromXYZ() const +{ + return m_fromXYZ; +} + QMatrix3x3 Colorimetry::toOther(const Colorimetry &other) const { // rendering intent is relative colorimetric, so adapt to the different whitepoint - if (white == other.white) { - return inverse(other.toXYZ()) * toXYZ(); - } else { - return inverse(other.toXYZ()) * chromaticAdaptationMatrix(this->white, other.white) * toXYZ(); - } + return other.fromXYZ() * chromaticAdaptationMatrix(this->white(), other.white()) * toXYZ(); } Colorimetry Colorimetry::adaptedTo(QVector2D newWhitepoint) const { - const auto mat = chromaticAdaptationMatrix(this->white, newWhitepoint); + const auto mat = chromaticAdaptationMatrix(this->white(), newWhitepoint); return Colorimetry{ - .red = xyzToXY(mat * xyToXYZ(red)), - .green = xyzToXY(mat * xyToXYZ(green)), - .blue = xyzToXY(mat * xyToXYZ(blue)), - .white = newWhitepoint, - .name = std::nullopt, + xyzToXY(mat * xyToXYZ(red())), + xyzToXY(mat * xyToXYZ(green())), + xyzToXY(mat * xyToXYZ(blue())), + newWhitepoint, }; } bool Colorimetry::operator==(const Colorimetry &other) const { - return (name || other.name) ? (name == other.name) - : (red == other.red && green == other.green && blue == other.blue && white == other.white); + return red() == other.red() && green() == other.green() && blue() == other.blue() && white() == other.white(); +} + +bool Colorimetry::operator==(NamedColorimetry name) const +{ + return *this == fromName(name); +} + +const QVector2D &Colorimetry::red() const +{ + return m_red; +} + +const QVector2D &Colorimetry::green() const +{ + return m_green; +} + +const QVector2D &Colorimetry::blue() const +{ + return m_blue; +} + +const QVector2D &Colorimetry::white() const +{ + return m_white; } static const Colorimetry BT709 = Colorimetry{ - .red = {0.64, 0.33}, - .green = {0.30, 0.60}, - .blue = {0.15, 0.06}, - .white = {0.3127, 0.3290}, - .name = NamedColorimetry::BT709, + QVector2D{0.64, 0.33}, + QVector2D{0.30, 0.60}, + QVector2D{0.15, 0.06}, + QVector2D{0.3127, 0.3290}, }; static const Colorimetry BT2020 = Colorimetry{ - .red = {0.708, 0.292}, - .green = {0.170, 0.797}, - .blue = {0.131, 0.046}, - .white = {0.3127, 0.3290}, - .name = NamedColorimetry::BT2020, + QVector2D{0.708, 0.292}, + QVector2D{0.170, 0.797}, + QVector2D{0.131, 0.046}, + QVector2D{0.3127, 0.3290}, }; -Colorimetry Colorimetry::fromName(NamedColorimetry name) +const Colorimetry &Colorimetry::fromName(NamedColorimetry name) { switch (name) { case NamedColorimetry::BT709: @@ -161,26 +207,15 @@ Colorimetry Colorimetry::fromName(NamedColorimetry name) Q_UNREACHABLE(); } -Colorimetry Colorimetry::fromXYZ(QVector3D red, QVector3D green, QVector3D blue, QVector3D white) -{ - return Colorimetry{ - .red = xyzToXY(red), - .green = xyzToXY(green), - .blue = xyzToXY(blue), - .white = xyzToXY(white), - .name = std::nullopt, - }; -} - const ColorDescription ColorDescription::sRGB = ColorDescription(NamedColorimetry::BT709, NamedTransferFunction::gamma22, 100, 0, 100, 100, 0); static Colorimetry sRGBColorimetry(double factor) { return Colorimetry{ - .red = BT709.red * (1 - factor) + BT2020.red * factor, - .green = BT709.green * (1 - factor) + BT2020.green * factor, - .blue = BT709.blue * (1 - factor) + BT2020.blue * factor, - .white = BT709.white, // whitepoint is the same + BT709.red() * (1 - factor) + BT2020.red() * factor, + BT709.green() * (1 - factor) + BT2020.green() * factor, + BT709.blue() * (1 - factor) + BT2020.blue() * factor, + BT709.white(), // whitepoint is the same }; } diff --git a/src/core/colorspace.h b/src/core/colorspace.h index 9694126db7..e39affd0d2 100644 --- a/src/core/colorspace.h +++ b/src/core/colorspace.h @@ -27,8 +27,7 @@ enum class NamedColorimetry { class KWIN_EXPORT Colorimetry { public: - static Colorimetry fromName(NamedColorimetry name); - static Colorimetry fromXYZ(QVector3D red, QVector3D green, QVector3D blue, QVector3D white); + static const Colorimetry &fromName(NamedColorimetry name); /** * @returns the XYZ representation of the xyY color passed in. Y is assumed to be one */ @@ -42,26 +41,43 @@ public: */ static QMatrix3x3 chromaticAdaptationMatrix(QVector2D sourceWhitepoint, QVector2D destinationWhitepoint); + static QMatrix3x3 calculateToXYZMatrix(QVector3D red, QVector3D green, QVector3D blue, QVector3D white); + + explicit Colorimetry(QVector2D red, QVector2D green, QVector2D blue, QVector2D white); + explicit Colorimetry(QVector3D red, QVector3D green, QVector3D blue, QVector3D white); + /** * @returns a matrix that transforms from the linear RGB representation of colors in this colorimetry to the XYZ representation */ - QMatrix3x3 toXYZ() const; + const QMatrix3x3 &toXYZ() const; + /** + * @returns a matrix that transforms from the XYZ representation to the linear RGB representation of colors in this colorimetry + */ + const QMatrix3x3 &fromXYZ() const; /** * @returns a matrix that transforms from linear RGB in this colorimetry to linear RGB in the other colorimetry * the rendering intent is relative colorimetric */ QMatrix3x3 toOther(const Colorimetry &colorimetry) const; bool operator==(const Colorimetry &other) const; + bool operator==(NamedColorimetry name) const; /** * @returns this colorimetry, adapted to the new whitepoint using the Bradford transform */ Colorimetry adaptedTo(QVector2D newWhitepoint) const; - QVector2D red; - QVector2D green; - QVector2D blue; - QVector2D white; - std::optional name; + const QVector2D &red() const; + const QVector2D &green() const; + const QVector2D &blue() const; + const QVector2D &white() const; + +private: + QVector2D m_red; + QVector2D m_green; + QVector2D m_blue; + QVector2D m_white; + QMatrix3x3 m_toXYZ; + QMatrix3x3 m_fromXYZ; }; /** diff --git a/src/core/iccprofile.cpp b/src/core/iccprofile.cpp index 9cec7b2127..4c2ac74372 100644 --- a/src/core/iccprofile.cpp +++ b/src/core/iccprofile.cpp @@ -315,7 +315,7 @@ std::unique_ptr IccProfile::load(const QString &path) // lut based profile, with relative colorimetric intent supported auto data = parseBToATag(handle, cmsSigBToA1Tag); if (data) { - return std::make_unique(handle, Colorimetry::fromXYZ(red, green, blue, white), std::move(*data), vcgt); + return std::make_unique(handle, Colorimetry(red, green, blue, white), std::move(*data), vcgt); } else { qCWarning(KWIN_CORE, "Parsing BToA1 tag failed"); return nullptr; @@ -325,7 +325,7 @@ std::unique_ptr IccProfile::load(const QString &path) // lut based profile, with perceptual intent. The ICC docs say to use this as a fallback auto data = parseBToATag(handle, cmsSigBToA0Tag); if (data) { - return std::make_unique(handle, Colorimetry::fromXYZ(red, green, blue, white), std::move(*data), vcgt); + return std::make_unique(handle, Colorimetry(red, green, blue, white), std::move(*data), vcgt); } else { qCWarning(KWIN_CORE, "Parsing BToA0 tag failed"); return nullptr; @@ -348,7 +348,7 @@ std::unique_ptr IccProfile::load(const QString &path) std::vector> stages; stages.push_back(std::make_unique(cmsStageAllocToneCurves(nullptr, 3, toneCurves))); const auto inverseEOTF = std::make_shared(std::move(stages)); - return std::make_unique(handle, Colorimetry::fromXYZ(red, green, blue, white), inverseEOTF, vcgt); + return std::make_unique(handle, Colorimetry(red, green, blue, white), inverseEOTF, vcgt); } } diff --git a/src/opengl/glshader.cpp b/src/opengl/glshader.cpp index c7a8e64b0e..834e015142 100644 --- a/src/opengl/glshader.cpp +++ b/src/opengl/glshader.cpp @@ -441,7 +441,7 @@ QMatrix4x4 GLShader::getUniformMatrix4x4(const char *name) bool GLShader::setColorspaceUniforms(const ColorDescription &src, const ColorDescription &dst) { - const auto &srcColorimetry = src.colorimetry().name == NamedColorimetry::BT709 ? dst.sdrColorimetry() : src.colorimetry(); + const auto &srcColorimetry = src.colorimetry() == NamedColorimetry::BT709 ? dst.sdrColorimetry() : src.colorimetry(); return setUniform(GLShader::MatrixUniform::ColorimetryTransformation, srcColorimetry.toOther(dst.colorimetry())) && setUniform(GLShader::IntUniform::SourceNamedTransferFunction, int(src.transferFunction())) && setUniform(GLShader::IntUniform::DestinationNamedTransferFunction, int(dst.transferFunction())) diff --git a/src/utils/edid.cpp b/src/utils/edid.cpp index a04c208be2..3c1222eb97 100644 --- a/src/utils/edid.cpp +++ b/src/utils/edid.cpp @@ -128,21 +128,14 @@ Edid::Edid(const void *data, uint32_t size) // colorimetry and HDR metadata const auto chromaticity = di_edid_get_chromaticity_coords(edid); if (chromaticity) { - m_colorimetry = { - .red = {chromaticity->red_x, chromaticity->red_y}, - .green = {chromaticity->green_x, chromaticity->green_y}, - .blue = {chromaticity->blue_x, chromaticity->blue_y}, - .white = {chromaticity->white_x, chromaticity->white_y}, + m_colorimetry = Colorimetry{ + QVector2D{chromaticity->red_x, chromaticity->red_y}, + QVector2D{chromaticity->green_x, chromaticity->green_y}, + QVector2D{chromaticity->blue_x, chromaticity->blue_y}, + QVector2D{chromaticity->white_x, chromaticity->white_y}, }; } else { - // assume sRGB - m_colorimetry = { - .red = {0.64, 0.33}, - .green = {0.30, 0.60}, - .blue = {0.15, 0.06}, - .white = {0.3127, 0.3290}, - .name = NamedColorimetry::BT709, - }; + m_colorimetry.reset(); } const di_edid_cta *cta = nullptr; @@ -246,7 +239,7 @@ QString Edid::hash() const return m_hash; } -Colorimetry Edid::colorimetry() const +std::optional Edid::colorimetry() const { return m_colorimetry; } diff --git a/src/utils/edid.h b/src/utils/edid.h index bf22a20654..33cc80f364 100644 --- a/src/utils/edid.h +++ b/src/utils/edid.h @@ -79,7 +79,7 @@ public: QString hash() const; - Colorimetry colorimetry() const; + std::optional colorimetry() const; double desiredMinLuminance() const; std::optional desiredMaxFrameAverageLuminance() const; @@ -100,7 +100,7 @@ private: QByteArray m_monitorName; QByteArray m_serialNumber; QString m_hash; - Colorimetry m_colorimetry; + std::optional m_colorimetry; struct HDRMetadata { double desiredContentMinLuminance; diff --git a/src/wayland/frog_colormanagement_v1.cpp b/src/wayland/frog_colormanagement_v1.cpp index 19e2736dfd..d61a07db6a 100644 --- a/src/wayland/frog_colormanagement_v1.cpp +++ b/src/wayland/frog_colormanagement_v1.cpp @@ -76,10 +76,10 @@ void FrogColorManagementSurfaceV1::setPreferredColorDescription(const ColorDescr { const auto &color = colorDescription.colorimetry(); send_preferred_metadata(kwinToFrogTransferFunction(colorDescription.transferFunction()), - encodePrimary(color.red.x()), encodePrimary(color.red.y()), - encodePrimary(color.green.x()), encodePrimary(color.green.y()), - encodePrimary(color.blue.x()), encodePrimary(color.blue.y()), - encodePrimary(color.white.x()), encodePrimary(color.white.y()), + encodePrimary(color.red().x()), encodePrimary(color.red().y()), + encodePrimary(color.green().x()), encodePrimary(color.green().y()), + encodePrimary(color.blue().x()), encodePrimary(color.blue().y()), + encodePrimary(color.white().x()), encodePrimary(color.white().y()), std::round(colorDescription.maxHdrHighlightBrightness()), std::round(colorDescription.minHdrBrightness() / 0.0001), std::round(colorDescription.maxFrameAverageBrightness()));