MCGR: ColorSteps handling moved to tooling

Due to the fact that handling of a vector GradientSteps
will get used more often with ongoing changes I moved
some handling to tooling, see sortAndCorrectColorSteps.
That method sorts and corrects a vector of ColorSteps
so that only valid entreis remain in a sorted order,
for details please refer to the docu at function
definition. I took the occasion to rearrange that to
work on the single provided vector which is better for
speed & ressources.

Also changed the ColorStep constructor to not
automatically correct constructed entries: While that
is formally correct, it moves an invalid entry to 0.0
or 1.0, thus creating additional wrong Start/EndColor
entries. Those may then 'overlay' the correct entry
when corrections are applied to the vector of entries
which leads to getting the wanted Start/EndColor to be
factically deleted, what is an error.

Also rearranged FillGradientAttribute to now work
initially with ColorSteps, no longer requires the
Start/EndColor, also adapted all usages. This securely
allows start/end and in-between single-color sections
in gradients.

Also needed to re-formulate GradientRect &
GradientElliptical ::appendTransformationsAndColors
method for correct 2D mapping(s) - always incrementing
from the start to end conflicts with the adaption of
the start value for the inner loop mainly because
increment is at start of inner loop (pre-increment).

Corrected errors from clang plugin, but also some
wrong initialiations for basegfx::ColorSteps.

Change-Id: I292592ed9abcfa591f68a680479f4fcdda46cbeb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/148196
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>

5 files changed, 51 insertions, 133 deletions

diff --git a/drawinglayer/qa/unit/vclpixelprocessor2d.cxx b/drawinglayer/qa/unit/vclpixelprocessor2d.cxx
index 573c4bdc32f0..343556aec6d1 100644
--- a/drawinglayer/qa/unit/vclpixelprocessor2d.cxx
+++ b/drawinglayer/qa/unit/vclpixelprocessor2d.cxx
@@ -18,6 +18,7 @@
 #include <drawinglayer/primitive2d/fillgradientprimitive2d.hxx>
 #include <drawinglayer/processor2d/baseprocessor2d.hxx>
 #include <drawinglayer/processor2d/processor2dtools.hxx>
+#include <basegfx/utils/gradienttools.hxx>
 
 using namespace drawinglayer;
 
@@ -59,8 +60,10 @@ public:
 
         basegfx::B2DRange definitionRange(0, 0, 100, 200);
         basegfx::B2DRange outputRange(0, 100, 100, 200); // Paint only lower half of the gradient.
+        const basegfx::ColorSteps aColorSteps{ basegfx::ColorStep(0.0, COL_WHITE.getBColor()),
+                                               basegfx::ColorStep(1.0, COL_BLACK.getBColor()) };
         attribute::FillGradientAttribute attributes(attribute::GradientStyle::Linear, 0, 0, 0, 0,
-                                                    COL_WHITE.getBColor(), COL_BLACK.getBColor());
+                                                    aColorSteps);
         rtl::Reference<primitive2d::FillGradientPrimitive2D> gradientPrimitive(
             new primitive2d::FillGradientPrimitive2D(outputRange, definitionRange, attributes));
         primitive2d::Primitive2DContainer primitives;
diff --git a/drawinglayer/source/attribute/fillgradientattribute.cxx b/drawinglayer/source/attribute/fillgradientattribute.cxx
index 07bc82e4e549..3d78273a6428 100644
--- a/drawinglayer/source/attribute/fillgradientattribute.cxx
+++ b/drawinglayer/source/attribute/fillgradientattribute.cxx
@@ -40,109 +40,31 @@ namespace drawinglayer::attribute
                 double fOffsetX,
                 double fOffsetY,
                 double fAngle,
-                const basegfx::BColor& rStartColor,
-                const basegfx::BColor& rEndColor,
-                const basegfx::ColorSteps* pColorSteps,
+                const basegfx::ColorSteps& rColorSteps,
                 sal_uInt16 nSteps)
             :   mfBorder(fBorder),
                 mfOffsetX(fOffsetX),
                 mfOffsetY(fOffsetY),
                 mfAngle(fAngle),
-                maColorSteps(),
+                maColorSteps(rColorSteps), // copy ColorSteps
                 meStyle(eStyle),
                 mnSteps(nSteps)
             {
-                // always add start color to guarantee a color at all. It's also just safer
-                // to have one and not an empty vector, that spares many checks in the using code
-                maColorSteps.emplace_back(0.0, rStartColor);
-
-                // if we have given ColorSteps, integrate these
-                if(nullptr != pColorSteps && !pColorSteps->empty())
-                {
-                    // append early to local & sort to prepare the following correction(s)
-                    // and later processing
-                    maColorSteps.insert(maColorSteps.end(), pColorSteps->begin(), pColorSteps->end());
-
-                    // no need to sort knowingly lowest entry StartColor, also guarantees that
-                    // entry to stay at begin
-                    std::sort(maColorSteps.begin() + 1, maColorSteps.end());
-
-                    // use two r/w heads on the data band maColorSteps
-                    size_t curr(0), next(1);
-
-                    // during processing, check if all colors are the same. We know the
-                    // StartColor, so to all be the same, all also have to be equal to
-                    // StartColor (including EndColor, use to initialize)
-                    bool bAllTheSameColor(rStartColor == rEndColor);
-
-                    // remove entries <= 0.0, >= 1.0 and with equal offset. do
-                    // this inside the already sorted local vector by evtl.
-                    // moving entries closer to begin to keep and adapting size
-                    // at the end
-                    for(; next < maColorSteps.size(); next++)
-                    {
-                        const double fNextOffset(maColorSteps[next].getOffset());
-
-                        // check for < 0.0 (should not really happen, see ::ColorStep)
-                        // also check for == 0.0 which would mean than an implicit
-                        // StartColor was given in ColorSteps - ignore that, we want
-                        // the explicitly given StartColor to always win
-                        if(basegfx::fTools::lessOrEqual(fNextOffset, 0.0))
-                            continue;
-
-                        // check for > 1.0 (should not really happen, see ::ColorStep)
-                        // also check for == 1.0 which would mean than an implicit
-                        // EndColor was given in ColorSteps - ignore that, we want
-                        // the explicitly given EndColor to always win
-                        if(basegfx::fTools::moreOrEqual(fNextOffset, 1.0))
-                            continue;
-
-                        // check for equal current offset
-                        const double fCurrOffset(maColorSteps[curr].getOffset());
-                        if(basegfx::fTools::equal(fNextOffset, fCurrOffset))
-                            continue;
-
-                        // next is > 0.0, < 1.0 and != curr, so a valid entry.
-                        // take over by evtl. have to move it left
-                        curr++;
-                        if(curr != next)
-                        {
-                            maColorSteps[curr] = maColorSteps[next];
-                        }
-
-                        // new valid entry detected, check it for all the same color
-                        bAllTheSameColor = bAllTheSameColor && maColorSteps[curr].getColor() == rStartColor;
-                    }
-
-                    if(bAllTheSameColor)
-                    {
-                        // if all are the same color, reset to StartColor only
-                        maColorSteps.resize(1);
-                    }
-                    else
-                    {
-                        // adapt size to detected useful entries
-                        curr++;
-                        if(curr != maColorSteps.size())
-                        {
-                            maColorSteps.resize(curr);
-                        }
-
-                        // add EndColor if in-between colors were added
-                        // or StartColor != EndColor
-                        if(curr > 1 || rStartColor != rEndColor)
-                        {
-                            maColorSteps.emplace_back(1.0, rEndColor);
-                        }
-                    }
-                }
-                else
+                // Correct the local ColorSteps. That will guarantee that the
+                // content does contain no offsets < 0.0, > 1.0 or double
+                // ones, also secures sorted arrangement and checks for
+                // double colors, too (see there for more information).
+                // This is what the usages of this in primitives need.
+                // Since FillGradientAttribute is read-only doing this
+                // once here in the constructor is sufficient
+                basegfx::utils::sortAndCorrectColorSteps(maColorSteps);
+
+                // sortAndCorrectColorSteps is rigid and can return
+                // an empty result. To keep things simple, add a single
+                // fallback value
+                if (maColorSteps.empty())
                 {
-                    // add EndColor if different from StartColor
-                    if(rStartColor != rEndColor)
-                    {
-                        maColorSteps.emplace_back(1.0, rEndColor);
-                    }
+                    maColorSteps.emplace_back(0.0, basegfx::BColor());
                 }
             }
 
@@ -204,12 +126,10 @@ namespace drawinglayer::attribute
             double fOffsetX,
             double fOffsetY,
             double fAngle,
-            const basegfx::BColor& rStartColor,
-            const basegfx::BColor& rEndColor,
-            const basegfx::ColorSteps* pColorSteps,
+            const basegfx::ColorSteps& rColorSteps,
             sal_uInt16 nSteps)
         :   mpFillGradientAttribute(ImpFillGradientAttribute(
-                eStyle, fBorder, fOffsetX, fOffsetY, fAngle, rStartColor, rEndColor, pColorSteps, nSteps))
+                eStyle, fBorder, fOffsetX, fOffsetY, fAngle, rColorSteps, nSteps))
         {
         }
 
diff --git a/drawinglayer/source/primitive3d/textureprimitive3d.cxx b/drawinglayer/source/primitive3d/textureprimitive3d.cxx
index ba998c7f50bb..549932e93049 100644
--- a/drawinglayer/source/primitive3d/textureprimitive3d.cxx
+++ b/drawinglayer/source/primitive3d/textureprimitive3d.cxx
@@ -20,6 +20,7 @@
 #include <primitive3d/textureprimitive3d.hxx>
 #include <drawinglayer/primitive3d/drawinglayer_primitivetypes3d.hxx>
 #include <basegfx/color/bcolor.hxx>
+#include <basegfx/utils/gradienttools.hxx>
 #include <utility>
 
 
@@ -92,7 +93,13 @@ namespace drawinglayer::primitive3d
             {
                 // create TransparenceTexturePrimitive3D with fixed transparence as replacement
                 const basegfx::BColor aGray(getTransparence(), getTransparence(), getTransparence());
-                const attribute::FillGradientAttribute aFillGradient(attribute::GradientStyle::Linear, 0.0, 0.0, 0.0, 0.0, aGray, aGray);
+
+                // create ColorSteps with StartColor == EndCoOlor == aGray
+                const basegfx::ColorSteps aColorSteps {
+                    basegfx::ColorStep(0.0, aGray),
+                    basegfx::ColorStep(1.0, aGray) };
+
+                const attribute::FillGradientAttribute aFillGradient(attribute::GradientStyle::Linear, 0.0, 0.0, 0.0, 0.0, aColorSteps);
                 const Primitive3DReference xRef(new TransparenceTexturePrimitive3D(aFillGradient, getChildren(), getTextureSize()));
                 return { xRef };
             }
diff --git a/drawinglayer/source/texture/texture.cxx b/drawinglayer/source/texture/texture.cxx
index 788594f3b8eb..375e83f9221c 100644
--- a/drawinglayer/source/texture/texture.cxx
+++ b/drawinglayer/source/texture/texture.cxx
@@ -466,11 +466,9 @@ namespace drawinglayer::texture
             if (mnColorSteps.size() < 2)
                 return;
 
-            // need to start with the offsets *outside* of all loops, these will
-            // get modified non-linear below, now in more than one gradient section,
-            // but *constantly* over the whole range
-            double fAllWidth(0.0);
-            double fAllHeight(0.0);
+            // prepare vars dependent on aspect ratio
+            const double fAR(maGradientInfo.getAspectRatio());
+            const bool bMTO(fAR > 1.0);
 
             // outer loop over ColorSteps, each is from cs_l to cs_r
             for (auto cs_l(mnColorSteps.begin()), cs_r(cs_l + 1); cs_r != mnColorSteps.end(); cs_l++, cs_r++)
@@ -486,26 +484,21 @@ namespace drawinglayer::texture
                 const double fOffsetEnd(cs_r->getOffset());
                 const double fStripeWidth((fOffsetEnd - fOffsetStart) / nSteps);
 
-                // prepare individual increments for X/Y dependent on aspect ratio
-                const double fAR(maGradientInfo.getAspectRatio());
-                const bool bMTO(fAR > 1.0);
-                const double fIncrementX(bMTO ? fStripeWidth / fAR : fStripeWidth);
-                const double fIncrementY(bMTO ? fStripeWidth : fStripeWidth * fAR);
-
                 // get correct start for inner loop (see above)
                 const sal_uInt32 nStartInnerLoop(cs_l == mnColorSteps.begin() ? 1 : 0);
 
                 for (sal_uInt32 innerLoop(nStartInnerLoop); innerLoop < nSteps; innerLoop++)
                 {
-                    // next step, actively adapt outer-loop w/h values
-                    fAllWidth += fIncrementX;
-                    fAllHeight += fIncrementY;
+                    // calculate offset position for entry
+                    const double fSize(fOffsetStart + (fStripeWidth * innerLoop));
 
                     // set and add at target
                     B2DHomMatrixAndBColor aB2DHomMatrixAndBColor;
 
                     aB2DHomMatrixAndBColor.maB2DHomMatrix = maGradientInfo.getTextureTransform()
-                        * basegfx::utils::createScaleB2DHomMatrix(1.0 - fAllWidth, 1.0 - fAllHeight);
+                        * basegfx::utils::createScaleB2DHomMatrix(
+                            1.0 - (bMTO ? fSize / fAR : fSize),
+                            1.0 - (bMTO ? fSize : fSize * fAR));
                     aB2DHomMatrixAndBColor.maBColor = interpolate(aCStart, aCEnd, double(innerLoop) / double(nSteps - 1));
                     rEntries.push_back(aB2DHomMatrixAndBColor);
                 }
@@ -656,11 +649,9 @@ namespace drawinglayer::texture
             if (mnColorSteps.size() < 2)
                 return;
 
-            // need to start with the offsets *outside* of all loops, these will
-            // get modified non-linear below, now in more than one gradient section,
-            // but *constantly* over the whole range
-            double fAllWidth(0.0);
-            double fAllHeight(0.0);
+            // prepare vars dependent on aspect ratio
+            const double fAR(maGradientInfo.getAspectRatio());
+            const bool bMTO(fAR > 1.0);
 
             // outer loop over ColorSteps, each is from cs_l to cs_r
             for (auto cs_l(mnColorSteps.begin()), cs_r(cs_l + 1); cs_r != mnColorSteps.end(); cs_l++, cs_r++)
@@ -676,26 +667,21 @@ namespace drawinglayer::texture
                 const double fOffsetEnd(cs_r->getOffset());
                 const double fStripeWidth((fOffsetEnd - fOffsetStart) / nSteps);
 
-                // prepare individual increments for X/Y dependent on aspect ratio
-                const double fAR(maGradientInfo.getAspectRatio());
-                const bool bMTO(fAR > 1.0);
-                const double fIncrementX(bMTO ? fStripeWidth / fAR : fStripeWidth);
-                const double fIncrementY(bMTO ? fStripeWidth : fStripeWidth * fAR);
-
                 // get correct start for inner loop (see above)
                 const sal_uInt32 nStartInnerLoop(cs_l == mnColorSteps.begin() ? 1 : 0);
 
                 for (sal_uInt32 innerLoop(nStartInnerLoop); innerLoop < nSteps; innerLoop++)
                 {
-                    // next step, actively adapt outer-loop w/h values
-                    fAllWidth += fIncrementX;
-                    fAllHeight += fIncrementY;
+                    // calculate offset position for entry
+                    const double fSize(fOffsetStart + (fStripeWidth * innerLoop));
 
                     // set and add at target
                     B2DHomMatrixAndBColor aB2DHomMatrixAndBColor;
 
                     aB2DHomMatrixAndBColor.maB2DHomMatrix = maGradientInfo.getTextureTransform()
-                        * basegfx::utils::createScaleB2DHomMatrix(1.0 - fAllWidth, 1.0 - fAllHeight);
+                        * basegfx::utils::createScaleB2DHomMatrix(
+                            1.0 - (bMTO ? fSize / fAR : fSize),
+                            1.0 - (bMTO ? fSize : fSize * fAR));
                     aB2DHomMatrixAndBColor.maBColor = interpolate(aCStart, aCEnd, double(innerLoop) / double(nSteps - 1));
                     rEntries.push_back(aB2DHomMatrixAndBColor);
                 }
diff --git a/drawinglayer/source/tools/wmfemfhelper.cxx b/drawinglayer/source/tools/wmfemfhelper.cxx
index 0189c621fda3..27e6a6c94e24 100644
--- a/drawinglayer/source/tools/wmfemfhelper.cxx
+++ b/drawinglayer/source/tools/wmfemfhelper.cxx
@@ -711,15 +711,17 @@ namespace wmfemfhelper
             }
         }
 
+        const basegfx::ColorSteps aColorSteps {
+            basegfx::ColorStep(0.0, aStart),
+            basegfx::ColorStep(1.0, aEnd) };
+
         return drawinglayer::attribute::FillGradientAttribute(
             aGradientStyle,
             static_cast<double>(rGradient.GetBorder()) * 0.01,
             static_cast<double>(rGradient.GetOfsX()) * 0.01,
             static_cast<double>(rGradient.GetOfsY()) * 0.01,
             toRadians(rGradient.GetAngle()),
-            aStart,
-            aEnd,
-            nullptr,
+            aColorSteps,
             rGradient.GetSteps());
     }