include/oox/drawingml/shape.hxx | 7 ++++ oox/source/drawingml/diagram/diagramlayoutatoms.cxx | 30 ++++++++++++++++++++ oox/source/drawingml/shape.cxx | 1 sd/qa/unit/data/pptx/smartart-org-chart.pptx |binary sd/qa/unit/import-tests-smartart.cxx | 21 ++++++++++++-- 5 files changed, 56 insertions(+), 3 deletions(-)
New commits: commit 22086e70c4f3bb41620ff81ecaf57fd2af6ccbce Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Wed Jan 9 17:49:14 2019 +0100 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Jan 10 09:07:04 2019 +0100 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. Change-Id: Ib81f3666fb092ed3b036d830f69ba7e1b94f8331 Reviewed-on: https://gerrit.libreoffice.org/66048 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/include/oox/drawingml/shape.hxx b/include/oox/drawingml/shape.hxx index 21972666f5f6..9dd643b34ae8 100644 --- a/include/oox/drawingml/shape.hxx +++ b/include/oox/drawingml/shape.hxx @@ -215,6 +215,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 & @@ -335,6 +339,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/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx index 056164276283..dd762a9bc77a 100644 --- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx +++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx @@ -99,6 +99,24 @@ sal_Int32 getConnectorType(const oox::drawingml::LayoutNode* pNode) 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; +} } namespace oox { namespace drawingml { @@ -534,6 +552,15 @@ void AlgAtom::layoutShape( const ShapePtr& rShape, sal_Int32 nCount = rShape->getChildren().size(); + 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]); + } + awt::Size aChildSize = rShape->getSize(); if (nDir == XML_fromT) aChildSize.Height /= nCount; @@ -972,6 +999,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()) { @@ -980,6 +1008,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) diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx index 8449082bc774..a3891cb44746 100644 --- a/oox/source/drawingml/shape.cxx +++ b/oox/source/drawingml/shape.cxx @@ -178,6 +178,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 index 3be3473fd6c4..259a9f5a1d13 100644 Binary files a/sd/qa/unit/data/pptx/smartart-org-chart.pptx 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 1cde30c4e140..45cc9b6e5523 100644 --- a/sd/qa/unit/import-tests-smartart.cxx +++ b/sd/qa/unit/import-tests-smartart.cxx @@ -719,14 +719,14 @@ void SdImportTestSmartArt::testOrgChart() // Make sure that the manager has 2 employees. // Without the accompanying fix in place, this test would have failed with // 'Expected: 2; Actual : 1'. - uno::Reference<drawing::XShapes> xEmployees(getChildShape(getChildShape(xGroup, 0), 1), + uno::Reference<drawing::XShapes> xEmployees(getChildShape(getChildShape(xGroup, 0), 2), uno::UNO_QUERY); CPPUNIT_ASSERT(xEmployees.is()); CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2), xEmployees->getCount()); uno::Reference<text::XText> xEmployee( getChildShape( - getChildShape(getChildShape(getChildShape(getChildShape(xGroup, 0), 1), 0), 0), 0), + getChildShape(getChildShape(getChildShape(getChildShape(xGroup, 0), 2), 0), 0), 0), uno::UNO_QUERY); CPPUNIT_ASSERT(xEmployee.is()); CPPUNIT_ASSERT_EQUAL(OUString("Employee"), xEmployee->getString()); @@ -747,7 +747,7 @@ void SdImportTestSmartArt::testOrgChart() // the second employee was below the first one. uno::Reference<text::XText> xEmployee2( getChildShape( - getChildShape(getChildShape(getChildShape(getChildShape(xGroup, 0), 1), 1), 0), 0), + getChildShape(getChildShape(getChildShape(getChildShape(xGroup, 0), 2), 1), 0), 0), uno::UNO_QUERY); CPPUNIT_ASSERT(xEmployee2.is()); CPPUNIT_ASSERT_EQUAL(OUString("Employee2"), xEmployee2->getString()); @@ -758,6 +758,21 @@ void SdImportTestSmartArt::testOrgChart() awt::Point aEmployee2Pos = xEmployee2Shape->getPosition(); 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); + xDocShRef->DoClose(); } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits