AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

macOS: Prevent backingstore image detach during color space assignment

Browse Source 
The call to CGImageCreateCopyWithColorSpace took a naked toCGImage(),
which left the resulting CGImageRef without a release, causing the
extra ref by toCGImage() to never be derefed, and a subsequent detach
of the image data on the next paint event.

Wrapping the call in a QCFType<CGImageRef> solves the problem. The code
has also been moved directly into QCocoaBackingStore::flush(), as there
is no need to keep the CGImageRef a member.

A local autorelease pool has been added to QCocoaBackingStore::flush(),
so that the NSImage used for blitting the backingstore is released upon
exit of the function, thereby releasing the corresponding CGImageRef.

Note that for layered mode, the QImage will still detach, as the view's
layer.contents property keeps a reference to the image data until being
replaced in a subsequent flush.

Task-number: QTBUG-63559
Change-Id: I06b9298f65a84deae7cc2eff617ba75c92ec3b87
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
This commit is contained in:
Tor Arne Vestbø 2017-10-03 16:55:39 +02:00
parent 1b473ee676
commit 5b4cf7af6a
2 changed files with 11 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/plugins/platforms/cocoa/qcocoabackingstore.h
View File

@ -52,16 +52,12 @@ public:
QCocoaBackingStore(QWindow *window);
~QCocoaBackingStore();
void beginPaint(const QRegion &) override;
void endPaint() override;
void flush(QWindow *, const QRegion &, const QPoint &) Q_DECL_OVERRIDE;
private:
bool windowHasUnifiedToolbar() const;
QImage::Format format() const Q_DECL_OVERRIDE;
void redrawRoundedBottomCorners(CGRect) const;
QCFType<CGImageRef> m_cgImage;
};
QT_END_NAMESPACE

34
src/plugins/platforms/cocoa/qcocoabackingstore.mm
View File

@ -48,7 +48,6 @@ Q_LOGGING_CATEGORY(lcCocoaBackingStore, "qt.qpa.cocoa.backingstore");
QCocoaBackingStore::QCocoaBackingStore(QWindow *window)
: QRasterBackingStore(window)
, m_cgImage(nullptr)
{
}
@ -70,26 +69,6 @@ QImage::Format QCocoaBackingStore::format() const
return QRasterBackingStore::format();
}
void QCocoaBackingStore::beginPaint(const QRegion &region)
{
m_cgImage = nullptr;
QRasterBackingStore::beginPaint(region);
}
void QCocoaBackingStore::endPaint()
{
QRasterBackingStore::endPaint();
// Prevent potentially costly color conversion by assiging the display
// color space to the backingstore image.
NSView *view = static_cast<QCocoaWindow *>(window()->handle())->view();
CGColorSpaceRef displayColorSpace = view.window.screen.colorSpace.CGColorSpace;
QCFType<CGImageRef> displayColorSpaceImage =
CGImageCreateCopyWithColorSpace(m_image.toCGImage(), displayColorSpace);
m_cgImage = displayColorSpaceImage;
}
#if !QT_MACOS_PLATFORM_SDK_EQUAL_OR_ABOVE(__MAC_10_12)
static const NSCompositingOperation NSCompositingOperationCopy = NSCompositeCopy;
static const NSCompositingOperation NSCompositingOperationSourceOver = NSCompositeSourceOver;
@ -111,6 +90,9 @@ void QCocoaBackingStore::flush(QWindow *window, const QRegion &region, const QPo
if (m_image.isNull())
return;
// Use local pool so that any stale image references are cleaned up after flushing
QMacAutoReleasePool pool;
const QWindow *topLevelWindow = this->window();
Q_ASSERT(topLevelWindow->handle() && window->handle());
@ -128,6 +110,12 @@ void QCocoaBackingStore::flush(QWindow *window, const QRegion &region, const QPo
qCDebug(lcCocoaBackingStore) << "Flushing" << region << "of" << view << qPrintable(targetViewDescription);
}
// Prevent potentially costly color conversion by assigning the display color space
// to the backingstore image. This does not copy the underlying image data.
CGColorSpaceRef displayColorSpace = view.window.screen.colorSpace.CGColorSpace;
QCFType<CGImageRef> cgImage = CGImageCreateCopyWithColorSpace(
QCFType<CGImageRef>(m_image.toCGImage()), displayColorSpace);
if (view.layer) {
// In layer-backed mode, locking focus on a view does not give the right
// view transformation, and doesn't give us a graphics context to render
@ -137,7 +125,7 @@ void QCocoaBackingStore::flush(QWindow *window, const QRegion &region, const QPo
// we then directly set the layer's backingstore (content) to our backingstore,
// masked to the part of the subview that is relevant.
// FIXME: Figure out if there's a way to do partial updates
view.layer.contents = (__bridge id)static_cast<CGImageRef>(m_cgImage);
view.layer.contents = (__bridge id)static_cast<CGImageRef>(cgImage);
if (view != topLevelView) {
view.layer.contentsRect = CGRectApplyAffineTransform(
[view convertRect:view.bounds toView:topLevelView],
@ -196,7 +184,7 @@ void QCocoaBackingStore::flush(QWindow *window, const QRegion &region, const QPo
"Focusing the view should give us a current graphics context");
// Create temporary image to use for blitting, without copying image data
NSImage *backingStoreImage = [[[NSImage alloc] initWithCGImage:m_cgImage size:NSZeroSize] autorelease];
NSImage *backingStoreImage = [[[NSImage alloc] initWithCGImage:cgImage size:NSZeroSize] autorelease];
QRegion clippedRegion = region;
for (QWindow *w = window; w; w = w->parent()) {