[Solved] Keeping a menubar for repeated use in QMainWindow::setMenuBar
-
In my user interface, I would like to be able to switch the menubar of QMainWindow among several predefined menus.
Using QMainWindow::setMenuBar() on a menu can only work once with any QMenuBar* pointer, as the documentation for setMenuBar says this :
"Note: QMainWindow takes ownership of the menuBar pointer and deletes it at the appropriate time".This seems to be the case, since when I tried, the second time that the same QMenuBar* pointer was used, setMenuBar crashed on invalid memory reference.
I have solved the problem by recreating the menubar before each call to setMenuBar.
However, is there a way of preserving the menubar, or of copying it without writing complex recursive code, so it could be more easily reset multiple times?
-
@raven-worx
There is nothing special about my code : Just doing setMenuBar with pointer1, followed by pointer2, then by pointer1 again, causes the crash.I don't suppose that there is any way to prevent setMenuBar from deleting the menubar, short of modifying Qt code, so my question is probably about copying a menubar easily.
If there is no better solution, I will keep on recreating the menubar at each time. It just doesn't seem very elegant.
-
@raven-worx
There is nothing special about my code : Just doing setMenuBar with pointer1, followed by pointer2, then by pointer1 again, causes the crash.I don't suppose that there is any way to prevent setMenuBar from deleting the menubar, short of modifying Qt code, so my question is probably about copying a menubar easily.
If there is no better solution, I will keep on recreating the menubar at each time. It just doesn't seem very elegant.
@Harry123 said:
There is nothing special about my code : Just doing setMenuBar with pointer1, followed by pointer2, then by pointer1 again, causes the crash.
nevermind, it seems i rushed too fast over your first post.
I don't suppose that there is any way to prevent setMenuBar from deleting the menubar, short of modifying Qt code, so my question is probably about copying a menubar easily.
no, since you already mentioned the ownership of the .
If there is no better solution, I will keep on recreating the menubar at each time. It just doesn't seem very elegant.
as i said it's probably way better to reuse the one menubar and just reinitialize it with the desired actions everytime. So avoid unnecessary memory allocations and deletions.
-
@Harry123 said:
There is nothing special about my code : Just doing setMenuBar with pointer1, followed by pointer2, then by pointer1 again, causes the crash.
nevermind, it seems i rushed too fast over your first post.
I don't suppose that there is any way to prevent setMenuBar from deleting the menubar, short of modifying Qt code, so my question is probably about copying a menubar easily.
no, since you already mentioned the ownership of the .
If there is no better solution, I will keep on recreating the menubar at each time. It just doesn't seem very elegant.
as i said it's probably way better to reuse the one menubar and just reinitialize it with the desired actions everytime. So avoid unnecessary memory allocations and deletions.
The menubar itself is just one class instance. Creating the actions requires allocating many more class instances, so just economizing on one allocate/delete, that of one QMenuBar, does not seem significant.
In any case, I'm not at all sure that clear() will not also crash. It seems to me that calling setMenuBar then clear may cause clear to crash on freeing memory that was already freed. Calling clear then setMenuBar may cause setMenuBar to crash.
-
The menubar itself is just one class instance. Creating the actions requires allocating many more class instances, so just economizing on one allocate/delete, that of one QMenuBar, does not seem significant.
In any case, I'm not at all sure that clear() will not also crash. It seems to me that calling setMenuBar then clear may cause clear to crash on freeing memory that was already freed. Calling clear then setMenuBar may cause setMenuBar to crash.
@Harry123
Yes, unfortunatelyQMainWindow::setMenuBarwill callQObject::deleteLaterfor the old menu. I don't think that this was a good design choice, but well, it's how it's implemented currently. You could try intercepting theQEvent::DeferredDeleteevents with an event filter and discard their processing for your menus. At least I see no other way, beside creating your menu bars anew.Kind regards.
-
The menubar itself is just one class instance. Creating the actions requires allocating many more class instances, so just economizing on one allocate/delete, that of one QMenuBar, does not seem significant.
In any case, I'm not at all sure that clear() will not also crash. It seems to me that calling setMenuBar then clear may cause clear to crash on freeing memory that was already freed. Calling clear then setMenuBar may cause setMenuBar to crash.
@Harry123 said:
The menubar itself is just one class instance. Creating the actions requires allocating many more class instances, so just economizing on one allocate/delete, that of one QMenuBar, does not seem significant.
i never said delete the actions. Unlike with the menubar the ownership of QActions isn't transferred to the widget when you call addAction().
So just create every instance once and compose them together as you need.
In any case, I'm not at all sure that clear() will not also crash. It seems to me that calling setMenuBar then clear may cause clear to crash on freeing memory that was already freed. Calling clear then setMenuBar may cause setMenuBar to crash.
what makes you think that?!
-
It would be nice if there was
takeMenuBar(), but that's not a big problem. It's 2 lines of code.
Before you set new bar make sure to set the parent of the current one to null i.e.if(mainWindow->menuBar()) mainWindow->menuBar()->setParent(nullptr); mainWindow->setMenuBar(anotherBar);One thing to note is that with this you need to keep track of the lifetime of the bars because the unused ones are no longer managed by a parent. This is as easy as putting them in some container and calling
qDeleteAll(barsContainer)at the end. -
It would be nice if there was
takeMenuBar(), but that's not a big problem. It's 2 lines of code.
Before you set new bar make sure to set the parent of the current one to null i.e.if(mainWindow->menuBar()) mainWindow->menuBar()->setParent(nullptr); mainWindow->setMenuBar(anotherBar);One thing to note is that with this you need to keep track of the lifetime of the bars because the unused ones are no longer managed by a parent. This is as easy as putting them in some container and calling
qDeleteAll(barsContainer)at the end.@Chris-Kawa
this is not a matter of QObject parent-child relationship.
The mainwindow keeps the menubar as a pointer and deletes it when a new one is set. -
@raven-worx Setting a null parent "detaches" the bar from main window. It won't be deleted by the consecutive
setMenuBar(). I'm not guessing. I've checked ;) -
@raven-worx Setting a null parent "detaches" the bar from main window. It won't be deleted by the consecutive
setMenuBar(). I'm not guessing. I've checked ;)@Chris-Kawa
i see. Because it's contained in a layout.
Sry for that. -
@raven-worx Setting a null parent "detaches" the bar from main window. It won't be deleted by the consecutive
setMenuBar(). I'm not guessing. I've checked ;)@Chris-Kawa
That's a very simple solution to the problem.
Thank you, I'll be trying it out very soon. -
@Chris-Kawa
i see. Because it's contained in a layout.
Sry for that.@raven-worx, @chris-kawa
http://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/widgets/qmainwindow.cpp?h=5.5#n517
I'm sorry to argue, but look up here. Line 535 explicitly deletes the menu! -
@kshegunov That's true, but note that deletion happens in
if (topLayout->menuBar() ....block.
When you set parent of the bar to nullptr it is taken out of the layout. But don't take my word for it, check:qDebug() << mainWindow->layout()->menuBar(); //prints out some pointer mainWindow->menuBar()->setParent(nullptr); qDebug() << mainWindow->layout()->menuBar(); //prints 0, the bar was taken out -
@kshegunov That's true, but note that deletion happens in
if (topLayout->menuBar() ....block.
When you set parent of the bar to nullptr it is taken out of the layout. But don't take my word for it, check:qDebug() << mainWindow->layout()->menuBar(); //prints out some pointer mainWindow->menuBar()->setParent(nullptr); qDebug() << mainWindow->layout()->menuBar(); //prints 0, the bar was taken out@Chris-Kawa
Ah! Yes, you're correct of course. This was my initial idea (reparenting the menu bar) but I discarded it because I didn't realize that the menu will be taken out of the layout, silly me! :)Kind regards.
-
Solution verified as simple and perfectly working.
What I did was to override QMainWindow::setMenuBar as follows :
void myMainWindow::setMenuBar(QMenuBar * bar) { if (menuBar()) menuBar()->setParent(0); // detach current menubar so it won't get deleted QMainWindow::setMenuBar(bar); } -
Solution verified as simple and perfectly working.
What I did was to override QMainWindow::setMenuBar as follows :
void myMainWindow::setMenuBar(QMenuBar * bar) { if (menuBar()) menuBar()->setParent(0); // detach current menubar so it won't get deleted QMainWindow::setMenuBar(bar); }@Harry123
please be advised that this only works as long as the instance is of the type myMainWindow, since setMenuBar() is not virtual.Meaning the following wont work, even the object behind the pointer has the correct type:
QMainWindow* mw = new myMainWindow; // pointer-type is different from instance-type! mw->setMenuBar(...); -
@Harry123
please be advised that this only works as long as the instance is of the type myMainWindow, since setMenuBar() is not virtual.Meaning the following wont work, even the object behind the pointer has the correct type:
QMainWindow* mw = new myMainWindow; // pointer-type is different from instance-type! mw->setMenuBar(...);I have "mw = this;" in the constructor for myMainWindow, where mw is a global variable, and I can now successfully do "mw->setMenuBar(..)" from several other classes with no problem.
It currently does work - why shouldn't it ?
-
@Harry123 It might work for your case but is very dangerous in general. As @raven-worx mentioned
setMenuBar()is not virtual. This means that if anyone calls it through a pointer to base class the base class version will be called (which deletes the bar).
In general you can't guarantee that your class will only by accessed via pointer to derived class and in fact you should never assume that.
Example:QMainWindow* gotcha = mw; mw->setMenuBar(someBar); //ok gotcha->setMenuBar(someOtherBar); //someBar is deleted!Long story short - don't override non-virtual methods. Create a new one with descriptive name.
-
@Harry123 It might work for your case but is very dangerous in general. As @raven-worx mentioned
setMenuBar()is not virtual. This means that if anyone calls it through a pointer to base class the base class version will be called (which deletes the bar).
In general you can't guarantee that your class will only by accessed via pointer to derived class and in fact you should never assume that.
Example:QMainWindow* gotcha = mw; mw->setMenuBar(someBar); //ok gotcha->setMenuBar(someOtherBar); //someBar is deleted!Long story short - don't override non-virtual methods. Create a new one with descriptive name.
@Chris-Kawa
I don't have this gotcha - my variable is of type myMainWindow*.
This can only not work if there is a bug in the C++ compiler.
But perhaps I will rename it, just in case. -
@Harry123 said:
This can only not work if there is a bug in the C++ compiler.
As I said - it works here and now for you. But who's to say in a month or a year another person will for whatever reason create a local variable of type QMainWindow* and copy your pointer. The point is that because this method is not virtual a different method will be called depending on the type of pointer you access it through. That's a time bomb type of bug. It does not explode here and now for you but it will for someone else or future you.
