Index: src/Plugins/Qt/qt_menu.cpp =================================================================== --- src/Plugins/Qt/qt_menu.cpp (revision 2798) +++ src/Plugins/Qt/qt_menu.cpp (working copy) @@ -34,21 +34,48 @@ extern char *slot_name(slot s); // from qt_widget.cpp +// REMARK on memory management. +// the hierarchy of a QMenu has parents correctly set to the proper supermenu +// this guarantees that deletion of the root menu correclty deletes all the tree below it. +// the root menu itself (without parent QObject) is "owned" by the associate qt_menu_rep instance +// and it is deallocated by it. This ensure correct memory management between TeXmacs and Qt +// since qt_menu_rep is sometimes cached at TeXmacs level. +// this also mean that when we install some menu in the GUI (in the main menu or in the toolbar) +// we should just add actions and not reroot the qt parent hierarchy since even if the menu will +// be eventually removed from the GUI it has some chance to still be cached in the TeXmacs side +// conventions are as follows: +// - the method as_qaction passes the ownership of the action and the eventual submenu to the caller +// responsibility. When creating menu hierachy (eg. via the scheme interface) you should use this method +// to retrieve the relevant Qt objects. +// - the method qt_menu_rep::get_menu preserve the onwership of the menu to the called qt_menu_rep +// (to guarantee correct caching). when installing menus in the gui you should use this method. + + + // this custom action frees its menu if it does not already have an owner. class QTMAction : public QAction { public: - QTMAction(QObject *parent = NULL) : QAction(parent) {} - ~QTMAction() { if (menu() && !(menu()->parent())) delete menu(); } //menu()->deleteLater(); } + QTMAction(QObject *parent = NULL) : QAction(parent) { } + ~QTMAction() { + if (menu() && !(menu()->parent())) delete menu(); + } }; + class qt_menu_rep: public qt_widget_rep { public: - bool flag; - QPointer item; - qt_menu_rep (QAction* _item): flag (false), item (_item) {} - ~qt_menu_rep () { if (item && !flag) delete item; } + QAction *item; + qt_menu_rep (QAction* _item) : item (_item ? _item : new QTMAction (NULL)) { } + ~qt_menu_rep () { + delete item; // the submenu is usually also deleted since item is a QTMAction + } + QMenu *get_qmenu() { return (item ? item->menu() : NULL); } + // get_menu doest not give ownership of the menu to the caller + // this allow menu caching at the TeXmacs level + // get_qmenu is called only by code which attach root menus in the GUI elements + virtual void send (slot s, blackbox val); virtual widget make_popup_widget (); virtual widget popup_window_widget (string s); @@ -60,9 +87,10 @@ // FIXME: the convention is that as_qaction give ownership of // the action to the caller. However in this case we do not want // to replicate the action so we must be sure to be called only once. - if (flag) cout << "THIS MUST NOT HAPPEN (CALLED TWICE)!!\n"; - flag = true; - return item; + if (!item) cout << "THIS MUST NOT HAPPEN TWICE" << LF; + QAction *ret = item; + item = NULL; + return ret; } widget @@ -371,19 +399,12 @@ QMenu* to_qmenu(widget w) { - QAction *a = concrete(w)->as_qaction(); - QMenu *m = a->menu(); // the menu has no parent - m->setParent(NULL); - a->setMenu(NULL); // otherwise the deletion of a (really a QTMAction) triggers deletion of m - delete a; + //FIXME: the static cast is not sure in general, how we can test for + // the right widget type? + QMenu *m = static_cast(w.rep)->get_qmenu(); return m; } -QAction* -to_qaction(widget w) { - return concrete(w)->as_qaction(); -} - QPixmap impress (simple_widget_rep* wid) { if (wid) { @@ -423,15 +444,30 @@ } void -QTMLazyMenu::force () { - if (!forced) { - widget w= pm (); - qt_menu_rep* wid= (qt_menu_rep*) (w.rep); - QMenu* menu2= wid->item->menu (); - replaceActions (this, menu2); - delete (wid->item); - wid->item= NULL; - //pm= promise(); - //forced= true; +rerootActions (QWidget* dest, QWidget* src) { + QList list = dest->actions(); + while (!list.isEmpty()) { + QAction* a= list.takeFirst(); + dest->removeAction (a); + // the following forces the deletion of the submenu since in the relevant cases the + // action is actually a QTMAction + if (a->menu()) a->menu()->setParent(NULL); + delete a; } + list = src->actions(); + while (!list.isEmpty()) { + QAction* a= list.takeFirst(); + dest->addAction (a); + a->setParent (dest); + // we set the parent of the submenu to its new supermenu + if (a->menu()) a->menu()->setParent (dest); + } } + + +void +QTMLazyMenu::force () { + widget w= pm (); + QMenu *menu2 = to_qmenu(w); + rerootActions (this, menu2); +} Index: src/Plugins/Qt/qt_menu.hpp =================================================================== --- src/Plugins/Qt/qt_menu.hpp (revision 2798) +++ src/Plugins/Qt/qt_menu.hpp (working copy) @@ -17,6 +17,5 @@ #include QMenu* to_qmenu (widget w); -QAction* to_qaction (widget w); #endif // defined QT_MENU_HPP Index: src/Plugins/Qt/qt_other_widgets.hpp =================================================================== --- src/Plugins/Qt/qt_other_widgets.hpp (revision 2798) +++ src/Plugins/Qt/qt_other_widgets.hpp (working copy) @@ -37,6 +37,8 @@ bool visibility[5]; command quit; + + widget main_menu_widget, main_icons_widget, context_icons_widget, user_icons_widget; public: qt_tm_widget_rep (int mask, command _quit); Index: src/Plugins/Qt/qt_widget.cpp =================================================================== --- src/Plugins/Qt/qt_widget.cpp (revision 2798) +++ src/Plugins/Qt/qt_widget.cpp (working copy) @@ -602,28 +602,18 @@ void replaceActions (QWidget* dest, QWidget* src) { + //NOTE: the parent hierachy of the actions is not modified while installing + // the menu in the GUI (see qt_menu.cpp for this memory management policy) QList list = dest->actions(); while (!list.isEmpty()) { QAction* a= list.takeFirst(); dest->removeAction (a); - delete a; -// a->deleteLater(); } - // cout << "replaceActions n:" << src->actions().count() << LF; list = src->actions(); while (!list.isEmpty()) { QAction* a= list.takeFirst(); dest->addAction (a); - a->setParent (dest); } - - // dest->addActions(src->actions()); -#if 0 - for(int i=0; isetMenuRole (QAction::ApplicationSpecificRole); - } -#endif } extern void @@ -659,10 +649,10 @@ case SLOT_MAIN_MENU: check_type_void (index, "SLOT_MAIN_MENU"); { + main_menu_widget = w; QMenu* m= to_qmenu (w); if (m) { replaceActions (tm_mainwindow()->menuBar (), m); - delete m; } } break; @@ -671,29 +661,27 @@ check_type_void (index, "SLOT_MAIN_ICONS"); { //cout << "widget :" << (void*)w.rep << LF; + main_icons_widget = w; QMenu* m= to_qmenu (w); - //mainToolBar->setUpdatesEnabled (false); replaceButtons (mainToolBar, m); - delete m; } break; case SLOT_CONTEXT_ICONS: check_type_void (index, "SLOT_CONTEXT_ICONS"); { + context_icons_widget = w; QMenu* m= to_qmenu (w); replaceButtons (contextToolBar, m); - //mainToolBar->setUpdatesEnabled (true); - delete m; } break; case SLOT_USER_ICONS: check_type_void (index, "SLOT_USER_ICONS"); { + user_icons_widget = w; QMenu* m= to_qmenu (w); replaceButtons (userToolBar, m); - delete m; } break; Index: src/Plugins/Qt/QTMMenuHelper.hpp =================================================================== --- src/Plugins/Qt/QTMMenuHelper.hpp (revision 2798) +++ src/Plugins/Qt/QTMMenuHelper.hpp (working copy) @@ -37,11 +37,10 @@ class QTMLazyMenu: public QMenu { Q_OBJECT promise pm; - bool forced; public: inline QTMLazyMenu (promise _pm): - pm (_pm), forced (false) { + pm (_pm) { QObject::connect (this, SIGNAL (aboutToShow ()), this, SLOT (force ())); } Index: src/Texmacs/Window/tm_window.cpp =================================================================== --- src/Texmacs/Window/tm_window.cpp (revision 2798) +++ src/Texmacs/Window/tm_window.cpp (working copy) @@ -90,11 +90,9 @@ if (menu_cache->contains (xmenu)) { if (menu_current[which] == xmenu) return false; menu_current (which)= xmenu; -#ifndef QTTEXMACS //cout << "Cached " << menu << "\n"; w= menu_cache [xmenu]; return true; -#endif } //cout << "Compute " << menu << "\n"; object umenu= eval ("'" * menu);