|
Patch for OS X Native Menus |
Todd Cope
Member #998
November 2000
|
A few weeks ago I brought up an issue with the native menus on the Mac OS X port of Allegro. I had some time to look into it today and was able to get it working. The previous implementation made an invalid assumption about the order of menu creation. Because of this, menus created manually with al_create_menu() and related functions had a high likelihood of causing a crash. To fix this, I've changed the way the menus are created. Instead of hopping around in the vector of menus, I simply recursively go over the menus and add items and sub-menus as I come to them. I've attached a patch with the new implementation. The patch also includes a few formatting fixes. |
SiegeLord
Member #7,827
October 2006
|
Does this still imply the changes in semantics of al_set_display_menu like that old patch did? How does this relate to that old patch? Sorry the old patch didn't get applied right away... "For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
Todd Cope
Member #998
November 2000
|
No, I started over with what was already there on Github. My old patch didn't work with nested menus. I didn't want to mess with the other issues yet. I figure they are separate issues that will require additional patching. |
SiegeLord
Member #7,827
October 2006
|
I tried your patch on my OSX 10.8 with that example code from the other thread (I paste it below for convenience), and while it fixed the crash, I didn't actually see it create the menu either. Does it work on your end? 1#include <allegro5/allegro5.h>
2#include <allegro5/allegro_native_dialog.h>
3
4int main(int argc, char * argv[])
5{
6 ALLEGRO_DISPLAY * display;
7 ALLEGRO_MENU * test_menu;
8
9 if(!al_init())
10 {
11 return -1;
12 }
13 if(!al_init_native_dialog_addon())
14 {
15 return -1;
16 }
17 al_set_new_display_flags(ALLEGRO_GTK_TOPLEVEL);
18 display = al_create_display(640, 480);
19 if(!display)
20 {
21 return -1;
22 }
23 test_menu = al_create_menu();
24 if(!test_menu)
25 {
26 return -1;
27 }
28 al_append_menu_item(test_menu, "Item 1", 1, 0, NULL, NULL);
29 al_append_menu_item(test_menu, "Item 2", 2, 0, NULL, NULL);
30 al_append_menu_item(test_menu, "Item 3", 3, 0, NULL, NULL);
31 al_append_menu_item(test_menu, "Item 4", 4, 0, NULL, NULL);
32 al_append_menu_item(test_menu, "Item 5", 5, 0, NULL, NULL);
33 al_append_menu_item(test_menu, "Item 6", 6, 0, NULL, NULL);
34 if(!al_set_display_menu(display, test_menu))
35 {
36 return -1;
37 }
38 al_rest(1.0);
39 al_set_display_menu(display, NULL);
40 return 0;
41}
"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
Todd Cope
Member #998
November 2000
|
That's interesting. It works fine with the example program and my own programs that use it... Just tested and it does work if I add a sub menu to test_menu. I wonder if the main menu can have items that don't lead to sub menus on OS X. Append: The documentation only talks about main menu items that lead to sub menus. It shouldn't be a problem. I don't think I've ever used a program that had a menu option on the main menu that didn't have a sub menu. The test program was just something I whipped up to demonstrate the crashing issue. Maybe it should be stated in Allegro's documentation that top level items in a non-popup menu are required to have sub menus or the behavior is undefined. |
SiegeLord
Member #7,827
October 2006
|
Alright, that sounds fine. I've committed this patch, thanks! "For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
|