From da319a87cd26a0c38f198e9c59382921a6cf2510 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 27 Mar 2014 00:02:28 +0000 Subject: [PATCH] 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 | 1 + include/wx/msw/menuitem.h | 30 ++++++---------- src/msw/menu.cpp | 74 +++++++++++++++++++++------------------ src/msw/menuitem.cpp | 38 +++++++++++--------- 4 files changed, 72 insertions(+), 71 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index d4614c3acc..84a1af607b 100644 --- a/docs/changes.txt +++ b/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: diff --git a/include/wx/msw/menuitem.h b/include/wx/msw/menuitem.h index ad1380e22c..8444e6169c 100644 --- a/include/wx/msw/menuitem.h +++ b/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_bmpDisabled; + m_bmpUnchecked; // (checked is used also for 'uncheckable' items) +#if wxUSE_OWNER_DRAWN + wxBitmap m_bmpDisabled; #endif // wxUSE_OWNER_DRAWN // Give wxMenu access to our MSWMustUseOwnerDrawn() and GetHBitmapForMenu(). diff --git a/src/msw/menu.cpp b/src/msw/menu.cpp index fd941cecd2..3101620146 100644 --- a/src/msw/menu.cpp +++ b/src/msw/menu.cpp @@ -62,9 +62,7 @@ // other standard headers #include -#if wxUSE_OWNER_DRAWN - #include "wx/dynlib.h" -#endif +#include "wx/dynlib.h" #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() ) - { -#ifndef __DMC__ +#endif // wxUSE_OWNER_DRAWN - if ( !m_ownerDrawn && !pItem->IsSeparator() ) - { - // use InsertMenuItem() if possible as it's guaranteed to look - // correct while our owner-drawn code is not - if ( !pItem->MSWMustUseOwnerDrawn() ) + if ( +#if wxUSE_OWNER_DRAWN + !pItem->IsOwnerDrawn() && +#endif + !pItem->IsSeparator() ) { +#if wxUSE_MENUITEMINFO WinStruct 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)")); } } - - // tell the item that it's not really owner-drawn but only - // needs to draw its bitmap, the rest is done by Windows - pItem->SetOwnerDrawn(false); } - } +#else + if ( pItem->GetBitmap().IsOk() ) + { + flags |= MF_BITMAP; + pData = reinterpret_cast(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 } -#endif // __DMC__ - if ( !ok ) +#if wxUSE_OWNER_DRAWN + if ( pItem->IsOwnerDrawn() || makeItemOwnerDrawn ) { // 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(pItem), TRUE); } diff --git a/src/msw/menuitem.cpp b/src/msw/menuitem.cpp index 0d0a4c0c36..5577320ea7 100644 --- a/src/msw/menuitem.cpp +++ b/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 - // support instead of making the item owner-drawn - SetOwnerDrawn(true); - if ( MSWMustUseOwnerDrawn() ) + { + SetOwnerDrawn(true); + + // 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 // ----------------------------------------------------------------------------