Kaydet (Commit) 53c3cbd7 authored tarafından Miklos Vajna's avatar Miklos Vajna Kaydeden (comit) Caolán McNamara

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 3103f9f9)

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 dcd378bf)

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
89206c47 (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 5dfd5755)

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 22086e70)

This is the commit #5:

oox smartart, org chart: handle multiple paragraphs on data node

This problem was similar to the one fixed in
cfa76f53 (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 dfc97dd3)

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 8638cc1b)

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 3a655975)

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 ae34f471)

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 1791e08e)

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/66693
Tested-by: Jenkins
Reviewed-by: 's avatarCaolán McNamara <caolanm@redhat.com>
Tested-by: 's avatarCaolán McNamara <caolanm@redhat.com>
üst e65b94e6
......@@ -214,6 +214,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 &
......@@ -333,6 +337,9 @@ private:
/// Z-Order offset.
sal_Int32 mnZOrderOff = 0;
/// Type of data node for an in-diagram shape.
sal_Int32 mnDataNodeType = 0;
};
} }
......
......@@ -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 );
......
......@@ -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;
......
......@@ -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 );
}
......
......@@ -59,7 +59,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
......
......@@ -177,6 +177,7 @@ Shape::Shape( const ShapePtr& pSourceShape )
, maDiagramDoms( pSourceShape->maDiagramDoms )
, mnZOrder(pSourceShape->mnZOrder)
, mnZOrderOff(pSourceShape->mnZOrderOff)
, mnDataNodeType(pSourceShape->mnDataNodeType)
{}
Shape::~Shape()
......
......@@ -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:
......@@ -47,6 +64,7 @@ public:
void testTableList();
void testAccentProcess();
void testContinuousBlockProcess();
void testOrgChart();
CPPUNIT_TEST_SUITE(SdImportTestSmartArt);
......@@ -76,6 +94,7 @@ public:
CPPUNIT_TEST(testTableList);
CPPUNIT_TEST(testAccentProcess);
CPPUNIT_TEST(testContinuousBlockProcess);
CPPUNIT_TEST(testOrgChart);
CPPUNIT_TEST_SUITE_END();
};
......@@ -576,6 +595,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();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment