sd/qa/unit/data/pptx/onemaster-twolayouts.pptx |binary
sd/qa/unit/export-tests-ooxml2.cxx | 10 -
sd/qa/unit/export-tests-ooxml4.cxx | 37 ++++++
sd/source/filter/eppt/epptooxml.hxx | 5
sd/source/filter/eppt/pptx-epptooxml.cxx | 133 ++++++++++++++++++++-----
5 files changed, 153 insertions(+), 32 deletions(-)
New commits:
commit 3c8c537b3cfc4cf812e8728f6e6e46c0596ba7f6
Author: Jaume Pujantell <jaume.pujant...@collabora.com>
AuthorDate: Tue Jan 7 09:22:46 2025 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Jan 9 12:17:13 2025 +0100
sd: de-duplicate masters on save to pptx follow-up
After commit 2d9c808d1952c18daa007b41e70fc7e63f6c5712 "sd: de-duplicate
slide masters on save to pptx", there was some loss of data on save
since when saving only the layouts for de-duplicated masters, not enough
of the shape tree was being saved.
Ideally we may want a clever algorithm that best separates the shape
tree between master and layout, but just saving all the shape tree on
the layout files when de-duplicating masters also works.
Change-Id: I8b932708ef9212a4d5034713066299065083da8d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/179861
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/sd/qa/unit/data/pptx/onemaster-twolayouts.pptx
b/sd/qa/unit/data/pptx/onemaster-twolayouts.pptx
new file mode 100644
index 000000000000..f27260f2e583
Binary files /dev/null and b/sd/qa/unit/data/pptx/onemaster-twolayouts.pptx
differ
diff --git a/sd/qa/unit/export-tests-ooxml2.cxx
b/sd/qa/unit/export-tests-ooxml2.cxx
index a17b7b29b249..7a1df1662769 100644
--- a/sd/qa/unit/export-tests-ooxml2.cxx
+++ b/sd/qa/unit/export-tests-ooxml2.cxx
@@ -1343,7 +1343,7 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest2, testTdf106867)
"/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:cmd/"
"p:cBhvr/p:tgtEl/p:spTgt"_ostr,
- "spid"_ostr, "14");
+ "spid"_ostr, "59");
}
CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest2, testTdf112280)
@@ -1936,10 +1936,10 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest2,
testTdf59323_slideFooters)
}
// Test placeholder indexes
- xmlDocUniquePtr pXmlDocMaster =
parseExport("ppt/slideMasters/slideMaster1.xml");
- assertXPath(pXmlDocMaster, "//p:ph [@type='dt']"_ostr, "idx"_ostr, "1");
- assertXPath(pXmlDocMaster, "//p:ph [@type='ftr']"_ostr, "idx"_ostr, "2");
- assertXPath(pXmlDocMaster, "//p:ph [@type='sldNum']"_ostr, "idx"_ostr,
"3");
+ xmlDocUniquePtr pXmlDocLayout =
parseExport("ppt/slideLayouts/slideLayout1.xml");
+ assertXPath(pXmlDocLayout, "//p:ph [@type='dt']"_ostr, "idx"_ostr, "1");
+ assertXPath(pXmlDocLayout, "//p:ph [@type='ftr']"_ostr, "idx"_ostr, "2");
+ assertXPath(pXmlDocLayout, "//p:ph [@type='sldNum']"_ostr, "idx"_ostr,
"3");
xmlDocUniquePtr pXmlDocSlide1 = parseExport("ppt/slides/slide1.xml");
assertXPath(pXmlDocSlide1, "//p:ph [@type='dt']"_ostr, "idx"_ostr, "1");
diff --git a/sd/qa/unit/export-tests-ooxml4.cxx
b/sd/qa/unit/export-tests-ooxml4.cxx
index dee24a51b509..f53f959388b9 100644
--- a/sd/qa/unit/export-tests-ooxml4.cxx
+++ b/sd/qa/unit/export-tests-ooxml4.cxx
@@ -1160,6 +1160,43 @@ CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4,
testTdf159931_slideLayouts)
bool(xNameAccess->hasByName("ppt/slideLayouts/" +
sSlideLayoutName2)));
}
+CPPUNIT_TEST_FIXTURE(SdOOXMLExportTest4, testDeduplicateMasters)
+{
+ createSdImpressDoc("pptx/onemaster-twolayouts.pptx");
+ saveAndReload("Impress Office Open XML");
+
+ // Check that the document still has one master and two layouts
+ xmlDocUniquePtr pXmlDocContent = parseExport("ppt/presentation.xml");
+ assertXPath(pXmlDocContent,
"/p:presentation/p:sldMasterIdLst/p:sldMasterId"_ostr, 1);
+ pXmlDocContent = parseExport("ppt/slideMasters/slideMaster1.xml");
+ assertXPath(pXmlDocContent,
"/p:sldMaster/p:sldLayoutIdLst/p:sldLayoutId"_ostr, 2);
+
+ // Check that both background colors have been preserved
+ uno::Reference<drawing::XMasterPagesSupplier> xDoc(mxComponent,
uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xDoc.is());
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xDoc->getMasterPages()->getCount());
+
+ uno::Reference<drawing::XDrawPage>
xPage(xDoc->getMasterPages()->getByIndex(0),
+ uno::UNO_QUERY_THROW);
+ uno::Reference<beans::XPropertySet> xPropSet(xPage, uno::UNO_QUERY);
+ uno::Any aAny = xPropSet->getPropertyValue("Background");
+ CPPUNIT_ASSERT(aAny.hasValue());
+ uno::Reference<beans::XPropertySet> aXBackgroundPropSet;
+ aAny >>= aXBackgroundPropSet;
+ Color nColor;
+ CPPUNIT_ASSERT(aXBackgroundPropSet->getPropertyValue("FillColor") >>=
nColor);
+ CPPUNIT_ASSERT_EQUAL(Color(0x0E2841), nColor);
+
+ uno::Reference<drawing::XDrawPage>
xPage1(xDoc->getMasterPages()->getByIndex(1),
+ uno::UNO_QUERY_THROW);
+ uno::Reference<beans::XPropertySet> xPropSet1(xPage1, uno::UNO_QUERY);
+ aAny = xPropSet1->getPropertyValue("Background");
+ CPPUNIT_ASSERT(aAny.hasValue());
+ aAny >>= aXBackgroundPropSet;
+ CPPUNIT_ASSERT(aXBackgroundPropSet->getPropertyValue("FillColor") >>=
nColor);
+ CPPUNIT_ASSERT_EQUAL(Color(0x000000), nColor);
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sd/source/filter/eppt/epptooxml.hxx
b/sd/source/filter/eppt/epptooxml.hxx
index e36fd3eac739..92ab55e1debd 100644
--- a/sd/source/filter/eppt/epptooxml.hxx
+++ b/sd/source/filter/eppt/epptooxml.hxx
@@ -90,6 +90,9 @@ private:
virtual void ImplWriteNotes( sal_uInt32 nPageNum ) override;
virtual void ImplWriteSlideMaster( sal_uInt32 nPageNum,
css::uno::Reference< css::beans::XPropertySet > const & aXBackgroundPropSet )
override;
void ImplWritePPTXLayout( sal_Int32 nOffset, sal_uInt32 nMasterNum, const
OUString& aSlideName );
+ void ImplWritePPTXLayoutWithContent(
+ sal_Int32 nOffset, sal_uInt32 nMasterNum, const OUString& aSlideName,
+ css::uno::Reference<css::beans::XPropertySet> const&
aXBackgroundPropSet);
static void WriteDefaultColorSchemes(const FSHelperPtr& pFS);
void WriteTheme( sal_Int32 nThemeNum, model::Theme* pTheme );
@@ -145,7 +148,7 @@ private:
LayoutInfo mLayoutInfo[OOXML_LAYOUT_SIZE];
// Pairs of masters and layouts as used by Impress
std::vector<std::pair<SdrPage*, sal_Int32>> maMastersLayouts;
- // For each Impress master, which master will represent it on the exported
file (themselves by default)
+ // For each Impress master, which master will represent it on the exported
file (SAL_MAX_UINT32 if not in an equivalency group)
std::vector<sal_uInt32> maEquivalentMasters;
std::vector< ::sax_fastparser::FSHelperPtr > mpSlidesFSArray;
sal_Int32 mnLayoutFileIdMax;
diff --git a/sd/source/filter/eppt/pptx-epptooxml.cxx
b/sd/source/filter/eppt/pptx-epptooxml.cxx
index b6a025e8a78a..a9f2fa4603ab 100644
--- a/sd/source/filter/eppt/pptx-epptooxml.cxx
+++ b/sd/source/filter/eppt/pptx-epptooxml.cxx
@@ -1513,10 +1513,9 @@ void PowerPointExport::FindEquivalentMasterPages()
css::uno::Reference<css::drawing::XDrawPages> xDrawPages(
mXMasterPagesSupplier->getMasterPages());
maMastersLayouts.resize(mnMasterPages);
- maEquivalentMasters.resize(mnMasterPages);
+ maEquivalentMasters.resize(mnMasterPages, SAL_MAX_UINT32);
for (sal_uInt32 i = 0; i < mnMasterPages; i++)
{
- maEquivalentMasters[i] = i;
css::uno::Reference<css::drawing::XDrawPage> xDrawPage;
uno::Any aAny(xDrawPages->getByIndex(i));
aAny >>= xDrawPage;
@@ -1537,11 +1536,11 @@ void PowerPointExport::FindEquivalentMasterPages()
for (sal_uInt32 i = 0; i < mnMasterPages; i++)
{
- if (!maMastersLayouts[i].first || maEquivalentMasters[i] != i)
+ if (!maMastersLayouts[i].first || maEquivalentMasters[i] !=
SAL_MAX_UINT32)
continue;
for (sal_uInt32 j = i + 1; j < mnMasterPages; j++)
{
- if (!maMastersLayouts[j].first || maEquivalentMasters[j] != j)
+ if (!maMastersLayouts[j].first || maEquivalentMasters[j] !=
SAL_MAX_UINT32)
continue;
if (lcl_ComparePageProperties(maMastersLayouts[i].first,
maMastersLayouts[j].first)
@@ -1550,6 +1549,7 @@ void PowerPointExport::FindEquivalentMasterPages()
// If both masters have the same properties and objects,
// we assume they are the same and only export the first one
maEquivalentMasters[j] = i;
+ maEquivalentMasters[i] = i;
}
}
}
@@ -1564,9 +1564,22 @@ sal_uInt32
PowerPointExport::GetEquivalentMasterPage(sal_uInt32 nMasterPage)
void PowerPointExport::ImplWriteSlideMaster(sal_uInt32 nPageNum, Reference<
XPropertySet > const& aXBackgroundPropSet)
{
- if (nPageNum != GetEquivalentMasterPage(nPageNum))
+ SAL_INFO("sd.eppt", "write master slide: " << nPageNum << "
--------------");
+
+ if (nPageNum != GetEquivalentMasterPage(nPageNum)
+ && GetEquivalentMasterPage(nPageNum) != SAL_MAX_UINT32)
{
- // We already exported it's layout on an equivalent master so do
nothing
+ // It's equivalent to an already written master, write only the layout
file
+ if (maMastersLayouts[nPageNum].second != -1)
+ {
+ OUString aSlideName;
+ Reference<XNamed> xNamed(mXDrawPage, UNO_QUERY);
+ if (xNamed.is())
+ aSlideName = xNamed->getName();
+ ImplWritePPTXLayoutWithContent(maMastersLayouts[nPageNum].second,
nPageNum, aSlideName,
+ aXBackgroundPropSet);
+ }
+
// Close the list tag if it was the last one
if (nPageNum == mnMasterPages - 1)
mPresentationFS->endElementNS(XML_p, XML_sldMasterIdLst);
@@ -1574,8 +1587,6 @@ void PowerPointExport::ImplWriteSlideMaster(sal_uInt32
nPageNum, Reference< XPro
return;
}
- SAL_INFO("sd.eppt", "write master slide: " << nPageNum << "
--------------");
-
// slides list
if (nPageNum == 0)
mPresentationFS->startElementNS(XML_p, XML_sldMasterIdLst);
@@ -1614,9 +1625,19 @@ void PowerPointExport::ImplWriteSlideMaster(sal_uInt32
nPageNum, Reference< XPro
pFS->startElementNS(XML_p, XML_cSld);
- if (aXBackgroundPropSet)
- ImplWriteBackground(pFS, aXBackgroundPropSet);
- WriteShapeTree(pFS, MASTER, true);
+ if (GetEquivalentMasterPage(nPageNum) != nPageNum)
+ {
+ if (aXBackgroundPropSet)
+ ImplWriteBackground(pFS, aXBackgroundPropSet);
+ WriteShapeTree(pFS, MASTER, true);
+ }
+ else
+ {
+ // Minimal shape tree, the actual one will be written in the layout
file.
+ pFS->startElementNS(XML_p, XML_spTree);
+ pFS->write(MAIN_GROUP);
+ pFS->endElementNS(XML_p, XML_spTree);
+ }
pFS->endElementNS(XML_p, XML_cSld);
@@ -1733,16 +1754,24 @@ void PowerPointExport::ImplWriteSlideMaster(sal_uInt32
nPageNum, Reference< XPro
for (auto nLayout : aLayouts)
{
- ImplWritePPTXLayout(nLayout, nPageNum, aSlideName);
+ if (GetEquivalentMasterPage(nPageNum) == nPageNum)
+ ImplWritePPTXLayoutWithContent(nLayout, nPageNum, aSlideName,
aXBackgroundPropSet);
+ else
+ ImplWritePPTXLayout(nLayout, nPageNum, aSlideName);
AddLayoutIdAndRelation(pFS, GetLayoutFileId(nLayout, nPageNum));
}
- // Export layouts of other Impress masters that came from a sinlge pptx
master with multiple layouts
+ // Add layouts of other Impress masters that came from a sinlge pptx
master with multiple layouts
for (sal_uInt32 i = 0; i < mnMasterPages; i++)
{
if (i != nPageNum && maEquivalentMasters[i] == nPageNum &&
maMastersLayouts[i].second != -1)
{
- ImplWritePPTXLayout(maMastersLayouts[i].second, i, aSlideName);
+ // Reserve layout file Id to be writen later
+ if (mLayoutInfo[maMastersLayouts[i].second].mnFileIdArray.size() <
mnMasterPages)
+
mLayoutInfo[maMastersLayouts[i].second].mnFileIdArray.resize(mnMasterPages);
+ mLayoutInfo[maMastersLayouts[i].second].mnFileIdArray[i] =
mnLayoutFileIdMax;
+ mnLayoutFileIdMax++;
+
AddLayoutIdAndRelation(pFS,
GetLayoutFileId(maMastersLayouts[i].second, i));
}
}
@@ -1800,9 +1829,9 @@ void PowerPointExport::ImplWritePPTXLayout(sal_Int32
nOffset, sal_uInt32 nMaster
"application/vnd.openxmlformats-officedocument.presentationml.slideLayout+xml");
// add implicit relation of slide layout to slide master
- addRelation(pFS->getOutputStream(),
oox::getRelationship(Relationship::SLIDEMASTER),
- Concat2View("../slideMasters/slideMaster"
- +
OUString::number(GetEquivalentMasterPage(nMasterNum) + 1) + ".xml"));
+ addRelation(
+ pFS->getOutputStream(),
oox::getRelationship(Relationship::SLIDEMASTER),
+ Concat2View("../slideMasters/slideMaster" +
OUString::number(nMasterNum + 1) + ".xml"));
pFS->startElementNS(XML_p, XML_sldLayout,
PNMSS,
@@ -1835,6 +1864,57 @@ void PowerPointExport::ImplWritePPTXLayout(sal_Int32
nOffset, sal_uInt32 nMaster
pFS->endDocument();
}
+void PowerPointExport::ImplWritePPTXLayoutWithContent(
+ sal_Int32 nOffset, sal_uInt32 nMasterNum, const OUString& aSlideName,
+ Reference<XPropertySet> const& aXBackgroundPropSet)
+{
+ SAL_INFO("sd.eppt", "write layout: " << nOffset);
+
+ if (mLayoutInfo[nOffset].mnFileIdArray.size() < mnMasterPages)
+ {
+ mLayoutInfo[nOffset].mnFileIdArray.resize(mnMasterPages);
+ }
+
+ if (mLayoutInfo[nOffset].mnFileIdArray[nMasterNum] == 0)
+ {
+ mLayoutInfo[nOffset].mnFileIdArray[nMasterNum] = mnLayoutFileIdMax;
+ mnLayoutFileIdMax++;
+ }
+ sal_Int32 nLayoutFileId = mLayoutInfo[nOffset].mnFileIdArray[nMasterNum];
+
+ FSHelperPtr pFS = openFragmentStreamWithSerializer(
+ "ppt/slideLayouts/slideLayout" + OUString::number(nLayoutFileId) +
".xml",
+
"application/vnd.openxmlformats-officedocument.presentationml.slideLayout+xml");
+
+ // add implicit relation of slide layout to slide master
+ addRelation(pFS->getOutputStream(),
oox::getRelationship(Relationship::SLIDEMASTER),
+ Concat2View("../slideMasters/slideMaster"
+ +
OUString::number(GetEquivalentMasterPage(nMasterNum) + 1) + ".xml"));
+
+ pFS->startElementNS(XML_p, XML_sldLayout, PNMSS, XML_type,
aLayoutInfo[nOffset].sType,
+ XML_preserve, "1");
+
+ if (!aSlideName.isEmpty())
+ {
+ pFS->startElementNS(XML_p, XML_cSld, XML_name, aSlideName);
+ }
+ else
+ {
+ pFS->startElementNS(XML_p, XML_cSld, XML_name,
aLayoutInfo[nOffset].sName);
+ }
+
+ if (aXBackgroundPropSet)
+ ImplWriteBackground(pFS, aXBackgroundPropSet);
+
+ WriteShapeTree(pFS, MASTER, true);
+
+ pFS->endElementNS(XML_p, XML_cSld);
+
+ pFS->endElementNS(XML_p, XML_sldLayout);
+
+ pFS->endDocument();
+}
+
void PowerPointExport::WriteShapeTree(const FSHelperPtr& pFS, PageType
ePageType, bool bMaster)
{
PowerPointShapeExport aDML(pFS, &maShapeMap, this);
@@ -2457,25 +2537,26 @@ Reference<XShape>
PowerPointExport::GetReferencedPlaceholderXShape(const Placeho
}
if (ePresObjKind != PresObjKind::NONE)
{
- SdPage* pMasterPage;
+ SdrPage* pPage;
if (ePageType == LAYOUT)
{
// since Layout pages do not have drawpages themselves -
mXDrawPage is still the master they reference to..
- pMasterPage = SdPage::getImplementation(mXDrawPage);
+ pPage = SdPage::getImplementation(mXDrawPage);
}
else
{
- SdrPage* pPage =
&SdPage::getImplementation(mXDrawPage)->TRG_GetMasterPage();
- for (sal_uInt32 i = 0; i < mnMasterPages; i++)
+ pPage =
&SdPage::getImplementation(mXDrawPage)->TRG_GetMasterPage();
+ }
+ for (sal_uInt32 i = 0; i < mnMasterPages; i++)
+ {
+ if (maMastersLayouts[i].first == pPage)
{
- if (maMastersLayouts[i].first == pPage)
- {
+ if (maEquivalentMasters[i] < mnMasterPages)
pPage = maMastersLayouts[maEquivalentMasters[i]].first;
- break;
- }
+ break;
}
- pMasterPage = dynamic_cast<SdPage*>(pPage);
}
+ SdPage* pMasterPage = dynamic_cast<SdPage*>(pPage);
if (SdrObject* pMasterFooter
= (pMasterPage ? pMasterPage->GetPresObj(ePresObjKind) : nullptr))
return GetXShapeForSdrObject(pMasterFooter);
