include/oox/drawingml/shape.hxx | 5 +++++ oox/source/drawingml/shape.cxx | 6 +++++- oox/source/ppt/pptshapegroupcontext.cxx | 4 +++- sd/qa/unit/data/pptx/tdf127964.pptx |binary sd/qa/unit/import-tests.cxx | 23 +++++++++++++++++++++++ 5 files changed, 36 insertions(+), 2 deletions(-)
New commits: commit 6f9407c0d872675dbcbd0dc52fd7a0fdb098804b Author: Miklos Vajna <[email protected]> AuthorDate: Mon Oct 14 21:49:32 2019 +0200 Commit: Xisco Faulí <[email protected]> CommitDate: Tue Oct 15 10:52:30 2019 +0200 tdf#127964 PPTX import: fix shape fill handling: style vs slide background Regression from commit 943a534ac7cb3df513583e226c986dafd8ba246b (tdf#123684 PPTX import: fix wrong background color for <p:sp useBgFill="1">, 2019-04-23), the problem was that we didn't handle the case when a shape had an XML fragment like this: <p:sp useBgFill="1"> <p:style> <a:fillRef idx="1"> <a:schemeClr val="accent1"/> </a:fillRef> </p:style> </p:sp> i.e. the shape both wants to use background fill and it has a style declaring how to fill it as well. We gave the style a priority, while PowerPoint gives the background fill a priority. Fix the problem by not setting the fill from the style in case the background fill is already set. Change-Id: Ie1b56e5615219138a5b7ddd7a2b25295b991bc05 Reviewed-on: https://gerrit.libreoffice.org/80804 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> (cherry picked from commit 46d630f98f1c07ec2048da35d1a4804181148ac5) Reviewed-on: https://gerrit.libreoffice.org/80807 Reviewed-by: Xisco Faulí <[email protected]> diff --git a/include/oox/drawingml/shape.hxx b/include/oox/drawingml/shape.hxx index 5aa6f00318a8..4abf973d8cb7 100644 --- a/include/oox/drawingml/shape.hxx +++ b/include/oox/drawingml/shape.hxx @@ -228,6 +228,8 @@ public: void setVerticalShapesCount(sal_Int32 nVerticalShapesCount) { mnVerticalShapesCount = nVerticalShapesCount; } sal_Int32 getVerticalShapesCount() const { return mnVerticalShapesCount; } + void setUseBgFill(bool bUseBgFill) { mbUseBgFill = bUseBgFill; } + /// Changes reference semantics to value semantics for fill properties. void cloneFillProperties(); @@ -367,6 +369,9 @@ private: /// Number of child shapes to be layouted vertically inside org chart in-diagram shape. sal_Int32 mnVerticalShapesCount = 0; + + /// The shape fill should be set to that of the slide background surface. + bool mbUseBgFill = false; }; } } diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx index 3525ad7d0317..16b0c5b04824 100644 --- a/oox/source/drawingml/shape.cxx +++ b/oox/source/drawingml/shape.cxx @@ -183,6 +183,7 @@ Shape::Shape( const ShapePtr& pSourceShape ) , mnZOrderOff(pSourceShape->mnZOrderOff) , mnDataNodeType(pSourceShape->mnDataNodeType) , mfAspectRatio(pSourceShape->mfAspectRatio) +, mbUseBgFill(pSourceShape->mbUseBgFill) {} Shape::~Shape() @@ -975,7 +976,10 @@ Reference< XShape > const & Shape::createAndInsert( } if( const ShapeStyleRef* pFillRef = getShapeStyleRef( XML_fillRef ) ) { - nFillPhClr = pFillRef->maPhClr.getColor( rGraphicHelper ); + if (!mbUseBgFill) + { + nFillPhClr = pFillRef->maPhClr.getColor(rGraphicHelper); + } OUString sColorScheme = pFillRef->maPhClr.getSchemeName(); if( !sColorScheme.isEmpty() ) diff --git a/oox/source/ppt/pptshapegroupcontext.cxx b/oox/source/ppt/pptshapegroupcontext.cxx index 6535e12d3f81..745a9b8e847e 100644 --- a/oox/source/ppt/pptshapegroupcontext.cxx +++ b/oox/source/ppt/pptshapegroupcontext.cxx @@ -101,7 +101,9 @@ ContextHandlerRef PPTShapeGroupContext::onCreateContext( sal_Int32 aElementToken case PPT_TOKEN( sp ): // Shape { std::shared_ptr<PPTShape> pShape( new PPTShape( meShapeLocation, "com.sun.star.drawing.CustomShape" ) ); - if( rAttribs.getBool( XML_useBgFill, false ) ) + bool bUseBgFill = rAttribs.getBool(XML_useBgFill, false); + pShape->setUseBgFill(bUseBgFill); + if (bUseBgFill) { oox::drawingml::FillPropertiesPtr pBackgroundPropertiesPtr = mpSlidePersistPtr->getBackgroundProperties(); if (!pBackgroundPropertiesPtr) diff --git a/sd/qa/unit/data/pptx/tdf127964.pptx b/sd/qa/unit/data/pptx/tdf127964.pptx new file mode 100644 index 000000000000..89482a4ce99c Binary files /dev/null and b/sd/qa/unit/data/pptx/tdf127964.pptx differ diff --git a/sd/qa/unit/import-tests.cxx b/sd/qa/unit/import-tests.cxx index 6252ca27161f..28a908197fdc 100644 --- a/sd/qa/unit/import-tests.cxx +++ b/sd/qa/unit/import-tests.cxx @@ -207,6 +207,7 @@ public: void testTdf122899(); void testOOXTheme(); void testCropToShape(); + void testTdf127964(); CPPUNIT_TEST_SUITE(SdImportTest); @@ -299,6 +300,7 @@ public: CPPUNIT_TEST(testTdf122899); CPPUNIT_TEST(testOOXTheme); CPPUNIT_TEST(testCropToShape); + CPPUNIT_TEST(testTdf127964); CPPUNIT_TEST_SUITE_END(); }; @@ -2823,6 +2825,27 @@ void SdImportTest::testCropToShape() CPPUNIT_ASSERT_EQUAL(css::drawing::BitmapMode_STRETCH, bitmapmode); } +void SdImportTest::testTdf127964() +{ + sd::DrawDocShellRef xDocShRef + = loadURL(m_directories.getURLFromSrc("sd/qa/unit/data/pptx/tdf127964.pptx"), PPTX); + const SdrPage* pPage = GetPage(1, xDocShRef); + const SdrObject* pObj = pPage->GetObj(0); + auto& rFillStyleItem + = dynamic_cast<const XFillStyleItem&>(pObj->GetMergedItem(XATTR_FILLSTYLE)); + CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_SOLID, rFillStyleItem.GetValue()); + + auto& rFillColorItem + = dynamic_cast<const XFillColorItem&>(pObj->GetMergedItem(XATTR_FILLCOLOR)); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 4294967295 + // - Actual : 5210557 + // i.e. instead of transparent (which then got rendered as white), the shape fill color was + // blue. + CPPUNIT_ASSERT_EQUAL(COL_TRANSPARENT, rFillColorItem.GetColorValue()); + xDocShRef->DoClose(); +} + CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTest); CPPUNIT_PLUGIN_IMPLEMENT(); _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
