From 99c70de9a6943ed3f0a81be7b2a395f24ab567ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Sat, 4 Nov 2023 11:17:30 +0100 Subject: [PATCH] Add dedicated child and parent events for QWindow reparenting When a QWindow is parented into another window, either via its constructor, or via setParent(), we also update the QObject parent of the window. This in turn sends ChildAdded and ChildRemoved events to the new and old parent of the window, as well as ParentAboutToChange and ParentChange to the window itself. But at the point when these events are sent, the QWindow parent relationship has not been fully established. We have not updated d->parentWindow, nor have we propagated the parent relationship to the platform window. This is problematic because clients can not use these events to react to window parent changes synchronously. For example. trying to raise a child window when added to a parent is not going to work, because at that point the child window isn't a native child of the parent. Instead of re-using the QObject events for QWindow, like QWidget does, by delaying the events or sending them manually at a point when the window parent relationship has been fully formed, we opt to add new dedicated events. Change-Id: I019c14eba444861f537e4f68ab3a82297f843cf7 Reviewed-by: Volker Hilsheimer Reviewed-by: Qt CI Bot --- src/corelib/kernel/qcoreevent.cpp | 4 + src/corelib/kernel/qcoreevent.h | 5 + src/gui/kernel/qevent.cpp | 40 +++++ src/gui/kernel/qevent.h | 11 ++ src/gui/kernel/qwindow.cpp | 34 +++- src/gui/kernel/qwindow_p.h | 2 +- tests/auto/gui/kernel/qwindow/tst_qwindow.cpp | 162 ++++++++++++++++++ 7 files changed, 253 insertions(+), 5 deletions(-) diff --git a/src/corelib/kernel/qcoreevent.cpp b/src/corelib/kernel/qcoreevent.cpp index 65dd8d99247..811a3f75992 100644 --- a/src/corelib/kernel/qcoreevent.cpp +++ b/src/corelib/kernel/qcoreevent.cpp @@ -79,6 +79,8 @@ Q_TRACE_POINT(qtcore, QEvent_dtor, QEvent *event, QEvent::Type type); \value ChildAdded An object gets a child (QChildEvent). \value ChildPolished A widget child gets polished (QChildEvent). \value ChildRemoved An object loses a child (QChildEvent). + \value [since 6.7] ChildWindowAdded A child window was added to the window. + \value [since 6.7] ChildWindowRemoved A child window was removed from the window. \value Clipboard The clipboard contents have changed. \value Close Widget was closed (QCloseEvent). \value CloseSoftwareInputPanel A widget wants to close the software input panel (SIP). @@ -164,6 +166,8 @@ Q_TRACE_POINT(qtcore, QEvent_dtor, QEvent *event, QEvent::Type type); Only sent to some object types, such as QWidget. \value ParentChange The object parent has changed. Only sent to some object types, such as QWidget. + \value [since 6.7] ParentWindowAboutToChange The parent window is about to change. + \value [since 6.7] ParentWindowChange The parent window has changed. \value PlatformPanel A platform specific panel has been requested. \value PlatformSurface A native platform surface has been created or is about to be destroyed (QPlatformSurfaceEvent). \omitvalue Pointer diff --git a/src/corelib/kernel/qcoreevent.h b/src/corelib/kernel/qcoreevent.h index e8e4b4bba6e..5c520471222 100644 --- a/src/corelib/kernel/qcoreevent.h +++ b/src/corelib/kernel/qcoreevent.h @@ -286,6 +286,11 @@ public: DevicePixelRatioChange = 222, + ChildWindowAdded = 223, + ChildWindowRemoved = 224, + ParentWindowAboutToChange = 225, + ParentWindowChange = 226, + // 512 reserved for Qt Jambi's MetaCall event // 513 reserved for Qt Jambi's DeleteOnMainThread event diff --git a/src/gui/kernel/qevent.cpp b/src/gui/kernel/qevent.cpp index b5727956ae8..48a4c42151b 100644 --- a/src/gui/kernel/qevent.cpp +++ b/src/gui/kernel/qevent.cpp @@ -4759,6 +4759,46 @@ Q_IMPL_EVENT_COMMON(QApplicationStateChangeEvent) Returns the state of the application. */ +/*! + \class QChildWindowEvent + \inmodule QtGui + \since 6.7 + \brief The QChildWindowEvent class contains event parameters for + child window changes. + + \ingroup events + + Child window events are sent to windows when children are + added or removed. + + In both cases you can only rely on the child being a QWindow + — not any subclass thereof. This is because in the + QEvent::ChildWindowAdded case the subclass is not yet fully + constructed, and in the QEvent::ChildWindowRemoved case it + might have already been destructed. +*/ + +/*! + Constructs a child window event object of a particular \a type + for the \a childWindow. + + \a type can be QEvent::ChildWindowAdded or QEvent::ChildWindowRemoved. + + \sa child() +*/ +QChildWindowEvent::QChildWindowEvent(Type type, QWindow *childWindow) + : QEvent(type), c(childWindow) +{ +} + +Q_IMPL_EVENT_COMMON(QChildWindowEvent) + +/*! + \fn QWindow *QChildWindowEvent::child() const + + Returns the child window that was added or removed. +*/ + QMutableTouchEvent::~QMutableTouchEvent() = default; diff --git a/src/gui/kernel/qevent.h b/src/gui/kernel/qevent.h index bf77cc3a734..f42fb449be6 100644 --- a/src/gui/kernel/qevent.h +++ b/src/gui/kernel/qevent.h @@ -1020,6 +1020,17 @@ private: Qt::ApplicationState m_applicationState; }; +class Q_GUI_EXPORT QChildWindowEvent : public QEvent +{ + Q_DECL_EVENT_COMMON(QChildWindowEvent) +public: + QChildWindowEvent(Type type, QWindow *childWindow); + QWindow *child() const { return c; } + +private: + QWindow *c; +}; + QT_END_NAMESPACE #endif // QEVENT_H diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp index ca3e5f274da..b514795da7f 100644 --- a/src/gui/kernel/qwindow.cpp +++ b/src/gui/kernel/qwindow.cpp @@ -128,7 +128,7 @@ QWindow::QWindow(QScreen *targetScreen) , QSurface(QSurface::Window) { Q_D(QWindow); - d->init(targetScreen); + d->init(nullptr, targetScreen); } static QWindow *nonDesktopParent(QWindow *parent) @@ -169,11 +169,11 @@ QWindow::QWindow(QWindow *parent) \sa setParent() */ QWindow::QWindow(QWindowPrivate &dd, QWindow *parent) - : QObject(dd, nonDesktopParent(parent)) + : QObject(dd, nullptr) , QSurface(QSurface::Window) { Q_D(QWindow); - d->init(); + d->init(nonDesktopParent(parent)); } /*! @@ -183,6 +183,8 @@ QWindow::~QWindow() { Q_D(QWindow); d->destroy(); + // Decouple from parent before window goes under + setParent(nullptr); QGuiApplicationPrivate::window_list.removeAll(this); if (!QGuiApplicationPrivate::is_app_closing) QGuiApplicationPrivate::instance()->modalWindowList.removeOne(this); @@ -206,10 +208,12 @@ QWindowPrivate::QWindowPrivate() QWindowPrivate::~QWindowPrivate() = default; -void QWindowPrivate::init(QScreen *targetScreen) +void QWindowPrivate::init(QWindow *parent, QScreen *targetScreen) { Q_Q(QWindow); + q->QObject::setParent(parent); + isWindow = true; parentWindow = static_cast(q->QObject::parent()); @@ -242,6 +246,11 @@ void QWindowPrivate::init(QScreen *targetScreen) #endif updateDevicePixelRatio(); }); + + if (parentWindow) { + QChildWindowEvent childAddedEvent(QEvent::ChildWindowAdded, q); + QCoreApplication::sendEvent(parentWindow, &childAddedEvent); + } } /*! @@ -771,6 +780,10 @@ void QWindow::setParent(QWindow *parent) return; } + QEvent parentAboutToChangeEvent(QEvent::ParentWindowAboutToChange); + QCoreApplication::sendEvent(this, &parentAboutToChangeEvent); + + const auto previousParent = d->parentWindow; QObject::setParent(parent); d->parentWindow = parent; @@ -793,6 +806,19 @@ void QWindow::setParent(QWindow *parent) } QGuiApplicationPrivate::updateBlockedStatus(this); + + if (previousParent) { + QChildWindowEvent childRemovedEvent(QEvent::ChildWindowRemoved, this); + QCoreApplication::sendEvent(previousParent, &childRemovedEvent); + } + + if (parent) { + QChildWindowEvent childAddedEvent(QEvent::ChildWindowAdded, this); + QCoreApplication::sendEvent(parent, &childAddedEvent); + } + + QEvent parentChangedEvent(QEvent::ParentWindowChange); + QCoreApplication::sendEvent(this, &parentChangedEvent); } /*! diff --git a/src/gui/kernel/qwindow_p.h b/src/gui/kernel/qwindow_p.h index 5de3546392b..cc1c120f6bd 100644 --- a/src/gui/kernel/qwindow_p.h +++ b/src/gui/kernel/qwindow_p.h @@ -44,7 +44,7 @@ public: QWindowPrivate(); ~QWindowPrivate() override; - void init(QScreen *targetScreen = nullptr); + void init(QWindow *parent, QScreen *targetScreen = nullptr); #ifndef QT_NO_CURSOR void setCursor(const QCursor *c = nullptr); diff --git a/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp b/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp index b0571714b70..a6642d8d224 100644 --- a/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp +++ b/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -101,6 +102,8 @@ private slots: void enterLeaveOnWindowShowHide(); #endif void windowExposedAfterReparent(); + void childEvents(); + void parentEvents(); private: QPoint m_availableTopLeft; @@ -3058,6 +3061,165 @@ void tst_QWindow::windowExposedAfterReparent() QVERIFY(QTest::qWaitForWindowExposed(&child)); } +struct ParentWindow : public QWindow +{ + bool event(QEvent *event) override + { + [&]() -> void { + if (event->type() == QEvent::ChildWindowAdded + || event->type() == QEvent::ChildWindowRemoved) { + // We should not receive child events after the window has been destructed + QVERIFY(this->isWindowType()); + + auto *parentWindow = this; + auto *childEvent = static_cast(event); + auto *childWindow = childEvent->child(); + + if (event->type() == QEvent::ChildWindowAdded) { + QVERIFY(childWindow->parent()); + QVERIFY(parentWindow->isAncestorOf(childWindow)); + if (childWindow->handle()) + QVERIFY(childWindow->handle()->parent() == parentWindow->handle()); + + } else { + QVERIFY(!childWindow->parent()); + QVERIFY(!parentWindow->isAncestorOf(childWindow)); + if (childWindow->handle()) + QVERIFY(childWindow->handle()->parent() != parentWindow->handle()); + } + } + }(); + + return QWindow::event(event); + } +}; + +void tst_QWindow::childEvents() +{ + ParentWindow parent; + + { + // ChildAdded via constructor + QWindow constructorChild(&parent); + if (QTest::currentTestFailed()) return; + // ChildRemoved via destructor + } + + if (QTest::currentTestFailed()) return; + + // ChildAdded and ChildRemoved via setParent + QWindow child; + child.setParent(&parent); + if (QTest::currentTestFailed()) return; + child.setParent(nullptr); + if (QTest::currentTestFailed()) return; + + parent.create(); + child.create(); + + // ChildAdded and ChildRemoved after creation + child.setParent(&parent); + if (QTest::currentTestFailed()) return; + child.setParent(nullptr); + if (QTest::currentTestFailed()) return; +} + +struct ChildWindowPrivate; +struct ChildWindow : public QWindow +{ + ChildWindow(QWindow *parent = nullptr); +}; + +struct ChildWindowPrivate : public QWindowPrivate +{ + ChildWindowPrivate() : QWindowPrivate() + { + receiveParentEvents = true; + } +}; + +ChildWindow::ChildWindow(QWindow *parent) + : QWindow(*new ChildWindowPrivate, parent) +{} + +struct ParentEventTester : public QObject +{ + bool eventFilter(QObject *object, QEvent *event) override + { + [&]() -> void { + if (event->type() == QEvent::ParentWindowAboutToChange + || event->type() == QEvent::ParentWindowChange) { + // We should not receive parent events after the window has been destructed + QVERIFY(object->isWindowType()); + auto *window = static_cast(object); + + if (event->type() == QEvent::ParentWindowAboutToChange) { + QVERIFY(window->parent() != nextExpectedParent); + if (window->handle()) { + QVERIFY(window->handle()->parent() != + (nextExpectedParent ? nextExpectedParent->handle() : nullptr)); + } + } else { + QVERIFY(window->parent() == nextExpectedParent); + if (window->handle()) { + QVERIFY(window->handle()->parent() == + (nextExpectedParent ? nextExpectedParent->handle() : nullptr)); + } + } + } + }(); + + return QObject::eventFilter(object, event); + } + + QWindow *nextExpectedParent = nullptr; +}; + + + +void tst_QWindow::parentEvents() +{ + QWindow parent; + + { + ParentEventTester tester; + + { + // We can't hook in early enough to get the parent change during + // QObject construction. + ChildWindow child(&parent); + + // But we can observe the one during destruction + child.installEventFilter(&tester); + tester.nextExpectedParent = nullptr; + } + } + if (QTest::currentTestFailed()) return; + + ParentEventTester tester; + ChildWindow child; + child.installEventFilter(&tester); + + tester.nextExpectedParent = &parent; + child.setParent(&parent); + if (QTest::currentTestFailed()) return; + + tester.nextExpectedParent = nullptr; + child.setParent(nullptr); + if (QTest::currentTestFailed()) return; + + parent.create(); + child.create(); + + tester.nextExpectedParent = &parent; + child.setParent(&parent); + if (QTest::currentTestFailed()) return; + + tester.nextExpectedParent = nullptr; + child.setParent(nullptr); + if (QTest::currentTestFailed()) return; +} + #include QTEST_MAIN(tst_QWindow)