[Libreoffice-commits] core.git: Branch 'libreoffice-6-2' - oox/source sd/qa
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Thu Feb 21 21:33:10 UTC 2019
oox/source/drawingml/diagram/diagram.cxx | 11 +-
oox/source/drawingml/diagram/diagram.hxx | 8 +
oox/source/drawingml/diagram/diagramlayoutatoms.cxx | 71 ++++++++++++---
oox/source/drawingml/diagram/diagramlayoutatoms.hxx | 4
oox/source/drawingml/diagram/layoutnodecontext.cxx | 16 ++-
sd/qa/unit/data/pptx/smartart-cycle-matrix.pptx |binary
sd/qa/unit/import-tests-smartart.cxx | 93 ++++++++++++++++++++
7 files changed, 181 insertions(+), 22 deletions(-)
New commits:
commit a6d9ccf26910e35f22ccec8a75472e2fa947abe1
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Thu Feb 7 16:26:31 2019 +0100
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Thu Feb 21 22:32:44 2019 +0100
Related: tdf#117761 oox smartart: backport fixes related to cycle matrix
This is a combination of 6 commits.
This is the 1st commit:
oox smartart, cycle matrix: fix counting presentation children
The markup is:
<dgm:if name="Name6" axis="ch ch" ptType="node node" st="1 1" cnt="1 0" func="cnt" op="gte" val="1">
Where PowerPoint evaluated the condition to true, but Impress evaluated
to false. This means that the undocumented relation between the child
lists is "OR" (not "AND").
Also, our code assumed that "node" has to be a data node (not
presentation node), but it seems the only way this condition can be true
if presentation children is also counted. (The presentation node in
question is not a presentation of anything.)
(cherry picked from commit e3c6f249c10f7f1bcc528e643f5723288c514b29)
This is the commit #2:
oox smartart, cycle matrix: handle left/bottom constraint in composite algo
The bugdoc has 3 shapes in the "outer" circle which have a position
where either x or y is not 0. But these are defined using constraints
talking about the right or bottom edge of the shape.
Map that to top/left, given that we already know the shape size.
(cherry picked from commit b9b4e9223b6c0d6e0b48b694c9aabbe54a250660)
This is the commit #3:
oox smartart, cycle matrix: fix too large height in composite algo
The user-level problem was that the height of the entire smartart was
too large. The reason for this was that:
- composite algorithm gets the constraint height should be 77% of width,
this means 6096000 -> 4693920 EMUs
- at the same time the parent container is already smaller, 4064000 EMUs
- a few lines later we already limit the max height with std::min(), but
in the meantime an incorrect y position is calculated, exactly due to
the lack of early limited height
Solve the problem by making sure composite algorithm never works with a
height (even when using it to calculate vertical center) that exceeds
the height of the parent.
(cherry picked from commit 5b2e38e0cfc7006d6982f741cf158a8a98dc8630)
This is the commit #4:
oox smartart, cycle matrix: handle aspect ratio in composite algo
This way the 4 quadrant shapes in the center of the SmartArt form a
circle, as width is shrinking.
It's not height growing, as OOXML spec clearly says "ar" always just
shrinks one axis.
(>1 and <1 "ar" is to be handled when they are seen in action in an
actual document.)
(cherry picked from commit 34383064ac061497b0c46c449313877c6b6a2087)
This is the commit #5:
oox smartart, cycle matrix: handle destination order in connections
It is possible to have connections from multiple data nodes to the same
presentation node with a presOf type. We use to order these based on as
they appear in the data XML, but we need to order them according to the
destOrd attribute.
Introduce an std::map for that, so get ordering automatically as we
iterate. Turn the std::pair into a struct to make the code a bit more
readable.
(cherry picked from commit ecb733da58b74714eb66d2063a2835ce5c471870)
Conflicts:
oox/source/drawingml/diagram/diagramlayoutatoms.cxx
This is the commit #6:
oox smartart, cycle matrix: fix fill and line props of shape
The topmost shape may not have 0 depth, but something larger.
In that case at least it's safe to still use fill & line properties. The
B1 quadrant of the test file now has the proper orange background, and
B2's border is also properly orange.
(cherry picked from commit 8193e697d286595aa62859011761adeb002244e3)
Conflicts:
oox/source/drawingml/diagram/diagramlayoutatoms.cxx
Change-Id: Iccc5f6993693a0f1cf8f50d163003c24d3ad690e
Reviewed-on: https://gerrit.libreoffice.org/68144
Tested-by: Jenkins
Reviewed-by: Andras Timar <andras.timar at collabora.com>
diff --git a/oox/source/drawingml/diagram/diagram.cxx b/oox/source/drawingml/diagram/diagram.cxx
index 64dc5dde6d91..d1de1c72f94d 100644
--- a/oox/source/drawingml/diagram/diagram.cxx
+++ b/oox/source/drawingml/diagram/diagram.cxx
@@ -271,8 +271,10 @@ void Diagram::build( )
if( connection.mnType == XML_presOf )
{
DiagramData::StringMap::value_type::second_type& rVec=getData()->getPresOfNameMap()[connection.msDestId];
- rVec.emplace_back(
- connection.msSourceId,sal_Int32(0));
+ DiagramData::SourceIdAndDepth aSourceIdAndDepth;
+ aSourceIdAndDepth.msSourceId = connection.msSourceId;
+ aSourceIdAndDepth.mnDepth = 0;
+ rVec[connection.mnDestOrder] = aSourceIdAndDepth;
}
}
@@ -282,9 +284,8 @@ void Diagram::build( )
{
for (auto & elem : elemPresOf.second)
{
- const sal_Int32 nDepth=calcDepth(elem.first,
- getData()->getConnections());
- elem.second = nDepth != 0 ? nDepth : -1;
+ const sal_Int32 nDepth = calcDepth(elem.second.msSourceId, getData()->getConnections());
+ elem.second.mnDepth = nDepth != 0 ? nDepth : -1;
if (nDepth > getData()->getMaxDepth())
getData()->setMaxDepth(nDepth);
}
diff --git a/oox/source/drawingml/diagram/diagram.hxx b/oox/source/drawingml/diagram/diagram.hxx
index 242ff09fa152..a0955b124230 100644
--- a/oox/source/drawingml/diagram/diagram.hxx
+++ b/oox/source/drawingml/diagram/diagram.hxx
@@ -160,8 +160,14 @@ public:
typedef std::map< OUString,
std::vector<dgm::Point*> > PointsNameMap;
typedef std::map< OUString, const dgm::Connection* > ConnectionNameMap;
+ struct SourceIdAndDepth
+ {
+ OUString msSourceId;
+ sal_Int32 mnDepth = 0;
+ };
+ /// Tracks connections: destination id -> {destination order, details} map.
typedef std::map< OUString,
- std::vector<std::pair<OUString,sal_Int32> > > StringMap;
+ std::map<sal_Int32, SourceIdAndDepth > > StringMap;
DiagramData();
FillPropertiesPtr & getFillProperties()
diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 7758ad3a1910..869b02180a89 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -370,6 +370,14 @@ sal_Int32 ConditionAtom::getNodeCount() const
if (aCxn.mnType == XML_parOf && aCxn.msSourceId == sNodeId)
nCount++;
}
+ else
+ {
+ // No presentation child is a presentation of a model node: just
+ // count presentation children.
+ for (const auto& aCxn : mrLayoutNode.getDiagram().getData()->getConnections())
+ if (aCxn.mnType == XML_presParOf && aCxn.msSourceId == pPoint->msModelId)
+ nCount++;
+ }
}
return nCount;
}
@@ -483,12 +491,29 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
LayoutPropertyMap aProperties;
LayoutProperty& rParent = aProperties[""];
- rParent[XML_w] = rShape->getSize().Width;
- rParent[XML_h] = rShape->getSize().Height;
- rParent[XML_l] = 0;
- rParent[XML_t] = 0;
- rParent[XML_r] = rShape->getSize().Width;
- rParent[XML_b] = rShape->getSize().Height;
+
+ sal_Int32 nParentXOffset = 0;
+ if (mfAspectRatio != 1.0)
+ {
+ rParent[XML_w] = rShape->getSize().Width;
+ rParent[XML_h] = rShape->getSize().Height;
+ rParent[XML_l] = 0;
+ rParent[XML_t] = 0;
+ rParent[XML_r] = rShape->getSize().Width;
+ rParent[XML_b] = rShape->getSize().Height;
+ }
+ else
+ {
+ // Shrink width to be only as large as height.
+ rParent[XML_w] = std::min(rShape->getSize().Width, rShape->getSize().Height);
+ rParent[XML_h] = rShape->getSize().Height;
+ if (rParent[XML_w] < rShape->getSize().Width)
+ nParentXOffset = (rShape->getSize().Width - rParent[XML_w]) / 2;
+ rParent[XML_l] = nParentXOffset;
+ rParent[XML_t] = 0;
+ rParent[XML_r] = rShape->getSize().Width - rParent[XML_l];
+ rParent[XML_b] = rShape->getSize().Height;
+ }
for (const auto & rConstr : rConstraints)
{
@@ -527,25 +552,30 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
LayoutProperty::const_iterator it, it2;
if ( (it = rProp.find(XML_w)) != rProp.end() )
- aSize.Width = it->second;
+ aSize.Width = std::min(it->second, rShape->getSize().Width);
if ( (it = rProp.find(XML_h)) != rProp.end() )
- aSize.Height = it->second;
+ aSize.Height = std::min(it->second, rShape->getSize().Height);
if ( (it = rProp.find(XML_l)) != rProp.end() )
aPos.X = it->second;
else if ( (it = rProp.find(XML_ctrX)) != rProp.end() )
aPos.X = it->second - aSize.Width/2;
+ else if ((it = rProp.find(XML_r)) != rProp.end())
+ aPos.X = it->second - aSize.Width;
if ( (it = rProp.find(XML_t)) != rProp.end())
aPos.Y = it->second;
else if ( (it = rProp.find(XML_ctrY)) != rProp.end() )
aPos.Y = it->second - aSize.Height/2;
+ else if ((it = rProp.find(XML_b)) != rProp.end())
+ aPos.Y = it->second - aSize.Height;
if ( (it = rProp.find(XML_l)) != rProp.end() && (it2 = rProp.find(XML_r)) != rProp.end() )
aSize.Width = it2->second - it->second;
if ( (it = rProp.find(XML_t)) != rProp.end() && (it2 = rProp.find(XML_b)) != rProp.end() )
aSize.Height = it2->second - it->second;
+ aPos.X += nParentXOffset;
aSize.Width = std::min(aSize.Width, rShape->getSize().Width - aPos.X);
aSize.Height = std::min(aSize.Height, rShape->getSize().Height - aPos.Y);
}
@@ -1128,11 +1158,21 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
{
DiagramData::StringMap::value_type::second_type::const_iterator aVecIter=aNodeName->second.begin();
const DiagramData::StringMap::value_type::second_type::const_iterator aVecEnd=aNodeName->second.end();
+ // Calculate the depth of what is effectively the topmost element.
+ sal_Int32 nMinDepth = std::numeric_limits<sal_Int32>::max();
+ for (const auto& rPair : aNodeName->second)
+ {
+ if (rPair.second.mnDepth < nMinDepth)
+ nMinDepth = rPair.second.mnDepth;
+ }
+
while( aVecIter != aVecEnd )
{
+ const auto& rPair = *aVecIter;
+ const DiagramData::SourceIdAndDepth& rItem = rPair.second;
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);
+ DiagramData::PointNameMap::const_iterator aDataNode2 = rMap.find(rItem.msSourceId);
if (aDataNode2 == rMap.end())
{
//busted, skip it
@@ -1142,7 +1182,7 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
rShape->setDataNodeType(aDataNode2->second->mnType);
- if( aVecIter->second == 0 )
+ if( rItem.mnDepth == 0 )
{
// grab shape attr from topmost element(s)
rShape->getShapeProperties() = aDataNode2->second->mpShape->getShapeProperties();
@@ -1159,6 +1199,13 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
<< " added for layout node named \"" << msName
<< "\"");
}
+ else if (rItem.mnDepth == nMinDepth)
+ {
+ // If no real topmost element, then take properties from the one that's the closest
+ // to topmost.
+ rShape->getLineProperties() = aDataNode2->second->mpShape->getLineProperties();
+ rShape->getFillProperties() = aDataNode2->second->mpShape->getFillProperties();
+ }
// append text with right outline level
if( aDataNode2->second->mpShape->getTextBody() &&
@@ -1184,8 +1231,8 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
for (const auto& pSourceParagraph : rSourceParagraphs)
{
TextParagraph& rPara = pTextBody->addParagraph();
- if (aVecIter->second != -1)
- rPara.getProperties().setLevel(aVecIter->second);
+ if (rItem.mnDepth != -1)
+ rPara.getProperties().setLevel(rItem.mnDepth);
for (const auto& pRun : pSourceParagraph->getRuns())
rPara.addRun(pRun);
diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
index 440db0ef21ed..f056e4f7e637 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
@@ -169,9 +169,13 @@ public:
/// Gives access to <dgm:param type="..." val="..."/>.
const ParamMap& getMap() const { return maMap; }
+ void setAspectRatio(double fAspectRatio) { mfAspectRatio = fAspectRatio; }
+
private:
sal_Int32 mnType;
ParamMap maMap;
+ /// Aspect ratio is not integer, so not part of maMap.
+ double mfAspectRatio = 0;
};
typedef std::shared_ptr< AlgAtom > AlgAtomPtr;
diff --git a/oox/source/drawingml/diagram/layoutnodecontext.cxx b/oox/source/drawingml/diagram/layoutnodecontext.cxx
index 2cfeec2e8db6..ff508a24fcae 100644
--- a/oox/source/drawingml/diagram/layoutnodecontext.cxx
+++ b/oox/source/drawingml/diagram/layoutnodecontext.cxx
@@ -67,10 +67,18 @@ public:
{
case DGM_TOKEN( param ):
{
- const sal_Int32 nValTok = rAttribs.getToken( XML_val, 0 );
- mpNode->addParam(
- rAttribs.getToken( XML_type, 0 ),
- nValTok>0 ? nValTok : rAttribs.getInteger( XML_val, 0 ) );
+ sal_Int32 nType = rAttribs.getToken(XML_type, 0);
+ switch (nType)
+ {
+ case XML_ar:
+ mpNode->setAspectRatio(rAttribs.getDouble(XML_val, 0));
+ break;
+ default:
+ const sal_Int32 nValTok = rAttribs.getToken(XML_val, 0);
+ mpNode->addParam(nType, nValTok > 0 ? nValTok
+ : rAttribs.getInteger(XML_val, 0));
+ break;
+ }
break;
}
default:
diff --git a/sd/qa/unit/data/pptx/smartart-cycle-matrix.pptx b/sd/qa/unit/data/pptx/smartart-cycle-matrix.pptx
new file mode 100644
index 000000000000..fb1cb7ea2c2e
Binary files /dev/null and b/sd/qa/unit/data/pptx/smartart-cycle-matrix.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx
index a2f0a574af3b..68fed33c8c03 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -65,6 +65,7 @@ public:
void testAccentProcess();
void testContinuousBlockProcess();
void testOrgChart();
+ void testCycleMatrix();
CPPUNIT_TEST_SUITE(SdImportTestSmartArt);
@@ -95,6 +96,7 @@ public:
CPPUNIT_TEST(testAccentProcess);
CPPUNIT_TEST(testContinuousBlockProcess);
CPPUNIT_TEST(testOrgChart);
+ CPPUNIT_TEST(testCycleMatrix);
CPPUNIT_TEST_SUITE_END();
};
@@ -721,6 +723,97 @@ void SdImportTestSmartArt::testOrgChart()
xDocShRef->DoClose();
}
+void SdImportTestSmartArt::testCycleMatrix()
+{
+ sd::DrawDocShellRef xDocShRef = loadURL(
+ m_directories.getURLFromSrc("/sd/qa/unit/data/pptx/smartart-cycle-matrix.pptx"), PPTX);
+ uno::Reference<drawing::XShape> xGroup(getShapeFromPage(0, 0, xDocShRef), uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xGroup.is());
+
+ // Without the accompanying fix in place, this test would have failed: the height was 12162,
+ // which is not the mm100 equivalent of the 4064000 EMU in the input file.
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(11287), xGroup->getSize().Height);
+
+ uno::Reference<text::XText> xA1(getChildShape(getChildShape(xGroup, 1), 0), uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xA1.is());
+ CPPUNIT_ASSERT_EQUAL(OUString("A1"), xA1->getString());
+
+ // Test fill color of B1, should be orange.
+ uno::Reference<text::XText> xB1(getChildShape(getChildShape(xGroup, 1), 1), uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xB1.is());
+ CPPUNIT_ASSERT_EQUAL(OUString("B1"), xB1->getString());
+
+ uno::Reference<beans::XPropertySet> xB1Props(xB1, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xB1Props.is());
+ sal_Int32 nFillColor = 0;
+ xB1Props->getPropertyValue("FillColor") >>= nFillColor;
+ // Without the accompanying fix in place, this test would have failed: the background color was
+ // 0x4f81bd, i.e. blue, not orange.
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0xf79646), nFillColor);
+
+ // Without the accompanying fix in place, this test would have failed: the
+ // content of the "A2" shape was lost.
+ uno::Reference<text::XText> xA2(getChildShape(getChildShape(getChildShape(xGroup, 0), 0), 1),
+ uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xA2.is());
+ CPPUNIT_ASSERT_EQUAL(OUString("A2"), xA2->getString());
+
+ // Test that the layout of shapes is like this:
+ // A2 B2
+ // D2 C2
+
+ uno::Reference<drawing::XShape> xA2Shape(xA2, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xA2Shape.is());
+
+ uno::Reference<text::XText> xB2(getChildShape(getChildShape(getChildShape(xGroup, 0), 1), 1),
+ uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xB2.is());
+ CPPUNIT_ASSERT_EQUAL(OUString("B2"), xB2->getString());
+ uno::Reference<drawing::XShape> xB2Shape(xB2, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xB2Shape.is());
+
+ // Test line color of B2, should be orange.
+ uno::Reference<beans::XPropertySet> xB2Props(xB2, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xB2Props.is());
+ sal_Int32 nLineColor = 0;
+ xB2Props->getPropertyValue("LineColor") >>= nLineColor;
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0xf79646), nLineColor);
+
+ uno::Reference<text::XText> xC2(getChildShape(getChildShape(getChildShape(xGroup, 0), 2), 1),
+ uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xC2.is());
+ // Without the accompanying fix in place, this test would have failed, i.e. the order of the
+ // lines in the shape were wrong: C2-1\nC2-4\nC2-3\nC2-2.
+ CPPUNIT_ASSERT_EQUAL(OUString("C2-1\nC2-2\nC2-3\nC2-4"), xC2->getString());
+ uno::Reference<drawing::XShape> xC2Shape(xC2, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xC2Shape.is());
+
+ uno::Reference<text::XText> xD2(getChildShape(getChildShape(getChildShape(xGroup, 0), 3), 1),
+ uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xD2.is());
+ CPPUNIT_ASSERT_EQUAL(OUString("D2"), xD2->getString());
+ uno::Reference<drawing::XShape> xD2Shape(xD2, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xD2Shape.is());
+
+ // Without the accompanying fix in place, this test would have failed, i.e.
+ // the A2 and B2 shapes had the same horizontal position, while B2 should
+ // be on the right of A2.
+ CPPUNIT_ASSERT_GREATER(xA2Shape->getPosition().X, xB2Shape->getPosition().X);
+ CPPUNIT_ASSERT_EQUAL(xA2Shape->getPosition().Y, xB2Shape->getPosition().Y);
+ CPPUNIT_ASSERT_GREATER(xA2Shape->getPosition().X, xC2Shape->getPosition().X);
+ CPPUNIT_ASSERT_GREATER(xA2Shape->getPosition().Y, xC2Shape->getPosition().Y);
+ CPPUNIT_ASSERT_EQUAL(xA2Shape->getPosition().X, xD2Shape->getPosition().X);
+ CPPUNIT_ASSERT_GREATER(xA2Shape->getPosition().Y, xD2Shape->getPosition().Y);
+
+ // Without the accompanying fix in place, this test would have failed: width was expected to be
+ // 4887, was actually 7331.
+ uno::Reference<drawing::XShape> xA1Shape(xA1, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xA1Shape.is());
+ CPPUNIT_ASSERT_EQUAL(xA1Shape->getSize().Height, xA1Shape->getSize().Width);
+
+ xDocShRef->DoClose();
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTestSmartArt);
CPPUNIT_PLUGIN_IMPLEMENT();
More information about the Libreoffice-commits
mailing list