include/oox/export/drawingml.hxx | 3 + oox/source/export/drawingml.cxx | 7 +++- sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx |binary sd/qa/unit/export-tests-ooxml4.cxx | 31 +++++++++++++++++++ sd/source/filter/eppt/epptooxml.hxx | 6 ++- sd/source/filter/eppt/pptx-epptooxml.cxx | 10 ++++-- 6 files changed, 49 insertions(+), 8 deletions(-)
New commits: commit 2de71234b3829672505cf8f6222c27eaf4f0b7af Author: Aron Budea <[email protected]> AuthorDate: Thu Nov 6 00:59:25 2025 +1030 Commit: Aron Budea <[email protected]> CommitDate: Fri Nov 7 07:28:43 2025 +0100 tdf#168755 sd smartart: use unified ids for diagrams and shapes Diagrams (SmartArt) had their separate counters for ids, and when saving animation details, the shape to target wasn't found, resulting in tags like this, where negative spid is invalid: <p:spTgt spid="-1"/> See "19.5.72 spTgt (Shape Target)" in OOXML spec. Ids can't be completely unified, as it would affect naming in ppt/diagrams and on the UI to have large numbers (eg. in the 40-60 range), while users would expect them to start from 1. At least ids to "p:cNvPr" under "p:graphicFrame" should be unified. The large number is because shapes are numbered consecutively over all slides, while for the most part they could restart on each slide, changing that is not in scope for this fix. Change-Id: I9790036cc0743313e98f5926d4dc365c8051bd04 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193479 Reviewed-by: Mike Kaganski <[email protected]> Reviewed-by: Aron Budea <[email protected]> Tested-by: Jenkins diff --git a/include/oox/export/drawingml.hxx b/include/oox/export/drawingml.hxx index aabc8c1b5b23..b3346a823162 100644 --- a/include/oox/export/drawingml.hxx +++ b/include/oox/export/drawingml.hxx @@ -509,7 +509,8 @@ public: OOX_DLLPUBLIC 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 ); - OOX_DLLPUBLIC void WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId); + OOX_DLLPUBLIC void WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, + sal_Int32 nDiagramId, sal_Int32 nShapeId = -1); void writeDiagramRels(const css::uno::Sequence<css::uno::Sequence<css::uno::Any>>& xRelSeq, const css::uno::Reference<css::io::XOutputStream>& xOutStream, std::u16string_view sGrabBagProperyName, int nDiagramId); diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx index 2955062eefc9..08b0f73872a6 100644 --- a/oox/source/export/drawingml.cxx +++ b/oox/source/export/drawingml.cxx @@ -6599,7 +6599,7 @@ OString DrawingML::WriteWdpPicture( const OUString& rFileId, const Sequence< sal return OUStringToOString(aId, RTL_TEXTENCODING_UTF8); } -void DrawingML::WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId) +void DrawingML::WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rXShape, sal_Int32 nDiagramId, sal_Int32 nShapeId) { uno::Reference<beans::XPropertySet> xPropSet(rXShape, uno::UNO_QUERY); @@ -6643,7 +6643,10 @@ void DrawingML::WriteDiagram(const css::uno::Reference<css::drawing::XShape>& rX // generate a unique id rtl::Reference<sax_fastparser::FastAttributeList> pDocPrAttrList = sax_fastparser::FastSerializerHelper::createAttrList(); - pDocPrAttrList->add(XML_id, OString::number(nDiagramId)); + // id in cNvPr under cNvGraphicFramePr must be unique among shapes, + // but can be different from diagram's own id + const sal_Int32 nExpShapeId = nShapeId != -1 ? nShapeId : nDiagramId; + pDocPrAttrList->add(XML_id, OString::number(nExpShapeId)); OString sName = "Diagram" + OString::number(nDiagramId); pDocPrAttrList->add(XML_name, sName); diff --git a/sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx b/sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx new file mode 100644 index 000000000000..8003a1f50318 Binary files /dev/null and b/sd/qa/unit/data/pptx/tdf168755_anim_on_SmartArt.pptx differ diff --git a/sd/qa/unit/export-tests-ooxml4.cxx b/sd/qa/unit/export-tests-ooxml4.cxx index 427330fb6b80..c56b21e589cc 100644 --- a/sd/qa/unit/export-tests-ooxml4.cxx +++ b/sd/qa/unit/export-tests-ooxml4.cxx @@ -115,6 +115,37 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testSmartArtPreserve) "ContentType", u"application/vnd.openxmlformats-officedocument.drawingml.diagramStyle+xml"); } +CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testTdf168755_anim_on_SmartArt) +{ + createSdImpressDoc("pptx/tdf168755_anim_on_SmartArt.pptx"); + save(u"Impress Office Open XML"_ustr); + + xmlDocUniquePtr pXmlDocContent = parseExport(u"ppt/slides/slide1.xml"_ustr); + + const OUString aDiagramId = getXPath( + pXmlDocContent, "//p:sld/p:cSld/p:spTree/p:graphicFrame/p:nvGraphicFramePr/p:cNvPr", "id"); + OUString aSpTgtId + = getXPath(pXmlDocContent, + "//p:sld/p:timing/p:tnLst/p:par/p:cTn/p:childTnLst/p:seq/p:cTn/p:childTnLst/" + "p:par/p:cTn/p:childTnLst/p:par/p:cTn/p:childTnLst/p:par/p:cTn/" + "p:childTnLst/p:set/p:cBhvr/p:tgtEl/p:spTgt", + "spid"); + // Before the fix this would fail, as the "spid" attribute of the target was -1 + CPPUNIT_ASSERT_MESSAGE("Shape id in animation target can't be -1", aSpTgtId != "-1"); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Shape id in animation target differs from original", aDiagramId, + aSpTgtId); + // Repeat the same with a slightly different path (contains "animEffect" instead of "set") + aSpTgtId + = getXPath(pXmlDocContent, + "//p:sld/p:timing/p:tnLst/p:par/p:cTn/p:childTnLst/p:seq/p:cTn/p:childTnLst/" + "p:par/p:cTn/p:childTnLst/p:par/p:cTn/p:childTnLst/p:par/p:cTn/" + "p:childTnLst/p:animEffect/p:cBhvr/p:tgtEl/p:spTgt", + "spid"); + CPPUNIT_ASSERT_MESSAGE("Shape id in animation target can't be -1", aSpTgtId != "-1"); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Shape id in animation target differs from original", aDiagramId, + aSpTgtId); +} + CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testTdf125346) { // There are two themes in the test document, make sure we use the right theme diff --git a/sd/source/filter/eppt/epptooxml.hxx b/sd/source/filter/eppt/epptooxml.hxx index 3db4dc9e15b8..2dea61ff2e6f 100644 --- a/sd/source/filter/eppt/epptooxml.hxx +++ b/sd/source/filter/eppt/epptooxml.hxx @@ -123,7 +123,9 @@ private: virtual OUString SAL_CALL getImplementationName() override; - static void WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId); + static void WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, + const css::uno::Reference<css::drawing::XShape>& rXShape, + sal_Int32 nDiagramId); /** Create a new placeholder index for a master placeholder shape @@ -159,7 +161,7 @@ private: sal_uInt32 mnAnimationNodeIdMax; sal_uInt32 mnThemeIdMax; - sal_uInt32 mnDiagramId; + sal_Int32 mnDiagramId; std::vector<OUString> maRelId; diff --git a/sd/source/filter/eppt/pptx-epptooxml.cxx b/sd/source/filter/eppt/pptx-epptooxml.cxx index 15e81bb0de40..3f1ce75935e2 100644 --- a/sd/source/filter/eppt/pptx-epptooxml.cxx +++ b/sd/source/filter/eppt/pptx-epptooxml.cxx @@ -2864,11 +2864,15 @@ OUString PowerPointExport::getImplementationName() return u"com.sun.star.comp.Impress.oox.PowerPointExport"_ustr; } -void PowerPointExport::WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, const css::uno::Reference<css::drawing::XShape>& rXShape, int nDiagramId) +void PowerPointExport::WriteDiagram(const FSHelperPtr& pFS, PowerPointShapeExport& rDML, + const css::uno::Reference<css::drawing::XShape>& rXShape, + sal_Int32 nDiagramId) { - SAL_INFO("sd.eppt", "writing Diagram " + OUString::number(nDiagramId)); + sal_Int32 nShapeId = rDML.GetNewShapeID(rXShape); + SAL_INFO("sd.eppt", "writing Diagram " + OUString::number(nDiagramId) + " with Shape Id " + + OUString::number(nShapeId)); pFS->startElementNS(XML_p, XML_graphicFrame); - rDML.WriteDiagram(rXShape, nDiagramId); + rDML.WriteDiagram(rXShape, nDiagramId, nShapeId); pFS->endElementNS(XML_p, XML_graphicFrame); }
