From 3799188e704e48ff8e9aa728cd76101b15f21fd2 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Mon, 7 Sep 2015 14:35:47 +0200 Subject: Menu: Schedule popup deletion when it's about to hide This leads to serious memory/OpenGL context leaks on Windows. No such thing has been noticed on Linux which may hint to differences in the backend. Task-number: QTBUG-47682 Change-Id: I274ed98db348ffe2c78707f2c92b812f272c2723 Reviewed-by: Mitch Curtis --- src/controls/MenuBar.qml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/controls/MenuBar.qml') diff --git a/src/controls/MenuBar.qml b/src/controls/MenuBar.qml index 0d9f8ee8..be182e29 100644 --- a/src/controls/MenuBar.qml +++ b/src/controls/MenuBar.qml @@ -302,7 +302,7 @@ MenuBarPrivate { anchors.fill: parent hoverEnabled: Settings.hoverEnabled - onPositionChanged: updateCurrentItem(mouse, false) + onPositionChanged: updateCurrentItem(mouse) onPressed: { if (updateCurrentItem(mouse)) { d.preselectMenuItem = false -- cgit v1.2.1 From 79092d8ab8f5c02c5d880d33f9869c034f0f933d Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Thu, 24 Sep 2015 16:08:34 +0200 Subject: MenuBar: Refactor logic for setting the current menu Among other things, we removed one connection object per menu delegate. We also made the logic more predictable and less dependent on property changes order. Task-number: QTBUG-48382 Change-Id: Ic97a7bf9c4ac8ff07f98bdd4a420e19bd228a6f1 Reviewed-by: J-P Nurmi --- src/controls/MenuBar.qml | 81 +++++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 36 deletions(-) (limited to 'src/controls/MenuBar.qml') diff --git a/src/controls/MenuBar.qml b/src/controls/MenuBar.qml index be182e29..eddd48cd 100644 --- a/src/controls/MenuBar.qml +++ b/src/controls/MenuBar.qml @@ -146,11 +146,41 @@ MenuBarPrivate { property bool altPressedAgain: false property var mnemonicsMap: ({}) + function openMenuAtIndex(index) { + if (openedMenuIndex === index) + return; + + var oldIndex = openedMenuIndex + openedMenuIndex = index + + if (oldIndex !== -1) { + var menu = root.menus[oldIndex] + if (menu.__popupVisible) { + menu.__dismissMenu() + menu.__destroyAllMenuPopups() + } + } + + if (openedMenuIndex !== -1) { + menu = root.menus[openedMenuIndex] + if (menu.enabled) { + if (menu.__usingDefaultStyle) + menu.style = d.style.menuStyle + + var xPos = row.LayoutMirroring.enabled ? menuItemLoader.width : 0 + menu.__popup(Qt.rect(xPos, menuBarLoader.height - d.heightPadding, 0, 0), 0) + + if (preselectMenuItem) + menu.__currentIndex = 0 + } + } + } + function dismissActiveFocus(event, reason) { if (reason) { altPressedAgain = false altPressed = false - openedMenuIndex = -1 + openMenuAtIndex(-1) root.__contentItem.parent.forceActiveFocus() } else { event.accepted = false @@ -160,7 +190,7 @@ MenuBarPrivate { function maybeOpenFirstMenu(event) { if (altPressed && openedMenuIndex === -1) { preselectMenuItem = true - openedMenuIndex = 0 + openMenuAtIndex(0) } else { event.accepted = false } @@ -197,7 +227,7 @@ MenuBarPrivate { idx-- if (idx >= 0) { d.preselectMenuItem = true - d.openedMenuIndex = idx + d.openMenuAtIndex(idx) } } else { event.accepted = false; @@ -211,7 +241,7 @@ MenuBarPrivate { idx++ if (idx < root.menus.length) { d.preselectMenuItem = true - d.openedMenuIndex = idx + d.openMenuAtIndex(idx) } } else { event.accepted = false; @@ -233,7 +263,7 @@ MenuBarPrivate { Accessible.role: Accessible.MenuItem Accessible.name: StyleHelpers.removeMnemonics(opts.text) - Accessible.onPressAction: d.openedMenuIndex = opts.index + Accessible.onPressAction: d.openMenuAtIndex(opts.index) property var styleData: QtObject { id: opts @@ -254,35 +284,18 @@ MenuBarPrivate { visible: __menuItem.visible Connections { - target: d - onOpenedMenuIndexChanged: { - if (!__menuItem.enabled) - return; + target: __menuItem + onAboutToHide: { if (d.openedMenuIndex === index) { - if (__menuItem.__usingDefaultStyle) - __menuItem.style = d.style.menuStyle - __menuItem.__popup(Qt.rect(row.LayoutMirroring.enabled ? menuItemLoader.width : 0, - menuBarLoader.height - d.heightPadding, 0, 0), 0) - if (d.preselectMenuItem) - __menuItem.__currentIndex = 0 - } else if (__menuItem.__popupVisible) { - __menuItem.__dismissMenu() - __menuItem.__destroyAllMenuPopups() + d.openMenuAtIndex(-1) + menuMouseArea.hoveredItem = null } } } - Connections { - target: __menuItem - onPopupVisibleChanged: { - if (!__menuItem.__popupVisible && d.openedMenuIndex === index) - d.openedMenuIndex = -1 - } - } - Connections { target: __menuItem.__action - onTriggered: d.openedMenuIndex = __menuItemIndex + onTriggered: d.openMenuAtIndex(__menuItemIndex) } Component.onCompleted: { @@ -303,26 +316,22 @@ MenuBarPrivate { hoverEnabled: Settings.hoverEnabled onPositionChanged: updateCurrentItem(mouse) - onPressed: { - if (updateCurrentItem(mouse)) { - d.preselectMenuItem = false - d.openedMenuIndex = currentItem.__menuItemIndex - } - } + onPressed: updateCurrentItem(mouse) onExited: hoveredItem = null property Item currentItem: null property Item hoveredItem: null function updateCurrentItem(mouse) { var pos = mapToItem(row, mouse.x, mouse.y) - if (!hoveredItem || !hoveredItem.contains(Qt.point(pos.x - currentItem.x, pos.y - currentItem.y))) { + if (pressed || !hoveredItem + || !hoveredItem.contains(Qt.point(pos.x - currentItem.x, pos.y - currentItem.y))) { hoveredItem = row.childAt(pos.x, pos.y) if (!hoveredItem) return false; currentItem = hoveredItem - if (d.openedMenuIndex !== -1) { + if (pressed || d.openedMenuIndex !== -1) { d.preselectMenuItem = false - d.openedMenuIndex = currentItem.__menuItemIndex + d.openMenuAtIndex(currentItem.__menuItemIndex) } } return true; -- cgit v1.2.1 From b27a01a86e614207025e569926d0c419857e8965 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Thu, 24 Sep 2015 19:56:23 +0200 Subject: Menus: Clean popup hide and destroy logic When a menu popup gets closed it usually needs to be destroyed right away since we don't recycle its contents. There is an exception, however, and it's when he user triggers a menu item. In this case, we need to proceed in three steps. First, we hide the menu popup, then we emit the triggered signal, and when that one returns, the menu contents can be disposed. If we did all in a single step, we may end up with a crash since we don't support deleting a QtQuick item while it's running a signal handler. Delayed deletions don't work either in the case when the triggered handler ends up running the event loop. Task-number: QTBUG-45182 Task-number: QTBUG-47682 Task-number: QTBUG-48382 Change-Id: Ic39717e09f38df602f641250cd81cf4931863db6 Reviewed-by: J-P Nurmi --- src/controls/MenuBar.qml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/controls/MenuBar.qml') diff --git a/src/controls/MenuBar.qml b/src/controls/MenuBar.qml index eddd48cd..444185cc 100644 --- a/src/controls/MenuBar.qml +++ b/src/controls/MenuBar.qml @@ -155,10 +155,8 @@ MenuBarPrivate { if (oldIndex !== -1) { var menu = root.menus[oldIndex] - if (menu.__popupVisible) { - menu.__dismissMenu() - menu.__destroyAllMenuPopups() - } + if (menu.__popupVisible) + menu.__dismissAndDestroy() } if (openedMenuIndex !== -1) { -- cgit v1.2.1