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

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Thu Jan 24 19:05:59 UTC 2019


 include/oox/drawingml/shape.hxx                         |    7 
 oox/source/drawingml/diagram/datamodelcontext.cxx       |    2 
 oox/source/drawingml/diagram/diagram.hxx                |    3 
 oox/source/drawingml/diagram/diagramfragmenthandler.cxx |    2 
 oox/source/drawingml/diagram/diagramlayoutatoms.cxx     |  255 ++++++++++++++--
 oox/source/drawingml/diagram/layoutatomvisitors.cxx     |    3 
 oox/source/drawingml/shape.cxx                          |    1 
 sd/qa/unit/data/pptx/smartart-org-chart.pptx            |binary
 sd/qa/unit/import-tests-smartart.cxx                    |  145 +++++++++
 9 files changed, 390 insertions(+), 28 deletions(-)

New commits:
commit 0e97e05d57a55d0825b321c0d0402dc619f089bf
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Dec 21 17:51:17 2018 +0100
Commit:     Aron Budea <aron.budea at collabora.com>
CommitDate: Thu Jan 24 20:05:30 2019 +0100

    Related: tdf#117761 oox smartart: backport fixes related to org chart
    
    This is a combination of 10 commits.
    
    This is the 1st commit:
    
    oox smartart, org chart: add initial hierChild/Root algorithms
    
    hierChild is supposed to align and position its child layout nodes in a
    linear path under the hierRoot layout node, so initially just use a
    simple vertical layout algorithm.
    
    (cherry picked from commit 3103f9f9461f6eabb61a70be73862ef4be98010e)
    
    This is the commit #2:
    
    oox smartart, org chart: handle multiple elements in hierChild
    
    In case one manager has multiple employees, then we laid out only the
    first one. Recognize non-assistant type as the node type (as a start) to
    get the correct number of employees (when there are no assistants), and
    also render employees on a horizontal (and not on a vertical) path.
    
    With this, the 1 manager and multiple employees case looks reasonable.
    
    (cherry picked from commit dcd378bf614c99d705312259a0a0a25b40d88e2b)
    
    This is the commit #3:
    
    oox smartart, org chart: fix font color when defined with quick styles
    
    createStyleMatrixContext() assumed that <dgm:style> contains
    <dgm:fontRef>, but it contains <a:fontRef> instead.
    
    This resulted in a 0 mnThemedIdx, which meant that since commit
    89206c472ecf18bfde6824cea8004921cd404365 (bnc#862510: PPTX import: Wrong
    text color inside shape, 2014-12-21) we ignored the theme color in
    oox::drawingml::Shape::createAndInsert().
    
    (cherry picked from commit 5dfd5755c709e91d2903bd7be4582f7832e89780)
    
    This is the commit #4:
    
    oox smartart, org chart: fix vertical order of assistant nodes
    
    It seems the manager -> assistant -> employees ordering is not part of
    the file format. The order is stored twice in the file: the hierRoot
    algorithm has 3 layout nodes as a children, and also the data model has
    an order of the presentation nodes: both describe that employees go
    before assistant nodes.
    
    In contrast to that, PowerPoint orders XML_asst nodes before XML_node
    ones, so teach the hierRoot algorithm about this.
    
    This requires tracking the data model node type for each in-diagram
    drawingML shape, so that layout can determine if a hierRoot algorithm
    children has an assistant node or not.
    
    (cherry picked from commit 22086e70c4f3bb41620ff81ecaf57fd2af6ccbce)
    
    This is the commit #5:
    
    oox smartart, org chart: handle multiple paragraphs on data node
    
    This problem was similar to the one fixed in
    cfa76f538a44d4396574ece59e8a3953c22c6eb7 (oox smartart, accent process:
    handle multiple runs from a data point, 2018-11-21), but this there we
    handled multiple runs and this handles multiple paragraphs.
    
    It seems some smartart types allow multiple paragraphs in a diagram
    node, others only allow multiple runs. Org chart is in the former
    category.
    
    (cherry picked from commit dfc97dd381ef516ca4a7e99b29f9da1033a380f4)
    
    This is the commit #6:
    
    oox smartart, org chart: fix height of manager nodes without employees
    
    Employees and/or assistants reduce the height of managers -- this effect
    is wanted even if there are no employees/assistants.
    
    (cherry picked from commit 8638cc1b737195df16a160b148d2cd2c68131174)
    
    This is the commit #7:
    
    oox smartart, org chart: improve width of non-manager nodes
    
    The default case is that all managers have assistants/employees, so
    nodes under a manager can only use the horizontal space under the
    manager to avoid overlapping.
    
    But in case the previous / next sibling of the manager have no child
    nodes (assistant/employee) then we can use that space to make the child
    nodes larger. This improves readability of the chart's text a lot and
    brings the layout closer to what PowerPoint does for the same input.
    
    Handle all this in the hierChild algorithm, i.e. the container for a
    list of assistants or a list of employees, which means "parent" in this
    context always refers to a manager node.
    
    (cherry picked from commit 3a655975e5ea43417885513d0752da3627dd25ed)
    
    This is the commit #8:
    
    oox smartart, org chart: implement support for hierBranch conditions
    
    The relevant part of the layout is the <dgm:layoutNode
    name="hierChild2"> element that has a <dgm:choose> with two branches:
    
    <dgm:if name="Name34" func="var" arg="hierBranch" op="equ" val="std">
    <dgm:if name="Name36" func="var" arg="hierBranch" op="equ" val="init">
    
    The connectors were missing as we took the first branch
    (ConditionAtom::getDecision() returned true if the arg was hierBranch),
    even hierBranch on the parent layout node was set to "init".
    
    With this, the correct number of connectors are created, previously all
    employee connectors were missing. Their size / position is still
    incorrect, though.
    
    (cherry picked from commit ae34f471030869dfc0da1784597cae6f9131f8c5)
    
    This is the commit #9:
    
    oox smartart, org chart: fix shape type of connectors
    
    PowerPoint renders these as bent connectors, not as arrow shapes.
    
    Also add a bit of vertical spacing between the nodes, otherwise the
    connectors have no way to be visible. Their position is still incorrect,
    though.
    
    (cherry picked from commit 1791e08e9f27453ac5563ef400c715e30c791133)
    
    This is the commit #10:
    
    oox smartart, org chart: fix position and size of connector shapes
    
    Finally the bugdoc rendering result is reasonable and even looks like a
    tree as it should.
    
    (cherry picked from commit 2bfb9117b287a371066a157267614b9bdb367d71)
    
    Change-Id: I4e7a729afd3d2c5af2e7f41903737bd56be406fa
    Reviewed-on: https://gerrit.libreoffice.org/66718
    Tested-by: Jenkins
    Reviewed-by: Aron Budea <aron.budea at collabora.com>

diff --git a/include/oox/drawingml/shape.hxx b/include/oox/drawingml/shape.hxx
index e04a58beb4a6..4a571213a090 100644
--- a/include/oox/drawingml/shape.hxx
+++ b/include/oox/drawingml/shape.hxx
@@ -222,6 +222,10 @@ public:
 
     sal_Int32 getZOrderOff() const { return mnZOrderOff; }
 
+    void setDataNodeType(sal_Int32 nDataNodeType) { mnDataNodeType = nDataNodeType; }
+
+    sal_Int32 getDataNodeType() const { return mnDataNodeType; }
+
 protected:
 
     css::uno::Reference< css::drawing::XShape > const &
@@ -341,6 +345,9 @@ private:
 
     /// Z-Order offset.
     sal_Int32 mnZOrderOff = 0;
+
+    /// Type of data node for an in-diagram shape.
+    sal_Int32 mnDataNodeType = 0;
 };
 
 } }
diff --git a/oox/source/drawingml/diagram/datamodelcontext.cxx b/oox/source/drawingml/diagram/datamodelcontext.cxx
index 326b4f933ed6..b98d0ee87ccf 100644
--- a/oox/source/drawingml/diagram/datamodelcontext.cxx
+++ b/oox/source/drawingml/diagram/datamodelcontext.cxx
@@ -111,7 +111,7 @@ public:
                 mrPoint.mnDirection = rAttribs.getToken( XML_val, XML_norm );
                 break;
             case DGM_TOKEN( hierBranch ):
-                mrPoint.mnHierarchyBranch = rAttribs.getToken( XML_val, XML_std );
+                mrPoint.moHierarchyBranch = rAttribs.getToken( XML_val );
                 break;
             case DGM_TOKEN( orgChart ):
                 mrPoint.mbOrgChartEnabled = rAttribs.getBool( XML_val, false );
diff --git a/oox/source/drawingml/diagram/diagram.hxx b/oox/source/drawingml/diagram/diagram.hxx
index 1a981a858c29..242ff09fa152 100644
--- a/oox/source/drawingml/diagram/diagram.hxx
+++ b/oox/source/drawingml/diagram/diagram.hxx
@@ -73,7 +73,6 @@ struct Point
         mnMaxChildren(-1),
         mnPreferredChildren(-1),
         mnDirection(XML_norm),
-        mnHierarchyBranch(XML_std),
         mnResizeHandles(XML_rel),
         mnCustomAngle(-1),
         mnPercentageNeighbourWidth(-1),
@@ -118,7 +117,7 @@ struct Point
     sal_Int32     mnMaxChildren;
     sal_Int32     mnPreferredChildren;
     sal_Int32     mnDirection;
-    sal_Int32     mnHierarchyBranch;
+    OptValue<sal_Int32> moHierarchyBranch;
     sal_Int32     mnResizeHandles;
     sal_Int32     mnCustomAngle;
     sal_Int32     mnPercentageNeighbourWidth;
diff --git a/oox/source/drawingml/diagram/diagramfragmenthandler.cxx b/oox/source/drawingml/diagram/diagramfragmenthandler.cxx
index 836faa309eca..616f8eb1392d 100644
--- a/oox/source/drawingml/diagram/diagramfragmenthandler.cxx
+++ b/oox/source/drawingml/diagram/diagramfragmenthandler.cxx
@@ -111,7 +111,7 @@ DiagramQStylesFragmentHandler::DiagramQStylesFragmentHandler( XmlFilterBase& rFi
     const AttributeList& rAttribs,
     ShapeStyleRef& o_rStyle )
 {
-    o_rStyle.mnThemedIdx = (nElement == DGM_TOKEN(fontRef)) ?
+    o_rStyle.mnThemedIdx = (nElement == A_TOKEN(fontRef)) ?
         rAttribs.getToken( XML_idx, XML_none ) : rAttribs.getInteger( XML_idx, 0 );
     return new ColorContext( *this, o_rStyle.maPhClr );
 }
diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 502470933e8f..513dc9926e82 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -29,6 +29,7 @@
 #include <drawingml/textparagraph.hxx>
 #include <drawingml/textrun.hxx>
 #include <drawingml/customshapeproperties.hxx>
+#include <tools/gen.hxx>
 
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::uno;
@@ -72,32 +73,130 @@ sal_Int32 getConnectorType(const oox::drawingml::LayoutNode* pNode)
     if (!pNode)
         return nType;
 
+    // This is cheaper than visiting the whole sub-tree.
+    if (pNode->getName().startsWith("hierChild"))
+        return oox::XML_bentConnector3;
+
     for (const auto& pChild : pNode->getChildren())
     {
         auto pAlgAtom = dynamic_cast<oox::drawingml::AlgAtom*>(pChild.get());
         if (!pAlgAtom)
             continue;
 
-        if (pAlgAtom->getType() != oox::XML_lin)
-            continue;
-
-        sal_Int32 nDir = oox::XML_fromL;
-        if (pAlgAtom->getMap().count(oox::XML_linDir))
-            nDir = pAlgAtom->getMap().find(oox::XML_linDir)->second;
-
-        switch (nDir)
+        switch (pAlgAtom->getType())
         {
-            case oox::XML_fromL:
-                nType = oox::XML_rightArrow;
+            case oox::XML_lin:
+            {
+                sal_Int32 nDir = oox::XML_fromL;
+                if (pAlgAtom->getMap().count(oox::XML_linDir))
+                    nDir = pAlgAtom->getMap().find(oox::XML_linDir)->second;
+
+                switch (nDir)
+                {
+                    case oox::XML_fromL:
+                        nType = oox::XML_rightArrow;
+                        break;
+                    case oox::XML_fromR:
+                        nType = oox::XML_leftArrow;
+                        break;
+                }
                 break;
-            case oox::XML_fromR:
-                nType = oox::XML_leftArrow;
+            }
+            case oox::XML_hierChild:
+            {
+                // TODO <dgm:param type="connRout" val="..."/> should be able
+                // to customize this.
+                nType = oox::XML_bentConnector3;
                 break;
+            }
         }
     }
 
     return nType;
 }
+
+/**
+ * Determines if pShape is (or contains) a presentation of a data node of type
+ * nType.
+ */
+bool containsDataNodeType(const oox::drawingml::ShapePtr& pShape, sal_Int32 nType)
+{
+    if (pShape->getDataNodeType() == nType)
+        return true;
+
+    for (const auto& pChild : pShape->getChildren())
+    {
+        if (containsDataNodeType(pChild, nType))
+            return true;
+    }
+
+    return false;
+}
+
+/**
+ * Calculates the offset and scaling for pShape (laid out with the hierChild
+ * algorithm) based on the siblings of pParent.
+ */
+void calculateHierChildOffsetScale(const oox::drawingml::ShapePtr& pShape,
+                                   const oox::drawingml::LayoutNode* pParent, sal_Int32& rXOffset,
+                                   double& rWidthScale)
+{
+    if (!pParent)
+        return;
+
+    const std::vector<oox::drawingml::ShapePtr>& rParents = pParent->getNodeShapes();
+    for (size_t nParent = 0; nParent < rParents.size(); ++nParent)
+    {
+        const oox::drawingml::ShapePtr& pParentShape = rParents[nParent];
+        const std::vector<oox::drawingml::ShapePtr>& rChildren = pParentShape->getChildren();
+        auto it = std::find_if(
+            rChildren.begin(), rChildren.end(),
+            [pShape](const oox::drawingml::ShapePtr& pChild) { return pChild == pShape; });
+        if (it == rChildren.end())
+            // This is not our parent.
+            continue;
+
+        if (nParent > 0)
+        {
+            if (rParents[nParent - 1]->getChildren().size() == 1)
+            {
+                // Previous sibling of our parent has no children: can use that
+                // space, so shift to the left and scale up.
+                rWidthScale += 1.0;
+                rXOffset -= pShape->getSize().Width;
+            }
+        }
+        if (nParent < rParents.size() - 1)
+        {
+            if (rParents[nParent + 1]->getChildren().size() == 1)
+                // Next sibling of our parent has no children: can use that
+                // space, so scale up.
+                rWidthScale += 1.0;
+        }
+    }
+}
+
+/// Sets the position and size of a connector inside a hierChild algorithm.
+void setHierChildConnPosSize(const oox::drawingml::ShapePtr& pShape)
+{
+    // Connect to the top center of the child.
+    awt::Point aShapePoint = pShape->getPosition();
+    awt::Size aShapeSize = pShape->getSize();
+    tools::Rectangle aRectangle(Point(aShapePoint.X, aShapePoint.Y),
+                                Size(aShapeSize.Width, aShapeSize.Height));
+    Point aTo = aRectangle.TopCenter();
+
+    // Connect from the bottom center of the parent.
+    Point aFrom = aTo;
+    aFrom.setY(aFrom.getY() - aRectangle.getHeight() * 0.3);
+
+    tools::Rectangle aRect(aFrom, aTo);
+    aRect.Justify();
+    aShapePoint = awt::Point(aRect.Left(), aRect.Top());
+    aShapeSize = awt::Size(aRect.getWidth(), aRect.getHeight());
+    pShape->setPosition(aShapePoint);
+    pShape->setSize(aShapeSize);
+}
 }
 
 namespace oox { namespace drawingml {
@@ -284,8 +383,32 @@ bool ConditionAtom::getDecision() const
     case XML_var:
     {
         const dgm::Point* pPoint = getPresNode();
-        if (pPoint && maCond.mnArg == XML_dir)
+        if (!pPoint)
+            break;
+
+        if (maCond.mnArg == XML_dir)
             return compareResult(maCond.mnOp, pPoint->mnDirection, maCond.mnVal);
+        else if (maCond.mnArg == XML_hierBranch)
+        {
+            sal_Int32 nHierarchyBranch = pPoint->moHierarchyBranch.get(XML_std);
+            if (!pPoint->moHierarchyBranch.has())
+            {
+                // If <dgm:hierBranch> is missing in the current presentation
+                // point, ask the parent.
+                OUString aParent = navigate(mrLayoutNode, XML_presParOf, pPoint->msModelId,
+                                            /*bSourceToDestination*/ false);
+                DiagramData::PointNameMap& rPointNameMap
+                    = mrLayoutNode.getDiagram().getData()->getPointNameMap();
+                auto it = rPointNameMap.find(aParent);
+                if (it != rPointNameMap.end())
+                {
+                    const dgm::Point* pParent = it->second;
+                    if (pParent->moHierarchyBranch.has())
+                        nHierarchyBranch = pParent->moHierarchyBranch.get();
+                }
+            }
+            return compareResult(maCond.mnOp, nHierarchyBranch, maCond.mnVal);
+        }
         break;
     }
 
@@ -445,6 +568,12 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
 
                 rShape->setSubType(nType);
                 rShape->getCustomShapeProperties()->setShapePresetType(nType);
+
+                if (nType == XML_bentConnector3)
+                {
+                    setHierChildConnPosSize(rShape);
+                    break;
+                }
             }
 
             // Parse constraints to adjust the size.
@@ -522,7 +651,81 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
 
         case XML_hierChild:
         case XML_hierRoot:
+        {
+            // hierRoot is the manager -> employees vertical linear path,
+            // hierChild is the first employee -> last employee horizontal
+            // linear path.
+            const sal_Int32 nDir = mnType == XML_hierRoot ? XML_fromT : XML_fromL;
+            if (rShape->getChildren().empty() || rShape->getSize().Width == 0
+                || rShape->getSize().Height == 0)
+                break;
+
+            sal_Int32 nCount = rShape->getChildren().size();
+
+            if (mnType == XML_hierChild)
+            {
+                // Connectors should not influence the size of non-connect
+                // shapes.
+                nCount = std::count_if(
+                    rShape->getChildren().begin(), rShape->getChildren().end(),
+                    [](const ShapePtr& pShape) { return pShape->getSubType() != XML_conn; });
+            }
+
+            // A manager node's height should be independent from if it has
+            // assistants and employees, compensate for that.
+            bool bTop = mnType == XML_hierRoot && rShape->getInternalName() == "hierRoot1";
+
+            // Add spacing, so connectors have a chance to be visible.
+            double fSpace = (nCount > 1 || bTop) ? 0.3 : 0;
+
+            double fHeightScale = 1.0;
+            if (mnType == XML_hierRoot && nCount < 3 && bTop)
+                fHeightScale = fHeightScale * nCount / 3;
+
+            if (mnType == XML_hierRoot && nCount == 3)
+            {
+                // Order assistant nodes above employee nodes.
+                std::vector<ShapePtr>& rChildren = rShape->getChildren();
+                if (!containsDataNodeType(rChildren[1], XML_asst)
+                    && containsDataNodeType(rChildren[2], XML_asst))
+                    std::swap(rChildren[1], rChildren[2]);
+            }
+
+            sal_Int32 nXOffset = 0;
+            double fWidthScale = 1.0;
+            if (mnType == XML_hierChild)
+                calculateHierChildOffsetScale(rShape, pParent, nXOffset, fWidthScale);
+
+            awt::Size aChildSize = rShape->getSize();
+            if (nDir == XML_fromT)
+            {
+                aChildSize.Height /= (nCount + nCount * fSpace);
+            }
+            else
+                aChildSize.Width /= nCount;
+            aChildSize.Height *= fHeightScale;
+            aChildSize.Width *= fWidthScale;
+
+            awt::Point aChildPos(nXOffset, 0);
+            for (auto& pChild : rShape->getChildren())
+            {
+                pChild->setPosition(aChildPos);
+                pChild->setSize(aChildSize);
+                pChild->setChildSize(aChildSize);
+
+                if (mnType == XML_hierChild && pChild->getSubType() == XML_conn)
+                    // Connectors should not influence the position of
+                    // non-connect shapes.
+                    continue;
+
+                if (nDir == XML_fromT)
+                    aChildPos.Y += aChildSize.Height + aChildSize.Height * fSpace;
+                else
+                    aChildPos.X += aChildSize.Width;
+            }
+
             break;
+        }
 
         case XML_lin:
         {
@@ -896,6 +1099,7 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
         while( aVecIter != aVecEnd )
         {
             DiagramData::PointNameMap& rMap = mrDgm.getData()->getPointNameMap();
+            // pPresNode is the presentation node of the aDataNode2 data node.
             DiagramData::PointNameMap::const_iterator aDataNode2 = rMap.find(aVecIter->first);
             if (aDataNode2 == rMap.end())
             {
@@ -904,6 +1108,8 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
                 continue;
             }
 
+            rShape->setDataNodeType(aDataNode2->second->mnType);
+
             if( aVecIter->second == 0 )
             {
                 // grab shape attr from topmost element(s)
@@ -941,16 +1147,19 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
                     rShape->setTextBody(pTextBody);
                 }
 
-                TextParagraph& rPara=pTextBody->addParagraph();
-                if( aVecIter->second != -1 )
-                    rPara.getProperties().setLevel(aVecIter->second);
-
-                std::shared_ptr<TextParagraph> pSourceParagraph
-                    = aDataNode2->second->mpShape->getTextBody()->getParagraphs().front();
-                for (const auto& pRun : pSourceParagraph->getRuns())
-                    rPara.addRun(pRun);
-                rPara.getProperties().apply(
-                    aDataNode2->second->mpShape->getTextBody()->getParagraphs().front()->getProperties());
+                const TextParagraphVector& rSourceParagraphs
+                    = aDataNode2->second->mpShape->getTextBody()->getParagraphs();
+                for (const auto& pSourceParagraph : rSourceParagraphs)
+                {
+                    TextParagraph& rPara = pTextBody->addParagraph();
+                    if (aVecIter->second != -1)
+                        rPara.getProperties().setLevel(aVecIter->second);
+
+                    for (const auto& pRun : pSourceParagraph->getRuns())
+                        rPara.addRun(pRun);
+                    const TextBodyPtr& rBody = aDataNode2->second->mpShape->getTextBody();
+                    rPara.getProperties().apply(rBody->getParagraphs().front()->getProperties());
+                }
             }
 
             ++aVecIter;
diff --git a/oox/source/drawingml/diagram/layoutatomvisitors.cxx b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
index 49a664c1e821..e2d6c4681b5c 100644
--- a/oox/source/drawingml/diagram/layoutatomvisitors.cxx
+++ b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
@@ -57,7 +57,8 @@ void ShapeCreationVisitor::visit(ForEachAtom& rAtom)
     const std::vector<LayoutAtomPtr>& rChildren=rAtom.getChildren();
 
     sal_Int32 nChildren=1;
-    if( rAtom.iterator().mnPtType == XML_node )
+    // Approximate the non-assistant type with the node type.
+    if (rAtom.iterator().mnPtType == XML_node || rAtom.iterator().mnPtType == XML_nonAsst)
     {
         // count child data nodes - check all child Atoms for "name"
         // attribute that is contained in diagram's
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index 16bc511743b1..55d154642960 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -177,6 +177,7 @@ Shape::Shape( const ShapePtr& pSourceShape )
 , maDiagramDoms( pSourceShape->maDiagramDoms )
 , mnZOrder(pSourceShape->mnZOrder)
 , mnZOrderOff(pSourceShape->mnZOrderOff)
+, mnDataNodeType(pSourceShape->mnDataNodeType)
 {}
 
 Shape::~Shape()
diff --git a/sd/qa/unit/data/pptx/smartart-org-chart.pptx b/sd/qa/unit/data/pptx/smartart-org-chart.pptx
new file mode 100644
index 000000000000..08c9a4fc916d
Binary files /dev/null and b/sd/qa/unit/data/pptx/smartart-org-chart.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx
index b83de20890b1..9de4061a072b 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -18,6 +18,23 @@
 
 using namespace ::com::sun::star;
 
+namespace
+{
+/// Gets one child of xShape, which one is specified by nIndex.
+uno::Reference<drawing::XShape> getChildShape(const uno::Reference<drawing::XShape>& xShape, sal_Int32 nIndex)
+{
+    uno::Reference<container::XIndexAccess> xGroup(xShape, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xGroup.is());
+
+    CPPUNIT_ASSERT(xGroup->getCount() > nIndex);
+
+    uno::Reference<drawing::XShape> xRet(xGroup->getByIndex(nIndex), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xRet.is());
+
+    return xRet;
+}
+}
+
 class SdImportTestSmartArt : public SdModelTestBase
 {
 public:
@@ -33,6 +50,7 @@ public:
     void testTableList();
     void testAccentProcess();
     void testContinuousBlockProcess();
+    void testOrgChart();
 
     CPPUNIT_TEST_SUITE(SdImportTestSmartArt);
 
@@ -48,6 +66,7 @@ public:
     CPPUNIT_TEST(testTableList);
     CPPUNIT_TEST(testAccentProcess);
     CPPUNIT_TEST(testContinuousBlockProcess);
+    CPPUNIT_TEST(testOrgChart);
 
     CPPUNIT_TEST_SUITE_END();
 };
@@ -402,6 +421,132 @@ void SdImportTestSmartArt::testContinuousBlockProcess()
     xDocShRef->DoClose();
 }
 
+void SdImportTestSmartArt::testOrgChart()
+{
+    // Simple org chart with 1 manager and 1 employee only.
+    sd::DrawDocShellRef xDocShRef = loadURL(
+        m_directories.getURLFromSrc("/sd/qa/unit/data/pptx/smartart-org-chart.pptx"),
+        PPTX);
+    uno::Reference<drawing::XShape> xGroup(getShapeFromPage(0, 0, xDocShRef), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xGroup.is());
+
+    uno::Reference<text::XText> xManager(
+        getChildShape(getChildShape(getChildShape(xGroup, 0), 0), 0), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xManager.is());
+    // Without the accompanying fix in place, this test would have failed: this
+    // was just "Manager", and the second paragraph was lost.
+    OUString aExpected("Manager\nSecond para");
+    CPPUNIT_ASSERT_EQUAL(aExpected, xManager->getString());
+
+    uno::Reference<container::XEnumerationAccess> xParaEnumAccess(xManager, uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xParaEnum = xParaEnumAccess->createEnumeration();
+    uno::Reference<text::XTextRange> xPara(xParaEnum->nextElement(), uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> xRunEnumAccess(xPara, uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xRunEnum = xRunEnumAccess->createEnumeration();
+    uno::Reference<beans::XPropertySet> xRun(xRunEnum->nextElement(), uno::UNO_QUERY);
+    sal_Int32 nActualColor = xRun->getPropertyValue("CharColor").get<sal_Int32>();
+    // Without the accompanying fix in place, this test would have failed: the
+    // "Manager" font color was black, not white.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0xffffff), nActualColor);
+
+    uno::Reference<drawing::XShape> xManagerShape(xManager, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xManagerShape.is());
+
+    awt::Point aManagerPos = xManagerShape->getPosition();
+    awt::Size aManagerSize = xManagerShape->getSize();
+
+    // Make sure that the manager has 2 employees.
+    uno::Reference<drawing::XShapes> xEmployees(getChildShape(getChildShape(xGroup, 0), 2),
+                                                uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xEmployees.is());
+    // 4 children: connector, 1st employee, connector, 2nd employee.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(4), xEmployees->getCount());
+
+    uno::Reference<text::XText> xEmployee(
+        getChildShape(
+            getChildShape(getChildShape(getChildShape(getChildShape(xGroup, 0), 2), 1), 0), 0),
+        uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xEmployee.is());
+    CPPUNIT_ASSERT_EQUAL(OUString("Employee"), xEmployee->getString());
+
+    uno::Reference<drawing::XShape> xEmployeeShape(xEmployee, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xEmployeeShape.is());
+
+    awt::Point aEmployeePos = xEmployeeShape->getPosition();
+    awt::Size aEmployeeSize = xEmployeeShape->getSize();
+
+    CPPUNIT_ASSERT_EQUAL(aManagerPos.X, aEmployeePos.X);
+
+    // Without the accompanying fix in place, this test would have failed: the
+    // two shapes were overlapping, i.e. "manager" was not above "employee".
+    CPPUNIT_ASSERT_GREATER(aManagerPos.Y, aEmployeePos.Y);
+
+    // Make sure that the second employee is on the right of the first one.
+    // Without the accompanying fix in place, this test would have failed, as
+    // the second employee was below the first one.
+    uno::Reference<text::XText> xEmployee2(
+        getChildShape(
+            getChildShape(getChildShape(getChildShape(getChildShape(xGroup, 0), 2), 3), 0), 0),
+        uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xEmployee2.is());
+    CPPUNIT_ASSERT_EQUAL(OUString("Employee2"), xEmployee2->getString());
+
+    uno::Reference<drawing::XShape> xEmployee2Shape(xEmployee2, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xEmployee2Shape.is());
+
+    awt::Point aEmployee2Pos = xEmployee2Shape->getPosition();
+    awt::Size aEmployee2Size = xEmployee2Shape->getSize();
+    CPPUNIT_ASSERT_GREATER(aEmployeePos.X, aEmployee2Pos.X);
+
+    // Make sure that assistant is above employees.
+    uno::Reference<text::XText> xAssistant(
+        getChildShape(
+            getChildShape(getChildShape(getChildShape(getChildShape(xGroup, 0), 1), 1), 0), 0),
+        uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("Assistant"), xAssistant->getString());
+
+    uno::Reference<drawing::XShape> xAssistantShape(xAssistant, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xAssistantShape.is());
+
+    awt::Point aAssistantPos = xAssistantShape->getPosition();
+    // Without the accompanying fix in place, this test would have failed: the
+    // assistant shape was below the employee shape.
+    CPPUNIT_ASSERT_GREATER(aAssistantPos.Y, aEmployeePos.Y);
+
+    // Make sure the connector of the assistant is above the shape.
+    uno::Reference<drawing::XShape> xAssistantConnector(
+        getChildShape(getChildShape(getChildShape(xGroup, 0), 1), 0), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xAssistantConnector.is());
+    awt::Point aAssistantConnectorPos = xAssistantConnector->getPosition();
+    // This failed, the vertical positions of the connector and the shape of
+    // the assistant were the same.
+    CPPUNIT_ASSERT_LESS(aAssistantPos.Y, aAssistantConnectorPos.Y);
+
+    // Make sure the height of xManager and xManager2 is the same.
+    uno::Reference<text::XText> xManager2(
+        getChildShape(getChildShape(getChildShape(xGroup, 1), 0), 0), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xManager2.is());
+    CPPUNIT_ASSERT_EQUAL(OUString("Manager2"), xManager2->getString());
+
+    uno::Reference<drawing::XShape> xManager2Shape(xManager2, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xManager2Shape.is());
+
+    awt::Size aManager2Size = xManager2Shape->getSize();
+    // Without the accompanying fix in place, this test would have failed:
+    // xManager2's height was 3 times larger than xManager's height.
+    CPPUNIT_ASSERT_EQUAL(aManagerSize.Height, aManager2Size.Height);
+
+    // Make sure the employee nodes use the free space on the right, since
+    // manager2 has no assistants / employees.
+    CPPUNIT_ASSERT_GREATER(aManagerSize.Width, aEmployeeSize.Width + aEmployee2Size.Width);
+
+    // Without the accompanying fix in place, this test would have failed: an
+    // employee was exactly the third of the total height, without any spacing.
+    CPPUNIT_ASSERT_LESS(xGroup->getSize().Height / 3, aEmployeeSize.Height);
+
+    xDocShRef->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTestSmartArt);
 
 CPPUNIT_PLUGIN_IMPLEMENT();


More information about the Libreoffice-commits mailing list