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.
This commit is contained in:
Guillaume Munch 2016-08-03 22:06:20 +01:00
parent 0779b3c36c
commit 4d99112056
6 changed files with 90 additions and 56 deletions

View File

@ -24,14 +24,32 @@
#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);
@ -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);
}

View File

@ -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_;
};

View File

@ -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;
}

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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