vcl/inc/skia/osx/gdiimpl.hxx | 4 -
vcl/osx/salgdiutils.cxx | 98 +++++++++++++++++++++--------
vcl/osx/salmacos.cxx | 7 ++
vcl/skia/osx/gdiimpl.cxx | 142 ++++++++++++++++++++++++++-----------------
4 files changed, 167 insertions(+), 84 deletions(-)
New commits:
commit 12dbf0e6b6b485a1d73c7e33bd0ecfb13e6efdac
Author: Patrick Luby <guibmac...@gmail.com>
AuthorDate: Wed Jun 19 16:03:06 2024 -0400
Commit: Patrick Luby <guibomac...@gmail.com>
CommitDate: Tue Jun 25 21:34:25 2024 +0200
tdf#159175 Do not allocate a CGLayer for each NSWindow when using Skia
Skia surfaces can be copied directly to an NSWindow's CGContextRef
so disable allocation of a CGLayer for each NSWindow to significantly
reduce memory usage when Skia is enabled.
Change-Id: I8e3001e4f2ae8dd36156c06db68447c6b1bc67df
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169242
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 b97245e86e11..b60280a09e6b 100644
--- a/vcl/inc/skia/osx/gdiimpl.hxx
+++ b/vcl/inc/skia/osx/gdiimpl.hxx
@@ -44,11 +44,13 @@ public:
virtual void Flush(const tools::Rectangle&) override;
virtual void WindowBackingPropertiesChanged() override;
+ CGImageRef createCGImageFromRasterSurface(const NSRect& rDirtyRect,
CGPoint& rImageOrigin,
+ bool& rImageFlipped);
+
private:
virtual int getWindowScaling() const override;
virtual void createWindowSurfaceInternal(bool forceRaster = false)
override;
virtual void flushSurfaceToWindowContext() override;
- void flushSurfaceToScreenCG();
static inline sk_sp<SkFontMgr> fontManager;
};
diff --git a/vcl/osx/salgdiutils.cxx b/vcl/osx/salgdiutils.cxx
index a9445293211c..d7f8ec48eaf0 100644
--- a/vcl/osx/salgdiutils.cxx
+++ b/vcl/osx/salgdiutils.cxx
@@ -29,6 +29,7 @@
#include <basegfx/range/b2irange.hxx>
#include <basegfx/vector/b2ivector.hxx>
#include <vcl/svapp.hxx>
+#include <vcl/skia/SkiaHelper.hxx>
#include <quartz/salgdi.h>
#include <quartz/utils.h>
@@ -37,7 +38,7 @@
#if HAVE_FEATURE_SKIA
#include <tools/sk_app/mac/WindowContextFactory_mac.h>
-#include <vcl/skia/SkiaHelper.hxx>
+#include <skia/osx/gdiimpl.hxx>
#endif
static bool bTotalScreenBounds = false;
@@ -233,6 +234,10 @@ bool AquaSharedAttributes::checkContext()
maLayer.set(nullptr);
}
+ // tdf#159175 no CGLayer is needed for an NSWindow when using Skia
+ if (SkiaHelper::isVCLSkiaEnabled() && mpFrame->getNSWindow())
+ return true;
+
if (!maContextHolder.isSet())
{
const int nBitmapDepth = 32;
@@ -297,7 +302,7 @@ bool AquaSharedAttributes::checkContext()
* associated window, if any; cf. drawRect event handling
* on the frame.
*/
-void AquaSalGraphics::UpdateWindow( NSRect& )
+void AquaSalGraphics::UpdateWindow( NSRect& rRect )
{
if (!maShared.mpFrame)
{
@@ -305,26 +310,65 @@ void AquaSalGraphics::UpdateWindow( NSRect& )
}
NSGraphicsContext* pContext = [NSGraphicsContext currentContext];
- if (maShared.maLayer.isSet() && pContext != nullptr)
+ if (!pContext)
+ {
+ SAL_WARN_IF(!maShared.mpFrame->mbInitShow, "vcl", "UpdateWindow called
with no NSGraphicsContext");
+ return;
+ }
+
+ CGImageRef img = nullptr;
+ CGPoint aImageOrigin = CGPointMake(0, 0);
+ bool bImageFlipped = false;
+#if HAVE_FEATURE_SKIA
+ if (SkiaHelper::isVCLSkiaEnabled())
+ {
+ // tdf#159175 no CGLayer is needed for an NSWindow when using Skia
+ // Get a CGImageRef directly from the Skia/Raster surface and draw
+ // that directly to the NSWindow.
+ // Note: Skia/Metal will always return a null CGImageRef since it
+ // draws directly to the NSWindow using the surface's CAMetalLayer.
+ AquaSkiaSalGraphicsImpl *pBackend =
static_cast<AquaSkiaSalGraphicsImpl*>(mpBackend.get());
+ if (pBackend)
+ img = pBackend->createCGImageFromRasterSurface(rRect,
aImageOrigin, bImageFlipped);
+ }
+ else
+#else
+ (void)rRect;
+#endif
+ if (maShared.maLayer.isSet())
{
+ maShared.applyXorContext();
+
+ const CGRect aRectPoints = { CGPointZero,
maShared.maLayer.getSizePixels() };
+ CGContextSetBlendMode(maShared.maCSContextHolder.get(),
kCGBlendModeCopy);
+ CGContextDrawLayerInRect(maShared.maCSContextHolder.get(),
aRectPoints, maShared.maLayer.get());
+
+ img = CGBitmapContextCreateImage(maShared.maCSContextHolder.get());
+ }
+
+ if (img)
+ {
+ const float fScale = sal::aqua::getWindowScaling();
CGContextHolder rCGContextHolder([pContext CGContext]);
rCGContextHolder.saveState();
- // Related: tdf#155092 translate Y coordinate for height differences
- // When in live resize, the NSView's height may have changed before
- // the CGLayer has been resized. This causes the CGLayer's content
- // to be drawn just above or below the top left corner of the view
- // so translate the Y coordinate by any difference between the
- // NSView's height and the CGLayer's height.
- NSView *pView = maShared.mpFrame->mpNSView;
- if (pView)
+ CGRect aRect = CGRectMake(aImageOrigin.x / fScale, aImageOrigin.y /
fScale, CGImageGetWidth(img) / fScale, CGImageGetHeight(img) / fScale);
+ if (bImageFlipped)
{
+ // Related: tdf#155092 translate Y coordinate of flipped images
+ // When in live resize, the NSView's height may have changed before
+ // the surface has been resized. This causes flipped content
+ // to be drawn just above or below the top left corner of the view
+ // so translate the Y coordinate using the NSView's height.
// Use the NSView's bounds, not its frame, to properly handle
// any rotation and/or scaling that might have been already
- // applied to the view
- CGFloat fTranslateY = [pView bounds].size.height -
maShared.maLayer.getSizePoints().height;
- CGContextTranslateCTM(rCGContextHolder.get(), 0, fTranslateY);
+ // applied to the view.
+ NSView *pView = maShared.mpFrame->mpNSView;
+ if (pView)
+ aRect.origin.y = [pView bounds].size.height - aRect.origin.y -
aRect.size.height;
+ else if (maShared.maLayer.isSet())
+ aRect.origin.y = maShared.maLayer.getSizePoints().height -
aRect.origin.y - aRect.size.height;
}
CGMutablePathRef rClip = maShared.mpFrame->getClipPath();
@@ -335,23 +379,23 @@ void AquaSalGraphics::UpdateWindow( NSRect& )
CGContextClip(rCGContextHolder.get());
}
- maShared.applyXorContext();
-
- const CGSize aSize = maShared.maLayer.getSizePoints();
- const CGRect aRect = CGRectMake(0, 0, aSize.width, aSize.height);
- const CGRect aRectPoints = { CGPointZero,
maShared.maLayer.getSizePixels() };
- CGContextSetBlendMode(maShared.maCSContextHolder.get(),
kCGBlendModeCopy);
- CGContextDrawLayerInRect(maShared.maCSContextHolder.get(),
aRectPoints, maShared.maLayer.get());
-
- CGImageRef img =
CGBitmapContextCreateImage(maShared.maCSContextHolder.get());
- CGImageRef displayColorSpaceImage =
CGImageCreateCopyWithColorSpace(img, [[maShared.mpFrame->getNSWindow()
colorSpace] CGColorSpace]);
CGContextSetBlendMode(rCGContextHolder.get(), kCGBlendModeCopy);
- CGContextDrawImage(rCGContextHolder.get(), aRect,
displayColorSpaceImage);
- CGImageRelease(img);
- CGImageRelease(displayColorSpaceImage);
+ NSWindow *pWindow = maShared.mpFrame->getNSWindow();
+ if (pWindow)
+ {
+ CGImageRef displayColorSpaceImage =
CGImageCreateCopyWithColorSpace(img, [[maShared.mpFrame->getNSWindow()
colorSpace] CGColorSpace]);
+ CGContextDrawImage(rCGContextHolder.get(), aRect,
displayColorSpaceImage);
+ CGImageRelease(displayColorSpaceImage);
+ }
+ else
+ {
+ CGContextDrawImage(rCGContextHolder.get(), aRect, img);
+ }
rCGContextHolder.restoreState();
+
+ CGImageRelease(img);
}
else
{
diff --git a/vcl/osx/salmacos.cxx b/vcl/osx/salmacos.cxx
index 700b252cf4f3..14e2a80695d9 100644
--- a/vcl/osx/salmacos.cxx
+++ b/vcl/osx/salmacos.cxx
@@ -26,6 +26,7 @@
#include <osl/diagnose.h>
#include <vcl/bitmap.hxx>
+#include <vcl/skia/SkiaHelper.hxx>
#include <quartz/salbmp.h>
#include <quartz/salgdi.h>
@@ -506,6 +507,12 @@ bool AquaSalVirtualDevice::SetSize(tools::Long nDX,
tools::Long nDY)
nFlags = uint32_t(kCGImageAlphaNoneSkipFirst) |
uint32_t(kCGBitmapByteOrder32Host);
}
+ if (SkiaHelper::isVCLSkiaEnabled())
+ {
+ mpGraphics->SetVirDevGraphics(this, maLayer, nullptr, mnBitmapDepth);
+ return true;
+ }
+
// Allocate buffer for virtual device graphics as bitmap context to store
graphics with highest required (scaled) resolution
size_t nScaledWidth = mnWidth * fScale;
diff --git a/vcl/skia/osx/gdiimpl.cxx b/vcl/skia/osx/gdiimpl.cxx
index 9b511ad4469b..ffe1ebc42d65 100644
--- a/vcl/skia/osx/gdiimpl.cxx
+++ b/vcl/skia/osx/gdiimpl.cxx
@@ -37,6 +37,24 @@
using namespace SkiaHelper;
+namespace
+{
+struct SnapshotImageData
+{
+ sk_sp<SkImage> image;
+ SkPixmap pixmap;
+};
+}
+
+static void SnapshotImageDataCallback(void* pInfo, const void* pData, size_t
nSize)
+{
+ (void)pData;
+ (void)nSize;
+
+ if (pInfo)
+ delete static_cast<SnapshotImageData*>(pInfo);
+}
+
AquaSkiaSalGraphicsImpl::AquaSkiaSalGraphicsImpl(AquaSalGraphics& rParent,
AquaSharedAttributes& rShared)
: SkiaSalGraphicsImpl(rParent, rShared.mpFrame)
@@ -101,117 +119,129 @@ void
AquaSkiaSalGraphicsImpl::WindowBackingPropertiesChanged() { windowBackingPr
void AquaSkiaSalGraphicsImpl::flushSurfaceToWindowContext()
{
if (!isGPU())
- flushSurfaceToScreenCG();
+ {
+ // tdf159175 mark dirty area in NSWindow for redrawing
+ // This will cause -[SalFrameView drawRect:] to be called. That,
+ // in turn, will draw a CGImageRef of the surface fetched from
+ // AquaSkiaSalGraphicsImpl::createCGImageFromRasterSurface().
+ mrShared.refreshRect(mDirtyRect.x(), mDirtyRect.y(),
mDirtyRect.width(),
+ mDirtyRect.height());
+ }
else
+ {
SkiaSalGraphicsImpl::flushSurfaceToWindowContext();
+ }
}
// For Raster we use our own screen blitting (see above).
-void AquaSkiaSalGraphicsImpl::flushSurfaceToScreenCG()
+CGImageRef AquaSkiaSalGraphicsImpl::createCGImageFromRasterSurface(const
NSRect& rDirtyRect,
+ CGPoint&
rImageOrigin,
+ bool&
rImageFlipped)
{
+ if (isGPU() || !mSurface)
+ return nullptr;
+
// Based on AquaGraphicsBackend::drawBitmap().
if (!mrShared.checkContext())
- return;
+ return nullptr;
+
+ NSRect aIntegralRect = NSIntegralRect(rDirtyRect);
+ if (NSIsEmptyRect(aIntegralRect))
+ return nullptr;
- assert(mSurface.get());
// Do not use sub-rect, it creates copies of the data.
- sk_sp<SkImage> image = makeCheckedImageSnapshot(mSurface);
- SkPixmap pixmap;
- if (!image->peekPixels(&pixmap))
+ SnapshotImageData* pInfo = new SnapshotImageData;
+ pInfo->image = makeCheckedImageSnapshot(mSurface);
+ if (!pInfo->image->peekPixels(&pInfo->pixmap))
abort();
- // If window scaling, then mDirtyRect is in VCL coordinates, mSurface has
screen size (=points,HiDPI),
- // maContextHolder has screen size but a scale matrix set so its inputs
are in VCL coordinates (see
- // its setup in AquaSharedAttributes::checkContext()).
+
+ SkIRect aDirtyRect = SkIRect::MakeXYWH(
+ aIntegralRect.origin.x * mScaling, aIntegralRect.origin.y * mScaling,
+ aIntegralRect.size.width * mScaling, aIntegralRect.size.height *
mScaling);
+ if (mrShared.isFlipped())
+ aDirtyRect = SkIRect::MakeXYWH(
+ aDirtyRect.x(), pInfo->pixmap.bounds().height() - aDirtyRect.y() -
aDirtyRect.height(),
+ aDirtyRect.width(), aDirtyRect.height());
+ if (!aDirtyRect.intersect(pInfo->pixmap.bounds()))
+ {
+ delete pInfo;
+ return nullptr;
+ }
+
+ // If window scaling, then aDirtyRect is in scaled VCL coordinates and
mSurface has
+ // screen size (=points,HiDPI).
// This creates the bitmap context from the cropped part,
writable_addr32() will get
- // the first pixel of mDirtyRect.topLeft(), and using pixmap.rowBytes()
ensures the following
+ // the first pixel of aDirtyRect.topLeft(), and using pixmap.rowBytes()
ensures the following
// pixel lines will be read from correct positions.
- if (pixmap.bounds() != mDirtyRect && pixmap.bounds().bottom() ==
mDirtyRect.bottom())
+ if (pInfo->pixmap.bounds() != aDirtyRect
+ && pInfo->pixmap.bounds().bottom() == aDirtyRect.bottom())
{
- // HACK for tdf#145843: If mDirtyRect includes the last line but not
the first pixel of it,
+ // HACK for tdf#145843: If aDirtyRect includes the last line but not
the first pixel of it,
// then the rowBytes() trick would lead to the CG* functions thinking
that even pixels after
// the pixmap data belong to the area (since the shifted
x()+rowBytes() points there) and
// at least on Intel Mac they would actually read those data, even
though I see no good reason
// to do that, as that's beyond the x()+width() for the last line.
That could be handled
// by creating a subset SkImage (which as is said above copies data),
or set the x coordinate
// to 0, which will then make rowBytes() match the actual data.
- mDirtyRect.fLeft = 0;
+ aDirtyRect.fLeft = 0;
// Related tdf#156630 pixmaps can be wider than the dirty rectangle
// This seems to most commonly occur when SAL_FORCE_HIDPI_SCALING=1
// and the native window scale is 2.
- assert(mDirtyRect.width() <= pixmap.bounds().width());
+ assert(aDirtyRect.width() <= pInfo->pixmap.bounds().width());
}
// tdf#145843 Do not use CGBitmapContextCreate() to create a bitmap context
// As described in the comment in the above code, CGBitmapContextCreate()
// and CGBitmapContextCreateWithData() will try to access pixels up to
- // mDirtyRect.x() + pixmap.bounds.width() for each row. When reading the
+ // aDirtyRect.x() + pixmap.bounds.width() for each row. When reading the
// last line in the SkPixmap, the buffer allocated for the SkPixmap ends at
- // mDirtyRect.x() + mDirtyRect.width() and mDirtyRect.width() is clamped to
- // pixmap.bounds.width() - mDirtyRect.x().
+ // aDirtyRect.x() + aDirtyRect.width() and aDirtyRect.width() is clamped to
+ // pixmap.bounds.width() - aDirtyRect.x().
// This behavior looks like an optimization within CGBitmapContextCreate()
// to draw with a single memcpy() so fix this bug by chaining the
// CGDataProvider(), CGImageCreate(), and CGImageCreateWithImageInRect()
// functions to create the screen image.
- CGDataProviderRef dataProvider = CGDataProviderCreateWithData(
- nullptr, pixmap.writable_addr32(0, 0), pixmap.computeByteSize(),
nullptr);
+ CGDataProviderRef dataProvider
+ = CGDataProviderCreateWithData(pInfo, pInfo->pixmap.writable_addr32(0,
0),
+ pInfo->pixmap.computeByteSize(),
SnapshotImageDataCallback);
if (!dataProvider)
{
+ delete pInfo;
SAL_WARN("vcl.skia", "flushSurfaceToScreenGC(): Failed to allocate
data provider");
- return;
+ return nullptr;
}
- CGImageRef fullImage = CGImageCreate(pixmap.bounds().width(),
pixmap.bounds().height(), 8,
- 8 *
image->imageInfo().bytesPerPixel(), pixmap.rowBytes(),
- GetSalData()->mxRGBSpace,
-
SkiaToCGBitmapType(image->colorType(), image->alphaType()),
- dataProvider, nullptr, false,
kCGRenderingIntentDefault);
+ CGImageRef fullImage
+ = CGImageCreate(pInfo->pixmap.bounds().width(),
pInfo->pixmap.bounds().height(), 8,
+ 8 * pInfo->image->imageInfo().bytesPerPixel(),
pInfo->pixmap.rowBytes(),
+ GetSalData()->mxRGBSpace,
+ SkiaToCGBitmapType(pInfo->image->colorType(),
pInfo->image->alphaType()),
+ dataProvider, nullptr, false,
kCGRenderingIntentDefault);
if (!fullImage)
{
CGDataProviderRelease(dataProvider);
SAL_WARN("vcl.skia", "flushSurfaceToScreenGC(): Failed to allocate
full image");
- return;
+ return nullptr;
}
CGImageRef screenImage = CGImageCreateWithImageInRect(
- fullImage, CGRectMake(mDirtyRect.x() * mScaling, mDirtyRect.y() *
mScaling,
- mDirtyRect.width() * mScaling,
mDirtyRect.height() * mScaling));
+ fullImage,
+ CGRectMake(aDirtyRect.x(), aDirtyRect.y(), aDirtyRect.width(),
aDirtyRect.height()));
if (!screenImage)
{
CGImageRelease(fullImage);
CGDataProviderRelease(dataProvider);
- SAL_WARN("vcl.skia", "flushSurfaceToScreenGC(): Failed to allocate
screen image");
- return;
+ SAL_WARN("vcl.skia", "createCGImageFromRasterSurface(): Failed to
allocate screen image");
+ return nullptr;
}
- mrShared.maContextHolder.saveState();
- // Drawing to the actual window has scaling active, so use unscaled
coordinates, the scaling matrix will scale them
- // to the proper screen coordinates. Unless the scaling is fake for
debugging, in which case scale them to draw
- // at the scaled size.
- int windowScaling = 1;
- static const char* env = getenv("SAL_FORCE_HIDPI_SCALING");
- if (env != nullptr)
- windowScaling = atoi(env);
- CGRect drawRect
- = CGRectMake(mDirtyRect.x() * windowScaling, mDirtyRect.y() *
windowScaling,
- mDirtyRect.width() * windowScaling, mDirtyRect.height() *
windowScaling);
- if (mrShared.isFlipped())
- {
- // I don't understand why, but apparently it's needed to explicitly to
flip the drawing, even though maContextHelper
- // has this set up, so this unsets the flipping.
- CGFloat invertedY = drawRect.origin.y + drawRect.size.height;
- CGContextTranslateCTM(mrShared.maContextHolder.get(), 0, invertedY);
- CGContextScaleCTM(mrShared.maContextHolder.get(), 1, -1);
- drawRect.origin.y = 0;
- }
- CGContextDrawImage(mrShared.maContextHolder.get(), drawRect, screenImage);
- mrShared.maContextHolder.restoreState();
+ rImageOrigin = CGPointMake(aDirtyRect.x(), aDirtyRect.y());
+ rImageFlipped = mrShared.isFlipped();
- CGImageRelease(screenImage);
CGImageRelease(fullImage);
CGDataProviderRelease(dataProvider);
- // This is also in VCL coordinates.
- mrShared.refreshRect(mDirtyRect.x(), mDirtyRect.y(), mDirtyRect.width(),
mDirtyRect.height());
+ return screenImage;
}
bool AquaSkiaSalGraphicsImpl::drawNativeControl(ControlType nType, ControlPart
nPart,
