From 32228c4f2b3419a35d1623377050ef72edf73c92 Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen Date: Wed, 6 Apr 2011 11:07:08 +0200 Subject: Cocoa: p1 bug fix: revert use of subWindowStacking This reverts commit 3c2373d7ea9bc91bb537c0725984d19ad0fbab01. After finding yet another bug related to cocoa child windows (QTBUG-17738), we have no other option than to admit it was a wrong move to use the API in the first place. Had we only known how many side-effects and hidden bugs it would introduce. The original problem we tried to solve were the cases where a stays-on-top parent window executed a modal child dialog. This child should always stay on top of its parent, but Cocoa would insist on pushing the window down to the modal window level upon activating/deactivating the application. Some window systems will always stack a window child on top of the parent, while others (X11) seems to be more selective on this issue. On Mac, we already stack windows a bit differently, thinking first and foremost on tool windows. Since this change is going into a patch release (which is debatable, since this changes behaviour, but p1 is a p1), we choose to add in a backdoor for those users who by chance depend on this behaviour. Setting the env var QT_MAC_USE_CHILDWINDOWS=1 will give you the old code path, but we plan to remove this for Qt-4.8. Also, this patch does fix the original bug described above by overriding the setLevel method in NSWindow, and refuse Cocoa to level down stays-on-top modal windows. --- src/corelib/global/qnamespace.h | 1 - src/corelib/global/qnamespace.qdoc | 8 ------- src/gui/kernel/qcocoasharedwindowmethods_mac_p.h | 14 +++++++++++ src/gui/kernel/qwidget_mac.mm | 30 ++++++++++++------------ 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/corelib/global/qnamespace.h b/src/corelib/global/qnamespace.h index 864d4a5..4d70744 100644 --- a/src/corelib/global/qnamespace.h +++ b/src/corelib/global/qnamespace.h @@ -525,7 +525,6 @@ public: #endif WA_X11DoNotAcceptFocus = 132, - WA_MacNoCocoaChildWindow = 133, // Add new attributes before this line WA_AttributeCount diff --git a/src/corelib/global/qnamespace.qdoc b/src/corelib/global/qnamespace.qdoc index 224d9b6..a79411b 100644 --- a/src/corelib/global/qnamespace.qdoc +++ b/src/corelib/global/qnamespace.qdoc @@ -981,14 +981,6 @@ the brushed metal style as supported by the windowing system. This attribute is only applicable to Mac OS X. - \value WA_MacNoCocoaChildWindow Indicates the widget should not be added - as a Cocoa child window of it's parent window. This will free the window - from being moved around together with the parent. However, this - will also allow it to stack/hide behind it's parent (if they are on - the same window level, e.g both windows are dialogs). This can cause problems if - both windows are modal, as the child can then block input to the parent - while hiding behind it. This attribute is only applicable to Mac OS X. - \omitvalue WA_MacMetalStyle \value WA_Mapped Indicates that the widget is mapped on screen. diff --git a/src/gui/kernel/qcocoasharedwindowmethods_mac_p.h b/src/gui/kernel/qcocoasharedwindowmethods_mac_p.h index 6254061..406e6d4 100644 --- a/src/gui/kernel/qcocoasharedwindowmethods_mac_p.h +++ b/src/gui/kernel/qcocoasharedwindowmethods_mac_p.h @@ -157,6 +157,20 @@ QT_END_NAMESPACE [NSApp terminate:sender]; } +- (void)setLevel:(NSInteger)windowLevel +{ + // Cocoa will upon activating/deactivating applications level modal + // windows up and down, regardsless of any explicit set window level. + // To ensure that modal stays-on-top dialogs actually stays on top after + // the application is activated (and therefore stacks in front of + // other stays-on-top windows), we need to add this little special-case override: + QWidget *widget = [[QT_MANGLE_NAMESPACE(QCocoaWindowDelegate) sharedDelegate] qt_qwidgetForWindow:self]; + if (widget && widget->isModal() && (widget->windowFlags() & Qt::WindowStaysOnTopHint)) + [super setLevel:NSPopUpMenuWindowLevel]; + else + [super setLevel:windowLevel]; +} + - (void)sendEvent:(NSEvent *)event { if ([event type] == NSApplicationDefined) { diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm index 7105d23..0f4abfa 100644 --- a/src/gui/kernel/qwidget_mac.mm +++ b/src/gui/kernel/qwidget_mac.mm @@ -2794,14 +2794,18 @@ void QWidgetPrivate::transferChildren() #ifdef QT_MAC_USE_COCOA void QWidgetPrivate::setSubWindowStacking(bool set) { + // After hitting too many unforeseen bugs trying to put Qt on top of the cocoa child + // window API, we have decided to revert this behaviour. For compability + // reasons we leave the code here for applications depending on it: + static bool use_behaviour_qt473 = !qgetenv("QT_MAC_USE_CHILDWINDOWS").isEmpty(); + if (use_behaviour_qt473 == false) + return; + // This will set/remove a visual relationship between parent and child on screen. // The reason for doing this is to ensure that a child always stacks infront of - // its parent (expecially if both windows are modal, and the child blocks input to - // the parent while at the same time hiding behind it). Unfortunatly is turns out - // that [NSWindow addChildWindow] has + // its parent. Unfortunatly is turns out that [NSWindow addChildWindow] has // several unwanted side-effects, one of them being the moving of a child when - // moving the parent, which we choose to accept (if this is unacceptable, consider - // using Qt::WA_MacNoCocoaChildWindow). A way tougher side-effect is + // moving the parent, which we choose to accept. A way tougher side-effect is // that Cocoa will hide the parent if you hide the child. And in the case of // a tool window, since it will normally hide when you deactivate the // application, Cocoa will hide the parent upon deactivate as well. The result often @@ -2824,14 +2828,12 @@ void QWidgetPrivate::setSubWindowStacking(bool set) if (QWidget *parent = q->parentWidget()) { if (NSWindow *pwin = [qt_mac_nativeview_for(parent) window]) { if (set) { - if (!q->testAttribute(Qt::WA_MacNoCocoaChildWindow)) { - Qt::WindowType ptype = parent->window()->windowType(); - if ([pwin isVisible] && (ptype == Qt::Window || ptype == Qt::Dialog) && ![qwin parentWindow]) { - NSInteger level = [qwin level]; - [pwin addChildWindow:qwin ordered:NSWindowAbove]; - if ([qwin level] < level) - [qwin setLevel:level]; - } + Qt::WindowType ptype = parent->window()->windowType(); + if ([pwin isVisible] && (ptype == Qt::Window || ptype == Qt::Dialog) && ![qwin parentWindow]) { + NSInteger level = [qwin level]; + [pwin addChildWindow:qwin ordered:NSWindowAbove]; + if ([qwin level] < level) + [qwin setLevel:level]; } } else { [pwin removeChildWindow:qwin]; @@ -2845,8 +2847,6 @@ void QWidgetPrivate::setSubWindowStacking(bool set) if (child && child->isWindow()) { if (NSWindow *cwin = [qt_mac_nativeview_for(child) window]) { if (set) { - if (child->testAttribute(Qt::WA_MacNoCocoaChildWindow)) - continue; Qt::WindowType ctype = child->window()->windowType(); if ([cwin isVisible] && (ctype == Qt::Window || ctype == Qt::Dialog) && ![cwin parentWindow]) { NSInteger level = [cwin level]; -- cgit v0.12 From 051a76c1d65d698f71dc75c89f91ae9021357eae Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Wed, 6 Apr 2011 15:46:35 +1000 Subject: Image w/ PreserveAspectFit has its width changed once more than needed. Avoid an extra setImplicitWidth/setImplicitHeight on image load. Change-Id: I8bec1c97244068000c7a7f5fb3e937f80f3b36f5 Task-number: QTBUG-18573 Reviewed-by: Michael Brasser --- src/declarative/graphicsitems/qdeclarativeimage.cpp | 20 ++++++++++++++++---- .../graphicsitems/qdeclarativeimagebase.cpp | 12 ++++++------ .../qdeclarativeimage/data/aspectratio.qml | 4 ++++ .../qdeclarativeimage/tst_qdeclarativeimage.cpp | 10 +++++++++- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/declarative/graphicsitems/qdeclarativeimage.cpp b/src/declarative/graphicsitems/qdeclarativeimage.cpp index a62d3d2..b776532 100644 --- a/src/declarative/graphicsitems/qdeclarativeimage.cpp +++ b/src/declarative/graphicsitems/qdeclarativeimage.cpp @@ -136,12 +136,10 @@ void QDeclarativeImagePrivate::setPixmap(const QPixmap &pixmap) Q_Q(QDeclarativeImage); pix.setPixmap(pixmap); - q->setImplicitWidth(pix.width()); - q->setImplicitHeight(pix.height()); + q->pixmapChange(); status = pix.isNull() ? QDeclarativeImageBase::Null : QDeclarativeImageBase::Ready; q->update(); - q->pixmapChange(); } /*! @@ -390,8 +388,11 @@ void QDeclarativeImage::updatePaintedGeometry() Q_D(QDeclarativeImage); if (d->fillMode == PreserveAspectFit) { - if (!d->pix.width() || !d->pix.height()) + if (!d->pix.width() || !d->pix.height()) { + setImplicitWidth(0); + setImplicitHeight(0); return; + } qreal w = widthValid() ? width() : d->pix.width(); qreal widthScale = w / qreal(d->pix.width()); qreal h = heightValid() ? height() : d->pix.height(); @@ -405,9 +406,13 @@ void QDeclarativeImage::updatePaintedGeometry() } if (widthValid() && !heightValid()) { setImplicitHeight(d->paintedHeight); + } else { + setImplicitHeight(d->pix.height()); } if (heightValid() && !widthValid()) { setImplicitWidth(d->paintedWidth); + } else { + setImplicitWidth(d->pix.width()); } } else if (d->fillMode == PreserveAspectCrop) { if (!d->pix.width() || !d->pix.height()) @@ -566,6 +571,13 @@ void QDeclarativeImage::paint(QPainter *p, const QStyleOptionGraphicsItem *, QWi void QDeclarativeImage::pixmapChange() { + Q_D(QDeclarativeImage); + // PreserveAspectFit calculates the implicit size differently so we + // don't call our superclass pixmapChange(), since that would + // result in the implicit size being set incorrectly, then updated + // in updatePaintedGeometry() + if (d->fillMode != PreserveAspectFit) + QDeclarativeImageBase::pixmapChange(); updatePaintedGeometry(); } diff --git a/src/declarative/graphicsitems/qdeclarativeimagebase.cpp b/src/declarative/graphicsitems/qdeclarativeimagebase.cpp index daebac4..81eac78 100644 --- a/src/declarative/graphicsitems/qdeclarativeimagebase.cpp +++ b/src/declarative/graphicsitems/qdeclarativeimagebase.cpp @@ -191,11 +191,9 @@ void QDeclarativeImageBase::load() d->pix.clear(this); d->status = Null; d->progress = 0.0; - setImplicitWidth(0); - setImplicitHeight(0); + pixmapChange(); emit progressChanged(d->progress); emit statusChanged(d->status); - pixmapChange(); update(); } else { QDeclarativePixmap::Options options; @@ -246,8 +244,7 @@ void QDeclarativeImageBase::requestFinished() d->progress = 1.0; - setImplicitWidth(d->pix.width()); - setImplicitHeight(d->pix.height()); + pixmapChange(); if (d->sourcesize.width() != d->pix.width() || d->sourcesize.height() != d->pix.height()) emit sourceSizeChanged(); @@ -256,7 +253,7 @@ void QDeclarativeImageBase::requestFinished() emit statusChanged(d->status); if (d->progress != oldProgress) emit progressChanged(d->progress); - pixmapChange(); + update(); } @@ -279,6 +276,9 @@ void QDeclarativeImageBase::componentComplete() void QDeclarativeImageBase::pixmapChange() { + Q_D(QDeclarativeImageBase); + setImplicitWidth(d->pix.width()); + setImplicitHeight(d->pix.height()); } QT_END_NAMESPACE diff --git a/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml b/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml index 739299d..cd092bc 100644 --- a/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml +++ b/tests/auto/declarative/qdeclarativeimage/data/aspectratio.qml @@ -1,6 +1,10 @@ import QtQuick 1.0 Image { + property int widthChange: 0 + property int heightChange: 0 source: "heart.png" fillMode: Image.PreserveAspectFit; + onWidthChanged: widthChange += 1 + onHeightChanged: heightChange += 1 } diff --git a/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp b/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp index c5a3d14..87e3347 100644 --- a/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp +++ b/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp @@ -248,14 +248,22 @@ void tst_qdeclarativeimage::preserveAspectRatio() canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/aspectratio.qml")); QDeclarativeImage *image = qobject_cast(canvas->rootObject()); QVERIFY(image != 0); + QCOMPARE(image->property("widthChange").toInt(), 1); + QCOMPARE(image->property("heightChange").toInt(), 1); image->setWidth(80.0); + QCOMPARE(image->property("widthChange").toInt(), 2); + QCOMPARE(image->property("heightChange").toInt(), 2); QCOMPARE(image->width(), 80.); QCOMPARE(image->height(), 80.); canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/aspectratio.qml")); image = qobject_cast(canvas->rootObject()); - image->setHeight(60.0); QVERIFY(image != 0); + QCOMPARE(image->property("widthChange").toInt(), 1); + QCOMPARE(image->property("heightChange").toInt(), 1); + image->setHeight(60.0); + QCOMPARE(image->property("widthChange").toInt(), 2); + QCOMPARE(image->property("heightChange").toInt(), 2); QCOMPARE(image->height(), 60.); QCOMPARE(image->width(), 60.); delete canvas; -- cgit v0.12 From a30decc01878ab2435a9a48f4963c07b5e08c5b6 Mon Sep 17 00:00:00 2001 From: Adrian Constantin Date: Thu, 7 Apr 2011 10:39:49 +0300 Subject: Revert "Remove SIGBUS emission from QNetworkSession destruction." This reverts commit b40d04a19f4c186bf47aad128b0618c629629e07. Reviewed-by: Shane Kearns It is usual for QXyzPrivate implementation to assume that the parent QXyz is a valid object. Reviewed-by: Cristiano di Flora --- src/network/bearer/qnetworksession.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/bearer/qnetworksession.cpp b/src/network/bearer/qnetworksession.cpp index 2613bf8..9503553 100644 --- a/src/network/bearer/qnetworksession.cpp +++ b/src/network/bearer/qnetworksession.cpp @@ -260,7 +260,7 @@ QNetworkSession::QNetworkSession(const QNetworkConfiguration& connectionConfig, */ QNetworkSession::~QNetworkSession() { - d->deleteLater(); + delete d; } /*! -- cgit v0.12 From 3f7d1dd5f323970f1047ab058159ec48d94012f7 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Mon, 28 Mar 2011 17:53:45 +0100 Subject: Ensure shared network session deleted from correct thread Due to threaded http, the shared QNetworkSession can have its last reference removed from a http delegate thread. To avoid this deadlocking use a deleteLater custom deleter so that the QNS is deleted from the thread it has affinity for. Reviewed-by: Markus Goetz Task-Number: QTBUG-17464 --- src/network/bearer/qsharednetworksession.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/network/bearer/qsharednetworksession.cpp b/src/network/bearer/qsharednetworksession.cpp index 28ca173..fcb0128 100644 --- a/src/network/bearer/qsharednetworksession.cpp +++ b/src/network/bearer/qsharednetworksession.cpp @@ -59,6 +59,11 @@ inline QSharedNetworkSessionManager* sharedNetworkSessionManager() return rv; } +static void doDeleteLater(QObject* obj) +{ + obj->deleteLater(); +} + QSharedPointer QSharedNetworkSessionManager::getSession(QNetworkConfiguration config) { QSharedNetworkSessionManager *m(sharedNetworkSessionManager()); @@ -69,7 +74,7 @@ QSharedPointer QSharedNetworkSessionManager::getSession(QNetwor return p; } //otherwise make one - QSharedPointer session(new QNetworkSession(config)); + QSharedPointer session(new QNetworkSession(config), doDeleteLater); m->sessions[config] = session; return session; } -- cgit v0.12 From baaa5ae00038c34c8a7539229cda083a8afde280 Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen Date: Thu, 7 Apr 2011 11:38:46 +0200 Subject: Cocoa: p1 bug fix: fix auto test regressions Ref: 32228c4f2b3419a35d1623377050ef72edf73c92 It seems that the change above broke some auto tests, which revealed a true problem. When it comes to modal dialog, children still needs to be stacked on top of modal parents, as they the user cannot use the mouse to raise it. So rather than removing subWindowStacking fully, we narrow it even further down to only be used for children of modal dialogs. All in all, this is close to removing it, but still us it for certain corner cases. Task-number: QTBUG-11481 Reviewed-by: msorvig --- src/gui/kernel/qwidget_mac.mm | 15 ++++++++++----- tests/auto/macnativeevents/tst_macnativeevents.cpp | 8 +++++++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm index 0f4abfa..aca1d53 100644 --- a/src/gui/kernel/qwidget_mac.mm +++ b/src/gui/kernel/qwidget_mac.mm @@ -2795,11 +2795,9 @@ void QWidgetPrivate::transferChildren() void QWidgetPrivate::setSubWindowStacking(bool set) { // After hitting too many unforeseen bugs trying to put Qt on top of the cocoa child - // window API, we have decided to revert this behaviour. For compability - // reasons we leave the code here for applications depending on it: + // window API, we have decided to revert this behaviour as much as we can. We + // therefore now only allow child windows to exist for children of modal dialogs. static bool use_behaviour_qt473 = !qgetenv("QT_MAC_USE_CHILDWINDOWS").isEmpty(); - if (use_behaviour_qt473 == false) - return; // This will set/remove a visual relationship between parent and child on screen. // The reason for doing this is to ensure that a child always stacks infront of @@ -2829,7 +2827,10 @@ void QWidgetPrivate::setSubWindowStacking(bool set) if (NSWindow *pwin = [qt_mac_nativeview_for(parent) window]) { if (set) { Qt::WindowType ptype = parent->window()->windowType(); - if ([pwin isVisible] && (ptype == Qt::Window || ptype == Qt::Dialog) && ![qwin parentWindow]) { + if ([pwin isVisible] + && (ptype == Qt::Window || ptype == Qt::Dialog) + && ![qwin parentWindow] + && (!use_behaviour_qt473 && parent->windowModality() == Qt::ApplicationModal)) { NSInteger level = [qwin level]; [pwin addChildWindow:qwin ordered:NSWindowAbove]; if ([qwin level] < level) @@ -2841,6 +2842,10 @@ void QWidgetPrivate::setSubWindowStacking(bool set) } } + // Only set-up child windows for q if q is modal: + if (set && !use_behaviour_qt473 && q->windowModality() != Qt::ApplicationModal) + return; + QObjectList widgets = q->children(); for (int i=0; i(widgets.at(i)); diff --git a/tests/auto/macnativeevents/tst_macnativeevents.cpp b/tests/auto/macnativeevents/tst_macnativeevents.cpp index ac7298c..731fe0a 100644 --- a/tests/auto/macnativeevents/tst_macnativeevents.cpp +++ b/tests/auto/macnativeevents/tst_macnativeevents.cpp @@ -68,7 +68,7 @@ private slots: void testMouseEnter(); void testChildDialogInFrontOfModalParent(); #ifdef QT_MAC_USE_COCOA - void testChildWindowInFrontOfParentWindow(); +// void testChildWindowInFrontOfParentWindow(); // void testChildToolWindowInFrontOfChildNormalWindow(); void testChildWindowInFrontOfStaysOnTopParentWindow(); #endif @@ -314,6 +314,11 @@ void tst_MacNativeEvents::testChildDialogInFrontOfModalParent() } #ifdef QT_MAC_USE_COCOA +#if 0 +// This test is disabled as of Qt-4.7.4 because we cannot do it +// unless we use the Cocoa sub window API. But using that opens up +// a world of side effects that we cannot live with. So we rather +// not support child-on-top-of-parent instead. void tst_MacNativeEvents::testChildWindowInFrontOfParentWindow() { // Test that a child window always stacks in front of its parent window. @@ -338,6 +343,7 @@ void tst_MacNativeEvents::testChildWindowInFrontOfParentWindow() QTest::qWait(100); QVERIFY(!child.isVisible()); } +#endif /* This test can be enabled once setStackingOrder has been fixed in qwidget_mac.mm void tst_MacNativeEvents::testChildToolWindowInFrontOfChildNormalWindow() -- cgit v0.12