include/oox/drawingml/color.hxx | 2 include/xmloff/xmltoken.hxx | 2 oox/qa/unit/data/theme-tint.pptx |binary oox/qa/unit/drawingml.cxx | 32 ++++++++++++ oox/source/drawingml/color.cxx | 2 oox/source/drawingml/fillproperties.cxx | 2 schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng | 16 ++++-- sd/source/core/stlsheet.cxx | 16 ++++++ svx/source/table/cell.cxx | 16 ++++++ svx/source/unodraw/unoshape.cxx | 16 ++++++ xmloff/qa/unit/data/refer-to-theme.odp |binary xmloff/qa/unit/draw.cxx | 19 +++++++ xmloff/source/core/xmltoken.cxx | 2 xmloff/source/draw/sdpropls.cxx | 2 xmloff/source/token/tokens.txt | 2 15 files changed, 123 insertions(+), 6 deletions(-)
New commits: commit 9f7587debb684688ddb29a90a172e9a3067d2ba1 Author: Miklos Vajna <[email protected]> AuthorDate: Thu May 5 20:18:06 2022 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Jul 1 15:16:21 2022 +0200 tdf#148929 sd theme: limit PPTX import for shape fill effects to lum mod/off Regression from 30735bdb5a0a81619000fdd24b2d0fbf45687f01 (sd theme: add PPTX import for shape fill color effects, 2022-04-27), the bugdoc's A2 cell lost its tinting (its background color is no longer lighter than A1) after saving back to PPTX + import again. The code assumed that in case a fill color has effects, it can only be luminance offset or modulation, since that's what the PowerPoint UI generates when setting a fill color explicitly. This did not take the table style case into account, which uses tinting to make a color lighter. Fix the problem by not importing the theme index / effects if tinting is used -- the current doc model is limited to theme index + lum mod/off with effects. This limitation can be removed while text color / fill color effects are not limited to lum mod/off, but also support tinting/shading. (cherry picked from commit f932b00f3a72dd802a6e50af84c3dc55072a22a0) Change-Id: I382cc0067518cc262e261a462999170cb7db261b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136731 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/include/oox/drawingml/color.hxx b/include/oox/drawingml/color.hxx index cc65c1346720..b28c926986ca 100644 --- a/include/oox/drawingml/color.hxx +++ b/include/oox/drawingml/color.hxx @@ -99,7 +99,7 @@ public: /** Returns the scheme name from the a:schemeClr element for interoperability purposes */ const OUString& getSchemeColorName() const { return msSchemeName; } sal_Int16 getSchemeColorIndex() const; - sal_Int16 getTintOrShade(); + sal_Int16 getTintOrShade() const; sal_Int16 getLumMod() const; sal_Int16 getLumOff() const; diff --git a/oox/qa/unit/data/theme-tint.pptx b/oox/qa/unit/data/theme-tint.pptx new file mode 100644 index 000000000000..23ab7589dea0 Binary files /dev/null and b/oox/qa/unit/data/theme-tint.pptx differ diff --git a/oox/qa/unit/drawingml.cxx b/oox/qa/unit/drawingml.cxx index 9ae434717fb8..4dc066f98039 100644 --- a/oox/qa/unit/drawingml.cxx +++ b/oox/qa/unit/drawingml.cxx @@ -29,6 +29,7 @@ #include <com/sun/star/drawing/TextHorizontalAdjust.hpp> #include <com/sun/star/drawing/XMasterPageTarget.hpp> #include <com/sun/star/text/XTextRange.hpp> +#include <com/sun/star/table/XCellRange.hpp> #include <unotools/mediadescriptor.hxx> #include <unotools/tempfile.hxx> @@ -509,6 +510,37 @@ CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testTdf132557_footerCustomShapes) xShapeSlideNum->getShapeType()); } +CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testThemeTint) +{ + // Given a document with a table style, using theme color with tinting in the A2 cell: + OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "theme-tint.pptx"; + + // When loading that document: + load(aURL); + + // Then make sure that we only import theming info to the doc model if the effects are limited + // to lum mod / off that we can handle (i.e. no tint/shade): + uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY); + uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0), + uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY); + uno::Reference<table::XCellRange> xTable; + CPPUNIT_ASSERT(xShape->getPropertyValue("Model") >>= xTable); + uno::Reference<beans::XPropertySet> xA1(xTable->getCellByPosition(0, 0), uno::UNO_QUERY); + sal_Int16 nFillColorTheme{}; + CPPUNIT_ASSERT(xA1->getPropertyValue("FillColorTheme") >>= nFillColorTheme); + // This is OK, no problematic effects: + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(4), nFillColorTheme); + uno::Reference<beans::XPropertySet> xA2(xTable->getCellByPosition(0, 1), uno::UNO_QUERY); + CPPUNIT_ASSERT(xA2->getPropertyValue("FillColorTheme") >>= nFillColorTheme); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: -1 + // - Actual : 4 + // i.e. we remembered the theme index, without being able to remember the tint effect, leading + // to a bad background color. + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(-1), nFillColorTheme); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/oox/source/drawingml/color.cxx b/oox/source/drawingml/color.cxx index 982b77ff4831..f810deecf2bf 100644 --- a/oox/source/drawingml/color.cxx +++ b/oox/source/drawingml/color.cxx @@ -479,7 +479,7 @@ void Color::clearTransparence() mnAlpha = MAX_PERCENT; } -sal_Int16 Color::getTintOrShade() +sal_Int16 Color::getTintOrShade() const { for(auto const& aTransform : maTransforms) { diff --git a/oox/source/drawingml/fillproperties.cxx b/oox/source/drawingml/fillproperties.cxx index 87b0e62a6fa4..9a39a475ca6c 100644 --- a/oox/source/drawingml/fillproperties.cxx +++ b/oox/source/drawingml/fillproperties.cxx @@ -403,7 +403,7 @@ void FillProperties::pushToPropMap( ShapePropertyMap& rPropMap, { rPropMap.setProperty(PROP_FillColorTheme, nPhClrTheme); } - else + else if (maFillColor.getTintOrShade() == 0) { rPropMap.setProperty(PROP_FillColorTheme, maFillColor.getSchemeColorIndex()); rPropMap.setProperty(PROP_FillColorLumMod, maFillColor.getLumMod()); commit ff0eb613d8379e68eaf1754273b314afd28e0b00 Author: Miklos Vajna <[email protected]> AuthorDate: Wed May 4 20:23:16 2022 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Jul 1 15:16:07 2022 +0200 sd theme: add ODP import/export for shape fill color effects Map a themed color with effects to: <style:graphic-properties draw:fill-color="..." loext:fill-theme-color="..." loext:fill-color-lum-mod="..." loext:fill-color-lum-off="..."> (cherry picked from commit 0c13e4768c3c7937c2fd71675c86ff8a0ca3fe50) Change-Id: I18d8ddf8d6050ef468a8d67a9e797a576f682e85 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136730 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/include/xmloff/xmltoken.hxx b/include/xmloff/xmltoken.hxx index 18f94dac4545..1b5a8eb0fd54 100644 --- a/include/xmloff/xmltoken.hxx +++ b/include/xmloff/xmltoken.hxx @@ -3484,6 +3484,8 @@ namespace xmloff::token { XML_FOLHLINK, XML_COLOR_LUM_MOD, XML_COLOR_LUM_OFF, + XML_FILL_COLOR_LUM_MOD, + XML_FILL_COLOR_LUM_OFF, XML_CONTENT_CONTROL, XML_SHOWING_PLACE_HOLDER, diff --git a/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng b/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng index 861b4481c1a6..959cc36e00b5 100644 --- a/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng +++ b/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng @@ -1762,12 +1762,22 @@ xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1. <rng:ref name="color"/> </rng:attribute> </rng:optional> - <!-- TODO no proposal for theme color of shape fill --> + <!-- TODO(vmiklos) no proposal for theme color of shape fill --> <rng:optional> <rng:attribute name="loext:fill-theme-color"> <rng:ref name="theme-color"/> </rng:attribute> </rng:optional> + <rng:optional> + <rng:attribute name="loext:fill-color-lum-mod"> + <rng:ref name="zeroToHundredPercent"/> + </rng:attribute> + </rng:optional> + <rng:optional> + <rng:attribute name="loext:fill-color-lum-off"> + <rng:ref name="zeroToHundredPercent"/> + </rng:attribute> + </rng:optional> <rng:optional> <rng:attribute name="draw:secondary-fill-color"> <rng:ref name="color"/> @@ -2264,7 +2274,7 @@ xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1. <rng:ref name="zeroToHundredPercent"/> </rng:attribute> </rng:optional> - <!-- TODO no proposal for theme color of shape text --> + <!-- TODO(vmiklos) no proposal for theme color of shape text --> <rng:optional> <rng:attribute name="loext:theme-color"> <rng:ref name="theme-color"/> @@ -3103,7 +3113,7 @@ xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1. <rng:ref name="office-forms"/> </rng:optional> <rng:optional> - <!-- TODO no proposal for defining a theme --> + <!-- TODO(vmiklos) no proposal for defining a theme --> <rng:ref name="loext-theme"/> </rng:optional> <rng:zeroOrMore> diff --git a/sd/source/core/stlsheet.cxx b/sd/source/core/stlsheet.cxx index feff63dbf69a..55b89637c6d1 100644 --- a/sd/source/core/stlsheet.cxx +++ b/sd/source/core/stlsheet.cxx @@ -1326,6 +1326,22 @@ PropertyState SAL_CALL SdStyleSheet::getPropertyState( const OUString& PropertyN eState = PropertyState_DEFAULT_VALUE; } } + else if (pEntry->nMemberId == MID_COLOR_LUM_MOD) + { + const XFillColorItem* pColor = rStyleSet.GetItem<XFillColorItem>(pEntry->nWID); + if (pColor->GetThemeColor().GetLumMod() == 10000) + { + eState = PropertyState_DEFAULT_VALUE; + } + } + else if (pEntry->nMemberId == MID_COLOR_LUM_OFF) + { + const XFillColorItem* pColor = rStyleSet.GetItem<XFillColorItem>(pEntry->nWID); + if (pColor->GetThemeColor().GetLumOff() == 0) + { + eState = PropertyState_DEFAULT_VALUE; + } + } break; } } diff --git a/svx/source/table/cell.cxx b/svx/source/table/cell.cxx index 5cb09e43bd35..a156174f56d6 100644 --- a/svx/source/table/cell.cxx +++ b/svx/source/table/cell.cxx @@ -1452,6 +1452,22 @@ PropertyState SAL_CALL Cell::getPropertyState( const OUString& PropertyName ) eState = PropertyState_DEFAULT_VALUE; } } + else if (pMap->nMemberId == MID_COLOR_LUM_MOD) + { + const XFillColorItem* pColor = rSet.GetItem<XFillColorItem>(pMap->nWID); + if (pColor->GetThemeColor().GetLumMod() == 10000) + { + eState = PropertyState_DEFAULT_VALUE; + } + } + else if (pMap->nMemberId == MID_COLOR_LUM_OFF) + { + const XFillColorItem* pColor = rSet.GetItem<XFillColorItem>(pMap->nWID); + if (pColor->GetThemeColor().GetLumOff() == 0) + { + eState = PropertyState_DEFAULT_VALUE; + } + } } } } diff --git a/svx/source/unodraw/unoshape.cxx b/svx/source/unodraw/unoshape.cxx index 78285121aa4d..4d7e00b42cdc 100644 --- a/svx/source/unodraw/unoshape.cxx +++ b/svx/source/unodraw/unoshape.cxx @@ -2049,6 +2049,22 @@ beans::PropertyState SvxShape::_getPropertyState( const OUString& PropertyName ) eState = beans::PropertyState_DEFAULT_VALUE; } } + else if (pMap->nMemberId == MID_COLOR_LUM_MOD) + { + const XFillColorItem* pColor = rSet.GetItem<XFillColorItem>(pMap->nWID); + if (pColor->GetThemeColor().GetLumMod() == 10000) + { + eState = beans::PropertyState_DEFAULT_VALUE; + } + } + else if (pMap->nMemberId == MID_COLOR_LUM_OFF) + { + const XFillColorItem* pColor = rSet.GetItem<XFillColorItem>(pMap->nWID); + if (pColor->GetThemeColor().GetLumOff() == 0) + { + eState = beans::PropertyState_DEFAULT_VALUE; + } + } break; } } diff --git a/xmloff/qa/unit/data/refer-to-theme.odp b/xmloff/qa/unit/data/refer-to-theme.odp index 5fe9832d3eee..2c413ef766f9 100644 Binary files a/xmloff/qa/unit/data/refer-to-theme.odp and b/xmloff/qa/unit/data/refer-to-theme.odp differ diff --git a/xmloff/qa/unit/draw.cxx b/xmloff/qa/unit/draw.cxx index 4e05b3f7f788..0b7c93c89073 100644 --- a/xmloff/qa/unit/draw.cxx +++ b/xmloff/qa/unit/draw.cxx @@ -224,6 +224,25 @@ CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testReferToTheme) // i.e. only the direct color was written, but not the theme reference. assertXPath(pXmlDoc, "//style:style[@style:name='gr2']/style:graphic-properties", "fill-theme-color", "accent1"); + + // Shape fill, 60% lighter. + assertXPath(pXmlDoc, "//style:style[@style:name='gr3']/style:graphic-properties", + "fill-theme-color", "accent1"); + // Without the accompanying fix in place, this test would have failed with: + // - XPath '//style:style[@style:name='gr3']/style:graphic-properties' no attribute 'fill-color-lum-mod' exist + // i.e. the themed color was fine, but its effects were lost. + assertXPath(pXmlDoc, "//style:style[@style:name='gr3']/style:graphic-properties", + "fill-color-lum-mod", "40%"); + assertXPath(pXmlDoc, "//style:style[@style:name='gr3']/style:graphic-properties", + "fill-color-lum-off", "60%"); + + // Shape fill, 25% darker. + assertXPath(pXmlDoc, "//style:style[@style:name='gr4']/style:graphic-properties", + "fill-theme-color", "accent1"); + assertXPath(pXmlDoc, "//style:style[@style:name='gr4']/style:graphic-properties", + "fill-color-lum-mod", "75%"); + assertXPathNoAttribute(pXmlDoc, "//style:style[@style:name='gr4']/style:graphic-properties", + "fill-color-lum-off"); } CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/xmloff/source/core/xmltoken.cxx b/xmloff/source/core/xmltoken.cxx index 1b0adb6d210d..10727ffb2928 100644 --- a/xmloff/source/core/xmltoken.cxx +++ b/xmloff/source/core/xmltoken.cxx @@ -3487,6 +3487,8 @@ namespace xmloff::token { TOKEN("folHlink", XML_FOLHLINK ), TOKEN("color-lum-mod", XML_COLOR_LUM_MOD ), TOKEN("color-lum-off", XML_COLOR_LUM_OFF ), + TOKEN("fill-color-lum-mod", XML_FILL_COLOR_LUM_MOD ), + TOKEN("fill-color-lum-off", XML_FILL_COLOR_LUM_OFF ), TOKEN("content-control", XML_CONTENT_CONTROL ), TOKEN("showing-place-holder", XML_SHOWING_PLACE_HOLDER ), diff --git a/xmloff/source/draw/sdpropls.cxx b/xmloff/source/draw/sdpropls.cxx index 49ed30583814..343436f71960 100644 --- a/xmloff/source/draw/sdpropls.cxx +++ b/xmloff/source/draw/sdpropls.cxx @@ -107,6 +107,8 @@ const XMLPropertyMapEntry aXMLSDProperties[] = GMAP_D("FillColor", XML_NAMESPACE_DRAW, XML_FILL_COLOR, XML_TYPE_COLOR, CTF_FILLCOLOR ), GMAP_D("FillColor2", XML_NAMESPACE_DRAW, XML_SECONDARY_FILL_COLOR, XML_TYPE_COLOR, 0), GMAPV("FillColorTheme", XML_NAMESPACE_LO_EXT, XML_FILL_THEME_COLOR, XML_TYPE_THEME_COLOR, 0, SvtSaveOptions::ODFSVER_FUTURE_EXTENDED), + GMAPV("FillColorLumMod", XML_NAMESPACE_LO_EXT, XML_FILL_COLOR_LUM_MOD, XML_TYPE_PERCENT100, 0, SvtSaveOptions::ODFSVER_FUTURE_EXTENDED), + GMAPV("FillColorLumOff", XML_NAMESPACE_LO_EXT, XML_FILL_COLOR_LUM_OFF, XML_TYPE_PERCENT100, 0, SvtSaveOptions::ODFSVER_FUTURE_EXTENDED), GMAP( "FillGradientName", XML_NAMESPACE_DRAW, XML_FILL_GRADIENT_NAME, XML_TYPE_STYLENAME|MID_FLAG_NO_PROPERTY_IMPORT, CTF_FILLGRADIENTNAME ), GMAP( "FillGradientStepCount", XML_NAMESPACE_DRAW, XML_GRADIENT_STEP_COUNT, XML_TYPE_NUMBER16, 0 ), GMAP( "FillHatchName", XML_NAMESPACE_DRAW, XML_FILL_HATCH_NAME, XML_TYPE_STYLENAME|MID_FLAG_NO_PROPERTY_IMPORT, CTF_FILLHATCHNAME ), diff --git a/xmloff/source/token/tokens.txt b/xmloff/source/token/tokens.txt index 1183988d9eae..44d71eb8d9fe 100644 --- a/xmloff/source/token/tokens.txt +++ b/xmloff/source/token/tokens.txt @@ -3231,6 +3231,8 @@ hlink folHlink color-lum-mod color-lum-off +fill-color-lum-mod +fill-color-lum-off content-control showing-place-holder checked-state
