vcl/inc/quartz/salgdi.h | 1
vcl/osx/salnativewidgets.cxx | 27 ++++++++++++---
vcl/skia/osx/gdiimpl.cxx | 74 ++++++++++++++++++++++++-------------------
3 files changed, 66 insertions(+), 36 deletions(-)
New commits:
commit 0807fe362819629259a9a80a48d2826e588d909c
Author: Patrick Luby <guibmac...@gmail.com>
AuthorDate: Sun Nov 24 20:25:59 2024 -0500
Commit: Patrick Luby <guibomac...@gmail.com>
CommitDate: Mon Nov 25 19:30:21 2024 +0100
Improve rendering speed for native controls when using Skia/Metal
While debugging tdf#163945, Xcode's Instruments application uncovered
the following performance bottlenecks when using Skia/Metal:
1. Very slow rendering an NSBox:
Many system colors have the NSColorTypeCatalog color type. For
some unkown 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.
2. Excessively large offscreen buffers when drawing native controls:
The temporary bitmap was set to the control region expanded by
50 * mScaling (e.g. both width and height were increased by 200
pixels when running on a Retina display). This caused temporary
bitmaps to be up to several times larger than needed. Also,
drawing NSBox objects to a CGBitmapContext is noticeably slow
so filling all that unneeded temporary bitmap area can slow down
performance when a large number of NSBox objects like the status
bar are redrawn in quick succession.
Using getNativeControlRegion() isn't perfect, but it does try to
account for the focus ring as well as the minimum width and/or
height of the native control so union the two regions set by
getNativeControlRegion() and add double the focus ring width on
each side just to be safe. In most cases, this should ensure
that the temporary bitmap is large enough to draw the entire
native control and a focus ring.
3. Unncessary copying of bitmap buffer when drawing native controls:
Let Skia own the CGBitmapContext's buffer so that an SkImage
can be created without Skia making a copy of the buffer.
Change-Id: Ibd3abb4b9d7045c47268319772fe97a5c4dba3c6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177225
Tested-by: Jenkins
Reviewed-by: Patrick Luby <guibomac...@gmail.com>
diff --git a/vcl/inc/quartz/salgdi.h b/vcl/inc/quartz/salgdi.h
index 40209e97b3ba..0b8e01b569ba 100644
--- a/vcl/inc/quartz/salgdi.h
+++ b/vcl/inc/quartz/salgdi.h
@@ -440,6 +440,7 @@ protected:
virtual bool drawNativeControl( ControlType nType, ControlPart
nPart, const tools::Rectangle& rControlRegion,
ControlState nState, const
ImplControlValue& aValue,
const OUString& aCaption, const
Color& rBackgroundColor ) override;
+public:
virtual bool getNativeControlRegion( ControlType nType,
ControlPart nPart, const tools::Rectangle& rControlRegion, ControlState nState,
const ImplControlValue&
aValue, const OUString& aCaption,
tools::Rectangle
&rNativeBoundingRegion, tools::Rectangle &rNativeContentRegion ) override;
diff --git a/vcl/osx/salnativewidgets.cxx b/vcl/osx/salnativewidgets.cxx
index 7ff36f0cf690..7dd94ac4b87e 100644
--- a/vcl/osx/salnativewidgets.cxx
+++ b/vcl/osx/salnativewidgets.cxx
@@ -380,6 +380,8 @@ bool AquaGraphicsBackend::drawNativeControl(ControlType
nType,
static void drawBox(CGContextRef context, const NSRect& rc, NSColor* pColor)
{
+ assert(pColor);
+
CGContextSaveGState(context);
CGContextTranslateCTM(context, rc.origin.x, rc.origin.y + rc.size.height);
CGContextScaleCTM(context, 1, -1);
@@ -388,12 +390,27 @@ 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];
- [pBox setFillColor: pColor];
- SAL_WNODEPRECATED_DECLARATIONS_PUSH // setBorderType first deprecated in
macOS 10.15
- [pBox setBorderType: NSNoBorder];
- SAL_WNODEPRECATED_DECLARATIONS_POP
+
+ // Related tdf#163945: Set fill color to NSColorTypeComponentBased color
type
+ // Many system colors have the NSColorTypeCatalog color type. For
+ // some unkown 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];
+
+ // -[NSBox setBorderType: NSNoBorder] is deprecated so hide the border
+ // by setting the border color to transparent
+ [pBox setBorderColor: [NSColor clearColor]];
+ [pBox setTitlePosition: NSNoTitle];
[pBox displayRectIgnoringOpacity: rect inContext: graphicsContext];
diff --git a/vcl/skia/osx/gdiimpl.cxx b/vcl/skia/osx/gdiimpl.cxx
index 45216340492c..7fa95e8d9fab 100644
--- a/vcl/skia/osx/gdiimpl.cxx
+++ b/vcl/skia/osx/gdiimpl.cxx
@@ -27,6 +27,7 @@
#include <quartz/CoreTextFont.hxx>
#include <quartz/SystemFontList.hxx>
+#include <osx/salnativewidgets.h>
#include <skia/quartz/cgutils.h>
#include <tools/window/mac/MacWindowInfo.h>
#include <tools/window/mac/GaneshMetalWindowContext_mac.h>
@@ -249,28 +250,51 @@ bool
AquaSkiaSalGraphicsImpl::drawNativeControl(ControlType nType, ControlPart n
return false;
// rControlRegion is not the whole area that the control should be painted
to (e.g. highlight
- // around focused lineedit is outside of it). Since we draw to a temporary
bitmap, we need tofind out
- // the real size. Using getNativeControlRegion() might seem like the
function to call, but we need
- // the other direction - what is called rControlRegion here is
rNativeContentRegion in that function
- // what's called rControlRegion there is what we need here. Moreover
getNativeControlRegion()
- // in some cases returns a fixed size that does not depend on its input,
so we have no way to
- // actually find out what the original size was (or maybe the function is
kind of broken, I don't know).
- // So, add a generous margin and hope it's enough.
+ // around focused lineedit is outside of it). Since we draw to a temporary
bitmap, we need to find out
+ // the real size.
+ // Related tdf#163945 Reduce the size of the temporary bitmap
+ // Previously, the temporary bitmap was set to the control region
+ // expanded by 50 * mScaling (e.g. both width and height were
+ // increased by 200 pixels when running on a Retina display). This
+ // caused temporary bitmaps to be up to several times larger than
+ // needed. Also, drawing NSBox objects to a CGBitmapContext is
+ // noticeably slow so filling all that unneeded temporary bitmap
+ // area can slow down performance when a large number of NSBox
+ // objects like the status bar are redrawn in quick succession.
+ // Using getNativeControlRegion() isn't perfect, but it does try to
+ // account for the focus ring as well as the minimum width and/or
+ // height of the native control so union the two regions set by
+ // getNativeControlRegion() and add double the focus ring width on
+ // each side just to be safe. In most cases, this should ensure
+ // that the temporary bitmap is large enough to draw the entire
+ // native control and a focus ring.
tools::Rectangle boundingRegion(rControlRegion);
- boundingRegion.expand(50 * mScaling);
+ tools::Rectangle rNativeBoundingRegion;
+ tools::Rectangle rNativeContentRegion;
+ AquaSalGraphics* pGraphics = mrShared.mpFrame->mpGraphics;
+ if (pGraphics
+ && pGraphics->getNativeControlRegion(nType, nPart, rControlRegion,
nState, aValue,
+ OUString(), rNativeBoundingRegion,
+ rNativeContentRegion))
+ {
+ boundingRegion.Union(rNativeBoundingRegion);
+ boundingRegion.Union(rNativeContentRegion);
+ }
+ boundingRegion.expand(2 * FOCUS_RING_WIDTH * mScaling);
+
// Do a scaled bitmap in HiDPI in order not to lose precision.
const tools::Long width = boundingRegion.GetWidth() * mScaling;
const tools::Long height = boundingRegion.GetHeight() * mScaling;
const size_t bytes = width * height * 4;
- sal_uInt8* data = new sal_uInt8[bytes];
- memset(data, 0, bytes);
+ // Let Skia own the CGBitmapContext's buffer so that an SkImage
+ // can be created without Skia making a copy of the buffer
+ sk_sp<SkData> data = SkData::MakeZeroInitialized(bytes);
CGContextRef context = CGBitmapContextCreate(
- data, width, height, 8, width * 4, GetSalData()->mxRGBSpace,
+ data->writable_data(), width, height, 8, width * 4,
GetSalData()->mxRGBSpace,
SkiaToCGBitmapType(mSurface->imageInfo().colorType(),
kPremul_SkAlphaType));
if (!context)
{
SAL_WARN("vcl.skia", "drawNativeControl(): Failed to allocate bitmap
context");
- delete[] data;
return false;
}
// Setup context state for drawing (performDrawNativeControl() e.g. fills
background in some cases).
@@ -302,14 +326,6 @@ bool
AquaSkiaSalGraphicsImpl::drawNativeControl(ControlType nType, ControlPart n
CGContextRelease(context);
if (bOK)
{
- // Let SkBitmap determine when it is safe to delete the pixel buffer
- SkBitmap bitmap;
- if (!bitmap.installPixels(SkImageInfo::Make(width, height,
-
mSurface->imageInfo().colorType(),
- kPremul_SkAlphaType),
- data, width * 4, nullptr, nullptr))
- abort();
-
preDraw();
SAL_INFO("vcl.skia.trace", "drawnativecontrol(" << this << "): " <<
rControlRegion << ":"
<< int(nType) << "/"
<< int(nPart));
@@ -322,23 +338,19 @@ bool
AquaSkiaSalGraphicsImpl::drawNativeControl(ControlType nType, ControlPart n
updateRect.GetWidth(),
updateRect.GetHeight()));
SkRect drawRect = SkRect::MakeXYWH(boundingRegion.getX(),
boundingRegion.getY(),
boundingRegion.GetWidth(),
boundingRegion.GetHeight());
- assert(drawRect.width() * mScaling == bitmap.width()); // no scaling
should be needed
- getDrawCanvas()->drawImageRect(bitmap.asImage(), drawRect,
SkSamplingOptions());
+ sk_sp<SkImage> image = SkImages::RasterFromData(
+ SkImageInfo::Make(width, height, mSurface->imageInfo().colorType(),
+ kPremul_SkAlphaType),
+ data, width * 4);
+ assert(image
+ && drawRect.width() * mScaling == image->width()); // no
scaling should be needed
+ getDrawCanvas()->drawImageRect(image, drawRect, SkSamplingOptions());
// Related: tdf#156881 flush the canvas after drawing the pixel buffer
if (auto dContext =
GrAsDirectContext(getDrawCanvas()->recordingContext()))
dContext->flushAndSubmit();
++pendingOperationsToFlush; // tdf#136369
postDraw();
}
- // Related: tdf#159529 eliminate possible memory leak
- // Despite confirming that the release function passed to
- // SkBitmap.bitmap.installPixels() does get called for every
- // data array that has been allocated, Apple's Instruments
- // indicates that the data is leaking. While it is likely a
- // false positive, it makes leak analysis difficult so leave
- // the bitmap mutable. That causes SkBitmap.asImage() to make
- // a copy of the data and the data can be safely deleted here.
- delete[] data;
return bOK;
}
