commit 4d99112056410762c720e8dfe2c859ff84c4a8b3
Author: Guillaume Munch <[email protected]>
Date: Wed Aug 3 22:06:20 2016 +0100
Action.cpp: replace a reference with a shared_ptr
Replace the member reference to FuncRequest in Action.cpp with a
shared_ptr. Compared to copying the FuncRequest, the shared_ptr has two
advantages:
* Recreating the menu each time creates a lot of new actions, so we avoid a
lot
of copies.
* FuncRequest can remain forward-declared in Action.h.
---
src/frontends/qt4/Action.cpp | 28 ++++++++++++---
src/frontends/qt4/Action.h | 15 ++++++--
src/frontends/qt4/GuiToolbar.cpp | 14 ++++----
src/frontends/qt4/Menus.cpp | 74 ++++++++++++++++++++------------------
src/frontends/qt4/Toolbars.cpp | 12 ++++---
src/frontends/qt4/Toolbars.h | 3 +-
6 files changed, 90 insertions(+), 56 deletions(-)
diff --git a/src/frontends/qt4/Action.cpp b/src/frontends/qt4/Action.cpp
index 71e41a4..bd102e6 100644
--- a/src/frontends/qt4/Action.cpp
+++ b/src/frontends/qt4/Action.cpp
@@ -24,15 +24,33 @@
#include "support/debug.h"
#include "support/lstrings.h"
+using namespace std;
+
+
namespace lyx {
namespace frontend {
-Action::Action(QIcon const & icon,
- QString const & text, FuncRequest const & func,
- QString const & tooltip, QObject * parent)
+Action::Action(FuncRequest func, QIcon const & icon, QString const & text,
+ QString const & tooltip, QObject * parent)
+ : QAction(parent), func_(make_shared<FuncRequest>(move(func)))
+{
+ init(icon, text, tooltip);
+}
+
+
+Action::Action(shared_ptr<FuncRequest const> func,
+ QIcon const & icon, QString const & text,
+ QString const & tooltip, QObject * parent)
: QAction(parent), func_(func)
{
+ init(icon, text, tooltip);
+}
+
+
+void Action::init(QIcon const & icon, QString const & text,
+ QString const & tooltip)
+{
// only Qt/Mac handles that
setMenuRole(NoRole);
setIcon(icon);
@@ -46,7 +64,7 @@ Action::Action(QIcon const & icon,
void Action::update()
{
- FuncStatus const status = getStatus(func_);
+ FuncStatus const status = getStatus(*func_);
if (status.onOff(true)) {
setCheckable(true);
@@ -66,7 +84,7 @@ void Action::action()
{
//LYXERR(Debug::ACTION, "calling lyx::dispatch: func_: ");
- lyx::dispatch(func_);
+ lyx::dispatch(*func_);
triggered(this);
}
diff --git a/src/frontends/qt4/Action.h b/src/frontends/qt4/Action.h
index 54954c6..8bc4a71 100644
--- a/src/frontends/qt4/Action.h
+++ b/src/frontends/qt4/Action.h
@@ -13,6 +13,7 @@
#define ACTION_H
#include <QAction>
+#include <memory>
class QIcon;
@@ -32,8 +33,15 @@ class Action : public QAction
Q_OBJECT
public:
- Action(QIcon const & icon, QString const & text,
- FuncRequest const & func, QString const & tooltip, QObject *
parent);
+ // Makes a copy of func
+ Action(FuncRequest func, QIcon const & icon, QString const & text,
+ QString const & tooltip, QObject * parent);
+
+ // Takes shared ownership of func.
+ // Use for perf-sensitive code such as populating menus.
+ Action(std::shared_ptr<FuncRequest const> func,
+ QIcon const & icon, QString const & text,
+ QString const & tooltip, QObject * parent);
void update();
@@ -45,7 +53,8 @@ private Q_SLOTS:
void action();
private:
- FuncRequest const & func_ ;
+ void init(QIcon const & icon, QString const & text, QString const &
tooltip);
+ std::shared_ptr<FuncRequest const> func_;
};
diff --git a/src/frontends/qt4/GuiToolbar.cpp b/src/frontends/qt4/GuiToolbar.cpp
index 2f0b510..36a7326 100644
--- a/src/frontends/qt4/GuiToolbar.cpp
+++ b/src/frontends/qt4/GuiToolbar.cpp
@@ -116,12 +116,12 @@ Action * GuiToolbar::addItem(ToolbarItem const & item)
QString text = toqstr(item.label_);
// Get the keys bound to this action, but keep only the
// first one later
- KeyMap::Bindings bindings =
theTopLevelKeymap().findBindings(item.func_);
+ KeyMap::Bindings bindings =
theTopLevelKeymap().findBindings(*item.func_);
if (!bindings.empty())
text += " [" +
toqstr(bindings.begin()->print(KeySequence::ForGui)) + "]";
- Action * act = new Action(getIcon(item.func_, false),
- text, item.func_, text, this);
+ Action * act = new Action(item.func_, getIcon(*item.func_, false), text,
+ text, this);
actions_.append(act);
return act;
}
@@ -148,7 +148,7 @@ public:
ToolbarInfo const * tbinfo =
guiApp->toolbars().info(tbitem_.name_);
if (tbinfo)
// use the icon of first action for the toolbar button
- setIcon(getIcon(tbinfo->items.begin()->func_, true));
+ setIcon(getIcon(*tbinfo->items.begin()->func_, true));
}
void mousePressEvent(QMouseEvent * e)
@@ -173,7 +173,7 @@ public:
ToolbarInfo::item_iterator it = tbinfo->items.begin();
ToolbarInfo::item_iterator const end = tbinfo->items.end();
for (; it != end; ++it)
- if (!getStatus(it->func_).unknown())
+ if (!getStatus(*it->func_).unknown())
panel->addButton(bar_->addItem(*it));
QToolButton::mousePressEvent(e);
@@ -228,7 +228,7 @@ void MenuButton::initialize()
ToolbarInfo::item_iterator it = tbinfo->items.begin();
ToolbarInfo::item_iterator const end = tbinfo->items.end();
for (; it != end; ++it)
- if (!getStatus(it->func_).unknown())
+ if (!getStatus(*it->func_).unknown())
m->add(bar_->addItem(*it));
setMenu(m);
}
@@ -311,7 +311,7 @@ void GuiToolbar::add(ToolbarItem const & item)
break;
}
case ToolbarItem::COMMAND: {
- if (!getStatus(item.func_).unknown())
+ if (!getStatus(*item.func_).unknown())
addAction(addItem(item));
break;
}
diff --git a/src/frontends/qt4/Menus.cpp b/src/frontends/qt4/Menus.cpp
index dce5c8c..11ea996 100644
--- a/src/frontends/qt4/Menus.cpp
+++ b/src/frontends/qt4/Menus.cpp
@@ -198,8 +198,8 @@ public:
QString const & submenu = QString(),
QString const & tooltip = QString(),
bool optional = false)
- : kind_(kind), label_(label), submenuname_(submenu),
- tooltip_(tooltip), optional_(optional)
+ : kind_(kind), label_(label), func_(make_shared<FuncRequest>()),
+ submenuname_(submenu), tooltip_(tooltip), optional_(optional)
{
LATTEST(kind == Submenu || kind == Help || kind == Info);
}
@@ -210,10 +210,10 @@ public:
QString const & tooltip = QString(),
bool optional = false,
FuncRequest::Origin origin = FuncRequest::MENU)
- : kind_(kind), label_(label), func_(func),
+ : kind_(kind), label_(label),
func_(make_shared<FuncRequest>(func)),
tooltip_(tooltip), optional_(optional)
{
- func_.setOrigin(origin);
+ func_->setOrigin(origin);
}
/// The label of a given menuitem
@@ -234,7 +234,7 @@ public:
/// The kind of entry
Kind kind() const { return kind_; }
/// the action (if relevant)
- FuncRequest const & func() const { return func_; }
+ shared_ptr<FuncRequest const> func() const { return func_; }
/// the tooltip
QString const & tooltip() const { return tooltip_; }
/// returns true if the entry should be omitted when disabled
@@ -253,13 +253,13 @@ public:
return QString();
// Get the keys bound to this action, but keep only the
// first one later
- KeyMap::Bindings bindings =
theTopLevelKeymap().findBindings(func_);
+ KeyMap::Bindings bindings =
theTopLevelKeymap().findBindings(*func_);
if (!bindings.empty())
return
toqstr(bindings.begin()->print(KeySequence::ForGui));
LYXERR(Debug::KBMAP, "No binding for "
- << lyxaction.getActionName(func_.action())
- << '(' << func_.argument() << ')');
+ << lyxaction.getActionName(func_->action())
+ << '(' << func_->argument() << ')');
return QString();
}
@@ -285,7 +285,7 @@ private:
///
QString label_;
///
- FuncRequest func_;
+ shared_ptr<FuncRequest> func_;// non-null
///
QString submenuname_;
///
@@ -398,7 +398,7 @@ void MenuDefinition::addWithStatusCheck(MenuItem const & i)
switch (i.kind()) {
case MenuItem::Command: {
- FuncStatus status = lyx::getStatus(i.func());
+ FuncStatus status = lyx::getStatus(*i.func());
if (status.unknown() || (!status.enabled() && i.optional()))
break;
items_.push_back(i);
@@ -691,7 +691,7 @@ MenuItem const & MenuDefinition::operator[](size_type i)
const
bool MenuDefinition::hasFunc(FuncRequest const & func) const
{
for (const_iterator it = begin(), et = end(); it != et; ++it)
- if (it->func() == func)
+ if (*it->func() == func)
return true;
return false;
}
@@ -741,7 +741,7 @@ bool MenuDefinition::searchMenu(FuncRequest const & func,
docstring_list & names
const_iterator m = begin();
const_iterator m_end = end();
for (; m != m_end; ++m) {
- if (m->kind() == MenuItem::Command && m->func() == func) {
+ if (m->kind() == MenuItem::Command && *m->func() == func) {
names.push_back(qstring_to_ucs4(m->label()));
return true;
}
@@ -1706,7 +1706,7 @@ struct Menu::Impl
{
/// populates the menu or one of its submenu
/// This is used as a recursive function
- void populate(QMenu & qMenu, MenuDefinition const & menu);
+ void populate(QMenu * qMenu, MenuDefinition const & menu);
/// Only needed for top level menus.
MenuDefinition * top_level_menu;
@@ -1739,7 +1739,7 @@ static QString label(MenuItem const & mi)
return label;
}
-void Menu::Impl::populate(QMenu & qMenu, MenuDefinition const & menu)
+void Menu::Impl::populate(QMenu * qMenu, MenuDefinition const & menu)
{
LYXERR(Debug::GUI, "populating menu " << menu.name());
if (menu.empty()) {
@@ -1747,21 +1747,26 @@ void Menu::Impl::populate(QMenu & qMenu, MenuDefinition
const & menu)
return;
}
LYXERR(Debug::GUI, " ***** menu entries " << menu.size());
- MenuDefinition::const_iterator m = menu.begin();
- MenuDefinition::const_iterator end = menu.end();
- for (; m != end; ++m) {
- if (m->kind() == MenuItem::Separator)
- qMenu.addSeparator();
- else if (m->kind() == MenuItem::Submenu) {
- QMenu * subMenu = qMenu.addMenu(label(*m));
- populate(*subMenu, m->submenu());
+ for (MenuItem const & m : menu)
+ switch (m.kind()) {
+ case MenuItem::Separator:
+ qMenu->addSeparator();
+ break;
+ case MenuItem::Submenu: {
+ QMenu * subMenu = qMenu->addMenu(label(m));
+ populate(subMenu, m.submenu());
subMenu->setEnabled(!subMenu->isEmpty());
- } else {
- // we have a MenuItem::Command
- qMenu.addAction(new Action(QIcon(), label(*m),
- m->func(), m->tooltip(), &qMenu));
+ break;
+ }
+ case MenuItem::Command:
+ default:
+ // FIXME: A previous comment assured that
MenuItem::Command was the
+ // only possible case in practice, but this is wrong.
It would be
+ // good to document which cases are actually treated
here.
+ qMenu->addAction(new Action(m.func(), QIcon(), label(m),
+ m.tooltip(), qMenu));
+ break;
}
- }
}
#if (defined(Q_OS_WIN) || defined(Q_CYGWIN_WIN)) && (QT_VERSION >= 0x040600)
@@ -1919,7 +1924,7 @@ void Menus::Impl::macxMenuBarInit(QMenuBar * qmb)
QAction::MenuRole role;
};
- static MacMenuEntry entries[] = {
+ static const MacMenuEntry entries[] = {
{LFUN_DIALOG_SHOW, "aboutlyx", "About LyX",
QAction::AboutRole},
{LFUN_DIALOG_SHOW, "prefs", "Preferences",
@@ -1948,11 +1953,10 @@ void Menus::Impl::macxMenuBarInit(QMenuBar * qmb)
// add the entries to a QMenu that will eventually be empty
// and therefore invisible.
QMenu * qMenu = qmb->addMenu("special");
- MenuDefinition::const_iterator cit = mac_special_menu_.begin();
- MenuDefinition::const_iterator end = mac_special_menu_.end();
- for (size_t i = 0 ; cit != end ; ++cit, ++i) {
- Action * action = new Action(QIcon(), cit->label(),
- cit->func(), QString(), qMenu);
+ size_t i = 0;
+ for (MenuItem const & m : mac_special_menu_) {
+ Action * action = new Action(m.func(), QIcon(), m.label(),
+ QString(), qMenu);
action->setMenuRole(entries[i].role);
qMenu->addAction(action);
}
@@ -2091,7 +2095,7 @@ void Menus::Impl::expand(MenuDefinition const & frommenu,
break;
case MenuItem::Command:
- if (!mac_special_menu_.hasFunc(cit->func()))
+ if (!mac_special_menu_.hasFunc(*cit->func()))
tomenu.addWithStatusCheck(*cit);
}
}
@@ -2330,7 +2334,7 @@ void Menus::updateMenu(Menu * qmenu)
if (qmenu->d->view)
bv = qmenu->d->view->currentBufferView();
d->expand(fromLyxMenu, *qmenu->d->top_level_menu, bv);
- qmenu->d->populate(*qmenu, *qmenu->d->top_level_menu);
+ qmenu->d->populate(qmenu, *qmenu->d->top_level_menu);
}
diff --git a/src/frontends/qt4/Toolbars.cpp b/src/frontends/qt4/Toolbars.cpp
index a9f90d4..b2a9c37 100644
--- a/src/frontends/qt4/Toolbars.cpp
+++ b/src/frontends/qt4/Toolbars.cpp
@@ -38,14 +38,16 @@ namespace frontend {
//
/////////////////////////////////////////////////////////////////////////
-ToolbarItem::ToolbarItem(Type type, FuncRequest const & func, docstring const
& label)
- : type_(type), func_(func), label_(label)
+ToolbarItem::ToolbarItem(Type type, FuncRequest const & func,
+ docstring const & label)
+ : type_(type), func_(make_shared<FuncRequest>(func)), label_(label)
{
}
-ToolbarItem::ToolbarItem(Type type, string const & name, docstring const &
label)
- : type_(type), label_(label), name_(name)
+ToolbarItem::ToolbarItem(Type type, string const & name,
+ docstring const & label)
+ : type_(type), func_(make_shared<FuncRequest>()), label_(label),
name_(name)
{
}
@@ -53,7 +55,7 @@ ToolbarItem::ToolbarItem(Type type, string const & name,
docstring const & label
void ToolbarInfo::add(ToolbarItem const & item)
{
items.push_back(item);
- items.back().func_.setOrigin(FuncRequest::TOOLBAR);
+ items.back().func_->setOrigin(FuncRequest::TOOLBAR);
}
diff --git a/src/frontends/qt4/Toolbars.h b/src/frontends/qt4/Toolbars.h
index dbaf3aa..a65d05e 100644
--- a/src/frontends/qt4/Toolbars.h
+++ b/src/frontends/qt4/Toolbars.h
@@ -17,6 +17,7 @@
#include <vector>
#include <map>
+#include <memory>
namespace lyx {
@@ -57,7 +58,7 @@ public:
/// item type
Type type_;
/// action
- FuncRequest func_;
+ std::shared_ptr<FuncRequest> func_; // non-null
/// label/tooltip
docstring label_;
/// name
