RepositoryExternal.mk | 1 +
vcl/inc/skia/utils.hxx | 4 ++++
vcl/skia/SkiaHelper.cxx | 35 ++++++++++++++++++++++++++++++++++-
vcl/skia/salbmp.cxx | 14 ++++++++++++--
4 files changed, 51 insertions(+), 3 deletions(-)
New commits:
commit 2c86b79e87bc8579f5213708954d5c85fe231407
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Thu Dec 9 14:30:15 2021 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Fri Dec 10 11:59:39 2021 +0100
cache Skia drawing based on checksum of bitmap content (tdf#146095)
Previously the caching was based on SkImage's uniqueID(), which
detects whether it's the same image. For caching to work based on
this it is required that the underlying image does not change,
which generally means using the same Bitmap(Ex) for repeated drawing.
But e.g. in tdf#146096 canvas (AFAICT) tries to cache bitmaps
by copying them around, which generates so many bitmaps that they all
do not fit in the cache (helped by the fact that the edit window
still animates them too, and bitmap caching in canvas being broken).
It feels kinda lame and unnecessary to checksum pixels of many bitmaps
to be drawn just to find out whether their drawing can be sped up,
and it really should be fixed higher up wherever it's broken, but
I've already failed several times trying to fix this in canvas,
so let's just roll with this.
This is done only for raster-based images, because GPU-backed drawing
is fast enough to deal even with expensive drawing (and fetching
GPU-backend pixels would be expensive).
On my machine this changes showing the slide from not being able
to quite keep up to about 20% CPU usage.
Change-Id: I25a362a02dc61e99b391cb305e2fdcd2feb67879
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126613
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/RepositoryExternal.mk b/RepositoryExternal.mk
index e63ab24dba27..fbb02d802142 100644
--- a/RepositoryExternal.mk
+++ b/RepositoryExternal.mk
@@ -128,6 +128,7 @@ $(call gb_LinkTarget_set_include,$(1),\
-I$(call gb_UnpackedTarball_get_dir,skia)/include/gpu \
-I$(call gb_UnpackedTarball_get_dir,skia)/include/config \
-I$(call gb_UnpackedTarball_get_dir,skia)/include/ports \
+ -I$(call gb_UnpackedTarball_get_dir,skia)/include/private \
-I$(call gb_UnpackedTarball_get_dir,skia)/include/third_party/vulkan \
-I$(call gb_UnpackedTarball_get_dir,skia)/tools/gpu \
-I$(call gb_UnpackedTarball_get_dir,skia) \
diff --git a/vcl/inc/skia/utils.hxx b/vcl/inc/skia/utils.hxx
index 35180c016e24..d2d4c81c3f94 100644
--- a/vcl/inc/skia/utils.hxx
+++ b/vcl/inc/skia/utils.hxx
@@ -119,6 +119,10 @@ sk_sp<SkImage> findCachedImage(const OString& key);
void removeCachedImage(sk_sp<SkImage> image);
tools::Long maxImageCacheSize();
+// Get checksum of the image content, only for raster images. Is cached,
+// but may still be somewhat expensive.
+uint32_t getSkImageChecksum(sk_sp<SkImage> image);
+
// SkSurfaceProps to be used by all Skia surfaces.
VCL_DLLPUBLIC const SkSurfaceProps* surfaceProps();
// Set pixel geometry to be used by SkSurfaceProps.
diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx
index 5b022d51b50d..2a4a7f1a6d7f 100644
--- a/vcl/skia/SkiaHelper.cxx
+++ b/vcl/skia/SkiaHelper.cxx
@@ -36,6 +36,7 @@ bool isVCLSkiaEnabled() { return false; }
#include <osl/file.hxx>
#include <tools/stream.hxx>
#include <list>
+#include <o3tl/lru_map.hxx>
#include <SkCanvas.h>
#include <SkPaint.h>
@@ -43,6 +44,7 @@ bool isVCLSkiaEnabled() { return false; }
#include <SkGraphics.h>
#include <GrDirectContext.h>
#include <SkRuntimeEffect.h>
+#include <SkOpts_spi.h>
#include <skia_compiler.hxx>
#include <skia_opts.hxx>
#include <tools/sk_app/VulkanWindowContext.h>
@@ -581,7 +583,7 @@ struct ImageCacheItem
} //namespace
// LRU cache, last item is the least recently used. Hopefully there won't be
that many items
-// to require a hash/map. Using o3tl::lru_cache would be simpler, but it
doesn't support
+// to require a hash/map. Using o3tl::lru_map would be simpler, but it doesn't
support
// calculating cost of each item.
static std::list<ImageCacheItem> imageCache;
static tools::Long imageCacheSize = 0; // sum of all ImageCacheItem.size
@@ -644,6 +646,37 @@ tools::Long maxImageCacheSize()
return officecfg::Office::Common::Cache::Skia::ImageCacheSize::get();
}
+static o3tl::lru_map<uint32_t, uint32_t> checksumCache(256);
+
+uint32_t computeSkPixmapChecksum(const SkPixmap& pixmap)
+{
+ // Use uint32_t because that's what SkOpts::hash_fn() returns.
+ static_assert(std::is_same_v<uint32_t, decltype(SkOpts::hash_fn(nullptr,
0, 0))>);
+ const size_t dataRowBytes = pixmap.width() << pixmap.shiftPerPixel();
+ if (dataRowBytes == pixmap.rowBytes())
+ return SkOpts::hash_fn(pixmap.addr(), pixmap.height() * dataRowBytes,
0);
+ uint32_t sum = 0;
+ for (int row = 0; row < pixmap.height(); ++row)
+ sum = SkOpts::hash_fn(pixmap.addr(0, row), dataRowBytes, sum);
+ return sum;
+}
+
+uint32_t getSkImageChecksum(sk_sp<SkImage> image)
+{
+ // Cache the checksums based on the uniqueID() (which should stay the same
+ // for the same image), because it may be still somewhat expensive.
+ uint32_t id = image->uniqueID();
+ auto it = checksumCache.find(id);
+ if (it != checksumCache.end())
+ return it->second;
+ SkPixmap pixmap;
+ if (!image->peekPixels(&pixmap))
+ abort(); // Fetching of GPU-based pixels is expensive, and
shouldn't(?) be needed anyway.
+ uint32_t checksum = computeSkPixmapChecksum(pixmap);
+ checksumCache.insert({ id, checksum });
+ return checksum;
+}
+
static sk_sp<SkBlender> invertBlender;
static sk_sp<SkBlender> xorBlender;
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index c18ad18f8428..b81e55221f34 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -1357,7 +1357,14 @@ OString SkiaSalBitmap::GetImageKey(DirectImage direct)
const
return OString::Concat("E") + ss.str().c_str();
}
assert(direct == DirectImage::No || mImage);
- return OString::Concat("I") +
OString::number(GetSkImage(direct)->uniqueID());
+ sk_sp<SkImage> image = GetSkImage(direct);
+ // In some cases drawing code may try to draw the same content but using
+ // different bitmaps (even underlying bitmaps), for example canvas
apparently
+ // copies the same things around in tdf#146095. For pixel-based images
+ // it should be still cheaper to compute a checksum and avoid re-caching.
+ if (!image->isTextureBacked())
+ return OString::Concat("C") +
OString::number(getSkImageChecksum(image));
+ return OString::Concat("I") + OString::number(image->uniqueID());
}
OString SkiaSalBitmap::GetAlphaImageKey(DirectImage direct) const
@@ -1370,7 +1377,10 @@ OString SkiaSalBitmap::GetAlphaImageKey(DirectImage
direct) const
return OString::Concat("E") + ss.str().c_str();
}
assert(direct == DirectImage::No || mAlphaImage);
- return OString::Concat("I") +
OString::number(GetAlphaSkImage(direct)->uniqueID());
+ sk_sp<SkImage> image = GetAlphaSkImage(direct);
+ if (!image->isTextureBacked())
+ return OString::Concat("C") +
OString::number(getSkImageChecksum(image));
+ return OString::Concat("I") + OString::number(image->uniqueID());
}
void SkiaSalBitmap::dump(const char* file) const
