include/oox/export/drawingml.hxx | 8 +
oox/source/export/drawingml.cxx | 207 +++++++++++++++++--------------
sc/qa/unit/data/ods/tdf91286.ods |binary
sc/qa/unit/subsequent_export_test2.cxx | 26 +++
sc/source/filter/excel/xestream.cxx | 4
sd/qa/unit/data/odp/tdf74670.odp |binary
sd/qa/unit/export-tests-ooxml3.cxx | 27 ++++
sd/source/filter/eppt/pptx-epptooxml.cxx | 2
8 files changed, 183 insertions(+), 91 deletions(-)
New commits:
commit cd4b20cd84018b7984826f42b878500cfd2d34c6
Author: Tünde Tóth <toth.tu...@nisz.hu>
AuthorDate: Tue Mar 22 09:47:57 2022 +0100
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Wed Mar 8 12:38:28 2023 +0000
tdf#74670 tdf#91286 PPTX XLSX export: save image once
Impress and Calc used to dump the same image file
as many times as it was featured in the document,
resulting redundant, sometimes huge documents.
Note: using only checksum to recognize image duplication
is a regression, because checksum collision results
image loss. This is a very unlikely event, and
the following commits have got the same problem.
The solution is comparing the images with the same
checksum byte for byte.
See also commit b484e9814c66d8d51cea974390963a6944bc9d73
"tdf#83227 oox: reuse RelId in DML/VML export for the same graphic"
and commit 797fef38612fb2fd62d1f6591619b9361e526bca
"tdf#118535 DOCX export: save header image once".
Change-Id: I9f233d521941381746634cf4f9b5991da0dadda9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131928
Tested-by: László Németh <nem...@numbertext.org>
Reviewed-by: László Németh <nem...@numbertext.org>
(cherry picked from commit aea8043bc5f5187498fa450505d6de9d6986e2a6)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/148270
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/include/oox/export/drawingml.hxx b/include/oox/export/drawingml.hxx
index 8e4fe156c0cd..7c79896a29b8 100644
--- a/include/oox/export/drawingml.hxx
+++ b/include/oox/export/drawingml.hxx
@@ -21,7 +21,9 @@
#define INCLUDED_OOX_EXPORT_DRAWINGML_HXX
#include <map>
+#include <stack>
#include <string_view>
+#include <unordered_map>
#include <vector>
#include <com/sun/star/beans/PropertyState.hpp>
@@ -41,6 +43,7 @@
#include <sax/fshelper.hxx>
#include <svx/msdffdef.hxx>
#include <vcl/checksum.hxx>
+#include <vcl/graph.hxx>
#include <tools/gen.hxx>
#include <vcl/mapmod.hxx>
#include <svx/EnhancedCustomShape2d.hxx>
@@ -149,6 +152,7 @@ private:
static std::map<OUString, OUString> maWdpCache;
static sal_Int32 mnDrawingMLCount;
static sal_Int32 mnVmlCount;
+ static std::stack<std::unordered_map<BitmapChecksum, OUString>>
maExportGraphics;
/// To specify where write eg. the images to (like 'ppt', or 'word' -
according to the OPC).
DocumentType meDocumentType;
@@ -344,9 +348,11 @@ public:
sal_Int32 getBulletMarginIndentation (const css::uno::Reference<
css::beans::XPropertySet >& rXPropSet,sal_Int16 nLevel, std::u16string_view
propName);
static void ResetCounters();
-
static void ResetMlCounters();
+ static void PushExportGraphics();
+ static void PopExportGraphics();
+
static sal_Int32 getNewDrawingUniqueId() { return ++mnDrawingMLCount; }
static sal_Int32 getNewVMLUniqueId() { return ++mnVmlCount; }
diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx
index 55a9f07e90fe..e843d51606b7 100644
--- a/oox/source/export/drawingml.cxx
+++ b/oox/source/export/drawingml.cxx
@@ -112,7 +112,6 @@
#include <tools/stream.hxx>
#include <unotools/fontdefs.hxx>
#include <vcl/cvtgrf.hxx>
-#include <vcl/graph.hxx>
#include <vcl/svapp.hxx>
#include <rtl/strbuf.hxx>
#include <filter/msfilter/escherex.hxx>
@@ -240,6 +239,7 @@ int DrawingML::mnWdpImageCounter = 1;
std::map<OUString, OUString> DrawingML::maWdpCache;
sal_Int32 DrawingML::mnDrawingMLCount = 0;
sal_Int32 DrawingML::mnVmlCount = 0;
+std::stack<std::unordered_map<BitmapChecksum, OUString>>
DrawingML::maExportGraphics;
sal_Int16 DrawingML::GetScriptType(const OUString& rStr)
{
@@ -278,6 +278,16 @@ void DrawingML::ResetMlCounters()
mnVmlCount = 0;
}
+void DrawingML::PushExportGraphics()
+{
+ maExportGraphics.emplace();
+}
+
+void DrawingML::PopExportGraphics()
+{
+ maExportGraphics.pop();
+}
+
bool DrawingML::GetProperty( const Reference< XPropertySet >& rXPropertySet,
const OUString& aName )
{
try
@@ -1243,113 +1253,130 @@ const char* DrawingML::GetRelationCompPrefix() const
OUString DrawingML::WriteImage( const Graphic& rGraphic , bool
bRelPathToMedia, OUString* pFileName )
{
GfxLink aLink = rGraphic.GetGfxLink ();
+ BitmapChecksum aChecksum = rGraphic.GetChecksum();
OUString sMediaType;
const char* pExtension = "";
OUString sRelId;
+ OUString sPath;
- SvMemoryStream aStream;
- const void* aData = aLink.GetData();
- std::size_t nDataSize = aLink.GetDataSize();
-
- switch ( aLink.GetType() )
+ // tdf#74670 tdf#91286 Save image only once (this is no problem for DOCX)
+ if (GetDocumentType() != DOCUMENT_DOCX && !maExportGraphics.empty())
{
- case GfxLinkType::NativeGif:
- sMediaType = "image/gif";
- pExtension = ".gif";
- break;
+ auto aIterator = maExportGraphics.top().find(aChecksum);
+ if (aIterator != maExportGraphics.top().end())
+ sPath = aIterator->second;
+ }
- // #i15508# added BMP type for better exports
- // export not yet active, so adding for reference (not checked)
- case GfxLinkType::NativeBmp:
- sMediaType = "image/bmp";
- pExtension = ".bmp";
- break;
+ if (sPath.isEmpty())
+ {
+ SvMemoryStream aStream;
+ const void* aData = aLink.GetData();
+ std::size_t nDataSize = aLink.GetDataSize();
- case GfxLinkType::NativeJpg:
- sMediaType = "image/jpeg";
- pExtension = ".jpeg";
- break;
- case GfxLinkType::NativePng:
- sMediaType = "image/png";
- pExtension = ".png";
- break;
- case GfxLinkType::NativeTif:
- sMediaType = "image/tiff";
- pExtension = ".tif";
- break;
- case GfxLinkType::NativeWmf:
- sMediaType = "image/x-wmf";
- pExtension = ".wmf";
- break;
- case GfxLinkType::NativeMet:
- sMediaType = "image/x-met";
- pExtension = ".met";
- break;
- case GfxLinkType::NativePct:
- sMediaType = "image/x-pict";
- pExtension = ".pct";
- break;
- case GfxLinkType::NativeMov:
- sMediaType = "application/movie";
- pExtension = ".MOV";
- break;
- default:
+ switch (aLink.GetType())
{
- GraphicType aType = rGraphic.GetType();
- if ( aType == GraphicType::Bitmap || aType ==
GraphicType::GdiMetafile)
+ case GfxLinkType::NativeGif:
+ sMediaType = "image/gif";
+ pExtension = ".gif";
+ break;
+
+ // #i15508# added BMP type for better exports
+ // export not yet active, so adding for reference (not checked)
+ case GfxLinkType::NativeBmp:
+ sMediaType = "image/bmp";
+ pExtension = ".bmp";
+ break;
+
+ case GfxLinkType::NativeJpg:
+ sMediaType = "image/jpeg";
+ pExtension = ".jpeg";
+ break;
+ case GfxLinkType::NativePng:
+ sMediaType = "image/png";
+ pExtension = ".png";
+ break;
+ case GfxLinkType::NativeTif:
+ sMediaType = "image/tiff";
+ pExtension = ".tif";
+ break;
+ case GfxLinkType::NativeWmf:
+ sMediaType = "image/x-wmf";
+ pExtension = ".wmf";
+ break;
+ case GfxLinkType::NativeMet:
+ sMediaType = "image/x-met";
+ pExtension = ".met";
+ break;
+ case GfxLinkType::NativePct:
+ sMediaType = "image/x-pict";
+ pExtension = ".pct";
+ break;
+ case GfxLinkType::NativeMov:
+ sMediaType = "application/movie";
+ pExtension = ".MOV";
+ break;
+ default:
{
- if ( aType == GraphicType::Bitmap )
+ GraphicType aType = rGraphic.GetType();
+ if (aType == GraphicType::Bitmap || aType ==
GraphicType::GdiMetafile)
{
- (void)GraphicConverter::Export( aStream, rGraphic,
ConvertDataFormat::PNG );
- sMediaType = "image/png";
- pExtension = ".png";
+ if (aType == GraphicType::Bitmap)
+ {
+ (void)GraphicConverter::Export(aStream, rGraphic,
ConvertDataFormat::PNG);
+ sMediaType = "image/png";
+ pExtension = ".png";
+ }
+ else
+ {
+ (void)GraphicConverter::Export(aStream, rGraphic,
ConvertDataFormat::EMF);
+ sMediaType = "image/x-emf";
+ pExtension = ".emf";
+ }
}
else
{
- (void)GraphicConverter::Export( aStream, rGraphic,
ConvertDataFormat::EMF );
- sMediaType = "image/x-emf";
- pExtension = ".emf";
+ SAL_WARN("oox.shape", "unhandled graphic type " <<
static_cast<int>(aType));
+ /*Earlier, even in case of unhandled graphic types we were
+ proceeding to write the image, which would eventually
+ write an empty image with a zero size, and return a valid
+ relationID, which is incorrect.
+ */
+ return sRelId;
}
- }
- else
- {
- SAL_WARN("oox.shape", "unhandled graphic type " <<
static_cast<int>(aType) );
- /*Earlier, even in case of unhandled graphic types we were
- proceeding to write the image, which would eventually
- write an empty image with a zero size, and return a valid
- relationID, which is incorrect.
- */
- return sRelId;
- }
- aData = aStream.GetData();
- nDataSize = aStream.GetEndOfData();
- break;
+ aData = aStream.GetData();
+ nDataSize = aStream.GetEndOfData();
+ break;
+ }
}
- }
- Reference< XOutputStream > xOutStream = mpFB->openFragmentStream(
OUStringBuffer()
-
.appendAscii( GetComponentDir() )
- .append(
"/media/image" +
-
OUString::number(mnImageCounter) )
-
.appendAscii( pExtension )
-
.makeStringAndClear(),
-
sMediaType );
- xOutStream->writeBytes( Sequence< sal_Int8 >( static_cast<const
sal_Int8*>(aData), nDataSize ) );
- xOutStream->closeOutput();
+ Reference<XOutputStream> xOutStream = mpFB->openFragmentStream(
+ OUStringBuffer()
+ .appendAscii(GetComponentDir())
+ .append("/media/image" + OUString::number(mnImageCounter))
+ .appendAscii(pExtension)
+ .makeStringAndClear(),
+ sMediaType);
+ xOutStream->writeBytes(Sequence<sal_Int8>(static_cast<const
sal_Int8*>(aData), nDataSize));
+ xOutStream->closeOutput();
+
+ const OString sRelPathToMedia = "media/image";
+ OString sRelationCompPrefix;
+ if (bRelPathToMedia)
+ sRelationCompPrefix = "../";
+ else
+ sRelationCompPrefix = GetRelationCompPrefix();
+ sPath = OUStringBuffer()
+ .appendAscii(sRelationCompPrefix.getStr())
+ .appendAscii(sRelPathToMedia.getStr())
+ .append(static_cast<sal_Int32>(mnImageCounter++))
+ .appendAscii(pExtension)
+ .makeStringAndClear();
+
+ if (GetDocumentType() != DOCUMENT_DOCX && !maExportGraphics.empty())
+ maExportGraphics.top()[aChecksum] = sPath;
+ }
- const OString sRelPathToMedia = "media/image";
- OString sRelationCompPrefix;
- if ( bRelPathToMedia )
- sRelationCompPrefix = "../";
- else
- sRelationCompPrefix = GetRelationCompPrefix();
- OUString sPath = OUStringBuffer()
- .appendAscii( sRelationCompPrefix.getStr() )
- .appendAscii( sRelPathToMedia.getStr() )
- .append( static_cast<sal_Int32>(mnImageCounter ++) )
- .appendAscii( pExtension )
- .makeStringAndClear();
sRelId = mpFB->addRelation( mpFS->getOutputStream(),
oox::getRelationship(Relationship::IMAGE),
sPath );
diff --git a/sc/qa/unit/data/ods/tdf91286.ods b/sc/qa/unit/data/ods/tdf91286.ods
new file mode 100644
index 000000000000..8bc260584e10
Binary files /dev/null and b/sc/qa/unit/data/ods/tdf91286.ods differ
diff --git a/sc/qa/unit/subsequent_export_test2.cxx
b/sc/qa/unit/subsequent_export_test2.cxx
index 329210a6d9eb..d9030eb24404 100644
--- a/sc/qa/unit/subsequent_export_test2.cxx
+++ b/sc/qa/unit/subsequent_export_test2.cxx
@@ -83,6 +83,7 @@
#include <com/sun/star/awt/XBitmap.hpp>
#include <com/sun/star/frame/Desktop.hpp>
#include <com/sun/star/graphic/GraphicType.hpp>
+#include <com/sun/star/packages/zip/ZipFileAccess.hpp>
#include <com/sun/star/sheet/GlobalSheetSettings.hpp>
#include <com/sun/star/sheet/XHeaderFooterContent.hpp>
#include <com/sun/star/text/XTextColumns.hpp>
@@ -220,6 +221,7 @@ public:
void testTdf130104_XLSXIndent();
void testWholeRowBold();
void testXlsxRowsOrder();
+ void testTdf91286();
void testTdf148820();
CPPUNIT_TEST_SUITE(ScExportTest2);
@@ -340,6 +342,7 @@ public:
CPPUNIT_TEST(testTdf130104_XLSXIndent);
CPPUNIT_TEST(testWholeRowBold);
CPPUNIT_TEST(testXlsxRowsOrder);
+ CPPUNIT_TEST(testTdf91286);
CPPUNIT_TEST(testTdf148820);
CPPUNIT_TEST_SUITE_END();
@@ -3165,6 +3168,29 @@ void ScExportTest2::testXlsxRowsOrder()
xDocSh->DoClose();
}
+void ScExportTest2::testTdf91286()
+{
+ ScDocShellRef xDocSh = loadDoc(u"tdf91286.", FORMAT_ODS);
+ CPPUNIT_ASSERT(xDocSh.is());
+ std::shared_ptr<utl::TempFile> pTemp = exportTo(*xDocSh, FORMAT_XLSX);
+ xDocSh->DoClose();
+
+ Reference<packages::zip::XZipFileAccess2> xNameAccess
+ =
packages::zip::ZipFileAccess::createWithURL(comphelper::getComponentContext(m_xSFactory),
+ pTemp->GetURL());
+ const Sequence<OUString> aNames(xNameAccess->getElementNames());
+ int nImageFiles = 0;
+ for (const auto& rElementName : aNames)
+ if (rElementName.startsWith("xl/media/image"))
+ nImageFiles++;
+
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 1
+ // - Actual : 2
+ // i.e. the embedded picture would have been saved twice.
+ CPPUNIT_ASSERT_EQUAL(1, nImageFiles);
+}
+
void ScExportTest2::testTdf148820()
{
ScDocShellRef xDocSh = loadDoc(u"tdf148820.", FORMAT_XLSX);
diff --git a/sc/source/filter/excel/xestream.cxx
b/sc/source/filter/excel/xestream.cxx
index 6de4db3b77ab..c761305d5996 100644
--- a/sc/source/filter/excel/xestream.cxx
+++ b/sc/source/filter/excel/xestream.cxx
@@ -1023,6 +1023,7 @@ bool XclExpXmlStream::exportDocument()
// Instead, write via XOutputStream instance.
tools::SvRef<SotStorage> rStorage = static_cast<SotStorage*>(nullptr);
drawingml::DrawingML::ResetMlCounters();
+ drawingml::DrawingML::PushExportGraphics();
XclExpRootData aData(
EXC_BIFF8, *pShell->GetMedium (), rStorage, rDoc,
@@ -1140,6 +1141,9 @@ bool XclExpXmlStream::exportDocument()
if (xStatusIndicator.is())
xStatusIndicator->end();
mpRoot = nullptr;
+
+ drawingml::DrawingML::PopExportGraphics();
+
return true;
}
diff --git a/sd/qa/unit/data/odp/tdf74670.odp b/sd/qa/unit/data/odp/tdf74670.odp
new file mode 100644
index 000000000000..98e223eb0725
Binary files /dev/null and b/sd/qa/unit/data/odp/tdf74670.odp differ
diff --git a/sd/qa/unit/export-tests-ooxml3.cxx
b/sd/qa/unit/export-tests-ooxml3.cxx
index 8655866b9e7b..e5a57b17055e 100644
--- a/sd/qa/unit/export-tests-ooxml3.cxx
+++ b/sd/qa/unit/export-tests-ooxml3.cxx
@@ -130,6 +130,7 @@ public:
void testTdf147121();
void testTdf140912_PicturePlaceholder();
void testEnhancedPathViewBox();
+ void testTdf74670();
void testTdf109169_OctagonBevel();
void testTdf109169_DiamondBevel();
void testTdf144092_emptyShapeTextProps();
@@ -211,10 +212,12 @@ public:
CPPUNIT_TEST(testTdf147121);
CPPUNIT_TEST(testTdf140912_PicturePlaceholder);
CPPUNIT_TEST(testEnhancedPathViewBox);
+ CPPUNIT_TEST(testTdf74670);
CPPUNIT_TEST(testTdf109169_OctagonBevel);
CPPUNIT_TEST(testTdf109169_DiamondBevel);
CPPUNIT_TEST(testTdf144092_emptyShapeTextProps);
CPPUNIT_TEST(testTdf94122_autoColor);
+
CPPUNIT_TEST_SUITE_END();
virtual void registerNamespaces(xmlXPathContextPtr& pXmlXPathCtx) override
@@ -1945,6 +1948,30 @@ void SdOOXMLExportTest3::testEnhancedPathViewBox()
CPPUNIT_ASSERT_EQUAL(sal_Int32(2045), aBoundRectangle.Width);
}
+void SdOOXMLExportTest3::testTdf74670()
+{
+ ::sd::DrawDocShellRef xDocShRef
+ =
loadURL(m_directories.getURLFromSrc(u"/sd/qa/unit/data/odp/tdf74670.odp"), ODP);
+ utl::TempFile tmpfile;
+ xDocShRef = saveAndReload(xDocShRef.get(), PPTX, &tmpfile);
+ xDocShRef->DoClose();
+
+ uno::Reference<packages::zip::XZipFileAccess2> xNameAccess
+ =
packages::zip::ZipFileAccess::createWithURL(comphelper::getComponentContext(m_xSFactory),
+ tmpfile.GetURL());
+ const uno::Sequence<OUString> aNames(xNameAccess->getElementNames());
+ int nImageFiles = 0;
+ for (const auto& rElementName : aNames)
+ if (rElementName.startsWith("ppt/media/image"))
+ nImageFiles++;
+
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 1
+ // - Actual : 2
+ // i.e. the embedded picture would have been saved twice.
+ CPPUNIT_ASSERT_EQUAL(1, nImageFiles);
+}
+
void SdOOXMLExportTest3::testTdf109169_OctagonBevel()
{
// The document has a shape 'Octagon Bevel'. It consists of an octagon
with 8 points and eight
diff --git a/sd/source/filter/eppt/pptx-epptooxml.cxx
b/sd/source/filter/eppt/pptx-epptooxml.cxx
index f4fab7ef1b52..601ce98076a8 100644
--- a/sd/source/filter/eppt/pptx-epptooxml.cxx
+++ b/sd/source/filter/eppt/pptx-epptooxml.cxx
@@ -428,6 +428,7 @@ bool PowerPointExport::importDocument() noexcept
bool PowerPointExport::exportDocument()
{
DrawingML::ResetCounters();
+ DrawingML::PushExportGraphics();
maShapeMap.clear();
mXModel = getModel();
@@ -511,6 +512,7 @@ bool PowerPointExport::exportDocument()
commitStorage();
+ DrawingML::PopExportGraphics();
maShapeMap.clear();
maAuthors.clear();
maRelId.clear();
