[Libreoffice-commits] core.git: include/oox oox/source sd/qa

[Libreoffice Gerrit user]

 include/oox/drawingml/shape.hxx                     |    3 +
 oox/source/drawingml/diagram/diagramlayoutatoms.cxx |    2 +
 oox/source/drawingml/diagram/layoutatomvisitors.cxx |    3 +
 oox/source/drawingml/shape.cxx                      |    6 +++
 sd/qa/unit/data/pptx/smartart-picture-strip.pptx    |binary
 sd/qa/unit/import-tests-smartart.cxx                |   38 ++++++++++++++++++++
 6 files changed, 52 insertions(+)

New commits:
commit 333e9ea15bb57cf1c87ac2ea150de1e3fd79cfcb
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Feb 22 17:12:04 2019 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Fri Feb 22 19:30:16 2019 +0100

    oox smartart, picture strip: handle bitmap fill of pres nodes
    
    There were two problems here:
    
    1) We did not import bitmap fill from presentation nodes.
    
    2) Presentation nodes contained properties with reference semantics, so
    if you set a bitmap fill for a first and a second shape, then at the end
    both shapes contained the second bitmap.
    
    With this, both bitmaps are imported exactly once.
    
    Change-Id: I098a006eb3f58a338400663d57888ca671ed9909
    Reviewed-on: https://gerrit.libreoffice.org/68218
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    Tested-by: Jenkins

diff --git a/include/oox/drawingml/shape.hxx b/include/oox/drawingml/shape.hxx
index 28a5e2907494..c8ffa1b278df 100644
--- a/include/oox/drawingml/shape.hxx
+++ b/include/oox/drawingml/shape.hxx
@@ -220,6 +220,9 @@ public:
 
     sal_Int32 getDataNodeType() const { return mnDataNodeType; }
 
+    /// Changes reference semantics to value semantics for fill properties.
+    void cloneFillProperties();
+
 protected:
 
     css::uno::Reference< css::drawing::XShape > const &
diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 1d6259ef698a..4cf36186322c 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -1258,6 +1258,8 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
                 " processing shape type "
                 << rShape->getCustomShapeProperties()->getShapePresetType()
                 << " for layout node named \"" << msName << "\"");
+        if (pPresNode->mpShape)
+            rShape->getFillProperties().assignUsed(pPresNode->mpShape->getFillProperties());
     }
 
     // TODO(Q1): apply styling & coloring - take presentation
diff --git a/oox/source/drawingml/diagram/layoutatomvisitors.cxx b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
index a73b6566bafe..ff37f816d789 100644
--- a/oox/source/drawingml/diagram/layoutatomvisitors.cxx
+++ b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
@@ -272,6 +272,9 @@ void ShapeTemplateVisitor::visit(ShapeAtom& rAtom)
     // TODO(F3): cloned shape shares all properties by reference,
     // don't change them!
     mpShape.reset(new Shape(pCurrShape));
+    // Fill properties have to be changed as sometimes only the presentation node contains the blip
+    // fill, unshare those.
+    mpShape->cloneFillProperties();
 }
 
 void ShapeLayoutingVisitor::defaultVisit(LayoutAtom const & rAtom)
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index b01aa8ac51ce..ac137e6f7d2f 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -1796,6 +1796,12 @@ uno::Sequence< uno::Sequence< uno::Any > >  Shape::resolveRelationshipsOfTypeFro
     return xRelListTemp;
 }
 
+void Shape::cloneFillProperties()
+{
+    auto pFillProperties = std::make_shared<FillProperties>();
+    pFillProperties->assignUsed(*mpFillPropertiesPtr);
+    mpFillPropertiesPtr = pFillProperties;
+}
 } }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sd/qa/unit/data/pptx/smartart-picture-strip.pptx b/sd/qa/unit/data/pptx/smartart-picture-strip.pptx
new file mode 100644
index 000000000000..4c379dfd396c
Binary files /dev/null and b/sd/qa/unit/data/pptx/smartart-picture-strip.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx
index 2faf360b5b72..22d781950176 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -67,6 +67,7 @@ public:
     void testContinuousBlockProcess();
     void testOrgChart();
     void testCycleMatrix();
+    void testPictureStrip();
 
     CPPUNIT_TEST_SUITE(SdImportTestSmartArt);
 
@@ -99,6 +100,7 @@ public:
     CPPUNIT_TEST(testContinuousBlockProcess);
     CPPUNIT_TEST(testOrgChart);
     CPPUNIT_TEST(testCycleMatrix);
+    CPPUNIT_TEST(testPictureStrip);
 
     CPPUNIT_TEST_SUITE_END();
 };
@@ -905,6 +907,42 @@ void SdImportTestSmartArt::testCycleMatrix()
     xDocShRef->DoClose();
 }
 
+void SdImportTestSmartArt::testPictureStrip()
+{
+    sd::DrawDocShellRef xDocShRef = loadURL(
+        m_directories.getURLFromSrc("/sd/qa/unit/data/pptx/smartart-picture-strip.pptx"), PPTX);
+    uno::Reference<drawing::XShape> xGroup(getShapeFromPage(0, 0, xDocShRef), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xGroup.is());
+
+    uno::Reference<beans::XPropertySet> xFirstImage(getChildShape(getChildShape(xGroup, 0), 1),
+                                                    uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xFirstImage.is());
+    drawing::FillStyle eFillStyle = drawing::FillStyle_NONE;
+    xFirstImage->getPropertyValue("FillStyle") >>= eFillStyle;
+    // Without the accompanying fix in place, this test would have failed: fill style was solid, not
+    // bitmap.
+    CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_BITMAP, eFillStyle);
+
+    uno::Reference<graphic::XGraphic> xGraphic;
+    xFirstImage->getPropertyValue("FillBitmap") >>= xGraphic;
+    Graphic aFirstGraphic(xGraphic);
+
+    uno::Reference<beans::XPropertySet> xSecondImage(getChildShape(getChildShape(xGroup, 1), 1),
+                                                     uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xSecondImage.is());
+    eFillStyle = drawing::FillStyle_NONE;
+    xSecondImage->getPropertyValue("FillStyle") >>= eFillStyle;
+    CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_BITMAP, eFillStyle);
+
+    xSecondImage->getPropertyValue("FillBitmap") >>= xGraphic;
+    Graphic aSecondGraphic(xGraphic);
+    // Without the accompanying fix in place, this test would have failed: both xFirstImage and
+    // xSecondImage had the bitmap fill from the second shape.
+    CPPUNIT_ASSERT(aFirstGraphic.GetChecksum() != aSecondGraphic.GetChecksum());
+
+    xDocShRef->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTestSmartArt);
 
 CPPUNIT_PLUGIN_IMPLEMENT();