include/vcl/graph.hxx | 7 +------
include/vcl/vectorgraphicdata.hxx | 1 +
vcl/inc/impgraph.hxx | 2 ++
vcl/source/gdi/bitmapex.cxx | 18 +++++++++++-------
vcl/source/gdi/impgraph.cxx | 27 +++++++++++----------------
vcl/source/gdi/vectorgraphicdata.cxx | 6 ++++++
6 files changed, 32 insertions(+), 29 deletions(-)
New commits:
commit 4d75bb89c60590b3dc7fb441960e75a57fda3ab2
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Mon May 20 19:08:19 2019 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Fri Apr 8 22:30:59 2022 +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.gran...@collabora.co.uk>
(cherry picked from commit 319c57d2af5d26d3910db4b02dca145d8881af44)
diff --git a/include/vcl/graph.hxx b/include/vcl/graph.hxx
index 3b91a3269478..8b42ca0cc3d7 100644
--- a/include/vcl/graph.hxx
+++ b/include/vcl/graph.hxx
@@ -197,11 +197,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);
@@ -249,7 +244,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 ea7615d2bdf6..159dc7afe286 100644
--- a/include/vcl/vectorgraphicdata.hxx
+++ b/include/vcl/vectorgraphicdata.hxx
@@ -103,6 +103,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 30c88594c5d6..133ed2d0de46 100644
--- a/vcl/inc/impgraph.hxx
+++ b/vcl/inc/impgraph.hxx
@@ -83,6 +83,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 ccf70a6c4dca..15a176924aec 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 9b6bf0da8d77..825f6b56b852 100644
--- a/vcl/source/gdi/impgraph.cxx
+++ b/vcl/source/gdi/impgraph.cxx
@@ -1671,6 +1671,9 @@ bool ImpGraphic::ImplIsLink() const
BitmapChecksum ImpGraphic::ImplGetChecksum() const
{
+ if (mnChecksum != 0)
+ return mnChecksum;
+
BitmapChecksum nRet = 0;
ensureAvailable();
@@ -1684,25 +1687,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;
@@ -1712,6 +1706,7 @@ BitmapChecksum ImpGraphic::ImplGetChecksum() const
}
}
+ mnChecksum = nRet;
return nRet;
}
diff --git a/vcl/source/gdi/vectorgraphicdata.cxx
b/vcl/source/gdi/vectorgraphicdata.cxx
index 08aebe0a1b3d..892f2864a1ab 100644
--- a/vcl/source/gdi/vectorgraphicdata.cxx
+++ b/vcl/source/gdi/vectorgraphicdata.cxx
@@ -287,4 +287,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: */
commit 45e98962327d2919d94a2190ee540a1043a75b82
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Tue Apr 30 10:49:44 2019 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Fri Apr 8 22:29:40 2022 +0200
fix bug in BitmapEx::operator==
Just because this image is transparent, does not mean it is equal to the
other image.
Similarly, just because this image has transparency color, does not mean
the other image has valid transparency color.
Also move the cheaper mbAlpha check before the more expensive
ShallowEquals check.
there since
commit 8ab086b6cc054501bfbf7ef6fa509c393691e860
Date: Mon Sep 18 16:07:07 2000 +0000
initial import
Change-Id: I63033bc8e7fed991513a171e637768e826eafad9
Reviewed-on: https://gerrit.libreoffice.org/71572
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
(cherry picked from commit 36f306d8891ef8cba53676e4a2a30434718228e4)
diff --git a/vcl/source/gdi/bitmapex.cxx b/vcl/source/gdi/bitmapex.cxx
index b89719c70b42..ccf70a6c4dca 100644
--- a/vcl/source/gdi/bitmapex.cxx
+++ b/vcl/source/gdi/bitmapex.cxx
@@ -187,13 +187,17 @@ bool BitmapEx::operator==( const BitmapEx& rBitmapEx )
const
if (GetSizePixel() != rBitmapEx.GetSizePixel())
return false;
- if (meTransparent == TransparentType::NONE)
- return true;
+ if (meTransparent != rBitmapEx.meTransparent)
+ return false;
- if (meTransparent == TransparentType::Color)
- return maTransparentColor == rBitmapEx.maTransparentColor;
+ if (meTransparent == TransparentType::Color
+ && maTransparentColor != rBitmapEx.maTransparentColor)
+ return false;
+
+ if (mbAlpha != rBitmapEx.mbAlpha)
+ return false;
- return maMask.ShallowEquals(rBitmapEx.maMask) && mbAlpha ==
rBitmapEx.mbAlpha;
+ return maMask.ShallowEquals(rBitmapEx.maMask);
}
bool BitmapEx::IsEmpty() const
