Fix a few issues related to Bitmaps in menus.

- fix missing icons.
- remove duplicate (and incorrect) code to set these icons in wxMenuItems

common/tool/action_menu.cpp

@@ -31,7 +31,7 @@
 #include <tool/tool_interactive.h>
 #include <tool/action_menu.h>
 #include <wx/log.h>
-#include <pgm_base.h>
+#include <menus_helpers.h>
 #include <id.h>
 
 
@@ -61,32 +61,6 @@ ACTION_MENU::~ACTION_MENU()
         parent->m_submenus.remove( this );
 }
 
-/*
- * Helper function.
- * Assigns an icon to the wxMenuItem aMenu.
- * aIcon is the icon to be assigned can be NULL.
- */
-static void set_wxMenuIcon( wxMenuItem* aMenu, const BITMAP_OPAQUE* aIcon )
-{
-    if( !Pgm().CommonSettings() )
-        return;
-
-#if defined(__WXGTK__)
-
-    // wxGTK doesn't support this for non-normal menu items
-    if( aMenu->GetKind() != wxITEM_NORMAL )
-        return;
-
-#endif
-
-    // Retrieve the global applicaton show icon option:
-    bool useImagesInMenus;
-    Pgm().CommonSettings()->Read( USE_ICONS_IN_MENUS_KEY, &useImagesInMenus );
-
-    if( aIcon && useImagesInMenus )
-        aMenu->SetBitmap( KiBitmap( aIcon ) );
-}
-
 
 void ACTION_MENU::SetIcon( const BITMAP_OPAQUE* aIcon )
 {
@@ -144,7 +118,7 @@ void ACTION_MENU::DisplayTitle( bool aDisplay )
             Insert( 0, new wxMenuItem( this, wxID_NONE, m_title, wxEmptyString, wxITEM_NORMAL ) );
 
             if( m_icon )
-                set_wxMenuIcon( FindItemByPosition( 0 ), m_icon );
+                AddBitmapToMenuItem( FindItemByPosition( 0 ), KiBitmap( m_icon ) );
 
             m_titleDisplayed = true;
         }
@@ -157,7 +131,9 @@ wxMenuItem* ACTION_MENU::Add( const wxString& aLabel, int aId, const BITMAP_OPAQ
     wxASSERT_MSG( FindItem( aId ) == nullptr, "Duplicate menu IDs!" );
 
     wxMenuItem* item = new wxMenuItem( this, aId, aLabel, wxEmptyString, wxITEM_NORMAL );
-    set_wxMenuIcon( item, aIcon );
+
+    if( aIcon )
+        AddBitmapToMenuItem( item, KiBitmap( aIcon ) );
 
     return Append( item );
 }
@@ -169,7 +145,9 @@ wxMenuItem* ACTION_MENU::Add( const wxString& aLabel, const wxString& aTooltip,
     wxASSERT_MSG( FindItem( aId ) == nullptr, "Duplicate menu IDs!" );
 
     wxMenuItem* item = new wxMenuItem( this, aId, aLabel, aTooltip, wxITEM_NORMAL );
-    set_wxMenuIcon( item, aIcon );
+
+    if( aIcon )
+        AddBitmapToMenuItem( item, KiBitmap( aIcon ) );
 
     return Append( item );
 }
@@ -184,7 +162,8 @@ wxMenuItem* ACTION_MENU::Add( const TOOL_ACTION& aAction, bool aIsCheckmarkEntry
                                        aAction.GetDescription(),
                                        aIsCheckmarkEntry ? wxITEM_CHECK : wxITEM_NORMAL );
 
-    set_wxMenuIcon( item, icon );
+    if( icon )
+        AddBitmapToMenuItem( item, KiBitmap( icon ) );
 
     m_toolActions[getMenuId( aAction )] = &aAction;
 
@@ -203,7 +182,7 @@ wxMenuItem* ACTION_MENU::Add( ACTION_MENU* aMenu )
     if( aMenu->m_icon )
     {
         wxMenuItem* newItem = new wxMenuItem( this, -1, menuCopy->m_title );
-        set_wxMenuIcon( newItem, aMenu->m_icon );
+        AddBitmapToMenuItem( newItem, KiBitmap( aMenu->m_icon ) );
         newItem->SetSubMenu( menuCopy );
         return Append( newItem );
     }
@@ -502,11 +481,7 @@ wxMenuItem* ACTION_MENU::appendCopy( const wxMenuItem* aSource )
     wxMenuItem* newItem = new wxMenuItem( this, aSource->GetId(), aSource->GetItemLabel(),
                                           aSource->GetHelp(), aSource->GetKind() );
 
-    bool useImagesInMenus;
+    AddBitmapToMenuItem( newItem, aSource->GetBitmap() );
-    Pgm().CommonSettings()->Read( USE_ICONS_IN_MENUS_KEY, &useImagesInMenus );
-
-    if( aSource->GetKind() == wxITEM_NORMAL && useImagesInMenus )
-        newItem->SetBitmap( aSource->GetBitmap() );
 
     if( aSource->IsSubMenu() )
     {

common/tool/conditional_menu.cpp

@@ -25,6 +25,7 @@
 
 #include <tool/conditional_menu.h>
 #include <tool/action_menu.h>
+#include <menus_helpers.h>
 
 
 CONDITIONAL_MENU::CONDITIONAL_MENU( bool isContextMenu, TOOL_INTERACTIVE* aTool ) :
@@ -70,9 +71,9 @@ void CONDITIONAL_MENU::AddItem( int aId, const wxString& aText, const wxString&
     wxMenuItem* item = new wxMenuItem( nullptr, aId, aText, aTooltip, wxITEM_NORMAL );
 
     if( aIcon )
-        item->SetBitmap( KiBitmap( aIcon ) );
+        AddBitmapToMenuItem( item, KiBitmap( aIcon ) );
 
-    addEntry( ENTRY( item, aCondition, aOrder, false ) );
+    addEntry( ENTRY( item, aIcon, aCondition, aOrder, false ) );
 }
 
 
@@ -82,14 +83,10 @@ void CONDITIONAL_MENU::AddCheckItem( int aId, const wxString& aText, const wxStr
 {
     wxMenuItem* item = new wxMenuItem( nullptr, aId, aText, aTooltip, wxITEM_CHECK );
 
-#if !defined(__WXGTK__)  // wxGTK does not support bitmaps on checkable menu items
-
     if( aIcon )
-        item->SetBitmap( KiBitmap( aIcon ) );
+        AddBitmapToMenuItem( item, KiBitmap( aIcon ) );
 
-#endif
+    addEntry( ENTRY( item, aIcon, aCondition, aOrder, true ) );
-
-    addEntry( ENTRY( item, aCondition, aOrder, true ) );
 }
 
 
@@ -144,8 +141,15 @@ void CONDITIONAL_MENU::Evaluate( SELECTION& aSelection )
                 break;
 
             case ENTRY::WXITEM:
-                menuItem = Append( entry.wxItem()->GetId(), entry.wxItem()->GetItemLabel(),
+                menuItem = new wxMenuItem( this, entry.wxItem()->GetId(), entry.wxItem()->GetItemLabel(),
                                    entry.wxItem()->GetHelp(), entry.wxItem()->GetKind() );
+
+                if( entry.GetIcon() )
+                    AddBitmapToMenuItem( menuItem, KiBitmap( entry.GetIcon() ) );
+
+                // the wxMenuItem must be append only after the bitmap is set:
+                Append( menuItem );
+
                 menu_count++;
                 break;
 

include/tool/conditional_menu.h

@@ -117,7 +117,7 @@ private:
     public:
         ENTRY( const TOOL_ACTION* aAction, SELECTION_CONDITION aCondition, int aOrder,
                bool aCheckmark ) :
-            m_type( ACTION ),
+            m_type( ACTION ), m_icon(nullptr),
             m_condition( aCondition ),
             m_order( aOrder ),
             m_isCheckmarkEntry( aCheckmark )
@@ -126,7 +126,7 @@ private:
         }
 
         ENTRY( ACTION_MENU* aMenu, SELECTION_CONDITION aCondition, int aOrder ) :
-            m_type( MENU ),
+            m_type( MENU ), m_icon(nullptr),
             m_condition( aCondition ),
             m_order( aOrder ),
             m_isCheckmarkEntry( false )
@@ -134,8 +134,9 @@ private:
             m_data.menu = aMenu;
         }
 
-        ENTRY( wxMenuItem* aItem, SELECTION_CONDITION aCondition, int aOrder, bool aCheckmark ) :
+        ENTRY( wxMenuItem* aItem, const BITMAP_OPAQUE* aWxMenuBitmap,
-            m_type( WXITEM ),
+                SELECTION_CONDITION aCondition, int aOrder, bool aCheckmark ) :
+            m_type( WXITEM ), m_icon( aWxMenuBitmap ),
             m_condition( aCondition ),
             m_order( aOrder ),
             m_isCheckmarkEntry( aCheckmark )
@@ -145,7 +146,7 @@ private:
 
         // Separator
         ENTRY( SELECTION_CONDITION aCondition, int aOrder ) :
-            m_type( SEPARATOR ),
+            m_type( SEPARATOR ), m_icon(nullptr),
             m_condition( aCondition ),
             m_order( aOrder ),
             m_isCheckmarkEntry( false )
@@ -165,6 +166,11 @@ private:
             return m_type;
         }
 
+        inline const BITMAP_OPAQUE* GetIcon() const
+        {
+            return m_icon;
+        }
+
         inline const TOOL_ACTION* Action() const
         {
             assert( m_type == ACTION );
@@ -205,6 +211,7 @@ private:
 
     private:
         ENTRY_TYPE m_type;
+        const BITMAP_OPAQUE* m_icon;
 
         union {
             const TOOL_ACTION* action;