[Libreoffice-commits] core.git: Branch 'libreoffice-6-3' - include/oox oox/source sd/qa

Miklos Vajna (via logerrit) logerrit at kemper.freedesktop.org
Tue Oct 15 08:53:22 UTC 2019


 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 <vmiklos at collabora.com>
AuthorDate: Mon Oct 14 21:49:32 2019 +0200
Commit:     Xisco FaulĂ­ <xiscofauli at libreoffice.org>
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 <vmiklos at collabora.com>
    (cherry picked from commit 46d630f98f1c07ec2048da35d1a4804181148ac5)
    Reviewed-on: https://gerrit.libreoffice.org/80807
    Reviewed-by: Xisco FaulĂ­ <xiscofauli at libreoffice.org>

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();


More information about the Libreoffice-commits mailing list