<table cellspacing="0" cellpadding="0" border="0" ><tr><td valign="top" style="font: inherit;"><br>hi,<br><br> I fear things are a lot more complicated<br><br> - There is not one popmenu menu but four independent popmenus (PopMenu, VideoPopupMenu, AudioPopupMenu, MiscPopupMenu)<br> (see dialogs_provider.cpp around line 162)<br><br> - today's implementation processes PopMenu in a certain way (with p_intf->p_sys->p_popup_menu ) and the three others in another way.<br><br> - 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.<br><br> - 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<br><br> - if you trace p_intf->p_sys->p_popup_menu, you'll see that use of this variable is quite weird<br><br> - In addition, there is still other QMenu situations apart from those 4 independent popupmenus,<br> where we need to ensure proper deallocation.<br><br> <br>To sum it up<br><br> - there is much work to do for menus<br> - Since it's feature freeze, my patch is still the shortest way to work out<br> the problem, yet not redesign the whole menu thing<br> - This patch is necessary because we cannot afford to leave vlc objects unreleased<br> - One alternative would be to just revert your patch that attached MenuItem to action .....<br> but I do agree this patch was
useful.<br><br><br>Regards<br>Erwan10<br> <br><br>--- En date de : <b>Mar 6.4.10, Jakob Leben <i><jakob.leben@gmail.com></i></b> a écrit :<br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px;"><br>De: Jakob Leben <jakob.leben@gmail.com><br>Objet: Re: [vlc-devel] [vlc-commits] commit: qt4: Don't create QMenu without parents (Erwan Tulou )<br>À: "Mailing list for VLC media player developers" <vlc-devel@videolan.org><br>Date: Mardi 6 avril 2010, 20h04<br><br><div id="yiv1808866540"><div class="gmail_quote">On Tue, Apr 6, 2010 at 5:51 PM, brezhoneg1 <span dir="ltr"><<a rel="nofollow" ymailto="mailto:brezhoneg1@yahoo.fr" target="_blank" href="/mc/compose?to=brezhoneg1@yahoo.fr">brezhoneg1@yahoo.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>The thing is there are not many solutions<br>
<br>
- QMenu needs a QWidget as parent<br>
- dialog_provider (as a dialog provider only) doesn't have any widget<br>
- Leaving QMenu without parent is just plain memory leak, and leads to vlc objects leak at times.<br><br></blockquote><div><br>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.<br>
p_intf->p_sys->p_popup_menu pointer is the device to make that possible and prevent "plain" memory leaks.<br><br></div></div>
</div></blockquote></td></tr></table><br>