vcl/inc/skia/osx/gdiimpl.hxx | 6 ++--
vcl/osx/salframe.cxx | 59 +------------------------------------------
vcl/skia/osx/gdiimpl.cxx | 54 +++++++++++++++++++++++++++++++++++++--
3 files changed, 57 insertions(+), 62 deletions(-)
New commits:
commit 9d973cdf107d1be4db806e6ec8a6ced49275c4e6
Author: Patrick Luby <guibmac...@gmail.com>
AuthorDate: Sat May 10 09:09:48 2025 -0400
Commit: Patrick Luby <guibomac...@gmail.com>
CommitDate: Sun May 11 23:43:01 2025 +0200
Related: tdf#163945 include Skia timer in Skia/Metal flushing rate limit
Previously, Skia/Metal flushes were only limited when called from
AquaSalFrame::doFlush(). While this fixed tdf#166258, this caused
tdf#163945 to reoccur on some machines. So expand the Skia/Metal
flushing rate limit to include flushes called from the Skia timer.
Change-Id: I299f17eb406ddf1c635d4f75d51241807f15f58c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185138
Tested-by: Jenkins
Reviewed-by: Patrick Luby <guibomac...@gmail.com>
diff --git a/vcl/inc/skia/osx/gdiimpl.hxx b/vcl/inc/skia/osx/gdiimpl.hxx
index b189898082ab..f1e2214d3924 100644
--- a/vcl/inc/skia/osx/gdiimpl.hxx
+++ b/vcl/inc/skia/osx/gdiimpl.hxx
@@ -41,8 +41,6 @@ public:
virtual void Flush(const tools::Rectangle&) override;
virtual void WindowBackingPropertiesChanged() override;
- void ScheduleFlush();
-
CGImageRef createCGImageFromRasterSurface(const NSRect& rDirtyRect,
CGPoint& rImageOrigin,
bool& rImageFlipped);
@@ -51,6 +49,10 @@ private:
virtual void createWindowSurfaceInternal(bool forceRaster = false)
override;
virtual void flushSurfaceToWindowContext() override;
static inline sk_sp<SkFontMgr> fontManager;
+ static inline AquaSkiaSalGraphicsImpl* lastFlushedGraphicsImpl = nullptr;
+ static inline CFAbsoluteTime lastFlushedGraphicsImplTime = 0;
+
+ bool mbInFlushSurfaceToWindowContext;
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/osx/salframe.cxx b/vcl/osx/salframe.cxx
index 37904e440bd3..e469882db3e2 100644
--- a/vcl/osx/salframe.cxx
+++ b/vcl/osx/salframe.cxx
@@ -57,10 +57,6 @@
#include <quartz/CGHelpers.hxx>
#include <postmac.h>
-#if HAVE_FEATURE_SKIA
-#include <vcl/skia/SkiaHelper.hxx>
-#endif
-
const int nMinBlinkCursorDelay = 500;
AquaSalFrame* AquaSalFrame::s_pCaptureFrame = nullptr;
@@ -1191,63 +1187,12 @@ bool AquaSalFrame::doFlush()
if( mbForceFlushScrolling || mbForceFlushProgressBar ||
ImplGetSVData()->maAppData.mnDispatchLevel <= 0 )
{
- // Related: tdf#163945 don't directly flush graphics with Skia/Metal
- // When dragging a selection box on an empty background in
- // Impress and only with Skia/Metal, the selection box
- // would not keep up with the pointer. The selection box
- // would repaint sporadically or not at all if the pointer
- // was dragged rapidly and the status bar was visible.
- // Apparently, flushing a graphics doesn't actually do much
- // of anything with Skia/Raster and Skia disabled so the
- // selection box repaints without any noticeable delay.
- // However, with Skia/Metal every flush of a graphics
- // creates and queues a new CAMetalLayer drawable. During
- // rapid dragging, this can lead to creating and queueing
- // up to 200 drawables per second leaving no spare time for
- // the Impress selection box painting timer to fire.
- // So with Skia/Metal, throttle the rate of flushing.
- bool bNeedsFlush = true;
- bool bFlushed = false;
-#if HAVE_FEATURE_SKIA
- // tdf#164428 Skia/Metal needs flush after drawing progress bar
- if (!mbForceFlushProgressBar && SkiaHelper::isVCLSkiaEnabled() &&
SkiaHelper::renderMethodToUse() != SkiaHelper::RenderRaster)
- {
- // Assume a certain frame rate is the fastest flushing rate
- // that can be handled with Skia/Metal. Note that the Skia
- // timer is running separately so the overall flushing rate
- // may still be faster than this limit. Previously, the
- // limit was set to 200 flushes per second but that caused
- // tdf#163945 to reappear so reduce the limit to 40 flushes
- // per second.
- static const CFAbsoluteTime fMinFlushInterval = 0.025;
- static CFAbsoluteTime fLastFlushTime = 0;
-
- CFAbsoluteTime fInterval = CFAbsoluteTimeGetCurrent() -
fLastFlushTime;
- if (fInterval >= 0.0f && fInterval < fMinFlushInterval)
- {
- // Do not schedule the Skia timer to run as it appears that
- // it might be part of the cause for the reappearance of
- // tdf#163945 on some machines.
- }
- else
- {
- mpGraphics->Flush();
- fLastFlushTime = CFAbsoluteTimeGetCurrent();
- bFlushed = true;
- }
- bNeedsFlush = false;
- }
-#endif
- if (bNeedsFlush)
- {
- mpGraphics->Flush();
- bFlushed = true;
- }
+ mpGraphics->Flush();
// Related: tdf#155266 skip redisplay of the view when forcing flush
// It appears that calling -[NSView display] overwhelms some Intel Macs
// so only flush the graphics and skip immediate redisplay of the view.
- bRet = !bFlushed || ImplGetSVData()->maAppData.mnDispatchLevel <= 0;
+ bRet = ImplGetSVData()->maAppData.mnDispatchLevel <= 0;
mbForceFlushScrolling = false;
mbForceFlushProgressBar = false;
diff --git a/vcl/skia/osx/gdiimpl.cxx b/vcl/skia/osx/gdiimpl.cxx
index 8e7e7b7beecf..7c0789bfe89f 100644
--- a/vcl/skia/osx/gdiimpl.cxx
+++ b/vcl/skia/osx/gdiimpl.cxx
@@ -59,11 +59,15 @@
AquaSkiaSalGraphicsImpl::AquaSkiaSalGraphicsImpl(AquaSalGraphics& rParent,
AquaSharedAttributes& rShared)
: SkiaSalGraphicsImpl(rParent, rShared.mpFrame)
, AquaGraphicsBackendBase(rShared, this)
+ , mbInFlushSurfaceToWindowContext(false)
{
}
AquaSkiaSalGraphicsImpl::~AquaSkiaSalGraphicsImpl()
{
+ if (lastFlushedGraphicsImpl == this)
+ lastFlushedGraphicsImpl = nullptr;
+
DeInit(); // mac code doesn't call DeInit()
}
@@ -125,12 +129,56 @@ void
AquaSkiaSalGraphicsImpl::flushSurfaceToWindowContext()
}
else
{
- SkiaSalGraphicsImpl::flushSurfaceToWindowContext();
+ // Related: tdf#163945 don't directly flush graphics with Skia/Metal
+ // When dragging a selection box on an empty background in
+ // Impress and only with Skia/Metal, the selection box would
+ // not keep up with the pointer. The selection box would
+ // repaint sporadically or not at all if the pointer was
+ // dragged rapidly and the status bar was visible.
+ // Apparently, flushing a graphics doesn't actually do much of
+ // anything with Skia/Raster and Skia disabled so the selection
+ // box repaints without any noticeable delay.
+ // However, with Skia/Metal every flush of a graphics creates
+ // and queues a new CAMetalLayer drawable. During rapid
+ // dragging, this can lead to creating and queueing up to 200
+ // drawables per second leaving no spare time for the Impress
+ // selection box painting timer to fire. So with Skia/Metal,
+ // throttle the rate of flushing.
+ // tdf#166258 Assume a certain maximum flushing rate with Skia/Metal
+ // Previously, the fix for tdf#163945 was done in the
+ // AquaSalFrame::Flush() methods, but that caused tdf#166258
+ // so include flushes done by the Skia timer in the throttling
+ // of the maximum flushing rate. Currently, the maximum flushing
+ // rate is 100 flushes per second.
+ static const CFAbsoluteTime fMinFlushInterval = 0.01;
+
+ // Related: tdf#166258 prevent recursion as calling scheduleFlush()
+ // may immediately recurse into performFlush() leading to infinite
+ // recursion. If scheduleFlush() is recursing, assume that a flush
+ // is necessary.
+ // Also assume that a flush is necessary if the current graphics
+ // is different than the last flushed graphics. Otherwise, the
+ // "tip of the day" dialog or other windows will fail to draw
+ // immediately after the window opens.
+ CFAbsoluteTime fInterval = CFAbsoluteTimeGetCurrent() -
lastFlushedGraphicsImplTime;
+ if (!mbInFlushSurfaceToWindowContext && lastFlushedGraphicsImpl ==
this && fInterval >= 0.0f
+ && fInterval < fMinFlushInterval)
+ {
+ // Just to be safe, enable the Skia timer so that the surface
+ // will eventually be flushed.
+ mbInFlushSurfaceToWindowContext = true;
+ scheduleFlush();
+ mbInFlushSurfaceToWindowContext = false;
+ }
+ else
+ {
+ lastFlushedGraphicsImpl = this;
+ SkiaSalGraphicsImpl::flushSurfaceToWindowContext();
+ lastFlushedGraphicsImplTime = CFAbsoluteTimeGetCurrent();
+ }
}
}
-void AquaSkiaSalGraphicsImpl::ScheduleFlush() { scheduleFlush(); }
-
// For Raster we use our own screen blitting (see above).
CGImageRef AquaSkiaSalGraphicsImpl::createCGImageFromRasterSurface(const
NSRect& rDirtyRect,
CGPoint&
rImageOrigin,
