tdf#84684 RTF import: fix ZOrder of child shapes inside group shape

Both ODF and OOXML's drawingML seem to just not write ZOrder for child
shapes (instead they just describe them in a bottom-up order), but RTF
does use dhgt, it seems. Given that ZOrder has different meaning inside
and outside a group shape use a stack of GraphicZOrderHelpers to get the
correct result.

Change-Id: I68dbd9f0ae59a1759da14bf414dc1d277d1c927d
(cherry picked from commit 5edebdf67720ac0867b088400e2b31806138dc25)
Reviewed-on: https://gerrit.libreoffice.org/29091
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

5 files changed, 430 insertions, 6 deletions

diff --git a/sw/qa/extras/rtfimport/data/tdf84684.rtf b/sw/qa/extras/rtfimport/data/tdf84684.rtf
new file mode 100644
index 000000000000..c9ce0a6028ec
--- /dev/null
+++ b/sw/qa/extras/rtfimport/data/tdf84684.rtf
@@ -0,0 +1,399 @@
+{\rtf1\adeflang1054\ansi\ansicpg1252\uc1\adeff31507\deff0\stshfdbch31505\stshfloch31506\stshfhich31506\stshfbi31507\deflang1033\deflangfe2052\themelang1033\themelangfe2052\themelangcs1054
+\pard\plain \ltrpar\ql \li0\ri0\sa160\sl259\slmult1\widctlpar\wrapdefault\aspalpha\aspnum\faauto\adjustright\rin0\lin0\itap0 \rtlch\fcs1 \af31507\afs28\alang1054 \ltrch\fcs0
+\fs22\lang1033\langfe2052\loch\af31506\hich\af31506\dbch\af31505\cgrid\langnp1033\langfenp2052
+{\rtlch\fcs1 \af31507 \ltrch\fcs0 \lang1024\langfe1024\noproof\insrsid11950139
+{\shpgrp
+{\*\shpinst\shpleft-780\shptop-825\shpright4800\shpbottom4785\shpfhdr0\shpbxcolumn\shpbxignore\shpbypara\shpbyignore\shpwr3\shpwrk0\shpfblwtxt0\shpz0\shplid1026
+{\sp
+{\sn groupLeft}
+{\sv 0}
+}
+{\sp
+{\sn groupTop}
+{\sv 0}
+}
+{\sp
+{\sn groupRight}
+{\sv 3543300}
+}
+{\sp
+{\sn groupBottom}
+{\sv 3562350}
+}
+{\sp
+{\sn fFlipH}
+{\sv 0}
+}
+{\sp
+{\sn fFlipV}
+{\sv 0}
+}
+{\sp
+{\sn rotation}
+{\sv 0}
+}
+{\sp
+{\sn wzName}
+{\sv Group 6}
+}
+{\sp
+{\sn dxWrapDistLeft}
+{\sv 114300}
+}
+{\sp
+{\sn dyWrapDistTop}
+{\sv 0}
+}
+{\sp
+{\sn dxWrapDistRight}
+{\sv 114300}
+}
+{\sp
+{\sn dyWrapDistBottom}
+{\sv 0}
+}
+{\sp
+{\sn lidRegroup}
+{\sv 0}
+}
+{\sp
+{\sn posh}
+{\sv 0}
+}
+{\sp
+{\sn posrelh}
+{\sv 2}
+}
+{\sp
+{\sn posv}
+{\sv 0}
+}
+{\sp
+{\sn posrelv}
+{\sv 2}
+}
+{\sp
+{\sn dhgt}
+{\sv 251665408}
+}
+{\sp
+{\sn fLayoutInCell}
+{\sv 1}
+}
+{\sp
+{\sn fAllowOverlap}
+{\sv 1}
+}
+{\sp
+{\sn fBehindDocument}
+{\sv 0}
+}
+{\sp
+{\sn fHidden}
+{\sv 0}
+}
+{\sp
+{\sn fLayoutInCell}
+{\sv 1}
+}
+{\shp
+{\*\shpinst\shplid1027
+{\sp
+{\sn relLeft}
+{\sv 0}
+}
+{\sp
+{\sn relTop}
+{\sv 0}
+}
+{\sp
+{\sn relRight}
+{\sv 3543300}
+}
+{\sp
+{\sn relBottom}
+{\sv 3562350}
+}
+{\sp
+{\sn fRelFlipH}
+{\sv 0}
+}
+{\sp
+{\sn fRelFlipV}
+{\sv 0}
+}
+{\sp
+{\sn shapeType}
+{\sv 1}
+}
+{\sp
+{\sn anchorText}
+{\sv 1}
+}
+{\sp
+{\sn fRotateText}
+{\sv 0}
+}
+{\sp
+{\sn fFitShapeToText}
+{\sv 0}
+}
+{\sp
+{\sn fillColor}
+{\sv 16777215}
+}
+{\sp
+{\sn fFilled}
+{\sv 1}
+}
+{\sp
+{\sn lineColor}
+{\sv 4697456}
+}
+{\sp
+{\sn lineWidth}
+{\sv 12700}
+}
+{\sp
+{\sn fInsetPen}
+{\sv 0}
+}
+{\sp
+{\sn fLine}
+{\sv 1}
+}
+{\sp
+{\sn wzName}
+{\sv Rectangle 1}
+}
+{\sp
+{\sn dhgt}
+{\sv 251659264}
+}
+{\sp
+{\sn fHidden}
+{\sv 0}
+}
+{\sp
+{\sn fLayoutInCell}
+{\sv 1}
+}
+}
+}
+{\shp
+{\*\shpinst\shplid1028
+{\sp
+{\sn relLeft}
+{\sv 1028700}
+}
+{\sp
+{\sn relTop}
+{\sv 1457325}
+}
+{\sp
+{\sn relRight}
+{\sv 1462638}
+}
+{\sp
+{\sn relBottom}
+{\sv 1858956}
+}
+{\sp
+{\sn fRelFlipH}
+{\sv 0}
+}
+{\sp
+{\sn fRelFlipV}
+{\sv 0}
+}
+{\sp
+{\sn shapeType}
+{\sv 0}
+}
+{\sp
+{\sn rotation}
+{\sv 2696765}
+}
+{\sp
+{\sn dxTextLeft}
+{\sv 91440}
+}
+{\sp
+{\sn dyTextTop}
+{\sv 45720}
+}
+{\sp
+{\sn dxTextRight}
+{\sv 91440}
+}
+{\sp
+{\sn dyTextBottom}
+{\sv 45720}
+}
+{\sp
+{\sn WrapText}
+{\sv 0}
+}
+{\sp
+{\sn anchorText}
+{\sv 1}
+}
+{\sp
+{\sn txflTextFlow}
+{\sv 0}
+}
+{\sp
+{\sn fRotateText}
+{\sv 0}
+}
+{\sp
+{\sn fFitShapeToText}
+{\sv 0}
+}
+{\sp
+{\sn geoLeft}
+{\sv 0}
+}
+{\sp
+{\sn geoTop}
+{\sv 0}
+}
+{\sp
+{\sn geoRight}
+{\sv 433938}
+}
+{\sp
+{\sn geoBottom}
+{\sv 401631}
+}
+{\sp
+{\sn shapePath}
+{\sv 4}
+}
+{\sp
+{\sn pVerticies}
+{\sv 8;12;(433938,200816);(433938,311724);(336798,401632);(216969,401632);(97140,401632);(0,311724)
+;(0,200816);(0,89908);(97140,0);(216969,0);(216969,200816);(433938,200816)}
+}
+{\sp
+{\sn pSegmentInfo}
+{\sv 2;8;16384;8193;8193;8193;1;1;24577;32768}
+}
+{\sp
+{\sn pConnectionSites}
+{\sv 8;6;(433938,200816);(216969,401632)
+;(0,200816);(216969,0);(216969,200816);(433938,200816)}
+}
+{\sp
+{\sn pConnectionSitesDir}
+{\sv 4;6;0;0;0;0;0;0}
+}
+{\sp
+{\sn cxk}
+{\sv 2}
+}
+{\sp
+{\sn f3DOK}
+{\sv 1}
+}
+{\sp
+{\sn fLineOK}
+{\sv 1}
+}
+{\sp
+{\sn fFillOK}
+{\sv 1}
+}
+{\sp
+{\sn fillType}
+{\sv 0}
+}
+{\sp
+{\sn fillColor}
+{\sv 13998939}
+}
+{\sp
+{\sn fillOpacity}
+{\sv 65536}
+}
+{\sp
+{\sn fFilled}
+{\sv 1}
+}
+{\sp
+{\sn lineColor}
+{\sv 7884063}
+}
+{\sp
+{\sn lineOpacity}
+{\sv 65536}
+}
+{\sp
+{\sn lineType}
+{\sv 0}
+}
+{\sp
+{\sn lineWidth}
+{\sv 12700}
+}
+{\sp
+{\sn lineMiterLimit}
+{\sv 524288}
+}
+{\sp
+{\sn lineStyle}
+{\sv 0}
+}
+{\sp
+{\sn lineDashing}
+{\sv 0}
+}
+{\sp
+{\sn lineJoinStyle}
+{\sv 1}
+}
+{\sp
+{\sn lineEndCapStyle}
+{\sv 2}
+}
+{\sp
+{\sn fInsetPen}
+{\sv 0}
+}
+{\sp
+{\sn fArrowheadsOK}
+{\sv 1}
+}
+{\sp
+{\sn fLine}
+{\sv 1}
+}
+{\sp
+{\sn wzName}
+{\sv Pie 2}
+}
+{\sp
+{\sn lidRegroup}
+{\sv 0}
+}
+{\sp
+{\sn dhgt}
+{\sv 251659264}
+}
+{\sp
+{\sn fHidden}
+{\sv 0}
+}
+{\sp
+{\sn fLayoutInCell}
+{\sv 1}
+}
+}
+}
+}
+}
+}
+}
+{\rtlch\fcs1 \af31507 \ltrch\fcs0 \insrsid7800591
+\par }
+}
diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx
index 3f87aa14213f..921228573d0f 100644
--- a/sw/qa/extras/rtfimport/rtfimport.cxx
+++ b/sw/qa/extras/rtfimport/rtfimport.cxx
@@ -1172,6 +1172,18 @@ DECLARE_RTFIMPORT_TEST(testDoDhgtOld, "do-dhgt-old.rtf")
     CPPUNIT_ASSERT_EQUAL(OUString("b"), xShape->getString());
 }
 
+DECLARE_RTFIMPORT_TEST(testTdf84684, "tdf84684.rtf")
+{
+    // The ZOrder of the two children of the group shape were swapped.
+    uno::Reference<drawing::XShapes> xGroup(getShape(1), uno::UNO_QUERY);
+    uno::Reference<container::XNamed> xChild1(xGroup->getByIndex(0), uno::UNO_QUERY);
+    // This was Pie 2.
+    CPPUNIT_ASSERT_EQUAL(OUString("Rectangle 1"), xChild1->getName());
+    uno::Reference<container::XNamed> xChild2(xGroup->getByIndex(1), uno::UNO_QUERY);
+    // This was Rectangle 1.
+    CPPUNIT_ASSERT_EQUAL(OUString("Pie 2"), xChild2->getName());
+}
+
 DECLARE_RTFIMPORT_TEST(testFdo61909, "fdo61909.rtf")
 {
     uno::Reference<text::XTextRange> xTextRange = getRun(getParagraph(1), 1);
@@ -2570,7 +2582,7 @@ DECLARE_RTFIMPORT_TEST(testTdf90097, "tdf90097.rtf")
 {
     // Get the second child of the group shape.
     uno::Reference<container::XIndexAccess> xGroup(getShape(1), uno::UNO_QUERY);
-    uno::Reference<beans::XPropertySet> xShape(xGroup->getByIndex(1), uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xShape(xGroup->getByIndex(0), uno::UNO_QUERY);
     uno::Sequence< uno::Sequence<awt::Point> > aPolyPolySequence;
     xShape->getPropertyValue("PolyPolygon") >>= aPolyPolySequence;
     uno::Sequence<awt::Point>& rPolygon = aPolyPolySequence[0];
diff --git a/writerfilter/source/dmapper/GraphicHelpers.cxx b/writerfilter/source/dmapper/GraphicHelpers.cxx
index 3d410b2e809a..0dccda708970 100644
--- a/writerfilter/source/dmapper/GraphicHelpers.cxx
+++ b/writerfilter/source/dmapper/GraphicHelpers.cxx
@@ -261,7 +261,7 @@ sal_Int32 GraphicZOrderHelper::findZOrder( sal_Int32 relativeHeight, bool bOldSt
         // std::map is iterated sorted by key
 
         // Old-style ordering differs in what should happen when there is already an item with the same z-order:
-        // we belong under it in case of new-style, but we belong below it in case of old-style.
+        // we belong under it in case of new-style, but we belong above it in case of old-style.
         bool bCond = bOldStyle ? (it->first > relativeHeight) : (it->first >= relativeHeight);
 
         if( bCond )
diff --git a/writerfilter/source/rtftok/rtfsdrimport.cxx b/writerfilter/source/rtftok/rtfsdrimport.cxx
index b3517b8590e4..4a4001337790 100644
--- a/writerfilter/source/rtftok/rtfsdrimport.cxx
+++ b/writerfilter/source/rtftok/rtfsdrimport.cxx
@@ -53,10 +53,13 @@ RTFSdrImport::RTFSdrImport(RTFDocumentImpl& rDocument,
     uno::Reference<drawing::XDrawPageSupplier> xDrawings(xDstDoc, uno::UNO_QUERY);
     if (xDrawings.is())
         m_aParents.push(xDrawings->getDrawPage());
+    m_aGraphicZOrderHelpers.push(writerfilter::dmapper::GraphicZOrderHelper());
 }
 
 RTFSdrImport::~RTFSdrImport()
 {
+    if (!m_aGraphicZOrderHelpers.empty())
+        m_aGraphicZOrderHelpers.pop();
     if (m_aParents.size())
         m_aParents.pop();
 }
@@ -113,18 +116,25 @@ std::vector<beans::PropertyValue> RTFSdrImport::getTextFrameDefaults(bool bNew)
 void RTFSdrImport::pushParent(uno::Reference<drawing::XShapes> const& xParent)
 {
     m_aParents.push(xParent);
+    m_aGraphicZOrderHelpers.push(writerfilter::dmapper::GraphicZOrderHelper());
 }
 
 void RTFSdrImport::popParent()
 {
+    if (!m_aGraphicZOrderHelpers.empty())
+        m_aGraphicZOrderHelpers.pop();
     m_aParents.pop();
 }
 
 void RTFSdrImport::resolveDhgt(uno::Reference<beans::XPropertySet> const& xPropertySet,
                                sal_Int32 const nZOrder, bool const bOldStyle)
 {
-    xPropertySet->setPropertyValue("ZOrder", uno::makeAny(m_aGraphicZOrderHelper.findZOrder(nZOrder, bOldStyle)));
-    m_aGraphicZOrderHelper.addItem(xPropertySet, nZOrder);
+    if (!m_aGraphicZOrderHelpers.empty())
+    {
+        writerfilter::dmapper::GraphicZOrderHelper& rHelper = m_aGraphicZOrderHelpers.top();
+        xPropertySet->setPropertyValue("ZOrder", uno::makeAny(rHelper.findZOrder(nZOrder, bOldStyle)));
+        rHelper.addItem(xPropertySet, nZOrder);
+    }
 }
 
 void RTFSdrImport::resolveLineColorAndWidth(bool bTextFrame, const uno::Reference<beans::XPropertySet>& xPropertySet, uno::Any& rLineColor, uno::Any& rLineWidth)
@@ -795,7 +805,10 @@ void RTFSdrImport::resolve(RTFShape& rShape, bool bClose, ShapeOrPict const shap
     {
         resolveLineColorAndWidth(m_bTextFrame, xPropertySet, aLineColor, aLineWidth);
         if (rShape.oZ)
-            resolveDhgt(xPropertySet, *rShape.oZ, /*bOldStyle=*/false);
+        {
+            bool bOldStyle = m_aParents.size() > 1;
+            resolveDhgt(xPropertySet, *rShape.oZ, bOldStyle);
+        }
         if (m_bTextFrame)
             // Writer textframes implement text::WritingMode2, which is a different data type.
             xPropertySet->setPropertyValue("WritingMode", uno::makeAny(sal_Int16(eWritingMode)));
diff --git a/writerfilter/source/rtftok/rtfsdrimport.hxx b/writerfilter/source/rtftok/rtfsdrimport.hxx
index afbbe4270b95..5e21b84d9946 100644
--- a/writerfilter/source/rtftok/rtfsdrimport.hxx
+++ b/writerfilter/source/rtftok/rtfsdrimport.hxx
@@ -73,7 +73,7 @@ private:
     bool m_bTextGraphicObject;
     /// if inside \pict, but actually it's a shape (not a picture)
     bool m_bFakePict;
-    writerfilter::dmapper::GraphicZOrderHelper m_aGraphicZOrderHelper;
+    std::stack<writerfilter::dmapper::GraphicZOrderHelper> m_aGraphicZOrderHelpers;
 };
 } // namespace rtftok
 } // namespace writerfilter