[Libreoffice-commits] core.git: include/vcl vcl/inc vcl/source

Noel Grandin (via logerrit) logerrit at kemper.freedesktop.org
Wed May 22 07:48:58 UTC 2019


 include/vcl/graph.hxx                |    7 +------
 include/vcl/vectorgraphicdata.hxx    |    1 +
 vcl/inc/impgraph.hxx                 |    2 ++
 vcl/source/gdi/bitmapex.cxx          |    8 ++++----
 vcl/source/gdi/impgraph.cxx          |   27 +++++++++++----------------
 vcl/source/gdi/vectorgraphicdata.cxx |    6 ++++++
 6 files changed, 25 insertions(+), 26 deletions(-)

New commits:
commit 319c57d2af5d26d3910db4b02dca145d8881af44
Author:     Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Mon May 20 19:08:19 2019 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Wed May 22 09:48:21 2019 +0200

    tdf#120837 File saving at least 5 times slower
    
    The problem here is that we never actually hit the maExportGraphics
    cache in SvXMLGraphicHelper, even though we are passing the same image
    down repeatedly.
    
    There are two bugs here:
    
    (1) BitmapEx::operator== does not return true if we instantiate 2
    Graphic objects from the same XGraphic, so change it to use the more
    expensive operator==. To mitigate the cost, move the expensive checks to
    the bottom of the method.
    
    (2) in order to use an object in std::unordered_map, the object must
    implement an equality function and a hash function. If two objects are
    equal THEY MUST have the same hash value. Using the Impl* as the hash
    value does not satisfy that condition, so rather use the checksum, which
    does.
    
    After these fixes, the save time drops to less than a second.
    
    Also make the checksum method look more like the operator== method,
    and add a checksum calculation method for SVG data that more accurately
    reflects the underlying SVG data.
    
    Change-Id: I4ca0c7bee60b2efa6fe42301e582c7b278022b46
    Reviewed-on: https://gerrit.libreoffice.org/72615
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/include/vcl/graph.hxx b/include/vcl/graph.hxx
index e8af0d2c06ce..d49a8a2ce512 100644
--- a/include/vcl/graph.hxx
+++ b/include/vcl/graph.hxx
@@ -195,11 +195,6 @@ public:
 
     BitmapChecksum  GetChecksum() const;
 
-    SAL_DLLPRIVATE std::size_t getHash() const
-    {
-        return reinterpret_cast<std::size_t>(ImplGetImpGraphic());
-    }
-
     OUString getOriginURL() const;
     void setOriginURL(OUString const & rOriginURL);
 
@@ -247,7 +242,7 @@ struct hash<Graphic>
 {
     std::size_t operator()(Graphic const & rGraphic) const
     {
-        return rGraphic.getHash();
+        return static_cast<std::size_t>(rGraphic.GetChecksum());
     }
 };
 
diff --git a/include/vcl/vectorgraphicdata.hxx b/include/vcl/vectorgraphicdata.hxx
index 113a733e296d..7a45533b8f6b 100644
--- a/include/vcl/vectorgraphicdata.hxx
+++ b/include/vcl/vectorgraphicdata.hxx
@@ -104,6 +104,7 @@ public:
     const basegfx::B2DRange& getRange() const;
     const std::deque< css::uno::Reference< css::graphic::XPrimitive2D > >& getPrimitive2DSequence() const;
     const BitmapEx& getReplacement() const;
+    BitmapChecksum GetChecksum() const;
 };
 
 typedef std::shared_ptr< VectorGraphicData > VectorGraphicDataPtr;
diff --git a/vcl/inc/impgraph.hxx b/vcl/inc/impgraph.hxx
index 457a33cfcddb..b97f736c770d 100644
--- a/vcl/inc/impgraph.hxx
+++ b/vcl/inc/impgraph.hxx
@@ -84,6 +84,8 @@ private:
     bool                         mbSwapOut;
     bool                         mbDummyContext;
     VectorGraphicDataPtr         maVectorGraphicData;
+    // cache checksum computation
+    mutable BitmapChecksum       mnChecksum = 0;
 
     /// The PDF stream from which this Graphic is rendered,
     /// as converted (version downgraded) from the original,
diff --git a/vcl/source/gdi/bitmapex.cxx b/vcl/source/gdi/bitmapex.cxx
index 2ea870e341e6..de9f2fb97762 100644
--- a/vcl/source/gdi/bitmapex.cxx
+++ b/vcl/source/gdi/bitmapex.cxx
@@ -181,9 +181,6 @@ bool BitmapEx::operator==( const BitmapEx& rBitmapEx ) const
     if (meTransparent != rBitmapEx.meTransparent)
         return false;
 
-    if (!maBitmap.ShallowEquals(rBitmapEx.maBitmap))
-        return false;
-
     if (GetSizePixel() != rBitmapEx.GetSizePixel())
         return false;
 
@@ -197,7 +194,10 @@ bool BitmapEx::operator==( const BitmapEx& rBitmapEx ) const
     if (mbAlpha != rBitmapEx.mbAlpha)
         return false;
 
-    return maMask.ShallowEquals(rBitmapEx.maMask);
+    if (maBitmap != rBitmapEx.maBitmap)
+        return false;
+
+    return maMask == rBitmapEx.maMask;
 }
 
 bool BitmapEx::IsEmpty() const
diff --git a/vcl/source/gdi/impgraph.cxx b/vcl/source/gdi/impgraph.cxx
index f99d44799aaa..3cd67b08ce33 100644
--- a/vcl/source/gdi/impgraph.cxx
+++ b/vcl/source/gdi/impgraph.cxx
@@ -1663,6 +1663,9 @@ bool ImpGraphic::ImplIsLink() const
 
 BitmapChecksum ImpGraphic::ImplGetChecksum() const
 {
+    if (mnChecksum != 0)
+        return mnChecksum;
+
     BitmapChecksum nRet = 0;
 
     ensureAvailable();
@@ -1676,25 +1679,16 @@ BitmapChecksum ImpGraphic::ImplGetChecksum() const
 
             case GraphicType::Bitmap:
             {
-                if(maVectorGraphicData.get() && maEx.IsEmpty())
-                {
-                    // use maEx as local buffer for rendered svg
-                    const_cast< ImpGraphic* >(this)->maEx = maVectorGraphicData->getReplacement();
-                }
-
-                if( mpAnimation )
-                {
-                    nRet = mpAnimation->GetChecksum();
-                }
-                else
-                {
-                    nRet = maEx.GetChecksum();
-                }
-
-                if (mpPdfData && mpPdfData->hasElements())
+                if(maVectorGraphicData)
+                    nRet = maVectorGraphicData->GetChecksum();
+                else if (mpPdfData && mpPdfData->hasElements())
                     // Include the PDF data in the checksum, so a metafile with
                     // and without PDF data is considered to be different.
                     nRet = vcl_get_checksum(nRet, mpPdfData->getConstArray(), mpPdfData->getLength());
+                else if( mpAnimation )
+                    nRet = mpAnimation->GetChecksum();
+                else
+                    nRet = maEx.GetChecksum();
             }
             break;
 
@@ -1704,6 +1698,7 @@ BitmapChecksum ImpGraphic::ImplGetChecksum() const
         }
     }
 
+    mnChecksum = nRet;
     return nRet;
 }
 
diff --git a/vcl/source/gdi/vectorgraphicdata.cxx b/vcl/source/gdi/vectorgraphicdata.cxx
index 0e6b610154a8..8c936f00cfd7 100644
--- a/vcl/source/gdi/vectorgraphicdata.cxx
+++ b/vcl/source/gdi/vectorgraphicdata.cxx
@@ -288,4 +288,10 @@ const BitmapEx& VectorGraphicData::getReplacement() const
     return maReplacement;
 }
 
+BitmapChecksum VectorGraphicData::GetChecksum() const
+{
+    BitmapChecksum nRet = 0;
+    return vcl_get_checksum(nRet, maVectorGraphicDataArray.getConstArray(), maVectorGraphicDataArray.getLength());
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */


More information about the Libreoffice-commits mailing list