Skip to content

Commit 74b130d

Browse files
committed
Refactor menu event dispatch to use direct C++ back-pointers
1 parent 07684e8 commit 74b130d

File tree

1 file changed

+33
-62
lines changed

1 file changed

+33
-62
lines changed

src/platform/macos/menu_macos.mm

Lines changed: 33 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@
1313

1414
// Forward declarations - moved to global scope
1515
@interface MenuItemTarget : NSObject
16-
@property (nonatomic, assign) nativeapi::MenuItemID itemId;
16+
@property (nonatomic, assign) void* cppMenuItem;
1717
- (void)menuItemClicked:(id)sender;
1818
@end
1919

2020
@interface MenuDelegate : NSObject <NSMenuDelegate>
21-
@property (nonatomic, assign) nativeapi::MenuID menuId;
21+
@property (nonatomic, assign) void* cppMenu;
2222
@end
2323

2424
namespace nativeapi {
@@ -27,9 +27,7 @@ @interface MenuDelegate : NSObject <NSMenuDelegate>
2727
static std::atomic<MenuItemID> g_next_menu_item_id{1};
2828
static std::atomic<MenuID> g_next_menu_id{1};
2929

30-
// Global registry to map native objects to C++ objects for event emission
31-
static std::unordered_map<MenuItemID, MenuItem*> g_menu_item_registry;
32-
static std::unordered_map<MenuID, Menu*> g_menu_registry;
30+
// Removed global registries; events are dispatched via direct back-pointers
3331

3432
// Helper function to convert KeyboardAccelerator to NSString and modifier mask
3533
std::pair<NSString*, NSUInteger> ConvertAccelerator(const KeyboardAccelerator& accelerator) {
@@ -92,14 +90,11 @@ @interface MenuDelegate : NSObject <NSMenuDelegate>
9290
@implementation MenuItemTarget
9391
- (void)menuItemClicked:(id)sender {
9492
NSMenuItem* menuItem = (NSMenuItem*)sender;
95-
96-
// Find the MenuItem object in registry
97-
auto menuItemIt = nativeapi::g_menu_item_registry.find(_itemId);
98-
if (menuItemIt == nativeapi::g_menu_item_registry.end()) {
99-
return; // Item not found in registry
93+
// Use direct back-pointer to the C++ MenuItem
94+
nativeapi::MenuItem* menuItemPtr = static_cast<nativeapi::MenuItem*>(_cppMenuItem);
95+
if (!menuItemPtr) {
96+
return;
10097
}
101-
102-
nativeapi::MenuItem* menuItemPtr = menuItemIt->second;
10398
nativeapi::MenuItemType itemType = menuItemPtr->GetType();
10499

105100
// Don't automatically handle state changes - let user code control the state
@@ -113,19 +108,13 @@ - (void)menuItemClicked:(id)sender {
113108
// Implementation of MenuDelegate - moved to global scope
114109
@implementation MenuDelegate
115110
- (void)menuWillOpen:(NSMenu *)menu {
116-
auto menuIt = nativeapi::g_menu_registry.find(_menuId);
117-
if (menuIt != nativeapi::g_menu_registry.end()) {
118-
nativeapi::Menu* menuPtr = menuIt->second;
119-
menuPtr->EmitOpenedEvent();
120-
}
111+
nativeapi::Menu* menuPtr = static_cast<nativeapi::Menu*>(_cppMenu);
112+
if (menuPtr) menuPtr->EmitOpenedEvent();
121113
}
122114

123115
- (void)menuDidClose:(NSMenu *)menu {
124-
auto menuIt = nativeapi::g_menu_registry.find(_menuId);
125-
if (menuIt != nativeapi::g_menu_registry.end()) {
126-
nativeapi::Menu* menuPtr = menuIt->second;
127-
menuPtr->EmitClosedEvent();
128-
}
116+
nativeapi::Menu* menuPtr = static_cast<nativeapi::Menu*>(_cppMenu);
117+
if (menuPtr) menuPtr->EmitClosedEvent();
129118
}
130119
@end
131120

@@ -200,12 +189,9 @@ - (void)menuDidClose:(NSMenu *)menu {
200189
auto item = std::shared_ptr<MenuItem>(new MenuItem(text, type));
201190
item->pimpl_ = std::make_unique<Impl>(nsItem, type);
202191
item->id = g_next_menu_item_id++;
203-
item->pimpl_->target_.itemId = item->id;
192+
[item->pimpl_->target_ setCppMenuItem:(void*)item.get()];
204193
item->pimpl_->text_ = text;
205194

206-
// Register the MenuItem for event emission
207-
g_menu_item_registry[item->id] = item.get();
208-
209195
return item;
210196
}
211197

@@ -216,25 +202,15 @@ - (void)menuDidClose:(NSMenu *)menu {
216202
MenuItem::MenuItem(void* native_item)
217203
: id(g_next_menu_item_id++)
218204
, pimpl_(std::make_unique<Impl>((__bridge NSMenuItem*)native_item, MenuItemType::Normal)) {
219-
pimpl_->target_.itemId = id;
220-
// Register the MenuItem for event emission
221-
g_menu_item_registry[id] = this;
205+
[pimpl_->target_ setCppMenuItem:(void*)this];
222206
}
223207

224208
MenuItem::MenuItem(const std::string& text, MenuItemType type)
225209
: id(0) { // Will be set in Create method
226210
}
227211

228212
MenuItem::~MenuItem() {
229-
// Safely remove from global registry
230-
try {
231-
auto it = g_menu_item_registry.find(id);
232-
if (it != g_menu_item_registry.end()) {
233-
g_menu_item_registry.erase(it);
234-
}
235-
} catch (...) {
236-
// Ignore exceptions during cleanup - the registry may be destroyed during shutdown
237-
}
213+
// No global registry cleanup required
238214
}
239215

240216
MenuItemType MenuItem::GetType() const {
@@ -373,15 +349,23 @@ - (void)menuDidClose:(NSMenu *)menu {
373349
}
374350
[pimpl_->ns_menu_item_ setState:nsState];
375351

376-
// Handle radio button group logic - uncheck others in the same group
352+
// Handle radio button group logic - uncheck siblings in the same NSMenu
377353
if (pimpl_->type_ == MenuItemType::Radio && state == MenuItemState::Checked && pimpl_->radio_group_ >= 0) {
378-
for (auto& pair : g_menu_item_registry) {
379-
MenuItem* otherItem = pair.second;
380-
if (otherItem != this &&
381-
otherItem->GetType() == MenuItemType::Radio &&
382-
otherItem->GetRadioGroup() == pimpl_->radio_group_) {
383-
otherItem->pimpl_->state_ = MenuItemState::Unchecked;
384-
[otherItem->pimpl_->ns_menu_item_ setState:NSControlStateValueOff];
354+
NSMenu* parentMenu = [pimpl_->ns_menu_item_ menu];
355+
if (parentMenu) {
356+
for (NSMenuItem* sibling in [parentMenu itemArray]) {
357+
if (sibling == pimpl_->ns_menu_item_) continue;
358+
NSObject* targetObj = [sibling target];
359+
if ([targetObj isKindOfClass:[MenuItemTarget class]]) {
360+
MenuItemTarget* siblingTarget = (MenuItemTarget*)targetObj;
361+
nativeapi::MenuItem* otherItem = static_cast<nativeapi::MenuItem*>([siblingTarget cppMenuItem]);
362+
if (otherItem &&
363+
otherItem->GetType() == MenuItemType::Radio &&
364+
otherItem->GetRadioGroup() == pimpl_->radio_group_) {
365+
otherItem->pimpl_->state_ = MenuItemState::Unchecked;
366+
[sibling setState:NSControlStateValueOff];
367+
}
368+
}
385369
}
386370
}
387371
}
@@ -474,36 +458,23 @@ - (void)menuDidClose:(NSMenu *)menu {
474458
auto menu = std::shared_ptr<Menu>(new Menu());
475459
menu->pimpl_ = std::make_unique<Impl>(nsMenu);
476460
menu->id = g_next_menu_id++;
477-
menu->pimpl_->delegate_.menuId = menu->id;
478-
479-
// Register the Menu for event emission
480-
g_menu_registry[menu->id] = menu.get();
461+
[menu->pimpl_->delegate_ setCppMenu:(void*)menu.get()];
481462

482463
return menu;
483464
}
484465

485466
Menu::Menu(void* native_menu)
486467
: id(g_next_menu_id++)
487468
, pimpl_(std::make_unique<Impl>((__bridge NSMenu*)native_menu)) {
488-
pimpl_->delegate_.menuId = id;
489-
// Register the Menu for event emission
490-
g_menu_registry[id] = this;
469+
[pimpl_->delegate_ setCppMenu:(void*)this];
491470
}
492471

493472
Menu::Menu()
494473
: id(0) { // Will be set in Create method
495474
}
496475

497476
Menu::~Menu() {
498-
// Safely remove from global registry
499-
try {
500-
auto it = g_menu_registry.find(id);
501-
if (it != g_menu_registry.end()) {
502-
g_menu_registry.erase(it);
503-
}
504-
} catch (...) {
505-
// Ignore exceptions during cleanup - the registry may be destroyed during shutdown
506-
}
477+
// No global registry cleanup required
507478
}
508479

509480
void Menu::AddItem(std::shared_ptr<MenuItem> item) {

0 commit comments

Comments
 (0)