Related: tdf#117761 oox smartart: backport fixes related to picture strip

This is a combination of 7 commits.

This is the 1st commit:

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.

(cherry picked from commit 333e9ea15bb57cf1c87ac2ea150de1e3fd79cfcb)

This is the commit #2:

oox smartart, picture strip: fetch # of children only once in snake algo

No functional changes intended.

(cherry picked from commit 90372d52fdcc378473b89f4e6f2de0e206c110ef)

This is the commit #3:

oox smartart, picture strip: expose aspect ratio of children for snake algo

The aspect ratio request of the Shape is not yet used in
AlgAtom::layoutShape(), though.

The heavy-lifting is needed, because the number of cols/rows in the
snake algorithm depends on the aspect ratio request from the child
algorithm, so need to transfer the aspect ratio from child algorithm ->
layout node -> shape -> parent algorithm.

Still no functional changes intended.

(cherry picked from commit a1e10b7968fbf4dba962349be8a6dfb0cb1d3176)

This is the commit #4:

oox smartart, picture strip: fix too many columns with aspect ratio request

The bugdoc has 3 items in the picture strip and PowerPoint laid this out
as a single column with 3 rows (as a snake algorithm). We used to put
the first two items to the first row and the third item to the second
row.

Improve out layout by taking into account what aspect ratio the child
algorithms request: this way it's obvious that we should use a single
column in case we have a large enough aspect ratio and few enough items.

(PowerPoint also uses multiple columns without the aspect ratio
request.)

(cherry picked from commit 159e33ec661b2ce038b2642b2f30600ce7901d1b)

This is the commit #5:

oox smartart, picture strip: fix lack of spacing around the picture list

The snake algorithm in PowerPoint seem to interpret spacing as follows:
if you have N elements, then there should be the requested amount of
spacing between the elements, and also double amount of spacing around
the actual list of elements.

With this, the SmartArt and the title shape in the bugdoc no longer
overlaps.

(cherry picked from commit 0a29c928afa74123bca05dc089c751603d368467)

This is the commit #6:

oox smartart, picture strip: fix lack of margin in text shapes

Shape text has two kind of spacing inside the shape's bounding box: the
shape-level margin and the paragraph-level one. Only the second was
handled in the tx algorithm so far, add support for the first.

The margins taken from constraints were way large by default: the only
explanation I found for that is that SmartArt layout sometimes
calculates in MMs, sometimes in Points, and the ratio between the two is
exactly the Impress / PowerPoint margin. So assume that indeed that unit
difference is the reason for the smaller in-PowerPoint margin values and
do the same on our side.

(cherry picked from commit 279c7f83a57c4d3991930ee80e9d9c287c21270a)

This is the commit #7:

oox smartart, picture strip: fix too wide child shapes

Once the constraints determine the size, the aspect ratio may shrink one
dimension to achieve the requested ratio. Implement the case where a >1
ratio shrinks the width, so the container of the image-text shape pair
has correct aspect ratio.

(cherry picked from commit f4fbb127897ea6afe27055d3b6cfcb0441080902)

Change-Id: I7bac764c031e80bac532c4f97ebd5b5096401096
Reviewed-on: https://gerrit.libreoffice.org/68687
Tested-by: Jenkins
Reviewed-by: Andras Timar <andras.timar@collabora.com>

5 files changed, 148 insertions, 27 deletions

diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 09edf0022259..247b99d0baad 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -65,6 +65,24 @@ bool isFontUnit(sal_Int32 nUnit)
     return nUnit == oox::XML_primFontSz || nUnit == oox::XML_secFontSz;
 }
 
+/// Determines which UNO property should be set for a given constraint type.
+sal_Int32 getPropertyFromConstraint(sal_Int32 nConstraint)
+{
+    switch (nConstraint)
+    {
+        case oox::XML_lMarg:
+            return oox::PROP_TextLeftDistance;
+        case oox::XML_rMarg:
+            return oox::PROP_TextRightDistance;
+        case oox::XML_tMarg:
+            return oox::PROP_TextUpperDistance;
+        case oox::XML_bMarg:
+            return oox::PROP_TextLowerDistance;
+    }
+
+    return 0;
+}
+
 /// Determines the connector shape type from a linear alg.
 sal_Int32 getConnectorType(const oox::drawingml::LayoutNode* pNode)
 {
@@ -246,7 +264,7 @@ void LayoutAtom::dump(int level)
         pAtom->dump(level + 1);
 }
 
-ForEachAtom::ForEachAtom(const LayoutNode& rLayoutNode, const Reference< XFastAttributeList >& xAttributes) :
+ForEachAtom::ForEachAtom(LayoutNode& rLayoutNode, const Reference< XFastAttributeList >& xAttributes) :
     LayoutAtom(rLayoutNode)
 {
     maIter.loadFromXAttr(xAttributes);
@@ -273,7 +291,7 @@ const std::vector<LayoutAtomPtr>& ChooseAtom::getChildren() const
     return maEmptyChildren;
 }
 
-ConditionAtom::ConditionAtom(const LayoutNode& rLayoutNode, bool isElse, const Reference< XFastAttributeList >& xAttributes) :
+ConditionAtom::ConditionAtom(LayoutNode& rLayoutNode, bool isElse, const Reference< XFastAttributeList >& xAttributes) :
     LayoutAtom(rLayoutNode),
     mIsElse(isElse)
 {
@@ -447,6 +465,21 @@ void ConstraintAtom::accept( LayoutAtomVisitor& rVisitor )
 void ConstraintAtom::parseConstraint(std::vector<Constraint>& rConstraints,
                                      bool bRequireForName) const
 {
+    // Whitelist for cases where empty forName is handled.
+    if (bRequireForName)
+    {
+        switch (maConstraint.mnType)
+        {
+            case XML_sp:
+            case XML_lMarg:
+            case XML_rMarg:
+            case XML_tMarg:
+            case XML_bMarg:
+                bRequireForName = false;
+                break;
+        }
+    }
+
     if (bRequireForName && maConstraint.msForName.isEmpty())
         return;
 
@@ -464,7 +497,7 @@ void AlgAtom::accept( LayoutAtomVisitor& rVisitor )
 }
 
 void AlgAtom::layoutShape( const ShapePtr& rShape,
-                           const std::vector<Constraint>& rOwnConstraints ) const
+                           const std::vector<Constraint>& rOwnConstraints )
 {
     // Algorithm result may depend on the parent constraints as well.
     std::vector<Constraint> aMergedConstraints;
@@ -860,6 +893,18 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
             if (rShape->getChildren().empty() || rShape->getSize().Width == 0 || rShape->getSize().Height == 0)
                 break;
 
+            // Parse constraints, only self spacing from height as a start.
+            double fSpaceFromConstraint = 0;
+            for (const auto& rConstr : rConstraints)
+            {
+                if (rConstr.mnRefType == XML_h)
+                {
+                    if (rConstr.mnType == XML_sp && rConstr.msForName.isEmpty())
+                        fSpaceFromConstraint = rConstr.mfFactor;
+                }
+            }
+            bool bSpaceFromConstraints = fSpaceFromConstraint != 0;
+
             const sal_Int32 nDir = maMap.count(XML_grDir) ? maMap.find(XML_grDir)->second : XML_tL;
             sal_Int32 nIncX = 1;
             sal_Int32 nIncY = 1;
@@ -871,38 +916,67 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
                 case XML_bR: nIncX = -1; nIncY = -1; break;
             }
 
-            // TODO: get values from constraints
             sal_Int32 nCount = rShape->getChildren().size();
-            double fSpace = 0.3;
+            // Defaults in case not provided by constraints.
+            double fSpace = bSpaceFromConstraints ? fSpaceFromConstraint : 0.3;
             double fAspectRatio = 0.54; // diagram should not spill outside, earlier it was 0.6
 
             sal_Int32 nCol = 1;
             sal_Int32 nRow = 1;
-            for ( ; nRow<nCount; nRow++)
+            double fChildAspectRatio = rShape->getChildren()[0]->getAspectRatio();
+            if (nCount <= fChildAspectRatio)
+                // Child aspect ratio request (width/height) is N, and we have at most N shapes.
+                // This means we don't need multiple columns.
+                nRow = nCount;
+            else
             {
-                nCol = (nCount+nRow-1) / nRow;
-                const double fShapeHeight = rShape->getSize().Height;
-                const double fShapeWidth = rShape->getSize().Width;
-                if ((fShapeHeight / nCol) / (fShapeWidth / nRow) >= fAspectRatio)
-                    break;
+                for ( ; nRow<nCount; nRow++)
+                {
+                    nCol = (nCount+nRow-1) / nRow;
+                    const double fShapeHeight = rShape->getSize().Height;
+                    const double fShapeWidth = rShape->getSize().Width;
+                    if ((fShapeHeight / nCol) / (fShapeWidth / nRow) >= fAspectRatio)
+                        break;
+                }
             }
 
             SAL_INFO("oox.drawingml", "Snake layout grid: " << nCol << "x" << nRow);
 
             sal_Int32 nWidth = rShape->getSize().Width / (nCol + (nCol-1)*fSpace);
-            const awt::Size aChildSize(nWidth, nWidth * fAspectRatio);
+            awt::Size aChildSize(nWidth, nWidth * fAspectRatio);
+            if (nCol == 1 && nRow > 1)
+            {
+                // We have a single column, so count the height based on the parent height, not
+                // based on width.
+                // Space occurs inside children; also double amount of space is needed outside (on
+                // both sides), if the factor comes from a constraint.
+                sal_Int32 nNumSpaces = -1;
+                if (bSpaceFromConstraints)
+                    nNumSpaces += 4;
+                sal_Int32 nHeight
+                    = rShape->getSize().Height / (nRow + (nRow + nNumSpaces) * fSpace);
+
+                if (fChildAspectRatio > 1)
+                {
+                    // Shrink width if the aspect ratio requires it.
+                    nWidth = std::min(rShape->getSize().Width,
+                                      static_cast<sal_Int32>(nHeight * fChildAspectRatio));
+                    aChildSize = awt::Size(nWidth, nHeight);
+                }
+            }
 
             awt::Point aCurrPos(0, 0);
             if (nIncX == -1)
                 aCurrPos.X = rShape->getSize().Width - aChildSize.Width;
             if (nIncY == -1)
                 aCurrPos.Y = rShape->getSize().Height - aChildSize.Height;
+            else if (bSpaceFromConstraints)
+                // Initial vertical offset to have upper spacing (outside, so double amount).
+                aCurrPos.Y = aChildSize.Height * fSpace * 2;
 
             sal_Int32 nStartX = aCurrPos.X;
             sal_Int32 nColIdx = 0,index = 0;
 
-            sal_Int32 num = rShape->getChildren().size();
-
             const sal_Int32 aContDir = maMap.count(XML_contDir) ? maMap.find(XML_contDir)->second : XML_sameDir;
 
             switch(aContDir)
@@ -922,7 +996,7 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
                     if(++nColIdx == nCol) // condition for next row
                     {
                         // if last row, then position children according to number of shapes.
-                        if((index+1)%nCol!=0 && (index+1)>=3 && ((index+1)/nCol+1)==nRow && num!=nRow*nCol)
+                        if((index+1)%nCol!=0 && (index+1)>=3 && ((index+1)/nCol+1)==nRow && nCount!=nRow*nCol)
                             // position first child of last row
                             aCurrPos.X = nStartX + (nIncX * (aChildSize.Width + fSpace*aChildSize.Width))/2;
                         else
@@ -961,10 +1035,10 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
                     if(++nColIdx == nCol) // condition for next row
                     {
                         // if last row, then position children according to number of shapes.
-                        if((index+1)%nCol!=0 && (index+1)>=4 && ((index+1)/nCol+1)==nRow && num!=nRow*nCol && ((index/nCol)+1)%2==0)
+                        if((index+1)%nCol!=0 && (index+1)>=4 && ((index+1)/nCol+1)==nRow && nCount!=nRow*nCol && ((index/nCol)+1)%2==0)
                             // position first child of last row
                             aCurrPos.X -= aChildSize.Width*3/2;
-                        else if((index+1)%nCol!=0 && (index+1)>=4 && ((index+1)/nCol+1)==nRow && num!=nRow*nCol && ((index/nCol)+1)%2!=0)
+                        else if((index+1)%nCol!=0 && (index+1)>=4 && ((index+1)/nCol+1)==nRow && nCount!=nRow*nCol && ((index/nCol)+1)%2!=0)
                             aCurrPos.X = nStartX + (nIncX * (aChildSize.Width + fSpace*aChildSize.Width))/2;
                         else if(((index/nCol)+1)%2!=0)
                             aCurrPos.X = nStartX;
@@ -997,6 +1071,29 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
         case XML_tx:
         {
             // adjust text alignment
+
+            // Parse constraints, only self margins as a start.
+            for (const auto& rConstr : rConstraints)
+            {
+                if (rConstr.mnRefType == XML_w)
+                {
+                    if (!rConstr.msForName.isEmpty())
+                        continue;
+
+                    sal_Int32 nProperty = getPropertyFromConstraint(rConstr.mnType);
+                    if (!nProperty)
+                        continue;
+
+                    // PowerPoint takes size as points, but gives margin as MMs.
+                    double fFactor = convertPointToMms(rConstr.mfFactor);
+
+                    // DrawingML works in EMUs, UNO API works in MM100s.
+                    sal_Int32 nValue = rShape->getSize().Width * fFactor / EMU_PER_HMM;
+
+                    rShape->getShapeProperties().setProperty(nProperty, nValue);
+                }
+            }
+
             // TODO: adjust text size to fit shape
             TextBodyPtr pTextBody = rShape->getTextBody();
             if (!pTextBody ||
@@ -1220,6 +1317,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/diagramlayoutatoms.hxx b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
index 75149a5cbecd..9b184f917f17 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
@@ -91,10 +91,10 @@ typedef std::shared_ptr< LayoutAtom > LayoutAtomPtr;
 class LayoutAtom
 {
 public:
-    LayoutAtom(const LayoutNode& rLayoutNode) : mrLayoutNode(rLayoutNode) {}
+    LayoutAtom(LayoutNode& rLayoutNode) : mrLayoutNode(rLayoutNode) {}
     virtual ~LayoutAtom() { }
 
-    const LayoutNode& getLayoutNode() const
+    LayoutNode& getLayoutNode()
         { return mrLayoutNode; }
 
     /** visitor acceptance
@@ -127,7 +127,7 @@ public:
     void dump(int level = 0);
 
 protected:
-    const LayoutNode&            mrLayoutNode;
+    LayoutNode&            mrLayoutNode;
     std::vector< LayoutAtomPtr > mpChildNodes;
     std::weak_ptr<LayoutAtom> mpParent;
     OUString                     msName;
@@ -137,7 +137,7 @@ class ConstraintAtom
     : public LayoutAtom
 {
 public:
-    ConstraintAtom(const LayoutNode& rLayoutNode) : LayoutAtom(rLayoutNode) {}
+    ConstraintAtom(LayoutNode& rLayoutNode) : LayoutAtom(rLayoutNode) {}
     virtual void accept( LayoutAtomVisitor& ) override;
     Constraint& getConstraint()
         { return maConstraint; }
@@ -150,7 +150,7 @@ class AlgAtom
     : public LayoutAtom
 {
 public:
-    AlgAtom(const LayoutNode& rLayoutNode) : LayoutAtom(rLayoutNode), mnType(0), maMap() {}
+    AlgAtom(LayoutNode& rLayoutNode) : LayoutAtom(rLayoutNode), mnType(0), maMap() {}
 
     typedef std::map<sal_Int32,sal_Int32> ParamMap;
 
@@ -161,7 +161,7 @@ public:
     void addParam( sal_Int32 nType, sal_Int32 nVal )
         { maMap[nType]=nVal; }
     void layoutShape( const ShapePtr& rShape,
-                      const std::vector<Constraint>& rConstraints ) const;
+                      const std::vector<Constraint>& rConstraints );
 
     /// Gives access to <dgm:alg type="..."/>.
     sal_Int32 getType() const { return mnType; }
@@ -171,6 +171,8 @@ public:
 
     void setAspectRatio(double fAspectRatio) { mfAspectRatio = fAspectRatio; }
 
+    double getAspectRatio() const { return mfAspectRatio; }
+
 private:
     sal_Int32 mnType;
     ParamMap  maMap;
@@ -184,7 +186,7 @@ class ForEachAtom
     : public LayoutAtom
 {
 public:
-    explicit ForEachAtom(const LayoutNode& rLayoutNode, const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttributes);
+    explicit ForEachAtom(LayoutNode& rLayoutNode, const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttributes);
 
     IteratorAttr & iterator()
         { return maIter; }
@@ -200,7 +202,7 @@ class ConditionAtom
     : public LayoutAtom
 {
 public:
-    explicit ConditionAtom(const LayoutNode& rLayoutNode, bool isElse, const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttributes);
+    explicit ConditionAtom(LayoutNode& rLayoutNode, bool isElse, const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttributes);
     virtual void accept( LayoutAtomVisitor& ) override;
     bool getDecision() const;
 private:
@@ -220,7 +222,7 @@ class ChooseAtom
     : public LayoutAtom
 {
 public:
-    ChooseAtom(const LayoutNode& rLayoutNode)
+    ChooseAtom(LayoutNode& rLayoutNode)
         : LayoutAtom(rLayoutNode)
 #if defined __clang__ && __clang_major__ == 3 && __clang_minor__ == 8
         , maEmptyChildren()
@@ -264,6 +266,10 @@ public:
 
     const LayoutNode* getParentLayoutNode() const;
 
+    void setAlgAtom(AlgAtomPtr pAlgAtom) { mpAlgAtom = pAlgAtom; }
+
+    AlgAtomPtr getAlgAtom() const { return mpAlgAtom.lock(); }
+
 private:
     const Diagram&               mrDgm;
     VarMap                       mVariables;
@@ -272,6 +278,7 @@ private:
     ShapePtr                     mpExistingShape;
     std::vector<ShapePtr>        mpNodeShapes;
     sal_Int32                    mnChildOrder;
+    std::weak_ptr<AlgAtom>       mpAlgAtom;
 };
 
 typedef std::shared_ptr< LayoutNode > LayoutNodePtr;
@@ -280,7 +287,7 @@ class ShapeAtom
     : public LayoutAtom
 {
 public:
-    ShapeAtom(const LayoutNode& rLayoutNode, const ShapePtr& pShape) : LayoutAtom(rLayoutNode), mpShapeTemplate(pShape) {}
+    ShapeAtom(LayoutNode& rLayoutNode, const ShapePtr& pShape) : LayoutAtom(rLayoutNode), mpShapeTemplate(pShape) {}
     virtual void accept( LayoutAtomVisitor& ) override;
     const ShapePtr& getShapeTemplate() const
         { return mpShapeTemplate; }
diff --git a/oox/source/drawingml/diagram/layoutatomvisitors.cxx b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
index e2d6c4681b5c..4e69e8c8725b 100644
--- a/oox/source/drawingml/diagram/layoutatomvisitors.cxx
+++ b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
@@ -131,6 +131,8 @@ void ShapeCreationVisitor::visit(LayoutNode& rAtom)
         if (rAtom.setupShape(pShape, pNewNode))
         {
             pShape->setInternalName(rAtom.getName());
+            if (AlgAtomPtr pAlgAtom = rAtom.getAlgAtom())
+                pShape->setAspectRatio(pAlgAtom->getAspectRatio());
             rAtom.addNodeShape(pShape);
         }
     }
@@ -151,6 +153,8 @@ void ShapeCreationVisitor::visit(LayoutNode& rAtom)
             if (rAtom.setupShape(pShape, pNewNode))
             {
                 pShape->setInternalName(rAtom.getName());
+                if (AlgAtomPtr pAlgAtom = rAtom.getAlgAtom())
+                    pShape->setAspectRatio(pAlgAtom->getAspectRatio());
                 pCurrParent->addChild(pShape);
                 pCurrParent = pShape;
                 rAtom.addNodeShape(pShape);
@@ -270,6 +274,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/diagram/layoutnodecontext.cxx b/oox/source/drawingml/diagram/layoutnodecontext.cxx
index 24264135478b..646ab5ecd3da 100644
--- a/oox/source/drawingml/diagram/layoutnodecontext.cxx
+++ b/oox/source/drawingml/diagram/layoutnodecontext.cxx
@@ -230,6 +230,7 @@ LayoutNodeContext::onCreateContext( ::sal_Int32 aElement,
         // CT_Algorithm
         AlgAtomPtr pAtom( new AlgAtom(mpNode->getLayoutNode()) );
         LayoutAtom::connect(mpNode, pAtom);
+        mpNode->getLayoutNode().setAlgAtom(pAtom);
         return new AlgorithmContext( *this, rAttribs, pAtom );
     }
     case DGM_TOKEN( choose ):
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index 55d154642960..4c552194b08a 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -178,6 +178,7 @@ Shape::Shape( const ShapePtr& pSourceShape )
 , mnZOrder(pSourceShape->mnZOrder)
 , mnZOrderOff(pSourceShape->mnZOrderOff)
 , mnDataNodeType(pSourceShape->mnDataNodeType)
+, mfAspectRatio(pSourceShape->mfAspectRatio)
 {}
 
 Shape::~Shape()
@@ -1806,6 +1807,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: */