tdf#99562: Do not ignore last column from matrix

Change-Id: I1dff65963e2c414d1771a1592159930150c513e0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154241
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

6 files changed, 92 insertions, 75 deletions

diff --git a/basegfx/source/color/bcolormodifier.cxx b/basegfx/source/color/bcolormodifier.cxx
index 4d3f277c6cc5..75c06f5cb8f3 100644
--- a/basegfx/source/color/bcolormodifier.cxx
+++ b/basegfx/source/color/bcolormodifier.cxx
@@ -155,23 +155,41 @@ namespace basegfx
             return false;
         }
 
-        return maMatrix == pCompare->maMatrix;
+        return maVector == pCompare->maVector;
     }
 
     ::basegfx::BColor BColorModifier_matrix::getModifiedColor(const ::basegfx::BColor& aSourceColor) const
     {
-        basegfx::B3DHomMatrix aColorMatrix;
-        aColorMatrix.set(0, 0, aSourceColor.getRed());
-        aColorMatrix.set(1, 0, aSourceColor.getGreen());
-        aColorMatrix.set(2, 0, aSourceColor.getBlue());
-        aColorMatrix.set(3, 0, 1.0);
-        // TODO: add support for alpha
+        if (maVector.size() != 20)
+            return aSourceColor;
+
+        const double aRed = maVector[0] * aSourceColor.getRed()
+            + maVector[1] * aSourceColor.getGreen()
+            + maVector[2] * aSourceColor.getBlue()
+            + maVector[3] * 1.0
+            + maVector[4];
+        const double aGreen = maVector[5] * aSourceColor.getRed()
+            + maVector[6] * aSourceColor.getGreen()
+            + maVector[7] * aSourceColor.getBlue()
+            + maVector[8] * 1.0
+            + maVector[9];
+        const double aBlue = maVector[10] * aSourceColor.getRed()
+            + maVector[11] * aSourceColor.getGreen()
+            + maVector[12] * aSourceColor.getBlue()
+            + maVector[13] * 1.0
+            + maVector[14];
+        /*TODO: add support for alpha
+        const double aAlpha = maVector[15] * aSourceColor.getRed()
+            + maVector[16] * aSourceColor.getGreen()
+            + maVector[17] * aSourceColor.getBlue()
+            + maVector[18] * 1.0
+            + maVector[19]);
+        */
 
-        aColorMatrix = maMatrix * aColorMatrix;
         return ::basegfx::BColor(
-                std::clamp(aColorMatrix.get(0, 0), 0.0, 1.0),
-                std::clamp(aColorMatrix.get(1, 0), 0.0, 1.0),
-                std::clamp(aColorMatrix.get(2, 0), 0.0, 1.0));
+                std::clamp(aRed, 0.0, 1.0),
+                std::clamp(aGreen, 0.0, 1.0),
+                std::clamp(aBlue, 0.0, 1.0));
     }
 
     OUString BColorModifier_matrix::getModifierName() const
diff --git a/basegfx/test/BColorModifierTest.cxx b/basegfx/test/BColorModifierTest.cxx
index 237f6a982266..4a84c3662a6b 100755
--- a/basegfx/test/BColorModifierTest.cxx
+++ b/basegfx/test/BColorModifierTest.cxx
@@ -272,26 +272,15 @@ public:
     void testMatrix()
     {
         // green matrix
-        basegfx::B3DHomMatrix aMatrix;
-        aMatrix.set(0, 0, 0.0);
-        aMatrix.set(0, 1, 0.0);
-        aMatrix.set(0, 2, 0.0);
-        aMatrix.set(0, 3, 0.0);
-        aMatrix.set(1, 0, 1.0);
-        aMatrix.set(1, 1, 1.0);
-        aMatrix.set(1, 2, 1.0);
-        aMatrix.set(1, 3, 1.0);
-        aMatrix.set(2, 0, 0.0);
-        aMatrix.set(2, 1, 0.0);
-        aMatrix.set(2, 2, 0.0);
-        aMatrix.set(2, 3, 0.0);
-        aMatrix.set(3, 0, 0.0);
-        aMatrix.set(3, 1, 0.0);
-        aMatrix.set(3, 2, 0.0);
-        aMatrix.set(3, 3, 1.0);
+        // clang-format off
+        std::vector<double> aVector = {0.0, 0.0, 0.0, 0.0, 0.0,
+                                       1.0, 1.0, 1.0, 1.0, 0.0,
+                                       0.0, 0.0, 0.0, 0.0, 0.0,
+                                       0.0, 0.0, 0.0, 1.0, 0.0};
+        // clang-format on
 
         const basegfx::BColorModifierSharedPtr aBColorModifier
-            = std::make_shared<basegfx::BColorModifier_matrix>(aMatrix);
+            = std::make_shared<basegfx::BColorModifier_matrix>(aVector);
 
         CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maWhite));
         CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maGray));
@@ -310,32 +299,54 @@ public:
         CPPUNIT_ASSERT(*aBColorModifier != *aBColorModifierInvert);
 
         const basegfx::BColorModifierSharedPtr aBColorModifier2
-            = std::make_shared<basegfx::BColorModifier_matrix>(aMatrix);
+            = std::make_shared<basegfx::BColorModifier_matrix>(aVector);
+        CPPUNIT_ASSERT(aBColorModifier->operator==(*aBColorModifier2));
+    }
+
+    void testMatrixShift()
+    {
+        // clang-format off
+        std::vector<double> aVector = {0.0, 0.0, 0.0, 0.0, 0.0,
+                                       0.0, 0.0, 0.0, 0.0, 1.0,
+                                       0.0, 0.0, 0.0, 0.0, 0.0,
+                                       0.0, 0.0, 0.0, 1.0, 0.0};
+        // clang-format on
+
+        const basegfx::BColorModifierSharedPtr aBColorModifier
+            = std::make_shared<basegfx::BColorModifier_matrix>(aVector);
+
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maWhite));
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maGray));
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maBlack));
+
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maRed));
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maGreen));
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maBlue));
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maYellow));
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maMagenta));
+        CPPUNIT_ASSERT_EQUAL(maGreen, aBColorModifier->getModifiedColor(maCyan));
+
+        CPPUNIT_ASSERT(aBColorModifier->operator==(*aBColorModifier));
+        const basegfx::BColorModifierSharedPtr aBColorModifierInvert
+            = std::make_shared<basegfx::BColorModifier_invert>();
+        CPPUNIT_ASSERT(*aBColorModifier != *aBColorModifierInvert);
+
+        const basegfx::BColorModifierSharedPtr aBColorModifier2
+            = std::make_shared<basegfx::BColorModifier_matrix>(aVector);
         CPPUNIT_ASSERT(aBColorModifier->operator==(*aBColorModifier2));
     }
 
     void testIdentityMatrix()
     {
-        basegfx::B3DHomMatrix aMatrix;
-        aMatrix.set(0, 0, 1.0);
-        aMatrix.set(0, 1, 0.0);
-        aMatrix.set(0, 2, 0.0);
-        aMatrix.set(0, 3, 0.0);
-        aMatrix.set(1, 0, 0.0);
-        aMatrix.set(1, 1, 1.0);
-        aMatrix.set(1, 2, 0.0);
-        aMatrix.set(1, 3, 0.0);
-        aMatrix.set(2, 0, 0.0);
-        aMatrix.set(2, 1, 0.0);
-        aMatrix.set(2, 2, 1.0);
-        aMatrix.set(2, 3, 0.0);
-        aMatrix.set(3, 0, 0.0);
-        aMatrix.set(3, 1, 0.0);
-        aMatrix.set(3, 2, 0.0);
-        aMatrix.set(3, 3, 1.0);
+        // clang-format off
+        std::vector<double> aVector = {1.0, 0.0, 0.0, 0.0, 0.0,
+                                       0.0, 1.0, 0.0, 0.0, 0.0,
+                                       0.0, 0.0, 1.0, 0.0, 0.0,
+                                       0.0, 1.0, 0.0, 1.0, 0.0};
+        // clang-format on
 
         const basegfx::BColorModifierSharedPtr aBColorModifier
-            = std::make_shared<basegfx::BColorModifier_matrix>(aMatrix);
+            = std::make_shared<basegfx::BColorModifier_matrix>(aVector);
 
         CPPUNIT_ASSERT_EQUAL(maWhite, aBColorModifier->getModifiedColor(maWhite));
         CPPUNIT_ASSERT_EQUAL(maGray, aBColorModifier->getModifiedColor(maGray));
@@ -354,7 +365,7 @@ public:
         CPPUNIT_ASSERT(*aBColorModifier != *aBColorModifierInvert);
 
         const basegfx::BColorModifierSharedPtr aBColorModifier2
-            = std::make_shared<basegfx::BColorModifier_matrix>(aMatrix);
+            = std::make_shared<basegfx::BColorModifier_matrix>(aVector);
         CPPUNIT_ASSERT(aBColorModifier->operator==(*aBColorModifier2));
     }
 
@@ -393,6 +404,7 @@ public:
     CPPUNIT_TEST(testLuminanceToAlpha);
     CPPUNIT_TEST(testHueRotate);
     CPPUNIT_TEST(testMatrix);
+    CPPUNIT_TEST(testMatrixShift);
     CPPUNIT_TEST(testIdentityMatrix);
     CPPUNIT_TEST(testBlackAndWhite);
     CPPUNIT_TEST_SUITE_END();
diff --git a/include/basegfx/color/bcolormodifier.hxx b/include/basegfx/color/bcolormodifier.hxx
index 6eb40b67e9d7..1c2c4c776a52 100644
--- a/include/basegfx/color/bcolormodifier.hxx
+++ b/include/basegfx/color/bcolormodifier.hxx
@@ -246,11 +246,11 @@ namespace basegfx
     class SAL_WARN_UNUSED BASEGFX_DLLPUBLIC BColorModifier_matrix final : public BColorModifier
     {
     private:
-        basegfx::B3DHomMatrix       maMatrix;
+        std::vector<double>       maVector;
 
     public:
-        BColorModifier_matrix(basegfx::B3DHomMatrix aMatrix)
-        :   maMatrix(aMatrix)
+        BColorModifier_matrix(std::vector<double> aVector)
+        :   maVector(aVector)
         {
         }
 
diff --git a/svgio/inc/svgtools.hxx b/svgio/inc/svgtools.hxx
index f556ed6cde02..c395702e5998 100644
--- a/svgio/inc/svgtools.hxx
+++ b/svgio/inc/svgtools.hxx
@@ -20,7 +20,6 @@
 #pragma once
 
 #include <basegfx/color/bcolor.hxx>
-#include <basegfx/range/b3drange.hxx>
 #include <basegfx/range/b2drange.hxx>
 #include <basegfx/vector/b2ivector.hxx>
 #include <rtl/ustrbuf.hxx>
@@ -110,7 +109,7 @@ namespace svgio::svgreader
         bool match_colorKeyword(basegfx::BColor& rColor, const OUString& rName);
         bool read_color(const OUString& rCandidate, basegfx::BColor& rColor, SvgNumber& rOpacity);
         basegfx::B2DRange readViewBox(std::u16string_view rCandidate, InfoProvider const & rInfoProvider);
-        basegfx::B3DHomMatrix readFilterMatrix(std::u16string_view rCandidate, InfoProvider const & rInfoProvider);
+        std::vector<double> readFilterMatrix(std::u16string_view rCandidate, InfoProvider const & rInfoProvider);
         basegfx::B2DHomMatrix readTransform(std::u16string_view rCandidate, InfoProvider const & rInfoProvider);
         bool readSingleNumber(std::u16string_view rCandidate, SvgNumber& aNum);
         bool readLocalLink(std::u16string_view rCandidate, OUString& rURL);
diff --git a/svgio/source/svgreader/svgfecolormatrixnode.cxx b/svgio/source/svgreader/svgfecolormatrixnode.cxx
index 0fed49ea6ca2..d08c5ea44280 100644
--- a/svgio/source/svgreader/svgfecolormatrixnode.cxx
+++ b/svgio/source/svgreader/svgfecolormatrixnode.cxx
@@ -105,11 +105,11 @@ void SvgFeColorMatrixNode::apply(drawinglayer::primitive2d::Primitive2DContainer
     }
     else if (maType == ColorType::Matrix)
     {
-        basegfx::B3DHomMatrix aMatrix = readFilterMatrix(maValuesContent, *this);
+        std::vector<double> aVector = readFilterMatrix(maValuesContent, *this);
 
         const drawinglayer::primitive2d::Primitive2DReference xRef(
             new drawinglayer::primitive2d::ModifiedColorPrimitive2D(
-                std::move(rTarget), std::make_shared<basegfx::BColorModifier_matrix>(aMatrix)));
+                std::move(rTarget), std::make_shared<basegfx::BColorModifier_matrix>(aVector)));
         rTarget = drawinglayer::primitive2d::Primitive2DContainer{ xRef };
     }
 }
diff --git a/svgio/source/svgreader/svgtools.cxx b/svgio/source/svgreader/svgtools.cxx
index f7bc5b951f97..ce8b4f99bd62 100644
--- a/svgio/source/svgreader/svgtools.cxx
+++ b/svgio/source/svgreader/svgtools.cxx
@@ -846,9 +846,9 @@ namespace svgio::svgreader
             return basegfx::B2DRange();
         }
 
-        basegfx::B3DHomMatrix readFilterMatrix(std::u16string_view rCandidate, InfoProvider const & rInfoProvider)
+        std::vector<double> readFilterMatrix(std::u16string_view rCandidate, InfoProvider const & rInfoProvider)
         {
-            basegfx::B3DHomMatrix aMatrix;
+            std::vector<double> aVector;
             const sal_Int32 nLen(rCandidate.size());
 
             sal_Int32 nPos(0);
@@ -856,28 +856,16 @@ namespace svgio::svgreader
 
             SvgNumber aVal;
 
-            // create a 4x4 matrix from the list of 20 matrix values.
-            for (sal_uInt16 nRow = 0; nRow < 4; ++nRow)
+            while (nPos < nLen)
             {
-                for (sal_uInt16 nColumn = 0; nColumn < 5; ++nColumn)
+                if(readNumberAndUnit(rCandidate, nPos, aVal, nLen))
                 {
-                    // return earlier if there are not enough values
-                    if (nPos >= nLen)
-                    {
-                        return basegfx::B3DHomMatrix();
-                    }
-
-                    if(readNumberAndUnit(rCandidate, nPos, aVal, nLen))
-                    {
-                        // ignore the last column
-                        if (nColumn < 4)
-                            aMatrix.set(nRow, nColumn, aVal.solve(rInfoProvider));
-                        skip_char(rCandidate, ' ', ',', nPos, nLen);
-                    }
+                    aVector.push_back(aVal.solve(rInfoProvider));
+                    skip_char(rCandidate, ' ', ',', nPos, nLen);
                 }
             }
 
-            return aMatrix;
+            return aVector;
         }
 
         basegfx::B2DHomMatrix readTransform(std::u16string_view rCandidate, InfoProvider const & rInfoProvider)