From c585b697b583fa0f8cdadeab594c31d270367ba7 Mon Sep 17 00:00:00 2001 From: Patrick Luby Date: Thu, 28 Nov 2024 19:56:45 -0500 Subject: 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 by calling display on the view. Also, with the reduced Skia/Metal flushing load from this fix, the color conversion when drawing to an NSBox appears to be no longer needed. The converted color appeared less saturated, at least to me, so accept the (hopefully now imperceptible) performance hit of drawing an NSBox with native colors. Change-Id: I55b4ab763bf20c6c2acad65587b703fc6f645264 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177530 Reviewed-by: Patrick Luby Tested-by: Jenkins --- vcl/osx/salframe.cxx | 62 ++++++++++++++++++++++++++++++++++++++++---- vcl/osx/salnativewidgets.cxx | 16 +----------- 2 files changed, 58 insertions(+), 20 deletions(-) (limited to 'vcl/osx') diff --git a/vcl/osx/salframe.cxx b/vcl/osx/salframe.cxx index 109ccfd16ba8..73ea3ce2262a 100644 --- a/vcl/osx/salframe.cxx +++ b/vcl/osx/salframe.cxx @@ -57,6 +57,9 @@ #include #include +#if HAVE_FEATURE_SKIA +#include +#endif const int nMinBlinkCursorDelay = 500; @@ -1007,6 +1010,15 @@ void AquaSalFrame::SetPointerPos( tools::Long nX, tools::Long nY ) CGDisplayMoveCursorToPoint( mainDisplayID, aPoint ); } +static bool lcl_ShouldDisplayInsteadOFFlush() +{ + bool bRet = false; +#if HAVE_FEATURE_SKIA + bRet = SkiaHelper::isVCLSkiaEnabled() && SkiaHelper::renderMethodToUse() != SkiaHelper::RenderRaster; +#endif + return bRet; +} + void AquaSalFrame::Flush() { if( !(mbGraphics && mpGraphics && mpNSView && mbShown) ) @@ -1022,11 +1034,31 @@ void AquaSalFrame::Flush() if( mbForceFlush || ImplGetSVData()->maAppData.mnDispatchLevel <= 0 ) { mbForceFlush = false; - mpGraphics->Flush(); + + // 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 by + // calling display on the view. + bool bDisplay = lcl_ShouldDisplayInsteadOFFlush(); + if (!bDisplay) + 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. - if( ImplGetSVData()->maAppData.mnDispatchLevel <= 0 ) + if( bDisplay || ImplGetSVData()->maAppData.mnDispatchLevel <= 0 ) [mpNSView display]; } } @@ -1048,12 +1080,32 @@ void AquaSalFrame::Flush( const tools::Rectangle& rRect ) if( mbForceFlush || ImplGetSVData()->maAppData.mnDispatchLevel <= 0 ) { mbForceFlush = false; - mpGraphics->Flush( rRect ); + + // 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 by + // calling display on the view. + bool bDisplay = lcl_ShouldDisplayInsteadOFFlush(); + if (!bDisplay) + 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. - if( ImplGetSVData()->maAppData.mnDispatchLevel <= 0 ) - [mpNSView display]; + if( bDisplay || ImplGetSVData()->maAppData.mnDispatchLevel <= 0 ) + [mpNSView displayRect: aNSRect]; } } diff --git a/vcl/osx/salnativewidgets.cxx b/vcl/osx/salnativewidgets.cxx index 113049b8eeda..d0ebad835f3d 100644 --- a/vcl/osx/salnativewidgets.cxx +++ b/vcl/osx/salnativewidgets.cxx @@ -400,21 +400,7 @@ static void drawBox(CGContextRef context, const NSRect& rc, NSColor* pColor) NSRect rect = { NSZeroPoint, NSMakeSize(rc.size.width, rc.size.height) }; NSBox* pBox = [[NSBox alloc] initWithFrame: rect]; [pBox setBoxType: NSBoxCustom]; - - // Related tdf#163945: Set fill color to NSColorTypeComponentBased color type - // Many system colors have the NSColorTypeCatalog color type. For - // some unknown reason, setting the NSBox's fill color to a color set - // to NSColorTypeCatalog causes drawing to take at least twice as long - // as when the fill color is set to NSColorTypeComponentBased. So, - // only draw with a fill color set to NSColorTypeComponentBased. - NSColor* pRGBColor = pColor; - if ([pColor type] != NSColorTypeComponentBased) - { - pRGBColor = [pColor colorUsingType: NSColorTypeComponentBased]; - if (!pRGBColor) - pRGBColor = pColor; - } - [pBox setFillColor: pRGBColor]; + [pBox setFillColor: pColor]; // -[NSBox setBorderType: NSNoBorder] is deprecated so hide the border // by setting the border color to transparent -- cgit