diff --git a/autotests/test_colorspaces.cpp b/autotests/test_colorspaces.cpp index 6e8d93731d..d11d155fbd 100644 --- a/autotests/test_colorspaces.cpp +++ b/autotests/test_colorspaces.cpp @@ -21,7 +21,7 @@ public: private Q_SLOTS: void roundtripConversion_data(); void roundtripConversion(); - void nonNormalizedPrimaries(); + void testXYZ_XYconversions(); void testIdentityTransformation_data(); void testIdentityTransformation(); void testColorPipeline_data(); @@ -82,16 +82,14 @@ void TestColorspaces::roundtripConversion() } } -void TestColorspaces::nonNormalizedPrimaries() +void TestColorspaces::testXYZ_XYconversions() { - // this test ensures that non-normalized primaries don't mess up the transformations between color spaces - const auto &from = ColorDescription::sRGB; - const ColorDescription to(Colorimetry(Colorimetry::xyToXYZ(from.containerColorimetry().red()) * 2, Colorimetry::xyToXYZ(from.containerColorimetry().green()) * 2, Colorimetry::xyToXYZ(from.containerColorimetry().blue()) * 2, Colorimetry::xyToXYZ(from.containerColorimetry().white()) * 2), from.transferFunction(), from.referenceLuminance(), from.minLuminance(), from.maxAverageLuminance(), from.maxHdrLuminance()); - - const auto convertedWhite = from.toOther(to, RenderingIntent::RelativeColorimetric) * QVector3D(1, 1, 1); - QCOMPARE_LE(std::abs(1 - convertedWhite.x()), s_resolution10bit); - QCOMPARE_LE(std::abs(1 - convertedWhite.y()), s_resolution10bit); - QCOMPARE_LE(std::abs(1 - convertedWhite.z()), s_resolution10bit); + // this test ensures that Colorimetry::xyzToXY and Colorimetry::xyToXYZ can handle weird inputs + // and don't cause crashes + QCOMPARE(Colorimetry::xyzToXY(QVector3D(0, 0, 0)), QVector2D(0, 0)); + QCOMPARE_LE(Colorimetry::xyzToXY(QVector3D(100, 100, 100)).y(), 1); + QCOMPARE(Colorimetry::xyToXYZ(QVector2D(0, 0)), QVector3D(0, 1, 0)); + QCOMPARE(Colorimetry::xyToXYZ(QVector2D(1, 0)), QVector3D(1, 1, 0)); } void TestColorspaces::testIdentityTransformation_data() diff --git a/src/core/colorspace.cpp b/src/core/colorspace.cpp index e78ce890eb..a41991db55 100644 --- a/src/core/colorspace.cpp +++ b/src/core/colorspace.cpp @@ -28,12 +28,20 @@ static QMatrix4x4 matrixFromColumns(const QVector3D &first, const QVector3D &sec QVector3D Colorimetry::xyToXYZ(QVector2D xy) { + if (xy.y() == 0) { + // special case for XYZ Colorimetry + // where xy.y == 0 is valid + return QVector3D(xy.x(), 1, 0); + } return QVector3D(xy.x() / xy.y(), 1, (1 - xy.x() - xy.y()) / xy.y()); } QVector2D Colorimetry::xyzToXY(QVector3D xyz) { - xyz /= xyz.y(); + if (xyz.y() == 0) { + // this is nonsense, but at least doesn't crash + return QVector2D(0, 0); + } return QVector2D(xyz.x() / (xyz.x() + xyz.y() + xyz.z()), xyz.y() / (xyz.x() + xyz.y() + xyz.z())); } @@ -76,11 +84,6 @@ QMatrix4x4 Colorimetry::chromaticAdaptationMatrix(QVector2D sourceWhitepoint, QV return inverseBradford * adaptation * bradford; } -static QVector3D normalizeToY1(const QVector3D &vect) -{ - return vect.y() == 0 ? vect : QVector3D(vect.x() / vect.y(), 1, vect.z() / vect.y()); -} - QMatrix4x4 Colorimetry::calculateToXYZMatrix(QVector3D red, QVector3D green, QVector3D blue, QVector3D white) { const auto component_scale = (matrixFromColumns(red, green, blue)).inverted() * white; @@ -107,16 +110,6 @@ Colorimetry::Colorimetry(QVector2D red, QVector2D green, QVector2D blue, QVector { } -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(normalizeToY1(red), normalizeToY1(green), normalizeToY1(blue), normalizeToY1(white))) - , m_fromXYZ(m_toXYZ.inverted()) -{ -} - const QMatrix4x4 &Colorimetry::toXYZ() const { return m_toXYZ; diff --git a/src/core/colorspace.h b/src/core/colorspace.h index ba0e24a49c..2be28c135c 100644 --- a/src/core/colorspace.h +++ b/src/core/colorspace.h @@ -68,7 +68,6 @@ public: static QMatrix4x4 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 diff --git a/src/core/iccprofile.cpp b/src/core/iccprofile.cpp index 34b5d5f900..ea8676e1da 100644 --- a/src/core/iccprofile.cpp +++ b/src/core/iccprofile.cpp @@ -263,11 +263,15 @@ std::unique_ptr IccProfile::load(const QString &path) qCWarning(KWIN_CORE, "profile is missing the wtpt tag"); return nullptr; } + if (whitepoint->Y == 0) { + qCWarning(KWIN_CORE, "profile has a zero luminance whitepoint"); + return nullptr; + } - QVector3D red; - QVector3D green; - QVector3D blue; - QVector3D white(whitepoint->X, whitepoint->Y, whitepoint->Z); + QVector2D red; + QVector2D green; + QVector2D blue; + QVector2D white = Colorimetry::xyzToXY(QVector3D(whitepoint->X, whitepoint->Y, whitepoint->Z)); std::optional chromaticAdaptationMatrix; if (cmsIsTag(handle, cmsSigChromaticAdaptationTag)) { // the chromatic adaptation tag is a 3x3 matrix that converts from the actual whitepoint to D50 @@ -284,12 +288,12 @@ std::unique_ptr IccProfile::load(const QString &path) return nullptr; } const QVector3D D50(0.9642, 1.0, 0.8249); - white = *chromaticAdaptationMatrix * D50; + white = Colorimetry::xyzToXY(*chromaticAdaptationMatrix * D50); } if (cmsCIExyYTRIPLE *chrmTag = static_cast(cmsReadTag(handle, cmsSigChromaticityTag))) { - red = Colorimetry::xyToXYZ(QVector2D(chrmTag->Red.x, chrmTag->Red.y)) * chrmTag->Red.Y; - green = Colorimetry::xyToXYZ(QVector2D(chrmTag->Green.x, chrmTag->Green.y)) * chrmTag->Green.Y; - blue = Colorimetry::xyToXYZ(QVector2D(chrmTag->Blue.x, chrmTag->Blue.y)) * chrmTag->Blue.Y; + red = QVector2D(chrmTag->Red.x, chrmTag->Red.y); + green = QVector2D(chrmTag->Green.x, chrmTag->Green.y); + blue = QVector2D(chrmTag->Blue.x, chrmTag->Blue.y); } else { const cmsCIEXYZ *r = static_cast(cmsReadTag(handle, cmsSigRedColorantTag)); const cmsCIEXYZ *g = static_cast(cmsReadTag(handle, cmsSigGreenColorantTag)); @@ -299,9 +303,9 @@ std::unique_ptr IccProfile::load(const QString &path) return nullptr; } if (chromaticAdaptationMatrix) { - red = *chromaticAdaptationMatrix * QVector3D(r->X, r->Y, r->Z); - green = *chromaticAdaptationMatrix * QVector3D(g->X, g->Y, g->Z); - blue = *chromaticAdaptationMatrix * QVector3D(b->X, b->Y, b->Z); + red = Colorimetry::xyzToXY(*chromaticAdaptationMatrix * QVector3D(r->X, r->Y, r->Z)); + green = Colorimetry::xyzToXY(*chromaticAdaptationMatrix * QVector3D(g->X, g->Y, g->Z)); + blue = Colorimetry::xyzToXY(*chromaticAdaptationMatrix * QVector3D(b->X, b->Y, b->Z)); } else { // if the chromatic adaptation tag isn't available, fall back to using the media whitepoint instead cmsCIEXYZ adaptedR{}; @@ -313,9 +317,9 @@ std::unique_ptr IccProfile::load(const QString &path) if (!success) { return nullptr; } - red = QVector3D(adaptedR.X, adaptedR.Y, adaptedR.Z); - green = QVector3D(adaptedG.X, adaptedG.Y, adaptedG.Z); - blue = QVector3D(adaptedB.X, adaptedB.Y, adaptedB.Z); + red = Colorimetry::xyzToXY(QVector3D(adaptedR.X, adaptedR.Y, adaptedR.Z)); + green = Colorimetry::xyzToXY(QVector3D(adaptedG.X, adaptedG.Y, adaptedG.Z)); + blue = Colorimetry::xyzToXY(QVector3D(adaptedB.X, adaptedB.Y, adaptedB.Z)); } }