Created attachment 201254 [details]
Xcode Instruments Performance trace during keypress

I can reproduce this bug in my local master build:

Version: 26.2.0.0.alpha0+ (AARCH64) / LibreOffice Community
Build ID: 8e6c53f057e48c0f5bf9c2ea14dd24e6cbd80209
CPU threads: 8; OS: macOS 15.5; UI render: Skia/Raster; VCL: osx
Locale: en-CA (en_CA.UTF-8); UI: en-US
Calc: threaded

I got the same output as your Instruments output and it looks like the image is getting drawn but the drawn canvas is not getting flushed to the screen.

So I added a flush after each "key up" event in the following debug patch and now I see the animated image move as I repeatedly press and release an arrow key. The debug patch isn't a fix since I see drawing artifacts that only get cleared when I pause briefly after entering an arrow key so I assume that some painting timer is being prevented from running when arrow key presses flood the event queue:

diff --git a/vcl/osx/salframeview.mm b/vcl/osx/salframeview.mm
index e4f474b8f050..99247dd2a4b3 100644
--- a/vcl/osx/salframeview.mm
+++ b/vcl/osx/salframeview.mm
@@ -1738,6 +1738,14 @@ -(void)keyDown: (NSEvent*)pEvent
     }
 }
 
+-(void)keyUp: (NSEvent*)pEvent
+{
+    SolarMutexGuard aGuard;
+
+    if( AquaSalFrame::isAlive( mpFrame ) )
+        mpFrame->Flush();
+}
+
 -(BOOL)handleKeyDownException:(NSEvent*)pEvent
 {
     // check for a very special set of modified characters

Created attachment 201281 [details]
Xcode Instruments trace on Mac Silicon

So I ran Instruments and I noticed that there is a difference between @Tolesto's Instruments trace and mine: in my trace, most of the CPU is done in a painting timer.

I need to investigate whether or not the painting timer is preventing the Skia timer from running. On macOS, the Skia timer runs at the next lower priority after painting timers.

Created attachment 201284 [details]
Xcode Instruments Performance

I redid the trace, Key Press isn't shown anymore. Odd. I suppose I'm doing something different this time, but what, no clue...

Anyhow: this case is pressing and holding the right arrow key

(In reply to Telesto from comment #4)
> Created attachment 201284 [details]
> Xcode Instruments Performance
> 
> I redid the trace, Key Press isn't shown anymore. Odd. I suppose I'm doing
> something different this time, but what, no clue...
> 
> Anyhow: this case is pressing and holding the right arrow key

I cannot reproduce this bug when pressing and holding an arrow key. Pressing and holding generates key press events at some constant rate determined by macOS. So my guess is that we can generate more key press events per second when rapidly pressing and releasing than when pressing and holding.

(In reply to Patrick (volunteer) from comment #3)
> I need to investigate whether or not the painting timer is preventing the
> Skia timer from running. On macOS, the Skia timer runs at the next lower
> priority after painting timers.

OK. I can confirm that the Skia timer is running. But the next lower priority - "post paint" - is not running when I press and release an arrow key repeatedly.

So, I think I need to see if I can figure out why the Skia draw bitmap calls are so slow with Skia/Raster. My current guess is that the slow draw bitmap calls are taking so much time that LibreOffice is backlogged in its work and so the "post paint" priority of timers never has a chance to run.

So I think I have found the cause of this bug: the Skia image cache is too small. In the following debug patch, I increased the Skia image cache from 64 MB to 512 MB and the bug disappears.

With macOS Retina displays, we need the cache to be at least 4 times larger to handle "each screen pixel is really backed by a 2x2 square of subpixels" to hold the same set of images as Windows and Linux. Still, 4 times larger wasn't enough and I had to double the cache since to 8 times larger.

The reason we need such a larger increase in the image cache size is that drawing images with a separate alpha channel is very slow with Skia/Raster. Specifically, the blending of the bitmap and a separate alpha channel in SkiaSalGraphicsImpl::mergeCacheBitmaps() is slow so such images are cached.

Problem is that the image that is being moved is large and once the cache is full, each image has to be reblended and drawn after every single key press.

However, I am not sure if making the Skia image cache size so large good idea or not. I wonder if there is something else that can be done to cache the blended bitmap and alpha channels but maybe that would end up using a lot more memory than just increasing the cache size:


diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx
index 7dcd5c41bc5a..4d18dbe1dff8 100644
--- a/vcl/skia/SkiaHelper.cxx
+++ b/vcl/skia/SkiaHelper.cxx
@@ -730,7 +730,7 @@ void removeCachedImage(sk_sp<SkImage> image)
 tools::Long maxImageCacheSize()
 {
     // Defaults to 4x 2000px 32bpp images, 64MiB.
-    return officecfg::Office::Common::Cache::Skia::ImageCacheSize::get();
+    return officecfg::Office::Common::Cache::Skia::ImageCacheSize::get() * 8;
 }
 
 static o3tl::lru_map<uint32_t, uint32_t> checksumCache(256);

(In reply to Patrick (volunteer) from comment #7)
> 
> With macOS Retina displays, we need the cache to be at least 4 times larger
> to handle "each screen pixel is really backed by a 2x2 square of subpixels"
> to hold the same set of images as Windows and Linux. Still, 4 times larger
> wasn't enough and I had to double the cache since to 8 times larger.
> 

It is possible that in Skia/Raster+macOS+retine mode, we can use quick and dirty resampling when painting so we don't need such large bitmaps?

(In reply to Patrick (volunteer) from comment #7)
> So I think I have found the cause of this bug: the Skia image cache is too
> small. In the following debug patch, I increased the Skia image cache from
> 64 MB to 512 MB and the bug disappears.

Indirectly related. Increasing the cache solves the performance issue I encountered at bug 167005 (on Windows) with a hi-resolution image.

(In reply to Telesto from comment #9)
> Indirectly related. Increasing the cache solves the performance issue I
> encountered at bug 167005 (on Windows) with a hi-resolution image.

Hmm. For me, increasing the Skia image cache didn't stop tdf#167005 on my machine. Only increase GraphicMemoryLimit to 1024 MB worked.

So I think tdf#167005 and tdf#167007 are due to the GraphicMemoryLimit cache revoking a graphic and its underlying SkiaSalBitmps. But with this bug, the SkiaSalBitmaps don't appear to get deleted so this bug is due to having to reblend the bitmap and its alpha repeatedly for each of the animation frames.

I don't know if this is a feasible or if it will solve any of these bugs, but I wonder if we take up the reference count on the blended Skia image and cache it and its key in the SkiaSalBitmap, we can use the cached image if it exists and draw that instead of reblending.

That will cause a temporary increase in the Skia image cache size but by caching the blended Skia images in the bitmap, the Skia image cache should go back down when the document is closed or the image is revoked from the GraphicMemoryLimit cache.

When I get some time, I can see if I can get this to work.

I have implemented a quick and dirty way to fix this bug:

https://gerrit.libreoffice.org/c/core/+/186530

The tradeoff in this patch is that it moves cached alpha blended outside of the Skia image cache. This will cause the overall memory usage to increase as it keeps the cached image alive longer and it won't be deleted until a bitmap is revoked from the GraphicMemoryLimit cache or the bitmap's document is closed.

Don't know if there is a reliable way to reduce the increased number of cached images.

(In reply to Patrick (volunteer) from comment #10)
> (In reply to Telesto from comment #9)
> > Indirectly related. Increasing the cache solves the performance issue I
> > encountered at bug 167005 (on Windows) with a hi-resolution image.
> 
> Hmm. For me, increasing the Skia image cache didn't stop tdf#167005 on my
> machine. Only increase GraphicMemoryLimit to 1024 MB worked.

You're right. I was probably slightly to enthusiastic about the Skia cache idea that I wished it to be true. I likely didn't check it too carefully; oops

(In reply to Patrick (volunteer) from comment #11)
> I have implemented a quick and dirty way to fix this bug:
> 
> https://gerrit.libreoffice.org/c/core/+/186530
> 
> The tradeoff in this patch is that it moves cached alpha blended outside of
> the Skia image cache. This will cause the overall memory usage to increase
> as it keeps the cached image alive longer and it won't be deleted until a
> bitmap is revoked from the GraphicMemoryLimit cache or the bitmap's document
> is closed.
> 
> Don't know if there is a reliable way to reduce the increased number of
> cached images.

I ran the Instruments both with and without the patch and, if I open attachment 159163 [details], view each of the slides, run a slideshow, and then close the document, memory usage starts and ends with rough the same amount.

But the peak memory usage grew by more than a GB (1.62 GB without patch vs. 2.71 GB with patch).

So, when I get some some time, I will try another approach: defer removal of an image from the Skia image cache if that image has been drawn within the last N seconds. My hope is that N can be a short period of time (e.g. 5 - 10 seconds) which would allow temporary increase in the cache size but the temporary increase won't grow so high as it does in the patch.

I'll post when I have more news to report.