PPTX import: import SmartArt drawing into single GroupShape

before that there were imported two GroupShapes:
	- empty one with properties like id, name, InteropGrapBag
	- second one with actual shapes

also fixed tests that relyed on that behaviour

Change-Id: I2b94a53e21666b16725c4353448d75e916e4f9df
Reviewed-on: https://gerrit.libreoffice.org/69252
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

7 files changed, 21 insertions, 30 deletions

diff --git a/oox/source/ppt/extdrawingfragmenthandler.cxx b/oox/source/ppt/extdrawingfragmenthandler.cxx
index a0d40ea0412c..cf8d8e50ed3c 100644
--- a/oox/source/ppt/extdrawingfragmenthandler.cxx
+++ b/oox/source/ppt/extdrawingfragmenthandler.cxx
@@ -29,7 +29,7 @@ ExtDrawingFragmentHandler::ExtDrawingFragmentHandler( XmlFilterBase& rFilter,
      mpSlidePersistPtr (rSlidePersistPtr ),
      meShapeLocation( eShapeLocation ),
      mpGroupShapePtr( pGroupShapePtr ),
-     mpOrgShapePtr( pShapePtr )
+     mpShapePtr( pShapePtr )
 {
 }
 
@@ -49,10 +49,6 @@ ExtDrawingFragmentHandler::onCreateContext( ::sal_Int32 aElement,
     case DSP_TOKEN( drawing ):
         break;
     case DSP_TOKEN( spTree ):
-        mpShapePtr = oox::drawingml::ShapePtr( new PPTShape( meShapeLocation, "com.sun.star.drawing.GroupShape" ) );
-        mpShapePtr->setPosition( mpOrgShapePtr->getPosition() );
-        mpShapePtr->setName( mpOrgShapePtr->getName() );
-
         return new PPTShapeGroupContext(
                 *this, mpSlidePersistPtr, meShapeLocation, mpGroupShapePtr,
                 mpShapePtr );
diff --git a/oox/source/ppt/extdrawingfragmenthandler.hxx b/oox/source/ppt/extdrawingfragmenthandler.hxx
index 83dfcd216e8f..cdde03777bc1 100644
--- a/oox/source/ppt/extdrawingfragmenthandler.hxx
+++ b/oox/source/ppt/extdrawingfragmenthandler.hxx
@@ -34,7 +34,6 @@ private:
         const oox::ppt::SlidePersistPtr mpSlidePersistPtr;
         const oox::ppt::ShapeLocation   meShapeLocation;
         oox::drawingml::ShapePtr const  mpGroupShapePtr;
-        oox::drawingml::ShapePtr        mpOrgShapePtr;          // Original Shape data, if any
         oox::drawingml::ShapePtr        mpShapePtr;
 };
 
diff --git a/oox/source/ppt/pptshapegroupcontext.cxx b/oox/source/ppt/pptshapegroupcontext.cxx
index 199bb790efff..c53b69823c9b 100644
--- a/oox/source/ppt/pptshapegroupcontext.cxx
+++ b/oox/source/ppt/pptshapegroupcontext.cxx
@@ -71,9 +71,12 @@ ContextHandlerRef PPTShapeGroupContext::onCreateContext( sal_Int32 aElementToken
     {
     case PPT_TOKEN( cNvPr ):
     {
+        // don't override SmartArt properties for embedded drawing's spTree
         mpGroupShapePtr->setHidden( rAttribs.getBool( XML_hidden, false ) );
-        mpGroupShapePtr->setId( rAttribs.getString( XML_id ).get() );
-        mpGroupShapePtr->setName( rAttribs.getString( XML_name ).get() );
+        if (mpGroupShapePtr->getId().isEmpty())
+            mpGroupShapePtr->setId(rAttribs.getString(XML_id).get());
+        if (mpGroupShapePtr->getName().isEmpty())
+            mpGroupShapePtr->setName( rAttribs.getString( XML_name ).get() );
         break;
     }
     case PPT_TOKEN( ph ):
diff --git a/sd/qa/unit/data/xml/n819614_0.xml b/sd/qa/unit/data/xml/n819614_0.xml
index 8238da646086..fc109917c460 100644
--- a/sd/qa/unit/data/xml/n819614_0.xml
+++ b/sd/qa/unit/data/xml/n819614_0.xml
@@ -14,15 +14,7 @@
    <Line3 column1="0.000000" column2="0.000000" column3="1.000000"/>
   </Transformation>
  </XShape>
- <XShape positionX="0" positionY="0" sizeX="100" sizeY="100" type="com.sun.star.drawing.GroupShape" name="Content Placeholder 4">
-  <XShapes/>
-  <Transformation>
-   <Line1 column1="101.000000" column2="0.000000" column3="0.000000"/>
-   <Line2 column1="0.000000" column2="101.000000" column3="0.000000"/>
-   <Line3 column1="0.000000" column2="0.000000" column3="1.000000"/>
-  </Transformation>
- </XShape>
- <XShape positionX="182" positionY="2544" sizeX="25125" sizeY="16293" type="com.sun.star.drawing.GroupShape">
+ <XShape positionX="182" positionY="2544" sizeX="25125" sizeY="16293" type="com.sun.star.drawing.GroupShape" name="Content Placeholder 4">
   <XShapes>
    <XShape positionX="10751" positionY="3496" sizeX="1563" sizeY="253" type="com.sun.star.drawing.CustomShape" fontHeight="18.000000" fontColor="ffffffff" textAutoGrowHeight="false" textAutoGrowWidth="false" textContourFrame="false" textFitToSize="NONE" textHorizontalAdjust="BLOCK" textVerticalAdjust="TOP" textLeftDistance="250" textRightDistance="250" textUpperDistance="125" textLowerDistance="125" textMaximumFrameHeight="0" textMaximumFrameWidth="0" textMinimumFrameHeight="0" textMinimumFrameWidth="0" textAnimationAmount="0" textAnimationCount="0" textAnimationDelay="0" textAnimationDirection="LEFT" textAnimationKind="NONE" textAnimationStartInside="false" textAnimationStopInside="false" textWritingMode="LR_TB" fillStyle="NONE" fillColor="729fcf" fillTransparence="0" fillTransparenceGradientName="">
     <FillTransparenceGradient style="LINEAR" startColor="000000" endColor="ffffff" angle="0" border="0" xOffset="50" yOffset="50" startIntensity="100" endIntensity="100" stepCount="0"/>
diff --git a/sd/qa/unit/export-tests-ooxml1.cxx b/sd/qa/unit/export-tests-ooxml1.cxx
index e7a5ec7b0f16..7161b327e98b 100644
--- a/sd/qa/unit/export-tests-ooxml1.cxx
+++ b/sd/qa/unit/export-tests-ooxml1.cxx
@@ -223,7 +223,7 @@ void SdOOXMLExportTest1::testBnc870233_2()
 
     // First smart art has blue font color (direct formatting)
     {
-        const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(1));
+        const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(0));
         CPPUNIT_ASSERT(pObjGroup);
         const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(0));
         checkFontAttributes<Color, SvxColorItem>(pObj, Color(0x0000ff));
@@ -231,7 +231,7 @@ void SdOOXMLExportTest1::testBnc870233_2()
 
     // Second smart art has "dk2" font color (style)
     {
-        const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(3)); // FIXME should be 1, smartart import creates an additional empty group for some reason
+        const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(1));
         CPPUNIT_ASSERT(pObjGroup);
         const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(0));
         checkFontAttributes<Color, SvxColorItem>( pObj, Color(0x1F497D) );
@@ -239,7 +239,7 @@ void SdOOXMLExportTest1::testBnc870233_2()
 
     // Third smart art has white font color (style)
     {
-        const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(5)); // FIXME should be 2, smartart import creates an additional empty group for some reason
+        const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(2));
         CPPUNIT_ASSERT(pObjGroup);
         const SdrTextObj *pObj = dynamic_cast<SdrTextObj *>(pObjGroup->GetSubList()->GetObj(0));
         checkFontAttributes<Color, SvxColorItem>(pObj, Color(0xffffff));
@@ -367,14 +367,14 @@ void SdOOXMLExportTest1::testBnc880763()
 
     // Check z-order of the two shapes, use background color to identify them
     // First object in the background has blue background color
-    const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(1));
+    const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(0));
     CPPUNIT_ASSERT(pObjGroup);
     const SdrObject *pObj = pObjGroup->GetSubList()->GetObj(0);
     CPPUNIT_ASSERT_MESSAGE( "no object", pObj != nullptr);
     CPPUNIT_ASSERT_EQUAL( Color(0x0000ff),(static_cast< const XColorItem& >(pObj->GetMergedItem(XATTR_FILLCOLOR))).GetColorValue());
 
     // Second object at the front has green background color
-    pObj = pPage->GetObj(2); // FIXME should be 1, smartart import creates an additional empty group for some reason
+    pObj = pPage->GetObj(1);
     CPPUNIT_ASSERT_MESSAGE( "no object", pObj != nullptr);
     CPPUNIT_ASSERT_EQUAL( Color(0x00ff00),(static_cast< const XColorItem& >(pObj->GetMergedItem(XATTR_FILLCOLOR))).GetColorValue());
 
@@ -389,7 +389,7 @@ void SdOOXMLExportTest1::testBnc862510_5()
     const SdrPage *pPage = GetPage( 1, xDocShRef );
 
     // Same as testBnc870237, but here we check the horizontal spacing
-    const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(1));
+    const SdrObjGroup *pObjGroup = dynamic_cast<SdrObjGroup *>(pPage->GetObj(0));
     CPPUNIT_ASSERT(pObjGroup);
     const SdrObject* pObj = pObjGroup->GetSubList()->GetObj(1);
     CPPUNIT_ASSERT_MESSAGE( "no object", pObj != nullptr);
diff --git a/sd/qa/unit/export-tests-ooxml2.cxx b/sd/qa/unit/export-tests-ooxml2.cxx
index eda6827a6869..157e9029b75e 100644
--- a/sd/qa/unit/export-tests-ooxml2.cxx
+++ b/sd/qa/unit/export-tests-ooxml2.cxx
@@ -1413,10 +1413,10 @@ void SdOOXMLExportTest2::testGroupsPosition()
     xDocShRef->DoClose();
 
     xmlDocPtr pXmlDocContent = parseExport(tempFile, "ppt/slides/slide1.xml");
-    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[2]/p:sp[1]/p:spPr/a:xfrm/a:off", "x", "4040640");
-    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[2]/p:sp[1]/p:spPr/a:xfrm/a:off", "y", "4273920");
-    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[2]/p:sp[3]/p:spPr/a:xfrm/a:off", "x", "6796800");
-    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[2]/p:sp[3]/p:spPr/a:xfrm/a:off", "y", "4273920");
+    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[1]/p:sp[1]/p:spPr/a:xfrm/a:off", "x", "4040640");
+    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[1]/p:sp[1]/p:spPr/a:xfrm/a:off", "y", "4273920");
+    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[1]/p:sp[3]/p:spPr/a:xfrm/a:off", "x", "6796800");
+    assertXPath(pXmlDocContent, "/p:sld/p:cSld/p:spTree/p:grpSp[1]/p:sp[3]/p:spPr/a:xfrm/a:off", "y", "4273920");
 }
 
 void SdOOXMLExportTest2::testGroupsRotatedPosition()
diff --git a/sd/qa/unit/import-tests.cxx b/sd/qa/unit/import-tests.cxx
index ef62c5bfecdc..696a4e5ded83 100644
--- a/sd/qa/unit/import-tests.cxx
+++ b/sd/qa/unit/import-tests.cxx
@@ -447,7 +447,8 @@ void SdImportTest::testN862510_2()
 
     const SdrPage *pPage = GetPage( 1, xDocShRef );
     {
-        SdrObjGroup *pGrpObj = dynamic_cast<SdrObjGroup *>( pPage->GetObj( 1 ) );
+        CPPUNIT_ASSERT_EQUAL(size_t(1), pPage->GetObjCount());
+        SdrObjGroup *pGrpObj = dynamic_cast<SdrObjGroup *>( pPage->GetObj( 0 ) );
         CPPUNIT_ASSERT( pGrpObj );
         SdrObjCustomShape *pObj = dynamic_cast<SdrObjCustomShape *>( pGrpObj->GetSubList()->GetObj( 0 ) );
         CPPUNIT_ASSERT( pObj );
@@ -827,7 +828,7 @@ void SdImportTest::testBnc870237()
     const SdrPage *pPage = GetPage( 1, xDocShRef );
 
     // Simulate a:ext inside dsp:txXfrm with changing the lower distance
-    const SdrObjGroup* pObj = dynamic_cast<SdrObjGroup*>( pPage->GetObj( 1 ) );
+    const SdrObjGroup* pObj = dynamic_cast<SdrObjGroup*>( pPage->GetObj( 0 ) );
     CPPUNIT_ASSERT_MESSAGE( "no object", pObj != nullptr);
     CPPUNIT_ASSERT_EQUAL( sal_Int32(0), pObj->GetMergedItem(SDRATTR_TEXT_UPPERDIST).GetValue());
     CPPUNIT_ASSERT_EQUAL( sal_Int32(9919), pObj->GetMergedItem(SDRATTR_TEXT_LOWERDIST).GetValue());
@@ -1274,7 +1275,7 @@ void SdImportTest::testTdf93830()
     uno::Reference< drawing::XDrawPage > xPage( getPage( 0, xDocShRef ) );
 
     // Get the first text box from group shape
-    uno::Reference< container::XIndexAccess > xShape( xPage->getByIndex(1), uno::UNO_QUERY );
+    uno::Reference< container::XIndexAccess > xShape( xPage->getByIndex(0), uno::UNO_QUERY );
     uno::Reference< beans::XPropertySet > xPropSet( xShape->getByIndex(2), uno::UNO_QUERY );
     CPPUNIT_ASSERT_MESSAGE( "no textbox shape", xPropSet.is() );