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>
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 0000000..1f8f8e4e
--- /dev/null
+++ b/oox/qa/unit/data/tdf152896_WordArt_color_darken.docx
Binary files differ
diff --git a/oox/qa/unit/shape.cxx b/oox/qa/unit/shape.cxx
index 89a6da6..b03266b 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 767f380..9fb3812 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
// 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())
{
// 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)
{
sal_Int32 nThemeOrigColor = oox::AttributeConversion::decodeIntegerHex(sColorString);
aFillProperties.maFillColor.setSrgbClr(nThemeOrigColor);
bColorFound = true;
}
if (aCharInteropGrabBagMap.getValue("CharThemeColor") >>= sColorString)
{
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;
}
// 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();
// Theme color superseds direct color. textFill superseds 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)
{
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)
{
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));
}
}
// 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);
return aFillProperties;
}
