[vlc-devel] [vlc-commits] commit: qt4: Don't create QMenu without parents (Erwan Tulou )

[brezhoneg1]

hi,

   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.


Regards
Erwan10
     

--- 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...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100407/6b5c7368/attachment.html>