Refixed wxWidget Patch: menu items with icon not correctly aligned (since wx263)

[stahta01]

just keep on trying, it's a bug in the menu code [again menu code], but probably inside CB, see another thread [http://forums.codeblocks.org/index.php?topic=4754.0], with any plug-in.

I got it to turn off, I submitted a partial fix as a patch for the plugin disable issue also.

Tim S

[Pecan]

If you would like to try an experiment with keybinder:
At menuutils.cpp line 190 (keybinder) there are two RebuildMenuItem() routines.
The second one was the one that solved the missing icon problem in wx2.6.2 .

With your "fixed" wxWidgets, comment out the first RebuildMenuItem and uncomment the second to see if it solves the problem.

It may cause a crash since it hasnt been used/updated in a year, but it'll tell us if the missing icons are fixed.

Code

}//Update
// ----------------------------------------------------------------------------
// RebuildMenuitem
// ----------------------------------------------------------------------------
wxMenuItem* wxMenuCmd::RebuildMenuitem(wxMenuItem* pMnuItem)
{//+v0.4.25 WXMSW
   // Since wxWidgets 2.6.3, we don't have to rebuild the menuitem
   // to preserve the bitmapped menu icon.
    return pMnuItem;

}//RebuildMenuitem
// ----------------------------------------------------------------------------
// The following routine was used when wxWidgets would not SetText()
// without clobbering the menu Bitmap icon
// ----------------------------------------------------------------------------
//wxMenuItem* wxMenuCmd::RebuildMenuitem(wxMenuItem* pMnuItem)
//{//+v0.4.6 WXMSW
//	// ---------------------------------------------------------------
//	//  Do it the slow/hard way, remove and delete the menu item
//	// ---------------------------------------------------------------
//    wxMenu* pMenu = pMnuItem->GetMenu();
//    wxMenuItemList items = pMenu->GetMenuItems();
//    int pos = items.IndexOf(pMnuItem);
//   // rebuild the menuitem
//    wxMenuItem* pnewitem = new wxMenuItem(pMenu, m_nId, pMnuItem->GetText(),
//                pMnuItem->GetHelp(), pMnuItem->GetKind(),
//                pMnuItem->GetSubMenu() );
//    pnewitem->SetBitmap(pMnuItem->GetBitmap() );
//    pnewitem->SetFont(pMnuItem->GetFont() );
//    // remove the menuitem
//    pMenu->Destroy(pMnuItem);
//    // update keybinder array menu item pointer
//    m_pItem = pnewitem;
//    // put the menuitem back on the menu
//    pMenu->Insert(pos, pnewitem);
//    return pnewitem;
//
//}//RebuildMenuitem
#endif //#if defined( __WXMSW__ )
// ----------------------------------------------------------------------------

[stahta01]

If you would like to try an experiment with keybinder:
At menuutils.cpp line 190 (keybinder) there are two RebuildMenuItem() routines.
The second one was the one that solved the missing icon problem in wx2.6.2 .

With your "fixed" wxWidgets, comment out the first RebuildMenuItem and uncomment the second to see if it solves the problem.

It may cause a crash since it hasnt been used/updated in a year, but it'll tell us if the missing icons are fixed.

This was the right way to fix this as a work around, but we might wish to find the cause, I think the cause is in wxWidgets MenuItem class.

I added code to the method RebuildMenuItem and it helps under 2.8.0, I am compiling it for 2.6.3.4 now and see if it works. Tim S

[stahta01]

I think I have found a patch to wxWidgets that fixes the issue. I will have to re-test to be sure.
The re-test still has the bug where keybinder removes 90% of the icons.

Tim S

[stahta01]

Here's a snippet of the keybinder logging under debug and kbLOGGING, does it give any information or is it unrelated to anything. Tim S

Code

01:59:08: wxMenuCmd::Update - Removing shortcuts [Continue	Ctrl-F7] for [Continue]
01:59:08: wxMenuCmd::Update - Setting shortcuts for [Continue	Ctrl-F7]
01:59:08: wxMenuCmd::Update - Removing shortcuts [Toggle breakpoint	F5] for [Toggle breakpoint]
01:59:08: wxMenuCmd::Update - Setting shortcuts for [Toggle breakpoint	F5]
01:59:08: CreateNew()UnMatched id:1413 name:Edit startup script
01:59:08: Debug: keybinder.cpp(602): assert "ret" failed.
01:59:18: wxMenuCmd::Update - Removing shortcuts [Goto file...	Alt-G] for [Goto file...]
01:59:18: wxMenuCmd::Update - Setting shortcuts for [Goto file...	Alt-G]
01:59:18: wxMenuCmd::Update - Removing shortcuts [Activate next project	Alt-F6] for [Activate next project]
01:59:18: wxMenuCmd::Update - Setting shortcuts for [Activate next project	Alt-F6]
01:59:18: wxMenuCmd::Update - Removing shortcuts [Activate prior project	Alt-F5] for [Activate prior project]

[stahta01]

I added code to the method RebuildMenuItem and it helps under 2.8.0, I am compiling it for 2.6.3.4 now and see if it works. Tim S

It works for me under 2.6.3 with my menu items alignment patch applied to it.

[ Patch #1777 ] keybinder RebuildMenuitem patch
https://developer.berlios.de/patch/?func=detailpatch&patch_id=1777&group_id=5358

Tim S

[Pecan]

Here's a snippet of the keybinder logging under debug and kbLOGGING, does it give any information or is it unrelated to anything. Tim S

Code

01:59:08: wxMenuCmd::Update - Removing shortcuts [Continue	Ctrl-F7] for [Continue]
01:59:08: wxMenuCmd::Update - Setting shortcuts for [Continue	Ctrl-F7]
01:59:08: wxMenuCmd::Update - Removing shortcuts [Toggle breakpoint	F5] for [Toggle breakpoint]
01:59:08: wxMenuCmd::Update - Setting shortcuts for [Toggle breakpoint	F5]
01:59:08: CreateNew()UnMatched id:1413 name:Edit startup script
01:59:08: Debug: keybinder.cpp(602): assert "ret" failed.
01:59:18: wxMenuCmd::Update - Removing shortcuts [Goto file...	Alt-G] for [Goto file...]
01:59:18: wxMenuCmd::Update - Setting shortcuts for [Goto file...	Alt-G]
01:59:18: wxMenuCmd::Update - Removing shortcuts [Activate next project	Alt-F6] for [Activate next project]
01:59:18: wxMenuCmd::Update - Setting shortcuts for [Activate next project	Alt-F6]
01:59:18: wxMenuCmd::Update - Removing shortcuts [Activate prior project	Alt-F5] for [Activate prior project]

These messages are correct. keybinder can have multiple profiles, so if the user switches to a secondary profile, the shortcuts must be removed and replaced with those of the secondary profile; else the old shortcuts will "leak" into the new chosen profile.

So the idea here is that when any profile is applied, remove the current shortcuts and replace them with those from the selected profile (previously recorded on disk in cbKeyBinder10.ini) . This applies to empty menuItems also. So if the user deleted a shortcut and saved it to disk, it gets replace with a "blank" shortcut when the profile is applied.

This of course, clobbers the icon in 2.6.2, but not 2.6.3 .

Re: The original problem with missing icons. Noone could change the text/label/shortcut of a menuitem without clobbering the icon. So the whole damn menuitem had to be rebuilt.

This also mean that anyone storing a pointer to the rebuilt menuitem is in deep doo. Because their pointer is now invalid.

You probably saw that in the code. I don't mean to insult your intelligence.

[Pecan]

I added code to the method RebuildMenuItem and it helps under 2.8.0, I am compiling it for 2.6.3.4 now and see if it works. Tim S

It works for me under 2.6.3 with my menu items alignment patch applied to it.

[ Patch #1777 ] keybinder RebuildMenuitem patch
https://developer.berlios.de/patch/?func=detailpatch&patch_id=1777&group_id=5358

Tim S

Sorry, I don't get it. That patch is to menu.cpp . I thought you said it was a keybinder RebuildMenuItem patch.

[stahta01]

Look at the Raw Patch View

https://developer.berlios.de/patch/download.php?id=1777

Tim S

[killerbot]

damn berlios, can't get to the raw patch :-(

[stahta01]

damn berlios, can't get to the raw patch :-(

Code

Index: src/plugins/contrib/keybinder/menuutils.cpp
===================================================================
--- src/plugins/contrib/keybinder/menuutils.cpp	(revision 3442)
+++ src/plugins/contrib/keybinder/menuutils.cpp	(working copy)
@@ -190,40 +190,54 @@
 // ----------------------------------------------------------------------------
 // RebuildMenuitem
 // ----------------------------------------------------------------------------
-wxMenuItem* wxMenuCmd::RebuildMenuitem(wxMenuItem* pMnuItem)
-{//+v0.4.25 WXMSW
-   // Since wxWidgets 2.6.3, we don't have to rebuild the menuitem
-   // to preserve the bitmapped menu icon.
-    return pMnuItem;
-
-}//RebuildMenuitem
+//wxMenuItem* wxMenuCmd::RebuildMenuitem(wxMenuItem* pMnuItem)
+//{//+v0.4.25 WXMSW
+//   // Since wxWidgets 2.6.3, we don't have to rebuild the menuitem
+//   // to preserve the bitmapped menu icon.
+//    return pMnuItem;
+//
+//}//RebuildMenuitem
 // ----------------------------------------------------------------------------
 // The following routine was used when wxWidgets would not SetText()
 // without clobbering the menu Bitmap icon
 // ----------------------------------------------------------------------------
-//wxMenuItem* wxMenuCmd::RebuildMenuitem(wxMenuItem* pMnuItem)
-//{//+v0.4.6 WXMSW
-//	// ---------------------------------------------------------------
-//	//  Do it the slow/hard way, remove and delete the menu item
-//	// ---------------------------------------------------------------
-//    wxMenu* pMenu = pMnuItem->GetMenu();
-//    wxMenuItemList items = pMenu->GetMenuItems();
-//    int pos = items.IndexOf(pMnuItem);
-//   // rebuild the menuitem
-//    wxMenuItem* pnewitem = new wxMenuItem(pMenu, m_nId, pMnuItem->GetText(),
-//                pMnuItem->GetHelp(), pMnuItem->GetKind(),
-//                pMnuItem->GetSubMenu() );
-//    pnewitem->SetBitmap(pMnuItem->GetBitmap() );
-//    pnewitem->SetFont(pMnuItem->GetFont() );
-//    // remove the menuitem
-//    pMenu->Destroy(pMnuItem);
-//    // update keybinder array menu item pointer
-//    m_pItem = pnewitem;
-//    // put the menuitem back on the menu
-//    pMenu->Insert(pos, pnewitem);
-//    return pnewitem;
-//
-//}//RebuildMenuitem
+wxMenuItem* wxMenuCmd::RebuildMenuitem(wxMenuItem* pMnuItem)
+{   //+v0.4.6 WXMSW
+	// ---------------------------------------------------------------
+	//  Do it the slow/hard way, remove and delete the menu item
+	// ---------------------------------------------------------------
+    wxMenu* pMenu = pMnuItem->GetMenu();
+    wxMenuItemList items = pMenu->GetMenuItems();
+    int pos = items.IndexOf(pMnuItem);
+    // rebuild the menuitem
+    wxMenuItem* pnewitem = new wxMenuItem(pMenu, m_nId, pMnuItem->GetText(),
+                pMnuItem->GetHelp(), pMnuItem->GetKind(),
+                pMnuItem->GetSubMenu() );
+    pnewitem->SetBitmap(pMnuItem->GetBitmap() );
+    pnewitem->SetFont(pMnuItem->GetFont() );
+#if wxUSE_OWNER_DRAWN
+    if ( pMnuItem->IsOwnerDrawn() )
+    {
+        pnewitem->SetOwnerDrawn(true);
+        pnewitem->SetMarginWidth(pMnuItem->GetMarginWidth());
+        pnewitem->SetDisabledBitmap(pMnuItem->GetDisabledBitmap());
+        if (pMnuItem->IsCheckable())
+        {
+            pnewitem->SetCheckable(true);
+            pnewitem->SetBitmaps(pMnuItem->GetBitmap(true), pMnuItem->GetBitmap(false));
+        }
+    }
+#endif
+
+    // remove the menuitem
+    pMenu->Destroy(pMnuItem);
+    // update keybinder array menu item pointer
+    m_pItem = pnewitem;
+    // put the menuitem back on the menu
+    pMenu->Insert(pos, pnewitem);
+    return pnewitem;
+
+}//RebuildMenuitem
 #endif //#if defined( __WXMSW__ )
 // ----------------------------------------------------------------------------
 bool wxMenuCmd::IsNumericMenuItem(wxMenuItem* pwxMenuItem)   //v0.2

Tim S

[Pecan]

Look at the Raw Patch View

https://developer.berlios.de/patch/download.php?id=1777

Tim S

Applied (w/verbage) SVN 3443.

I don't know what you want done with the menu.cpp patch.
I'll leave that to Lieven.

[stahta01]

I don't know what you want done with the menu.cpp patch.
I'll leave that to Lieven.

Sorry, that is just in case someone wanted to test it, and see it fixes the icon alignment bug.
Tim S

[killerbot]

yes, it works nicely. Well done guys.

[stahta01]

The wxWidgets people have not done any thing yet, so I am starting work on the patch submission process.
I am looking at the wxWidgets sample going to try to compile and verify problem exists with ownerdrw sample. If anyone has a better sample to demo the bug please email me which one is better. Tim S