tdf#146534 pptx import: make Z rotation work with rotation transform

Expands previous idea from a9c5c0d814a266096483572b84c72875ef8efd77
(tdf#133037 OOXML shape import: camera rotation along Z)
and uses it also for shapes that have a true bUseRotationTransform flag

Also fixes same Z rotation exporting twice from InteropGrabBag to
both spPr and textBody causing text overrotating on roundtrip.

Change-Id: If0f192af029ca86b932a63613f961be1f5003c5b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127880
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128219

8 files changed, 72 insertions, 30 deletions

diff --git a/include/oox/export/drawingml.hxx b/include/oox/export/drawingml.hxx
index af0a84ff8e16..63a32225172b 100644
--- a/include/oox/export/drawingml.hxx
+++ b/include/oox/export/drawingml.hxx
@@ -320,7 +320,11 @@ public:
     void WriteShapeStyle( const css::uno::Reference< css::beans::XPropertySet >& rXPropSet );
     void WriteShapeEffects( const css::uno::Reference< css::beans::XPropertySet >& rXPropSet );
     void WriteShapeEffect( std::u16string_view sName, const css::uno::Sequence< css::beans::PropertyValue >& aEffectProps );
-    void WriteShape3DEffects( const css::uno::Reference< css::beans::XPropertySet >& rXPropSet );
+    /** Populates scene3d tag
+        @param rXPropSet Prop set
+        @param bIsText True if the 3D effects are for a text body, false if it is for a shape
+     */
+    void Write3DEffects(const css::uno::Reference<css::beans::XPropertySet>& rXPropSet, bool bIsText);
     void WriteArtisticEffect( const css::uno::Reference< css::beans::XPropertySet >& rXPropSet );
     OString WriteWdpPicture( const OUString& rFileId, const css::uno::Sequence< sal_Int8 >& rPictureData );
     void WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId);
diff --git a/oox/qa/unit/data/camera-rotation-revolution-nonwps.pptx b/oox/qa/unit/data/camera-rotation-revolution-nonwps.pptx
new file mode 100755
index 000000000000..db26dc032caa
--- /dev/null
+++ b/oox/qa/unit/data/camera-rotation-revolution-nonwps.pptx
diff --git a/oox/qa/unit/drawingml.cxx b/oox/qa/unit/drawingml.cxx
index 0d6f367f02c7..ccfa0fa80633 100644
--- a/oox/qa/unit/drawingml.cxx
+++ b/oox/qa/unit/drawingml.cxx
@@ -294,6 +294,32 @@ CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testCameraRotationRevolution)
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(27000), nRotateAngle1);
 }
 
+CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testTdf146534_CameraRotationRevolutionNonWpsShapes)
+{
+    OUString aURL
+        = m_directories.getURLFromSrc(DATA_DIRECTORY) + "camera-rotation-revolution-nonwps.pptx";
+    load(aURL);
+
+    uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+                                                 uno::UNO_QUERY);
+    uno::Reference<container::XNamed> xShape0(xDrawPage->getByIndex(0), uno::UNO_QUERY);
+    uno::Reference<container::XNamed> xShape1(xDrawPage->getByIndex(1), uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xShapeProps0(xShape0, uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xShapeProps1(xShape1, uno::UNO_QUERY);
+    sal_Int32 nRotateAngle0;
+    sal_Int32 nRotateAngle1;
+    xShapeProps0->getPropertyValue("RotateAngle") >>= nRotateAngle0;
+    xShapeProps1->getPropertyValue("RotateAngle") >>= nRotateAngle1;
+
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 9000
+    // - Actual  : 0
+    // so the camera rotation would not have been factored into how the shape is displayed
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(9000), nRotateAngle0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(30500), nRotateAngle1);
+}
+
 CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testTableShadow)
 {
     auto verify = [](const uno::Reference<lang::XComponent>& xComponent) {
diff --git a/oox/qa/unit/export.cxx b/oox/qa/unit/export.cxx
index 280e052d09af..83bd05c6f286 100644
--- a/oox/qa/unit/export.cxx
+++ b/oox/qa/unit/export.cxx
@@ -326,6 +326,33 @@ CPPUNIT_TEST_FIXTURE(Test, testCustomShapeArrowExport)
                 "a:graphicData/wps:wsp/wps:spPr/a:prstGeom/a:avLst/a:gd[4]",
                 "fmla", "val 66660");
 }
+
+CPPUNIT_TEST_FIXTURE(Test, testCameraRevolutionGrabBag)
+{
+    // Given a PPTX file that contains camera revolution (rotation around z axis) applied shapes
+    OUString aURL
+        = m_directories.getURLFromSrc(DATA_DIRECTORY) + "camera-rotation-revolution-nonwps.pptx";
+
+    // When saving that document:
+    loadAndSave(aURL, "Impress Office Open XML");
+
+    std::unique_ptr<SvStream> pStream = parseExportStream(getTempFile(), "ppt/slides/slide1.xml");
+    xmlDocUniquePtr pXmlDoc = parseXmlStream(pStream.get());
+    // Then make sure the revolution is exported without a problem:
+    // First shape textbox:
+    assertXPath(pXmlDoc, "//p:sp[1]/p:spPr/a:scene3d/a:camera/a:rot", "rev", "5400000");
+
+    // Second shape rectangle:
+    assertXPath(pXmlDoc, "//p:sp[2]/p:spPr/a:scene3d/a:camera/a:rot", "rev", "18300000");
+
+    // Make sure Shape3DProperties don't leak under txBody
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 0
+    // - Actual  : 1
+    // - In <>, XPath '//p:sp[1]/p:txBody/a:bodyPr/a:scene3d/a:camera/a:rot' number of nodes is incorrect
+    assertXPath(pXmlDoc, "//p:sp[1]/p:txBody/a:bodyPr/a:scene3d/a:camera/a:rot", 0);
+    assertXPath(pXmlDoc, "//p:sp[2]/p:txBody/a:bodyPr/a:scene3d/a:camera/a:rot", 0);
+}
 }
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index 78a27f8a0c9c..d36f01ee7eab 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -875,8 +875,11 @@ Reference< XShape > const & Shape::createAndInsert(
     // The flip contained in aParentScale will affect orientation of object rotation angle.
     sal_Int16 nOrientation = ((aParentScale.getX() < 0) != (aParentScale.getY() < 0)) ? -1 : 1;
     // ToDo: Not sure about the restrictions given by bUseRotationTransform.
-    if (bUseRotationTransform && mnRotation != 0)
-        lcl_RotateAtCenter(aTransformation, nOrientation * mnRotation);
+    // Since LibreOffice doesn't have 3D camera options for 2D shapes, rotate the shape opposite of
+    // the camera Z axis rotation, in order to produce the same visual result from MSO
+    const sal_Int32 nCameraRotation = get3DProperties().maCameraRotation.mnRevolution.get(0);
+    if (bUseRotationTransform && (mnRotation != 0 || nCameraRotation != 0))
+        lcl_RotateAtCenter(aTransformation, nOrientation * (mnRotation - nCameraRotation));
 
     if (fParentRotate != 0.0)
         aTransformation.rotate(fParentRotate);
@@ -1601,8 +1604,6 @@ Reference< XShape > const & Shape::createAndInsert(
             getTextBody()->getTextProperties().pushVertSimulation();
 
         // tdf#133037: a bit hackish: force Shape to rotate in the opposite direction the camera would rotate
-        const sal_Int32 nCameraRotation = get3DProperties().maCameraRotation.mnRevolution.get(0);
-
         PropertySet aPropertySet(mxShape);
         if ( !bUseRotationTransform && (mnRotation != 0 || nCameraRotation != 0) )
         {
diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx
index 396288bd9940..4731f556967e 100644
--- a/oox/source/export/drawingml.cxx
+++ b/oox/source/export/drawingml.cxx
@@ -3473,7 +3473,7 @@ void DrawingML::WriteText(const Reference<XInterface>& rXIface, bool bBodyPr, bo
             }
         }
 
-        WriteShape3DEffects( rXPropSet );
+        Write3DEffects( rXPropSet, /*bIsText=*/true );
 
         mpFS->endElementNS((nXmlNamespace ? nXmlNamespace : XML_a), XML_bodyPr);
     }
@@ -4655,7 +4655,7 @@ bool DrawingML::HasEnhancedCustomShapeSegmentCommand(
     return false;
 }
 
-void DrawingML::WriteShape3DEffects( const Reference< XPropertySet >& xPropSet )
+void DrawingML::Write3DEffects( const Reference< XPropertySet >& xPropSet, bool bIsText )
 {
     // check existence of the grab bag
     if( !GetProperty( xPropSet, "InteropGrabBag" ) )
@@ -4664,8 +4664,10 @@ void DrawingML::WriteShape3DEffects( const Reference< XPropertySet >& xPropSet )
     // extract the relevant properties from the grab bag
     Sequence< PropertyValue > aGrabBag, aEffectProps, aLightRigProps, aShape3DProps;
     mAny >>= aGrabBag;
-    auto pShapeProp = std::find_if(std::cbegin(aGrabBag), std::cend(aGrabBag),
-        [](const PropertyValue& rProp) { return rProp.Name == "3DEffectProperties"; });
+
+    auto pShapeProp = std::find_if( std::cbegin(aGrabBag), std::cend(aGrabBag),
+        [bIsText](const PropertyValue& rProp)
+        { return rProp.Name == (bIsText ? u"Text3DEffectProperties" : u"3DEffectProperties"); });
     if (pShapeProp != std::cend(aGrabBag))
     {
         Sequence< PropertyValue > a3DEffectProps;
@@ -4681,24 +4683,6 @@ void DrawingML::WriteShape3DEffects( const Reference< XPropertySet >& xPropSet )
         }
     }
 
-    auto pTextProp = std::find_if(std::cbegin(aGrabBag), std::cend(aGrabBag),
-        [](const PropertyValue& rProp) { return rProp.Name == "Text3DEffectProperties"; });
-
-    if (pTextProp != std::cend(aGrabBag))
-    {
-        Sequence< PropertyValue > a3DEffectProps;
-        pTextProp->Value >>= a3DEffectProps;
-        for( const auto& r3DEffectProp : std::as_const(a3DEffectProps) )
-        {
-            if( r3DEffectProp.Name == "Camera" )
-                r3DEffectProp.Value >>= aEffectProps;
-            else if( r3DEffectProp.Name == "LightRig" )
-                r3DEffectProp.Value >>= aLightRigProps;
-            else if( r3DEffectProp.Name == "Shape3D" )
-                r3DEffectProp.Value >>= aShape3DProps;
-        }
-    }
-
     if( !aEffectProps.hasElements() && !aLightRigProps.hasElements() && !aShape3DProps.hasElements() )
         return;
 
diff --git a/oox/source/export/shapes.cxx b/oox/source/export/shapes.cxx
index 35b7a4bd4c9d..466b392c1a0e 100644
--- a/oox/source/export/shapes.cxx
+++ b/oox/source/export/shapes.cxx
@@ -1114,7 +1114,7 @@ ShapeExport& ShapeExport::WriteCustomShape( const Reference< XShape >& xShape )
                 bHas3DEffectinShape = true;
 
         if( bHas3DEffectinShape)
-            WriteShape3DEffects( rXPropSet );
+            Write3DEffects( rXPropSet, /*bIsText=*/false );
     }
 
     pFS->endElementNS( mnXmlNamespace, XML_spPr );
@@ -1420,7 +1420,7 @@ void ShapeExport::WriteGraphicObjectShapePart( const Reference< XShape >& xShape
     WriteOutline( xShapeProps );
 
     WriteShapeEffects( xShapeProps );
-    WriteShape3DEffects( xShapeProps );
+    Write3DEffects( xShapeProps, /*bIsText=*/false );
 
     pFS->endElementNS( mnXmlNamespace, XML_spPr );
 
diff --git a/sd/source/filter/eppt/pptx-epptooxml.cxx b/sd/source/filter/eppt/pptx-epptooxml.cxx
index d5787d6f4cb7..1e175d8bc564 100644
--- a/sd/source/filter/eppt/pptx-epptooxml.cxx
+++ b/sd/source/filter/eppt/pptx-epptooxml.cxx
@@ -1733,7 +1733,7 @@ ShapeExport& PowerPointShapeExport::WritePlaceholderShape(const Reference< XShap
                 bHas3DEffectinShape = true;
 
         if( bHas3DEffectinShape)
-            WriteShape3DEffects( xProps );
+            Write3DEffects( xProps, /*bIsText=*/false );
     }
     mpFS->endElementNS(XML_p, XML_spPr);