macOS: Keep reference to NSScreen via index instead of pointer

Manual revert of 73e68a9c0f, which was flawed. macOS can, and will,
dealloc and realloc NSScreen instances for a given screen index, for
example when deallocing an NSWindow, which has the screen as an
auxiliary resource. This is also documented for +[NSScreen screens],
which states that "The array should not be cached".

Task-number: QTBUG-58128
Change-Id: I926513a26cb7af52acd7fc5ee9380ef29ede65e6
Reviewed-by: Robin Burchell <robin.burchell@crimson.no>
This commit is contained in:
Tor Arne Vestbø 2017-01-17 15:56:05 +01:00
parent 04a74c362f
commit 2fe9f30512
3 changed files with 28 additions and 20 deletions

View File

@ -61,7 +61,7 @@ QT_BEGIN_NAMESPACE
class QCocoaScreen : public QPlatformScreen class QCocoaScreen : public QPlatformScreen
{ {
public: public:
QCocoaScreen(NSScreen *screen); QCocoaScreen(int screenIndex);
~QCocoaScreen(); ~QCocoaScreen();
// ---------------------------------------------------- // ----------------------------------------------------
@ -84,7 +84,7 @@ public:
// ---------------------------------------------------- // ----------------------------------------------------
// Additional methods // Additional methods
void setVirtualSiblings(const QList<QPlatformScreen *> &siblings) { m_siblings = siblings; } void setVirtualSiblings(const QList<QPlatformScreen *> &siblings) { m_siblings = siblings; }
NSScreen *nsScreen() const; NSScreen *nativeScreen() const;
void updateGeometry(); void updateGeometry();
QPointF mapToNative(const QPointF &pos) const { return flipCoordinate(pos); } QPointF mapToNative(const QPointF &pos) const { return flipCoordinate(pos); }
@ -97,7 +97,7 @@ private:
QRectF flipCoordinate(const QRectF &rect) const; QRectF flipCoordinate(const QRectF &rect) const;
public: public:
NSScreen *m_nsScreen; int m_screenIndex;
QRect m_geometry; QRect m_geometry;
QRect m_availableGeometry; QRect m_availableGeometry;
QDpi m_logicalDpi; QDpi m_logicalDpi;

View File

@ -69,8 +69,8 @@ static void initResources()
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
QCocoaScreen::QCocoaScreen(NSScreen *screen) QCocoaScreen::QCocoaScreen(int screenIndex)
: QPlatformScreen(), m_nsScreen(screen), m_refreshRate(60.0) : QPlatformScreen(), m_screenIndex(screenIndex), m_refreshRate(60.0)
{ {
updateGeometry(); updateGeometry();
m_cursor = new QCocoaCursor; m_cursor = new QCocoaCursor;
@ -81,9 +81,15 @@ QCocoaScreen::~QCocoaScreen()
delete m_cursor; delete m_cursor;
} }
NSScreen *QCocoaScreen::nsScreen() const NSScreen *QCocoaScreen::nativeScreen() const
{ {
return m_nsScreen; NSArray *screens = [NSScreen screens];
// Stale reference, screen configuration has changed
if (m_screenIndex < 0 || (NSUInteger)m_screenIndex >= [screens count])
return nil;
return [screens objectAtIndex:m_screenIndex];
} }
/*! /*!
@ -116,28 +122,29 @@ QRectF QCocoaScreen::flipCoordinate(const QRectF &rect) const
void QCocoaScreen::updateGeometry() void QCocoaScreen::updateGeometry()
{ {
if (!m_nsScreen) NSScreen *nsScreen = nativeScreen();
if (!nsScreen)
return; return;
// At this point the geometry is in native coordinates, but the size // At this point the geometry is in native coordinates, but the size
// is correct, which we take advantage of next when we map the native // is correct, which we take advantage of next when we map the native
// coordinates to the Qt coordinate system. // coordinates to the Qt coordinate system.
m_geometry = QRectF::fromCGRect(NSRectToCGRect(m_nsScreen.frame)).toRect(); m_geometry = QRectF::fromCGRect(NSRectToCGRect(nsScreen.frame)).toRect();
m_availableGeometry = QRectF::fromCGRect(NSRectToCGRect(m_nsScreen.visibleFrame)).toRect(); m_availableGeometry = QRectF::fromCGRect(NSRectToCGRect(nsScreen.visibleFrame)).toRect();
// The reference screen for the geometry is always the primary screen, but since // The reference screen for the geometry is always the primary screen, but since
// we may be in the process of creating and registering the primary screen, we // we may be in the process of creating and registering the primary screen, we
// must special-case that and assign it direcly. // must special-case that and assign it direcly.
QCocoaScreen *primaryScreen = (m_nsScreen == [[NSScreen screens] firstObject]) ? QCocoaScreen *primaryScreen = (nsScreen == [[NSScreen screens] firstObject]) ?
this : static_cast<QCocoaScreen*>(QGuiApplication::primaryScreen()->handle()); this : static_cast<QCocoaScreen*>(QGuiApplication::primaryScreen()->handle());
m_geometry = primaryScreen->mapFromNative(m_geometry).toRect(); m_geometry = primaryScreen->mapFromNative(m_geometry).toRect();
m_availableGeometry = primaryScreen->mapFromNative(m_availableGeometry).toRect(); m_availableGeometry = primaryScreen->mapFromNative(m_availableGeometry).toRect();
m_format = QImage::Format_RGB32; m_format = QImage::Format_RGB32;
m_depth = NSBitsPerPixelFromDepth([m_nsScreen depth]); m_depth = NSBitsPerPixelFromDepth([nsScreen depth]);
NSDictionary *devDesc = [m_nsScreen deviceDescription]; NSDictionary *devDesc = [nsScreen deviceDescription];
CGDirectDisplayID dpy = [[devDesc objectForKey:@"NSScreenNumber"] unsignedIntValue]; CGDirectDisplayID dpy = [[devDesc objectForKey:@"NSScreenNumber"] unsignedIntValue];
CGSize size = CGDisplayScreenSize(dpy); CGSize size = CGDisplayScreenSize(dpy);
m_physicalSize = QSizeF(size.width, size.height); m_physicalSize = QSizeF(size.width, size.height);
@ -164,7 +171,8 @@ void QCocoaScreen::updateGeometry()
qreal QCocoaScreen::devicePixelRatio() const qreal QCocoaScreen::devicePixelRatio() const
{ {
QMacAutoReleasePool pool; QMacAutoReleasePool pool;
return qreal(m_nsScreen ? [m_nsScreen backingScaleFactor] : 1.0); NSScreen *nsScreen = nativeScreen();
return qreal(nsScreen ? [nsScreen backingScaleFactor] : 1.0);
} }
QPlatformScreen::SubpixelAntialiasingType QCocoaScreen::subpixelAntialiasingTypeHint() const QPlatformScreen::SubpixelAntialiasingType QCocoaScreen::subpixelAntialiasingTypeHint() const
@ -443,7 +451,7 @@ void QCocoaIntegration::updateScreens()
// NSScreen documentation says do not cache the array returned from [NSScreen screens]. // NSScreen documentation says do not cache the array returned from [NSScreen screens].
// However in practice, we can identify a screen by its pointer: if resolution changes, // However in practice, we can identify a screen by its pointer: if resolution changes,
// the NSScreen object will be the same instance, just with different values. // the NSScreen object will be the same instance, just with different values.
if (existingScr->nsScreen() == scr) { if (existingScr->nativeScreen() == scr) {
screen = existingScr; screen = existingScr;
break; break;
} }
@ -451,7 +459,7 @@ void QCocoaIntegration::updateScreens()
remainingScreens.remove(screen); remainingScreens.remove(screen);
screen->updateGeometry(); screen->updateGeometry();
} else { } else {
screen = new QCocoaScreen(scr); screen = new QCocoaScreen(i);
mScreens.append(screen); mScreens.append(screen);
screenAdded(screen); screenAdded(screen);
} }
@ -468,7 +476,7 @@ void QCocoaIntegration::updateScreens()
foreach (QCocoaScreen* screen, remainingScreens) { foreach (QCocoaScreen* screen, remainingScreens) {
mScreens.removeOne(screen); mScreens.removeOne(screen);
// Prevent stale references to NSScreen during destroy // Prevent stale references to NSScreen during destroy
screen->m_nsScreen = nil; screen->m_screenIndex = -1;
destroyScreen(screen); destroyScreen(screen);
} }
} }
@ -483,7 +491,7 @@ QCocoaScreen *QCocoaIntegration::screenForNSScreen(NSScreen *nsScreen)
updateScreens(); updateScreens();
for (QCocoaScreen *screen : mScreens) { for (QCocoaScreen *screen : mScreens) {
if (screen->nsScreen() == nsScreen) if (screen->nativeScreen() == nsScreen)
return screen; return screen;
} }

View File

@ -1685,7 +1685,7 @@ QCocoaNSWindow *QCocoaWindow::createNSWindow(bool shouldBeChildNSWindow, bool sh
// Use NSPanel for popup-type windows. (Popup, Tool, ToolTip, SplashScreen) // Use NSPanel for popup-type windows. (Popup, Tool, ToolTip, SplashScreen)
// and dialogs // and dialogs
if (shouldBePanel) { if (shouldBePanel) {
QNSPanel *panel = [[QNSPanel alloc] initWithContentRect:frame screen:cocoaScreen->nsScreen() QNSPanel *panel = [[QNSPanel alloc] initWithContentRect:frame screen:cocoaScreen->nativeScreen()
styleMask: styleMask qPlatformWindow:this]; styleMask: styleMask qPlatformWindow:this];
if ((type & Qt::Popup) == Qt::Popup) if ((type & Qt::Popup) == Qt::Popup)
@ -1704,7 +1704,7 @@ QCocoaNSWindow *QCocoaWindow::createNSWindow(bool shouldBeChildNSWindow, bool sh
createdWindow = panel; createdWindow = panel;
} else { } else {
createdWindow = [[QNSWindow alloc] initWithContentRect:frame screen:cocoaScreen->nsScreen() createdWindow = [[QNSWindow alloc] initWithContentRect:frame screen:cocoaScreen->nativeScreen()
styleMask: styleMask qPlatformWindow:this]; styleMask: styleMask qPlatformWindow:this];
} }