[vlc-devel] [vlc-commits] commit: qt4: Don't create QMenu without parents (Erwan Tulou )
brezhoneg1 at yahoo.fr
Wed Apr 7 10:07:17 CEST 2010
I fear things are a lot more complicated
- There is not one popmenu menu but four independent popmenus (PopMenu, VideoPopupMenu, AudioPopupMenu, MiscPopupMenu)
(see dialogs_provider.cpp around line 162)
- today's implementation processes PopMenu in a certain way (with p_intf->p_sys->p_popup_menu ) and the three others in another way.
- p_intf->p_sys->p_popup_menu does bring a plus, which is to prevent accumulation of new QMenus (first deleting the previous one before creating a new one). So, if it were to be used, the three others popmenus should also use the same pattern.
- imo, p_intf->p_sys->p_popup_menu is an ugly implementation in C++. What we need is actually some kind of a singleton class, that automatically cleans after itself when needed
- if you trace p_intf->p_sys->p_popup_menu, you'll see that use of this variable is quite weird
- In addition, there is still other QMenu situations apart from those 4 independent popupmenus,
where we need to ensure proper deallocation.
To sum it up
- there is much work to do for menus
- Since it's feature freeze, my patch is still the shortest way to work out
the problem, yet not redesign the whole menu thing
- This patch is necessary because we cannot afford to leave vlc objects unreleased
- One alternative would be to just revert your patch that attached MenuItem to action .....
but I do agree this patch was useful.
--- En date de : Mar 6.4.10, Jakob Leben <jakob.leben at gmail.com> a écrit :
De: Jakob Leben <jakob.leben at gmail.com>
Objet: Re: [vlc-devel] [vlc-commits] commit: qt4: Don't create QMenu without parents (Erwan Tulou )
À: "Mailing list for VLC media player developers" <vlc-devel at videolan.org>
Date: Mardi 6 avril 2010, 20h04
On Tue, Apr 6, 2010 at 5:51 PM, brezhoneg1 <brezhoneg1 at yahoo.fr> wrote:
The thing is there are not many solutions
- QMenu needs a QWidget as parent
- dialog_provider (as a dialog provider only) doesn't have any widget
- Leaving QMenu without parent is just plain memory leak, and leads to vlc objects leak at times.
I agree that creating QObjects without parents is not the best practice and somewhat risky. I think though, that those popup menus are reasonable exception, if we can ensurPopMenue that they get deleted when necessary.
p_intf->p_sys->p_popup_menu pointer is the device to make that possible and prevent "plain" memory leaks.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel