docx export: Use checksum as key to cache graphic, not Graphic*

The problem is when we have multiple identical images in the
document and try to export. Without this change, the images will
be duplicated because the cache is using Graphic*, for which the
instance changes all the time (is not unique). Using the checksum
will make sure that all the images with the same content will be
saved as one image into the document.

Change-Id: I980af2dba51060ce4320571aca14d21e26ed8976
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103132
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>

2 files changed, 39 insertions, 13 deletions

diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx
index 59c75c6fa635..8142c6f0a667 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -4889,8 +4889,8 @@ void DocxAttributeOutput::PopRelIdCache()
 
 void DocxAttributeOutput::PushRelIdCache()
 {
-    m_aRelIdCache.push(std::map<const Graphic*, OString>());
-    m_aSdrRelIdCache.push(std::map<BitmapChecksum, OUString>());
+    m_aRelIdCache.emplace();
+    m_aSdrRelIdCache.emplace();
 }
 
 OUString DocxAttributeOutput::FindRelId(BitmapChecksum nChecksum)
@@ -4909,6 +4909,29 @@ void DocxAttributeOutput::CacheRelId(BitmapChecksum nChecksum, const OUString& r
         m_aSdrRelIdCache.top()[nChecksum] = rRelId;
 }
 
+OString DocxAttributeOutput::getExistingGraphicRelId(BitmapChecksum nChecksum)
+{
+    OString aResult;
+
+    if (m_aRelIdCache.empty())
+        return aResult;
+
+    auto pIterator = m_aRelIdCache.top().find(nChecksum);
+
+    if (pIterator != m_aRelIdCache.top().end())
+    {
+        aResult = pIterator->second;
+    }
+
+    return aResult;
+}
+
+void DocxAttributeOutput::cacheGraphicRelId(BitmapChecksum nChecksum, OString const & rRelId)
+{
+    if (!m_aRelIdCache.empty())
+        m_aRelIdCache.top().emplace(nChecksum, rRelId);
+}
+
 void DocxAttributeOutput::FlyFrameGraphic( const SwGrfNode* pGrfNode, const Size& rSize, const SwFlyFrameFormat* pOLEFrameFormat, SwOLENode* pOLENode, const SdrObject* pSdrObj )
 {
     SAL_INFO("sw.ww8", "TODO DocxAttributeOutput::FlyFrameGraphic( const SwGrfNode* pGrfNode, const Size& rSize, const SwFlyFrameFormat* pOLEFrameFormat, SwOLENode* pOLENode, const SdrObject* pSdrObj  ) - some stuff still missing" );
@@ -4948,25 +4971,24 @@ void DocxAttributeOutput::FlyFrameGraphic( const SwGrfNode* pGrfNode, const Size
     else
     {
         // inline, we also have to write the image itself
-        const Graphic* pGraphic = nullptr;
+        Graphic aGraphic;
         if (pGrfNode)
-            pGraphic = &pGrfNode->GetGrf();
+            aGraphic = pGrfNode->GetGrf();
         else
-            pGraphic = pOLENode->GetGraphic();
+            aGraphic = *pOLENode->GetGraphic();
 
-        if (!m_aRelIdCache.empty() && m_aRelIdCache.top().find(pGraphic) != m_aRelIdCache.top().end())
-            // We already have a RelId for this Graphic.
-            aRelId = m_aRelIdCache.top()[pGraphic];
-        else
+        BitmapChecksum aChecksum = aGraphic.GetChecksum();
+        aRelId = getExistingGraphicRelId(aChecksum);
+
+        if (aRelId.isEmpty())
         {
             // Not in cache, then need to write it.
             m_rDrawingML.SetFS( m_pSerializer ); // to be sure that we write to the right stream
 
-            OUString aImageId = m_rDrawingML.WriteImage( *pGraphic );
+            OUString aImageId = m_rDrawingML.WriteImage(aGraphic);
 
             aRelId = OUStringToOString( aImageId, RTL_TEXTENCODING_UTF8 );
-            if (!m_aRelIdCache.empty())
-                m_aRelIdCache.top()[pGraphic] = aRelId;
+            cacheGraphicRelId(aChecksum, aRelId);
         }
 
         nImageType = XML_embed;
diff --git a/sw/source/filter/ww8/docxattributeoutput.hxx b/sw/source/filter/ww8/docxattributeoutput.hxx
index 089914ea0b2b..b9278e7f771c 100644
--- a/sw/source/filter/ww8/docxattributeoutput.hxx
+++ b/sw/source/filter/ww8/docxattributeoutput.hxx
@@ -958,8 +958,12 @@ private:
     // store hardcoded value which was set during import.
     sal_Int32 m_nParaBeforeSpacing,m_nParaAfterSpacing;
 
+    OString getExistingGraphicRelId(BitmapChecksum aChecksum);
+    void cacheGraphicRelId(BitmapChecksum nChecksum, OString const & rRelId);
+
     /// RelId <-> Graphic* cache, so that in case of alternate content, the same graphic only gets written once.
-    std::stack< std::map<const Graphic*, OString> > m_aRelIdCache;
+    std::stack< std::map<BitmapChecksum, OString> > m_aRelIdCache;
+
     /// RelId <-> BitmapChecksum cache, similar to m_aRelIdCache, but used for non-Writer graphics, handled in oox.
     std::stack< std::map<BitmapChecksum, OUString> > m_aSdrRelIdCache;