summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGabriel de Dietrich <gabriel.dedietrich@qt.io>2017-08-09 11:10:40 +0700
committerGabriel de Dietrich <gabriel.dedietrich@qt.io>2017-08-15 01:21:37 +0000
commitf27d1ccbb24ec2fd4098f2976503478831006cc8 (patch)
treed9eb9dbc784529ba46f91a7d51cb94805cdfc347
parentc161c6db780b308a604875c3e0f7affb09e89fce (diff)
QCocoaMenu: De-pessimize the number of calls to validateMenuItem:
Calling -[NSMenu update] every time we add a new item can result in a quadratic behavior since the function itself will iterate over all the items in the menu. We solve this by using a 0-timer which will trigger the call to update the next time the event loop spins. Menurama manual test updated. Change-Id: Ic155d364515cc93eb81b1c8085c8e44c93799954 Task-number: QTBUG-62396 Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
-rw-r--r--src/plugins/platforms/cocoa/qcocoamenu.h4
-rw-r--r--src/plugins/platforms/cocoa/qcocoamenu.mm19
-rw-r--r--tests/manual/cocoa/menurama/mainwindow.cpp13
-rw-r--r--tests/manual/cocoa/menurama/mainwindow.ui92
4 files changed, 90 insertions, 38 deletions
diff --git a/src/plugins/platforms/cocoa/qcocoamenu.h b/src/plugins/platforms/cocoa/qcocoamenu.h
index b77071536b..06688dbf3d 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.h
+++ b/src/plugins/platforms/cocoa/qcocoamenu.h
@@ -96,14 +96,18 @@ public:
bool isOpen() const;
void setIsOpen(bool isOpen);
+ void timerEvent(QTimerEvent *e) Q_DECL_OVERRIDE;
+
private:
QCocoaMenuItem *itemOrNull(int index) const;
void insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem);
+ void scheduleUpdate();
QList<QCocoaMenuItem *> m_menuItems;
NSMenu *m_nativeMenu;
NSMenuItem *m_attachedItem;
quintptr m_tag;
+ int m_updateTimer;
bool m_enabled:1;
bool m_parentEnabled:1;
bool m_visible:1;
diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index 8e47974d12..e5681c0894 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -260,6 +260,7 @@ QT_BEGIN_NAMESPACE
QCocoaMenu::QCocoaMenu() :
m_attachedItem(0),
m_tag(0),
+ m_updateTimer(0),
m_enabled(true),
m_parentEnabled(true),
m_visible(true),
@@ -410,6 +411,20 @@ QCocoaMenuItem *QCocoaMenu::itemOrNull(int index) const
return m_menuItems.at(index);
}
+void QCocoaMenu::scheduleUpdate()
+{
+ if (!m_updateTimer)
+ m_updateTimer = startTimer(0);
+}
+
+void QCocoaMenu::timerEvent(QTimerEvent *e)
+{
+ if (e->timerId() == m_updateTimer) {
+ m_updateTimer = 0;
+ [m_nativeMenu update];
+ }
+}
+
void QCocoaMenu::syncMenuItem(QPlatformMenuItem *menuItem)
{
QMacAutoReleasePool pool;
@@ -436,9 +451,9 @@ void QCocoaMenu::syncMenuItem(QPlatformMenuItem *menuItem)
QCocoaMenuItem* beforeItem = itemOrNull(m_menuItems.indexOf(cocoaItem) + 1);
insertNative(cocoaItem, beforeItem);
} else {
- // Force NSMenuValidation to kick in. This is needed e.g.
+ // Schedule NSMenuValidation to kick in. This is needed e.g.
// when an item's enabled state changes after menuWillOpen:
- [m_nativeMenu update];
+ scheduleUpdate();
}
}
diff --git a/tests/manual/cocoa/menurama/mainwindow.cpp b/tests/manual/cocoa/menurama/mainwindow.cpp
index 5cccc16974..06867bd7c9 100644
--- a/tests/manual/cocoa/menurama/mainwindow.cpp
+++ b/tests/manual/cocoa/menurama/mainwindow.cpp
@@ -56,6 +56,19 @@ MainWindow::MainWindow(QWidget *parent) :
connect(ui->pushButton, &QPushButton::clicked, [=] {
menuApp->populateMenu(ui->menuOn_Click, true /*clear*/);
});
+
+ connect(ui->addManyButton, &QPushButton::clicked, [=] {
+ QMenu *menu = new QMenu(QLatin1String("Many More ") +
+ QString::number(ui->menuBar->actions().count()));
+ ui->menuBar->insertMenu(ui->menuDynamic_Stuff->menuAction(), menu);
+ for (int i = 0; i < 2000; i++) {
+ auto *action = menu->addAction(QLatin1String("Item ") + QString::number(i));
+ if (i & 0x1)
+ action->setEnabled(false);
+ if (i & 0x2)
+ action->setVisible(false);
+ }
+ });
}
MainWindow::~MainWindow()
diff --git a/tests/manual/cocoa/menurama/mainwindow.ui b/tests/manual/cocoa/menurama/mainwindow.ui
index d3caa6c608..18cded70d2 100644
--- a/tests/manual/cocoa/menurama/mainwindow.ui
+++ b/tests/manual/cocoa/menurama/mainwindow.ui
@@ -6,50 +6,60 @@
<rect>
<x>0</x>
<y>0</y>
- <width>429</width>
- <height>251</height>
+ <width>486</width>
+ <height>288</height>
</rect>
</property>
<property name="windowTitle">
<string>MainWindow</string>
</property>
<widget class="QWidget" name="centralWidget">
- <layout class="QVBoxLayout" name="verticalLayout_2">
+ <layout class="QVBoxLayout" name="verticalLayout">
<item>
- <layout class="QVBoxLayout" name="verticalLayout">
- <property name="spacing">
- <number>24</number>
- </property>
- <item>
- <widget class="QLabel" name="label">
- <property name="text">
- <string>The &quot;Help&quot; menu should NOT be visible.
+ <widget class="QLabel" name="label">
+ <property name="text">
+ <string>The &quot;Help&quot; menu should NOT be visible.
Click on &quot;Dynamic Stuff&quot; then move left and right to other menus. Disabled items should remain that way.</string>
- </property>
- <property name="scaledContents">
- <bool>false</bool>
- </property>
- <property name="alignment">
- <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop</set>
- </property>
- <property name="wordWrap">
- <bool>true</bool>
- </property>
- </widget>
- </item>
- <item>
- <widget class="QCheckBox" name="checkBox">
- <property name="text">
- <string>Enable &quot;Stuff&quot; Menu</string>
- </property>
- <property name="checked">
- <bool>true</bool>
- </property>
- </widget>
- </item>
+ </property>
+ <property name="scaledContents">
+ <bool>false</bool>
+ </property>
+ <property name="alignment">
+ <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop</set>
+ </property>
+ <property name="wordWrap">
+ <bool>true</bool>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QCheckBox" name="checkBox">
+ <property name="text">
+ <string>Enable &quot;Stuff&quot; Menu</string>
+ </property>
+ <property name="checked">
+ <bool>true</bool>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QPushButton" name="pushButton">
+ <property name="sizePolicy">
+ <sizepolicy hsizetype="Maximum" vsizetype="Fixed">
+ <horstretch>0</horstretch>
+ <verstretch>0</verstretch>
+ </sizepolicy>
+ </property>
+ <property name="text">
+ <string>Populate Dynamic Submenu</string>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <layout class="QHBoxLayout" name="horizontalLayout">
<item>
- <widget class="QPushButton" name="pushButton">
+ <widget class="QPushButton" name="addManyButton">
<property name="sizePolicy">
<sizepolicy hsizetype="Maximum" vsizetype="Fixed">
<horstretch>0</horstretch>
@@ -57,7 +67,17 @@ Click on &quot;Dynamic Stuff&quot; then move left and right to other menus. Disa
</sizepolicy>
</property>
<property name="text">
- <string>Populate Dynamic Submenu</string>
+ <string>Add Many Items</string>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QLabel" name="label_2">
+ <property name="text">
+ <string>Adding hundreds of items should not block the UI for noticeable periods of time. Odd numbered items should be disabled, those with 2nd LSB on should be hidden.</string>
+ </property>
+ <property name="wordWrap">
+ <bool>true</bool>
</property>
</widget>
</item>
@@ -70,7 +90,7 @@ Click on &quot;Dynamic Stuff&quot; then move left and right to other menus. Disa
<rect>
<x>0</x>
<y>0</y>
- <width>429</width>
+ <width>486</width>
<height>22</height>
</rect>
</property>