include/oox/export/drawingml.hxx | 6 ++- oox/qa/unit/data/camera-rotation-revolution-nonwps.pptx |binary oox/qa/unit/drawingml.cxx | 26 ++++++++++++++ oox/qa/unit/export.cxx | 27 +++++++++++++++ oox/source/drawingml/shape.cxx | 9 ++--- oox/source/export/drawingml.cxx | 28 +++------------- oox/source/export/shapes.cxx | 4 +- sd/source/filter/eppt/pptx-epptooxml.cxx | 2 - 8 files changed, 72 insertions(+), 30 deletions(-)
New commits: commit ebc5d02a0d4a716fb4efb4ce17a622eb21cf7c9c Author: Sarper Akdemir <sarper.akde...@collabora.com> AuthorDate: Mon Jan 3 05:45:17 2022 +0300 Commit: Xisco Fauli <xiscofa...@libreoffice.org> CommitDate: Tue Jan 11 10:05:15 2022 +0100 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 <vmik...@collabora.com> Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128219 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 Binary files /dev/null and b/oox/qa/unit/data/camera-rotation-revolution-nonwps.pptx differ 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);