tdf#152896 do not apply color shading twice

If a character theme color is shaded, Word writes this as w:themeShade
attribute of the w:color element. If the character also has
transparency, Word writes an additional w14:textFill element with
a w14:lumMod child element. In such cases the w14:textFill element
supersedes the w:color element.

The initial implementation of Fontwork import in commit
cbf30153a5c776e6d1ee26f2f83c8f77503eceb9 does it wrong. It replaces the
color itself but not the color transformation, so that the shading was
applied twice, once from w:themeShade attribute and the other time from
w14:lumMod.

The solution here is to reverse the order so that w:color is only
evaluated if w14:textFill is not present. Another solution would have
been to clear the color transformation vector before adding the values
from w14:textFill. I use reverse order here because it more clearly
reflects that w14:textFill supersedes w:color.

Change-Id: I3e700795167a34238ea619b9c4a691c10da357f4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145150
Tested-by: Jenkins
Reviewed-by: Regina Henschel <rb.henschel@t-online.de>

3 files changed, 79 insertions, 48 deletions

diff --git a/oox/qa/unit/data/tdf152896_WordArt_color_darken.docx b/oox/qa/unit/data/tdf152896_WordArt_color_darken.docx
new file mode 100644
index 000000000000..1f8f8e4e0edf
--- /dev/null
+++ b/oox/qa/unit/data/tdf152896_WordArt_color_darken.docx
diff --git a/oox/qa/unit/shape.cxx b/oox/qa/unit/shape.cxx
index 89a6da6905d2..b03266b553f6 100644
--- a/oox/qa/unit/shape.cxx
+++ b/oox/qa/unit/shape.cxx
@@ -559,6 +559,27 @@ CPPUNIT_TEST_FIXTURE(OoxShapeTest, testWriterShapeFillNonAccentColor)
     uno::Reference<beans::XPropertySet> xShape3Props(xDrawPage->getByIndex(3), uno::UNO_QUERY);
     CPPUNIT_ASSERT_EQUAL(uno::Any(sal_Int16(2)), xShape3Props->getPropertyValue(u"FillColorTheme"));
 }
+
+CPPUNIT_TEST_FIXTURE(OoxShapeTest, testWriterFontworkDarkenTransparency)
+{
+    loadFromURL(u"tdf152896_WordArt_color_darken.docx");
+    // The file contains a WordArt shape with theme colors "Background 2", shading mode "Darken 25%"
+    // and "20% Transparency". Word writes this as w:color element with additional w14:textFill
+    // element. In such case the w14:textFill element supersedes the w:color element. Error was, that
+    // the darkening was applied twice, once from w:color and the other time from w14:textFill.
+
+    uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+                                                 uno::UNO_QUERY);
+
+    uno::Reference<beans::XPropertySet> xShapeProps(xDrawPage->getByIndex(0), uno::UNO_QUERY);
+    // Without the fix in place the test would have failed with
+    // Expected: 13676402 (= 0xD0AF72 = rgb(208, 175, 114) => luminance 63.14%)
+    // Actual: 11897660 (= 0xB58B3C = rgb(181, 139, 60) => luminance 47.25% )
+    // The original "Background 2" is 0xEBDDC3 = rgb(235, 221, 195) => luminance 84.31%
+    CPPUNIT_ASSERT_EQUAL(uno::Any(Color(208, 175, 114)),
+                         xShapeProps->getPropertyValue(u"FillColor"));
+}
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/oox/source/shape/WpsContext.cxx b/oox/source/shape/WpsContext.cxx
index 767f3807607f..9fb3812923b2 100644
--- a/oox/source/shape/WpsContext.cxx
+++ b/oox/source/shape/WpsContext.cxx
@@ -381,62 +381,72 @@ lcl_generateFillPropertiesFromTextProps(const comphelper::SequenceAsHashMap& rTe
 {
     oox::drawingml::FillProperties aFillProperties;
     aFillProperties.moFillType = oox::XML_solidFill; // default
-    sal_Int32 aCharColor = 0;
-    if (rTextPropMap.getValue(u"CharColor") >>= aCharColor)
-        aFillProperties.maFillColor.setSrgbClr(aCharColor);
-    else
-        aFillProperties.maFillColor.setUnused();
-
-    // Theme color superseds direct color. textFill superseds theme color. Theme color and textfill
-    // are in CharInteropGrabBag
+    // Theme color supersedes direct color. textFill supersedes theme color. Theme color and textFill
+    // are in CharInteropGrabBag.
     uno::Sequence<beans::PropertyValue> aCharInteropGrabBagSeq;
-    if (!((rTextPropMap.getValue(u"CharInteropGrabBag") >>= aCharInteropGrabBagSeq)
-          && aCharInteropGrabBagSeq.hasElements()))
-        return aFillProperties;
-    comphelper::SequenceAsHashMap aCharInteropGrabBagMap(aCharInteropGrabBagSeq);
-
-    // Handle theme color, tint and shade.
-    OUString sColorString;
-    if (aCharInteropGrabBagMap.getValue("CharThemeOriginalColor") >>= sColorString)
-    {
-        sal_Int32 nThemeOrigColor = oox::AttributeConversion::decodeIntegerHex(sColorString);
-        aFillProperties.maFillColor.setSrgbClr(nThemeOrigColor);
-    }
-    if (aCharInteropGrabBagMap.getValue("CharThemeColor") >>= sColorString)
+    if ((rTextPropMap.getValue(u"CharInteropGrabBag") >>= aCharInteropGrabBagSeq)
+        && aCharInteropGrabBagSeq.hasElements())
     {
-        sal_Int32 nColorToken = oox::AttributeConversion::decodeToken(sColorString);
-        aFillProperties.maFillColor.setSchemeClr(nColorToken);
-        aFillProperties.maFillColor.setSchemeName(sColorString);
-        // A character color has shade and tint, a shape color has lumMod and lumOff.
-        OUString sTransformString;
-        if (aCharInteropGrabBagMap.getValue("CharThemeColorTint") >>= sTransformString)
+        // Handle case textFill
+        comphelper::SequenceAsHashMap aCharInteropGrabBagMap(aCharInteropGrabBagSeq);
+        beans::PropertyValue aProp;
+        if (aCharInteropGrabBagMap.getValue(u"CharTextFillTextEffect") >>= aProp)
+        {
+            uno::Sequence<beans::PropertyValue> aTextFillSeq;
+            if (aProp.Name == "textFill" && (aProp.Value >>= aTextFillSeq)
+                && aTextFillSeq.hasElements())
+            {
+                // Copy fill properties from aTextFillSeq to aFillProperties
+                lcl_getFillDetailsFromPropSeq(aTextFillSeq, aFillProperties);
+                return aFillProperties;
+            }
+        }
+
+        // no textFill, look for theme color, tint and shade
+        bool bColorFound(false);
+        OUString sColorString;
+        if (aCharInteropGrabBagMap.getValue("CharThemeOriginalColor") >>= sColorString)
         {
-            double fTint = oox::AttributeConversion::decodeIntegerHex(sTransformString);
-            fTint = fTint / 255.0 * oox::drawingml::MAX_PERCENT;
-            aFillProperties.maFillColor.addTransformation(OOX_TOKEN(w14, lumMod),
-                                                          static_cast<sal_Int32>(fTint + 0.5));
-            double fOff = oox::drawingml::MAX_PERCENT - fTint;
-            aFillProperties.maFillColor.addTransformation(OOX_TOKEN(w14, lumOff),
-                                                          static_cast<sal_Int32>(fOff + 0.5));
+            sal_Int32 nThemeOrigColor = oox::AttributeConversion::decodeIntegerHex(sColorString);
+            aFillProperties.maFillColor.setSrgbClr(nThemeOrigColor);
+            bColorFound = true;
         }
-        else if (aCharInteropGrabBagMap.getValue("CharThemeColorShade") >>= sTransformString)
+        if (aCharInteropGrabBagMap.getValue("CharThemeColor") >>= sColorString)
         {
-            double fShade = oox::AttributeConversion::decodeIntegerHex(sTransformString);
-            fShade = fShade / 255.0 * oox::drawingml::MAX_PERCENT;
-            aFillProperties.maFillColor.addTransformation(OOX_TOKEN(w14, lumMod),
-                                                          static_cast<sal_Int32>(fShade + 0.5));
+            sal_Int32 nColorToken = oox::AttributeConversion::decodeToken(sColorString);
+            aFillProperties.maFillColor.setSchemeClr(nColorToken);
+            aFillProperties.maFillColor.setSchemeName(sColorString);
+            bColorFound = true;
+            // A character color has shade or tint, a shape color has lumMod and lumOff.
+            OUString sTransformString;
+            if (aCharInteropGrabBagMap.getValue("CharThemeColorTint") >>= sTransformString)
+            {
+                double fTint = oox::AttributeConversion::decodeIntegerHex(sTransformString);
+                fTint = fTint / 255.0 * oox::drawingml::MAX_PERCENT;
+                aFillProperties.maFillColor.addTransformation(OOX_TOKEN(w14, lumMod),
+                                                              static_cast<sal_Int32>(fTint + 0.5));
+                double fOff = oox::drawingml::MAX_PERCENT - fTint;
+                aFillProperties.maFillColor.addTransformation(OOX_TOKEN(w14, lumOff),
+                                                              static_cast<sal_Int32>(fOff + 0.5));
+            }
+            else if (aCharInteropGrabBagMap.getValue("CharThemeColorShade") >>= sTransformString)
+            {
+                double fShade = oox::AttributeConversion::decodeIntegerHex(sTransformString);
+                fShade = fShade / 255.0 * oox::drawingml::MAX_PERCENT;
+                aFillProperties.maFillColor.addTransformation(OOX_TOKEN(w14, lumMod),
+                                                              static_cast<sal_Int32>(fShade + 0.5));
+            }
         }
+        if (bColorFound)
+            return aFillProperties;
     }
 
-    // Handle textFill
-    beans::PropertyValue aProp;
-    if (!(aCharInteropGrabBagMap.getValue(u"CharTextFillTextEffect") >>= aProp))
-        return aFillProperties;
-    uno::Sequence<beans::PropertyValue> aTextFillSeq;
-    if (!(aProp.Name == "textFill" && (aProp.Value >>= aTextFillSeq) && aTextFillSeq.hasElements()))
-        return aFillProperties;
-    // Copy fill properties from aTextFillSeq to aFillProperties
-    lcl_getFillDetailsFromPropSeq(aTextFillSeq, aFillProperties);
+    // Neither textFill nor theme color. Look for direct color.
+    sal_Int32 aCharColor = 0;
+    if (rTextPropMap.getValue(u"CharColor") >>= aCharColor)
+        aFillProperties.maFillColor.setSrgbClr(aCharColor);
+    else
+        aFillProperties.maFillColor.setUnused();
     return aFillProperties;
 }