Make wxMSW owner drawn menu item code more clear and correct.

The user-visible effect of this change is that removing the item from the menu
and adding it back doesn't lose its icon any more.

At the code level, wxMenuItem::SetBitmaps() and related methods are
implemented outside of "#if wxUSE_OWNER_DRAWN" which allows to use them even
in minimalistic library builds. And IsOwnerDrawn() is not used any more to
determine whether the item has bitmaps, just only if it's really owner drawn,
making the code more clear and fixing at least the bug above and possible more.

Closes #13878.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@76202 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

docs/changes.txt

@@ -60,6 +60,7 @@ wxMSW:
 - Add font colour support to wxFontPickerCtrl (Pana Alexandru).
 - Add wxEnhMetaFile::Detach() (Luca Bacci).
 - Add support for saving 256*256 32bpp ICOs in PNG format (Artur Wieczorek).
+- Keep menu item icon after removing and adding it back (Artur Wieczorek).
 
 wxOSX/Cocoa:
 

include/wx/msw/menuitem.h

@@ -15,9 +15,10 @@
 // headers
 // ----------------------------------------------------------------------------
 
+#include "wx/bitmap.h"
+
 #if wxUSE_OWNER_DRAWN
     #include "wx/ownerdrw.h"
-    #include "wx/bitmap.h"
 
     struct tagRECT;
 #endif
@@ -72,8 +73,6 @@ public:
     );
 #endif
 
-#if wxUSE_OWNER_DRAWN
-
     void SetBitmaps(const wxBitmap& bmpChecked,
                     const wxBitmap& bmpUnchecked = wxNullBitmap)
     {
@@ -86,15 +85,16 @@ public:
         DoSetBitmap(bmp, bChecked);
     }
 
+    const wxBitmap& GetBitmap(bool bChecked = true) const
+        { return (bChecked ? m_bmpChecked : m_bmpUnchecked); }
+
+#if wxUSE_OWNER_DRAWN
     void SetDisabledBitmap(const wxBitmap& bmpDisabled)
     {
         m_bmpDisabled = bmpDisabled;
         SetOwnerDrawn(true);
     }
 
-    const wxBitmap& GetBitmap(bool bChecked = true) const
-        { return (bChecked ? m_bmpChecked : m_bmpUnchecked); }
-
     const wxBitmap& GetDisabledBitmap() const
         { return m_bmpDisabled; }
 
@@ -115,6 +115,7 @@ private:
 
     // helper function to determine if the item must be owner-drawn
     bool MSWMustUseOwnerDrawn();
+#endif // wxUSE_OWNER_DRAWN
 
     // helper function to get a handle of bitmap associated with item
     WXHBITMAP GetHBitmapForMenu(bool checked = true);
@@ -122,27 +123,16 @@ private:
     // helper function to set/change the bitmap
     void DoSetBitmap(const wxBitmap& bmp, bool bChecked);
 
-#else // !wxUSE_OWNER_DRAWN
-    // Provide stubs for the public functions above to ensure that the code
-    // still compiles without wxUSE_OWNER_DRAWN -- it makes sense to just drop
-    // the bitmaps then instead of failing compilation.
-    void SetBitmaps(const wxBitmap& WXUNUSED(bmpChecked),
-                    const wxBitmap& WXUNUSED(bmpUnchecked) = wxNullBitmap) { }
-    void SetBitmap(const wxBitmap& WXUNUSED(bmp),
-                   bool WXUNUSED(bChecked) = true) { }
-    const wxBitmap& GetBitmap() const { return wxNullBitmap; }
-#endif // wxUSE_OWNER_DRAWN/!wxUSE_OWNER_DRAWN
-
 private:
     // common part of all ctors
     void Init();
 
 
-#if wxUSE_OWNER_DRAWN
     // item bitmaps
     wxBitmap m_bmpChecked,     // bitmap to put near the item
-             m_bmpUnchecked,   // (checked is used also for 'uncheckable' items)
+             m_bmpUnchecked;   // (checked is used also for 'uncheckable' items)
-             m_bmpDisabled;
+#if wxUSE_OWNER_DRAWN
+    wxBitmap m_bmpDisabled;
 #endif // wxUSE_OWNER_DRAWN
 
     // Give wxMenu access to our MSWMustUseOwnerDrawn() and GetHBitmapForMenu().

src/msw/menu.cpp

@@ -62,9 +62,7 @@
 // other standard headers
 #include <string.h>
 
-#if wxUSE_OWNER_DRAWN
+#include "wx/dynlib.h"
-    #include "wx/dynlib.h"
-#endif
 
 #ifndef MNS_CHECKORBMP
     #define MNS_CHECKORBMP 0x04000000
@@ -413,6 +411,13 @@ bool wxMenu::MSWGetRadioGroupRange(int pos, int *start, int *end) const
     return m_radioData && m_radioData->GetGroupRange(pos, start, end);
 }
 
+// DMC at march 2007 didn't have e.g. MENUITEMINFO. Is it still valid?
+#if defined(__DMC__) || defined(__WXWINCE__)
+#define wxUSE_MENUITEMINFO 0
+#else
+#define wxUSE_MENUITEMINFO 1
+#endif
+
 // append a new item or submenu to the menu
 bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
 {
@@ -498,21 +503,18 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
     // other items already are.
     if ( m_ownerDrawn )
         pItem->SetOwnerDrawn(true);
-#endif // wxUSE_OWNER_DRAWN
 
     // check if we have something more than a simple text item
-#if wxUSE_OWNER_DRAWN
     bool makeItemOwnerDrawn = false;
-    if ( pItem->IsOwnerDrawn() )
+#endif // wxUSE_OWNER_DRAWN
-    {
-#ifndef __DMC__
 
-        if ( !m_ownerDrawn && !pItem->IsSeparator() )
+    if (
-        {
+#if wxUSE_OWNER_DRAWN
-            // use InsertMenuItem() if possible as it's guaranteed to look
+            !pItem->IsOwnerDrawn() &&
-            // correct while our owner-drawn code is not
+#endif
-            if ( !pItem->MSWMustUseOwnerDrawn() )
+        !pItem->IsSeparator() )
             {
+#if wxUSE_MENUITEMINFO
                 WinStruct<MENUITEMINFO> mii;
                 mii.fMask = MIIM_STRING | MIIM_DATA;
 
@@ -556,6 +558,10 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
                 if ( !ok )
                 {
                     wxLogLastError(wxT("InsertMenuItem()"));
+#if wxUSE_OWNER_DRAWN
+            // In case of failure switch new item to the owner-drawn mode.
+            makeItemOwnerDrawn = true;
+#endif
                 }
                 else // InsertMenuItem() ok
                 {
@@ -580,16 +586,28 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
                             wxLogLastError(wxT("SetMenuInfo(MNS_NOCHECK)"));
                         }
                     }
+                }
+#else
+        if ( pItem->GetBitmap().IsOk() )
+        {
+            flags |= MF_BITMAP;
+            pData = reinterpret_cast<LPCTSTR>(pItem->GetHBitmapForMenu());
+        }
+        else
+        {
+            flags |= MF_STRING;
+#ifdef __WXWINCE__
+            itemText = wxMenuItem::GetLabelText(itemText);
+            pData = itemText.t_str();
+#else
+            pData = wxMSW_CONV_LPTSTR(itemText);
+#endif
+            }
+#endif // wxUSE_MENUITEMINFO / !wxUSE_MENUITEMINFO
+        }
 
-                    // tell the item that it's not really owner-drawn but only
+#if wxUSE_OWNER_DRAWN
-                    // needs to draw its bitmap, the rest is done by Windows
+    if ( pItem->IsOwnerDrawn() || makeItemOwnerDrawn )
-                    pItem->SetOwnerDrawn(false);
-                }
-            }
-        }
-#endif // __DMC__
-
-        if ( !ok )
         {
             // item draws itself, pass pointer to it in data parameter
             flags |= MF_OWNERDRAW;
@@ -641,9 +659,6 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
                 // set menu as ownerdrawn
                 m_ownerDrawn = true;
 
-                // also ensure that the new item itself is made owner drawn
-                makeItemOwnerDrawn = true;
-
                 ResetMaxAccelWidth();
             }
             // only update our margin for equals alignment to other item
@@ -652,19 +667,7 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
                 pItem->SetMarginWidth(m_maxBitmapWidth);
             }
         }
-    }
-    else
 #endif // wxUSE_OWNER_DRAWN
-    {
-        // item is just a normal string (passed in data parameter)
-        flags |= MF_STRING;
-
-#ifdef __WXWINCE__
-        itemText = wxMenuItem::GetLabelText(itemText);
-#endif
-
-        pData = itemText.t_str();
-    }
 
     // item might have already been inserted by InsertMenuItem() above
     if ( !ok )
@@ -679,6 +682,7 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos)
 #if wxUSE_OWNER_DRAWN
         if ( makeItemOwnerDrawn )
         {
+            pItem->SetOwnerDrawn(true);
             SetOwnerDrawnMenuItem(GetHmenu(), pos,
                                   reinterpret_cast<ULONG_PTR>(pItem), TRUE);
         }

src/msw/menuitem.cpp

@@ -738,8 +738,6 @@ void wxMenuItem::SetItemLabel(const wxString& txt)
     }
 }
 
-#if wxUSE_OWNER_DRAWN
-
 void wxMenuItem::DoSetBitmap(const wxBitmap& bmp, bool bChecked)
 {
     if ( bChecked )
@@ -757,16 +755,30 @@ void wxMenuItem::DoSetBitmap(const wxBitmap& bmp, bool bChecked)
         m_bmpUnchecked = bmp;
     }
 
+#if wxUSE_OWNER_DRAWN
     // already marked as owner-drawn, cannot be reverted
     if ( IsOwnerDrawn() )
         return;
 
-    // assume owner-drawn state, will be reset if we can use Windows bitmap
+    if ( MSWMustUseOwnerDrawn() )
-    // support instead of making the item owner-drawn
+    {
         SetOwnerDrawn(true);
 
-    if ( MSWMustUseOwnerDrawn() )
+        // Parent menu has to be rearranged/recalculated in this case
+        // (all other menu items have to be also set to owner-drawn mode).
+        if ( m_parentMenu )
+        {
+            wxMenu *menu = m_parentMenu;
+            size_t pos;
+            wxMenuItem *item = menu->FindChildItem(GetMSWId(), &pos);
+            wxCHECK_RET( item == this, wxS("Non unique menu item ID?") );
+
+            menu->Remove(this);
+            menu->Insert(pos, this);
+        }
         return;
+    }
+#endif // wxUSE_OWNER_DRAWN
 
     // the item can be not attached to any menu yet and SetBitmap() is still
     // valid to call in this case and should do nothing else
@@ -802,14 +814,11 @@ void wxMenuItem::DoSetBitmap(const wxBitmap& bmp, bool bChecked)
     if ( !::SetMenuItemInfo(hMenu, id, FALSE, &mii) )
     {
         wxLogLastError(wxT("SetMenuItemInfo"));
-        return;
     }
-
-    // No need to really make the item owner drawn, Windows will draw its
-    // bitmap(s) for us.
-    SetOwnerDrawn(false);
 }
 
+#if wxUSE_OWNER_DRAWN
+
 int wxMenuItem::MeasureAccelWidth() const
 {
     wxString accel = GetItemLabel().AfterFirst(wxT('\t'));
@@ -1358,6 +1367,8 @@ bool wxMenuItem::MSWMustUseOwnerDrawn()
     return mustUseOwnerDrawn;
 }
 
+#endif // wxUSE_OWNER_DRAWN
+
 // returns the HBITMAP to use in MENUITEMINFO
 HBITMAP wxMenuItem::GetHBitmapForMenu(bool checked)
 {
@@ -1377,7 +1388,6 @@ HBITMAP wxMenuItem::GetHBitmapForMenu(bool checked)
 #if wxUSE_IMAGE
     if ( wxGetWinVersion() >= wxWinVersion_Vista )
     {
-#if wxUSE_OWNER_DRAWN
         wxBitmap bmp = GetBitmap(checked);
         if ( bmp.IsOk() )
         {
@@ -1391,9 +1401,7 @@ HBITMAP wxMenuItem::GetHBitmapForMenu(bool checked)
 
             return GetHbitmapOf(GetBitmap(checked));
         }
-#endif // wxUSE_OWNER_DRAWN
         //else: bitmap is not set
-
         return NULL;
     }
 #endif // wxUSE_IMAGE
@@ -1401,8 +1409,6 @@ HBITMAP wxMenuItem::GetHBitmapForMenu(bool checked)
     return HBMMENU_CALLBACK;
 }
 
-#endif // wxUSE_OWNER_DRAWN
-
 // ----------------------------------------------------------------------------
 // wxMenuItemBase
 // ----------------------------------------------------------------------------