From cc1b6475a99994908cbd6309a08fd08614f18221 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 10 Aug 2010 16:30:18 +0100 Subject: Test backing store is deleted after: partial reveal, full reveal, hide After the following sequence: * widget starts hidden * Partially reveal widget * Fully reveal widget * Hide widget widget's backing store should be deleted, when running on Symbian. Task-number: QTBUG-12800 Reviewed-by: Jason Barron --- tests/auto/qwidget/tst_qwidget.cpp | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/auto/qwidget/tst_qwidget.cpp b/tests/auto/qwidget/tst_qwidget.cpp index f722f89..e6e8970 100644 --- a/tests/auto/qwidget/tst_qwidget.cpp +++ b/tests/auto/qwidget/tst_qwidget.cpp @@ -9758,6 +9758,43 @@ void tst_QWidget::destroyBackingStoreWhenHidden() QVERIFY(0 != backingStore(parent)); QVERIFY(0 == backingStore(child)); } + + // 6. Partial reveal followed by full reveal + { + QWidget upper; + upper.setAutoFillBackground(true); + upper.setPalette(Qt::red); + upper.setGeometry(50, 50, 100, 100); + + QWidget lower; + lower.setAutoFillBackground(true); + lower.setPalette(Qt::green); + lower.setGeometry(50, 50, 100, 100); + + lower.show(); + QTest::qWaitForWindowShown(&lower); + upper.show(); + QTest::qWaitForWindowShown(&upper); + upper.raise(); + + QVERIFY(0 != backingStore(upper)); + QVERIFY(0 == backingStore(lower)); + + // Check that upper obscures lower + QVERIFY(lower.visibleRegion().subtracted(upper.visibleRegion()).isEmpty()); + + // Partially reveal lower + upper.move(100, 100); + + // Completely reveal lower + upper.hide(); + + // Hide lower widget - this should cause its backing store to be deleted + lower.hide(); + + // Check that backing store was deleted + WAIT_AND_VERIFY(0 == backingStore(lower)); + } } #undef WAIT_AND_VERIFY -- cgit v0.12 From b5360eb223d5377beb62008fcc1da48f432dc8dd Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 10 Aug 2010 17:08:37 +0100 Subject: Test backing store is deleted after reparenting a visible native child widget After the following sequence: * Create a TLW, parent1 * Create a child of parent1, and make it a native widget * Create another TLW, parent2 * Show parent1 and parent2 * Reparent child so its parent is now parent2 * Hide parent1 parent1's backing store should be deleted, when running on Symbian. Task-number: QTBUG-12817 Reviewed-by: Jason Barron --- tests/auto/qwidget/tst_qwidget.cpp | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/auto/qwidget/tst_qwidget.cpp b/tests/auto/qwidget/tst_qwidget.cpp index e6e8970..ba0be3e 100644 --- a/tests/auto/qwidget/tst_qwidget.cpp +++ b/tests/auto/qwidget/tst_qwidget.cpp @@ -9795,6 +9795,43 @@ void tst_QWidget::destroyBackingStoreWhenHidden() // Check that backing store was deleted WAIT_AND_VERIFY(0 == backingStore(lower)); } + + // 7. Reparenting of visible native child widget + { + QWidget parent1; + parent1.setAutoFillBackground(true); + parent1.setPalette(Qt::green); + parent1.setGeometry(50, 50, 100, 100); + + QWidget *child = new QWidget(&parent1); + child->winId(); + child->setAutoFillBackground(true); + child->setPalette(Qt::red); + child->setGeometry(10, 10, 30, 30); + + QWidget parent2; + parent2.setAutoFillBackground(true); + parent2.setPalette(Qt::blue); + parent2.setGeometry(150, 150, 100, 100); + + parent1.show(); + QTest::qWaitForWindowShown(&parent1); + QVERIFY(0 != backingStore(parent1)); + + parent2.show(); + QTest::qWaitForWindowShown(&parent2); + QVERIFY(0 != backingStore(parent2)); + + child->setParent(&parent2); + child->setGeometry(10, 10, 30, 30); + child->show(); + + parent1.hide(); + WAIT_AND_VERIFY(0 == backingStore(parent1)); + + parent2.hide(); + WAIT_AND_VERIFY(0 == backingStore(parent2)); + } } #undef WAIT_AND_VERIFY -- cgit v0.12 From cb8d2dcc70846ccd9384a8b94b2a80821c0eb285 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 10 Aug 2010 16:34:19 +0100 Subject: Replaced backing store reference count with list of visible widgets Previously, the following sequence: 1. Widget is hidden 2. Widget partially revealed 3. Widget fully revealed resulted in the reference count of the backing store owned by the widget's window() being incremented twice. This patch replaces the simple reference count with a QSet which stores pointers to the native widgets which are descendents of the backing store owner, and which are currently visible. The sequence above therefore results in just a single insertion at step (2), with step (3) having no effect on the backing store. The QRefCountedWidgetBackingStore class has been renamed QWidgetBackingStoreTracker to better reflect its purpose. Task-number: QTBUG-12800 Task-number: QTBUG-12817 Reviewed-by: Jason Barron --- src/gui/kernel/qapplication_s60.cpp | 11 +++---- src/gui/kernel/qwidget.cpp | 61 +++++++++++++++++++++++++++---------- src/gui/kernel/qwidget_p.h | 17 ++++++----- 3 files changed, 58 insertions(+), 31 deletions(-) diff --git a/src/gui/kernel/qapplication_s60.cpp b/src/gui/kernel/qapplication_s60.cpp index 559bb6a..df93bc5 100644 --- a/src/gui/kernel/qapplication_s60.cpp +++ b/src/gui/kernel/qapplication_s60.cpp @@ -1926,27 +1926,24 @@ int QApplicationPrivate::symbianProcessWsEvent(const QSymbianEvent *symbianEvent QWidget *const window = w->window(); if (!window->d_func()->maybeTopData()) break; - QRefCountedWidgetBackingStore &backingStore = window->d_func()->maybeTopData()->backingStore; + QWidgetBackingStoreTracker &backingStore = window->d_func()->maybeTopData()->backingStore; if (visChangedEvent->iFlags & TWsVisibilityChangedEvent::ENotVisible) { #ifdef SYMBIAN_GRAPHICS_WSERV_QT_EFFECTS S60->wsSession().SendEffectCommand(ETfxCmdDeallocateLayer); #endif - // Decrement backing store reference count - backingStore.deref(); + backingStore.unregisterWidget(w); // In order to ensure that any resources used by the window surface // are immediately freed, we flush the WSERV command buffer. S60->wsSession().Flush(); } else if (visChangedEvent->iFlags & TWsVisibilityChangedEvent::EPartiallyVisible) { if (backingStore.data()) { - // Increment backing store reference count - backingStore.ref(); + backingStore.registerWidget(w); } else { #ifdef SYMBIAN_GRAPHICS_WSERV_QT_EFFECTS S60->wsSession().SendEffectCommand(ETfxCmdRestoreLayer); #endif - // Create backing store with an initial reference count of 1 backingStore.create(window); - backingStore.ref(); + backingStore.registerWidget(w); w->d_func()->invalidateBuffer(w->rect()); w->repaint(); } diff --git a/src/gui/kernel/qwidget.cpp b/src/gui/kernel/qwidget.cpp index aaa29a1..dad4848 100644 --- a/src/gui/kernel/qwidget.cpp +++ b/src/gui/kernel/qwidget.cpp @@ -162,47 +162,76 @@ static inline bool hasBackingStoreSupport() extern bool qt_sendSpontaneousEvent(QObject*, QEvent*); // qapplication.cpp extern QDesktopWidget *qt_desktopWidget; // qapplication.cpp +/*! + \internal + \class QWidgetBackingStoreTracker + \brief Class which allows tracking of which widgets are using a given backing store -QRefCountedWidgetBackingStore::QRefCountedWidgetBackingStore() + QWidgetBackingStoreTracker is a thin wrapper around a QWidgetBackingStore pointer, + which maintains a list of the QWidgets which are currently using the backing + store. This list is modified via the registerWidget and unregisterWidget functions. + */ + +QWidgetBackingStoreTracker::QWidgetBackingStoreTracker() : m_ptr(0) - , m_count(0) { } -QRefCountedWidgetBackingStore::~QRefCountedWidgetBackingStore() +QWidgetBackingStoreTracker::~QWidgetBackingStoreTracker() { delete m_ptr; } -void QRefCountedWidgetBackingStore::create(QWidget *widget) +/*! + \internal + Destroy the contained QWidgetBackingStore, if not null, and clear the list of + widgets using the backing store, then create a new QWidgetBackingStore, providing + the QWidget. + */ +void QWidgetBackingStoreTracker::create(QWidget *widget) { destroy(); m_ptr = new QWidgetBackingStore(widget); - m_count = 0; } -void QRefCountedWidgetBackingStore::destroy() +/*! + \internal + Destroy the contained QWidgetBackingStore, if not null, and clear the list of + widgets using the backing store. + */ +void QWidgetBackingStoreTracker::destroy() { delete m_ptr; m_ptr = 0; - m_count = 0; + m_widgets.clear(); } -void QRefCountedWidgetBackingStore::ref() +/*! + \internal + Add the widget to the list of widgets currently using the backing store. + If the widget was already in the list, this function is a no-op. + */ +void QWidgetBackingStoreTracker::registerWidget(QWidget *w) { Q_ASSERT(m_ptr); - ++m_count; + Q_ASSERT(w->internalWinId()); + Q_ASSERT(qt_widget_private(w)->maybeBackingStore() == m_ptr); + m_widgets.insert(w); } -void QRefCountedWidgetBackingStore::deref() +/*! + \internal + Remove the widget from the list of widgets currently using the backing store. + If the widget was in the list, and removing it causes the list to be empty, + the backing store is deleted. + If the widget was not in the list, this function is a no-op. + */ +void QWidgetBackingStoreTracker::unregisterWidget(QWidget *w) { - if (m_count) { - Q_ASSERT(m_ptr); - if (0 == --m_count) { - delete m_ptr; - m_ptr = 0; - } + if (m_widgets.remove(w) && m_widgets.isEmpty()) { + delete m_ptr; + m_ptr = 0; } } diff --git a/src/gui/kernel/qwidget_p.h b/src/gui/kernel/qwidget_p.h index 587d7fb..4a79dc7 100644 --- a/src/gui/kernel/qwidget_p.h +++ b/src/gui/kernel/qwidget_p.h @@ -110,17 +110,18 @@ class QWidgetItemV2; class QStyle; -class Q_AUTOTEST_EXPORT QRefCountedWidgetBackingStore +class Q_AUTOTEST_EXPORT QWidgetBackingStoreTracker { + public: - QRefCountedWidgetBackingStore(); - ~QRefCountedWidgetBackingStore(); + QWidgetBackingStoreTracker(); + ~QWidgetBackingStoreTracker(); void create(QWidget *tlw); void destroy(); - void ref(); - void deref(); + void registerWidget(QWidget *w); + void unregisterWidget(QWidget *w); inline QWidgetBackingStore* data() { @@ -143,11 +144,11 @@ public: } private: - Q_DISABLE_COPY(QRefCountedWidgetBackingStore) + Q_DISABLE_COPY(QWidgetBackingStoreTracker) private: QWidgetBackingStore* m_ptr; - int m_count; + QSet m_widgets; }; struct QTLWExtra { @@ -156,7 +157,7 @@ struct QTLWExtra { // Regular pointers (keep them together to avoid gaps on 64 bits architectures). QIcon *icon; // widget icon QPixmap *iconPixmap; - QRefCountedWidgetBackingStore backingStore; + QWidgetBackingStoreTracker backingStore; QWindowSurface *windowSurface; QPainter *sharedPainter; -- cgit v0.12 From ea85300f6456ef1bd9702296f3068edf0795736e Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 17 Aug 2010 16:05:01 +0100 Subject: Refactored handling of window visibility events on Symbian Task-number: QTBUG-12817 Reviewed-by: Jason Barron --- src/gui/kernel/qapplication_s60.cpp | 62 +++++++++++++++++++++---------------- src/gui/kernel/qt_s60_p.h | 1 + 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/gui/kernel/qapplication_s60.cpp b/src/gui/kernel/qapplication_s60.cpp index df93bc5..670bf34 100644 --- a/src/gui/kernel/qapplication_s60.cpp +++ b/src/gui/kernel/qapplication_s60.cpp @@ -135,6 +135,38 @@ void QS60Data::setStatusPaneAndButtonGroupVisibility(bool statusPaneVisible, boo } #endif +void QS60Data::controlVisibilityChanged(CCoeControl *control, bool visible) +{ + if (QWidgetPrivate::mapper && QWidgetPrivate::mapper->contains(control)) { + QWidget *const widget = QWidgetPrivate::mapper->value(control); + QWidget *const window = widget->window(); + if (QTLWExtra *topData = qt_widget_private(window)->maybeTopData()) { + QWidgetBackingStoreTracker &backingStore = topData->backingStore; + if (visible) { + if (backingStore.data()) { + backingStore.registerWidget(widget); + } else { +#ifdef SYMBIAN_GRAPHICS_WSERV_QT_EFFECTS + S60->wsSession().SendEffectCommand(ETfxCmdRestoreLayer); +#endif + backingStore.create(window); + backingStore.registerWidget(widget); + qt_widget_private(widget)->invalidateBuffer(widget->rect()); + widget->repaint(); + } + } else { +#ifdef SYMBIAN_GRAPHICS_WSERV_QT_EFFECTS + S60->wsSession().SendEffectCommand(ETfxCmdDeallocateLayer); +#endif + backingStore.unregisterWidget(widget); + // In order to ensure that any resources used by the window surface + // are immediately freed, we flush the WSERV command buffer. + S60->wsSession().Flush(); + } + } + } +} + bool qt_nograb() // application no-grab option { #if defined(QT_DEBUG) @@ -1922,32 +1954,10 @@ int QApplicationPrivate::symbianProcessWsEvent(const QSymbianEvent *symbianEvent if (callSymbianEventFilters(symbianEvent)) return 1; const TWsVisibilityChangedEvent *visChangedEvent = event->VisibilityChanged(); - QWidget *w = QWidgetPrivate::mapper->value(control); - QWidget *const window = w->window(); - if (!window->d_func()->maybeTopData()) - break; - QWidgetBackingStoreTracker &backingStore = window->d_func()->maybeTopData()->backingStore; - if (visChangedEvent->iFlags & TWsVisibilityChangedEvent::ENotVisible) { -#ifdef SYMBIAN_GRAPHICS_WSERV_QT_EFFECTS - S60->wsSession().SendEffectCommand(ETfxCmdDeallocateLayer); -#endif - backingStore.unregisterWidget(w); - // In order to ensure that any resources used by the window surface - // are immediately freed, we flush the WSERV command buffer. - S60->wsSession().Flush(); - } else if (visChangedEvent->iFlags & TWsVisibilityChangedEvent::EPartiallyVisible) { - if (backingStore.data()) { - backingStore.registerWidget(w); - } else { -#ifdef SYMBIAN_GRAPHICS_WSERV_QT_EFFECTS - S60->wsSession().SendEffectCommand(ETfxCmdRestoreLayer); -#endif - backingStore.create(window); - backingStore.registerWidget(w); - w->d_func()->invalidateBuffer(w->rect()); - w->repaint(); - } - } + if (visChangedEvent->iFlags & TWsVisibilityChangedEvent::ENotVisible) + S60->controlVisibilityChanged(control, false); + else if (visChangedEvent->iFlags & TWsVisibilityChangedEvent::EPartiallyVisible) + S60->controlVisibilityChanged(control, true); return 1; } break; diff --git a/src/gui/kernel/qt_s60_p.h b/src/gui/kernel/qt_s60_p.h index a18ea07..ad6a99a 100644 --- a/src/gui/kernel/qt_s60_p.h +++ b/src/gui/kernel/qt_s60_p.h @@ -164,6 +164,7 @@ public: static inline CEikButtonGroupContainer* buttonGroupContainer(); static void setStatusPaneAndButtonGroupVisibility(bool statusPaneVisible, bool buttonGroupVisible); #endif + static void controlVisibilityChanged(CCoeControl *control, bool visible); #ifdef Q_OS_SYMBIAN TTrapHandler *s60InstalledTrapHandler; -- cgit v0.12 From bc1c472967c78579178cf7e98ab999842fee3102 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 11 Aug 2010 12:58:29 +0100 Subject: Ensure native window is considered invisible when it gets destroyed The lifetime of the backing store depends, on Symbian, on the visibility of the native windows which are rendering from it. Specifically, once all such windows (i.e. the TLW window and those of any native widget descendents) have become invisible, the backing store is deleted. During re-parenting of a visible native child widget from parent1 to parent2, the following events occur: 1. QWidget::hide() is called on the child 2. The child widget's native control is deleted 3. A new native control is created, with its parent set as parent2->window()->effectiveWinId() Because there is no yield to the event loop between (1) and (2), the application does not receive a 'window hidden' notification as a result of the call to hide(). This means that the child widget is not removed from the list of visible widgets held in the backing store of parent1->window(). This patch ensures that the child is removed from this list during between steps (1) and (2). Task-number: QTBUG-12406 Task-number: QTBUG-12817 Reviewed-by: Jason Barron --- src/gui/kernel/qwidget_s60.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/gui/kernel/qwidget_s60.cpp b/src/gui/kernel/qwidget_s60.cpp index 56349ad..319f330 100644 --- a/src/gui/kernel/qwidget_s60.cpp +++ b/src/gui/kernel/qwidget_s60.cpp @@ -684,6 +684,12 @@ void QWidgetPrivate::setParent_sys(QWidget *parent, Qt::WindowFlags f) QSymbianControl *old_winid = static_cast(wasCreated ? data.winid : 0); if ((q->windowType() == Qt::Desktop)) old_winid = 0; + + // old_winid may not have received a 'not visible' visibility + // changed event before being destroyed; make sure that it is + // removed from the backing store's list of visible windows. + S60->controlVisibilityChanged(old_winid, false); + setWinId(0); // hide and reparent our own window away. Otherwise we might get -- cgit v0.12 From 0129e6e48613378a1a5548b2744ec7f9ae0d9961 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 17 Aug 2010 16:08:02 +0100 Subject: Removed QEXPECT_FAIL macros for test case which now passes Task-number: QTBUG-12406 Reviewed-by: Jason Barron --- tests/auto/qwidget/tst_qwidget.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/auto/qwidget/tst_qwidget.cpp b/tests/auto/qwidget/tst_qwidget.cpp index ba0be3e..ef05b91 100644 --- a/tests/auto/qwidget/tst_qwidget.cpp +++ b/tests/auto/qwidget/tst_qwidget.cpp @@ -9742,7 +9742,6 @@ void tst_QWidget::destroyBackingStoreWhenHidden() QVERIFY(0 != backingStore(child)); // Parent is obscured, therefore its backing store should be destroyed - QEXPECT_FAIL("", "QTBUG-12406", Continue); QVERIFY(0 == backingStore(parent)); // Disable full screen -- cgit v0.12