core/colorpipeline: improve optimization with differing reference luminances

This is done by
- fixing isFuzzyScalingOnly to not check the [3, 3] component, which is always 1
- making the comparison between transfer functions fuzzy, so small floating point
  errors don't prevent two practically identical functions to be optimized out
- switching manual optimizations to use addMatrix instead, which removes the
  matrix or replaces it with a multiplier

and the autotest is expanded to test transformations between color descriptions with
transfer functions and reference luminances that are just scaled versions of each
other

autotests/test_colorspaces.cpp

@@ -114,7 +114,9 @@ void TestColorspaces::testIdentityTransformation()
     QFETCH(NamedColorimetry, colorimetry);
     QFETCH(TransferFunction::Type, transferFunction);
     const TransferFunction tf(transferFunction);
-    const ColorDescription color(colorimetry, tf, 100, tf.minLuminance, tf.maxLuminance, tf.maxLuminance);
+    const ColorDescription src(colorimetry, tf, 100, tf.minLuminance, tf.maxLuminance, tf.maxLuminance);
+    const TransferFunction tf2(transferFunction, tf.minLuminance * 1.1, tf.maxLuminance * 1.1);
+    const ColorDescription dst(colorimetry, tf2, 110, tf2.minLuminance, tf2.maxLuminance, tf2.maxLuminance);
 
     constexpr std::array renderingIntents = {
         RenderingIntent::Perceptual,
@@ -123,7 +125,7 @@ void TestColorspaces::testIdentityTransformation()
         RenderingIntent::RelativeColorimetricWithBPC,
     };
     for (const RenderingIntent intent : renderingIntents) {
-        const auto pipeline = ColorPipeline::create(color, color, intent);
+        const auto pipeline = ColorPipeline::create(src, dst, intent);
         if (!pipeline.isIdentity()) {
             qWarning() << pipeline;
         }

src/core/colorpipeline.cpp

@@ -71,24 +71,26 @@ void ColorPipeline::addMultiplier(const QVector3D &factors)
     if (!ops.empty()) {
         auto *lastOp = &ops.back().operation;
         if (const auto mat = std::get_if<ColorMatrix>(lastOp)) {
-            mat->mat.scale(factors);
+            auto newMat = mat->mat;
-            ops.back().output = output;
+            newMat.scale(factors);
+            ops.erase(ops.end() - 1);
+            addMatrix(newMat, output);
             return;
         } else if (const auto mult = std::get_if<ColorMultiplier>(lastOp)) {
             mult->factors *= factors;
-            if (mult->factors == QVector3D(1, 1, 1)) {
+            if ((mult->factors - QVector3D(1, 1, 1)).lengthSquared() < s_maxResolution * s_maxResolution) {
                 ops.erase(ops.end() - 1);
             } else {
                 ops.back().output = output;
             }
             return;
-        } else if (factors.x() == factors.y() && factors.y() == factors.z()) {
+        } else if (std::abs(factors.x() - factors.y()) < s_maxResolution && std::abs(factors.x() - factors.z()) < s_maxResolution) {
-            if (const auto tf = std::get_if<ColorTransferFunction>(lastOp); tf && tf->tf.isRelative()) {
+            if (const auto tf = std::get_if<ColorTransferFunction>(lastOp)) {
                 tf->tf.minLuminance *= factors.x();
                 tf->tf.maxLuminance *= factors.x();
                 ops.back().output = output;
                 return;
-            } else if (const auto tf = std::get_if<InverseColorTransferFunction>(lastOp); tf && tf->tf.isRelative()) {
+            } else if (const auto tf = std::get_if<InverseColorTransferFunction>(lastOp)) {
                 tf->tf.minLuminance /= factors.x();
                 tf->tf.maxLuminance /= factors.x();
                 ops.back().output = output;
@@ -165,13 +167,10 @@ void ColorPipeline::addInverseTransferFunction(TransferFunction tf)
 
 static bool isFuzzyIdentity(const QMatrix4x4 &mat)
 {
-    // matrix calculations with floating point numbers can result in very small errors
-    // -> ignore them, as that just causes inefficiencies and more rounding errors
-    constexpr float maxResolution = 0.00001;
     for (int i = 0; i < 4; i++) {
         for (int j = 0; j < 4; j++) {
             const float targetValue = i == j ? 1 : 0;
-            if (std::abs(mat(i, j) - targetValue) > maxResolution) {
+            if (std::abs(mat(i, j) - targetValue) > ColorPipeline::s_maxResolution) {
                 return false;
             }
         }
@@ -181,15 +180,12 @@ static bool isFuzzyIdentity(const QMatrix4x4 &mat)
 
 static bool isFuzzyScalingOnly(const QMatrix4x4 &mat)
 {
-    // matrix calculations with floating point numbers can result in very small errors
-    // -> ignore them, as that just causes inefficiencies and more rounding errors
-    constexpr float maxResolution = 0.00001;
     for (int i = 0; i < 4; i++) {
         for (int j = 0; j < 4; j++) {
-            if (i < 3 && i == j) {
+            if (i == j) {
                 continue;
             }
-            if (std::abs(mat(i, j)) > maxResolution) {
+            if (std::abs(mat(i, j)) > ColorPipeline::s_maxResolution) {
                 return false;
             }
         }
@@ -205,17 +201,15 @@ void ColorPipeline::addMatrix(const QMatrix4x4 &mat, const ValueRange &output)
     if (!ops.empty()) {
         auto *lastOp = &ops.back().operation;
         if (const auto otherMat = std::get_if<ColorMatrix>(lastOp)) {
-            otherMat->mat = mat * otherMat->mat;
+            const auto newMat = mat * otherMat->mat;
-            ops.back().output = output;
+            ops.erase(ops.end() - 1);
+            addMatrix(newMat, output);
             return;
         } else if (const auto mult = std::get_if<ColorMultiplier>(lastOp)) {
             QMatrix4x4 scaled = mat;
             scaled.scale(mult->factors);
-            ops.back() = ColorOp{
+            ops.erase(ops.end() - 1);
-                .input = currentOutputRange(),
+            addMatrix(scaled, output);
-                .operation = ColorMatrix(scaled),
-                .output = output,
-            };
             return;
         }
     }

src/core/colorpipeline.h

@@ -77,6 +77,13 @@ public:
 class KWIN_EXPORT ColorPipeline
 {
 public:
+    /**
+     * matrix calculations with floating point numbers can result in very small errors
+     * this value is the minimum difference we actually care about; everything below
+     * can and should be optimized out
+     */
+    static constexpr float s_maxResolution = 0.00001;
+
     explicit ColorPipeline();
     explicit ColorPipeline(const ValueRange &inputRange);
 

src/core/colorspace.cpp

@@ -4,8 +4,9 @@
     SPDX-License-Identifier: GPL-2.0-or-later
 */
 #include "colorspace.h"
+#include "colorpipeline.h"
 
-#include <qassert.h>
+#include <QtAssert>
 
 namespace KWin
 {
@@ -434,6 +435,15 @@ double TransferFunction::defaultReferenceLuminanceFor(Type type)
     Q_UNREACHABLE();
 }
 
+bool TransferFunction::operator==(const TransferFunction &other) const
+{
+    // allow for a greater error with large max. luminance, as floating point errors get larger there
+    // and the effect of errors is smaller too
+    return type == other.type
+        && std::abs(other.minLuminance - minLuminance) < ColorPipeline::s_maxResolution
+        && std::abs(other.maxLuminance - maxLuminance) < ColorPipeline::s_maxResolution * maxLuminance;
+}
+
 TransferFunction::TransferFunction(Type tf)
     : TransferFunction(tf, defaultMinLuminanceFor(tf), defaultMaxLuminanceFor(tf))
 {

src/core/colorspace.h

@@ -120,7 +120,7 @@ public:
     explicit TransferFunction(Type tf);
     explicit TransferFunction(Type tf, double minLuminance, double maxLuminance);
 
-    auto operator<=>(const TransferFunction &) const = default;
+    bool operator==(const TransferFunction &) const;
 
     bool isRelative() const;
     TransferFunction relativeScaledTo(double referenceLuminance) const;