From 662e17d0ec75a746a330390ad3818e35bd2be234 Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Tue, 8 Aug 2023 19:32:27 +0300 Subject: [PATCH] refactor: impl drop for inner types (#92) * refactor: impl drop for inner types * macos * impl todos in macos .remove() * remove code * fix gtk accelerator not working after creating the context menu --- .changes/drop.md | 5 + .changes/set-text-windows.md | 5 + src/platform_impl/gtk/mod.rs | 472 +++++++++++++++++++++---------- src/platform_impl/macos/mod.rs | 367 +++++++++++++++--------- src/platform_impl/windows/mod.rs | 221 ++++++++++----- 5 files changed, 710 insertions(+), 360 deletions(-) create mode 100644 .changes/drop.md create mode 100644 .changes/set-text-windows.md diff --git a/.changes/drop.md b/.changes/drop.md new file mode 100644 index 0000000..77b9d61 --- /dev/null +++ b/.changes/drop.md @@ -0,0 +1,5 @@ +--- +"muda": "minor" +--- + +Add `Drop` implementation for the inner types to release memory and OS resources. diff --git a/.changes/set-text-windows.md b/.changes/set-text-windows.md new file mode 100644 index 0000000..e46b7c9 --- /dev/null +++ b/.changes/set-text-windows.md @@ -0,0 +1,5 @@ +--- +"muda": "patch" +--- + +On Windows, fix `.set_text()` sometimes adding gebberish characters after multiple calls. diff --git a/src/platform_impl/gtk/mod.rs b/src/platform_impl/gtk/mod.rs index c339290..79755bd 100644 --- a/src/platform_impl/gtk/mod.rs +++ b/src/platform_impl/gtk/mod.rs @@ -15,7 +15,7 @@ use crate::{ IsMenuItem, MenuEvent, MenuId, MenuItemKind, MenuItemType, Position, }; use accelerator::{from_gtk_mnemonic, parse_accelerator, to_gtk_mnemonic}; -use gtk::{prelude::*, Orientation}; +use gtk::{prelude::*, Container, Orientation}; use std::{ cell::RefCell, collections::HashMap, @@ -25,41 +25,59 @@ use std::{ static COUNTER: Counter = Counter::new(); -macro_rules! return_if_predefined_item_not_supported { - ($item:tt) => { +macro_rules! is_item_supported { + ($item:tt) => {{ let child = $item.child(); let child_ = child.borrow(); - match (&child_.item_type, &child_.predefined_item_type) { - ( - MenuItemType::Predefined, + let supported = if let Some(predefined_item_type) = &child_.predefined_item_type { + matches!( + predefined_item_type, PredefinedMenuItemType::Separator - | PredefinedMenuItemType::Copy - | PredefinedMenuItemType::Cut - | PredefinedMenuItemType::Paste - | PredefinedMenuItemType::SelectAll - | PredefinedMenuItemType::About(_), - ) => {} - ( - MenuItemType::Submenu - | MenuItemType::MenuItem - | MenuItemType::Check - | MenuItemType::Icon, - _, - ) => {} - _ => return Ok(()), - } + | PredefinedMenuItemType::Copy + | PredefinedMenuItemType::Cut + | PredefinedMenuItemType::Paste + | PredefinedMenuItemType::SelectAll + | PredefinedMenuItemType::About(_) + ) + } else { + true + }; drop(child_); + supported + }}; +} + +macro_rules! return_if_item_not_supported { + ($item:tt) => { + if !is_item_supported!($item) { + return Ok(()); + } }; } pub struct Menu { id: MenuId, children: Vec>>, + // TODO: maybe save a reference to the window? gtk_menubars: HashMap, accel_group: Option, gtk_menu: (u32, Option), // dedicated menu for tray or context menus } +impl Drop for Menu { + fn drop(&mut self) { + for (id, menu) in &self.gtk_menubars { + drop_children_from_menu_and_destroy(*id, menu, &self.children); + unsafe { menu.destroy() } + } + + if let (id, Some(menu)) = &self.gtk_menu { + drop_children_from_menu_and_destroy(*id, menu, &self.children); + unsafe { menu.destroy() } + } + } +} + impl Menu { pub fn new(id: Option) -> Self { Self { @@ -76,28 +94,28 @@ impl Menu { } pub fn add_menu_item(&mut self, item: &dyn crate::IsMenuItem, op: AddOp) -> crate::Result<()> { - return_if_predefined_item_not_supported!(item); - - for (menu_id, menu_bar) in &self.gtk_menubars { - let gtk_item = item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; - match op { - AddOp::Append => menu_bar.append(>k_item), - AddOp::Insert(position) => menu_bar.insert(>k_item, position as i32), - } - gtk_item.show(); - } - - { - let (menu_id, menu) = &self.gtk_menu; - if let Some(menu) = menu { + if is_item_supported!(item) { + for (menu_id, menu_bar) in &self.gtk_menubars { let gtk_item = item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; match op { - AddOp::Append => menu.append(>k_item), - AddOp::Insert(position) => menu.insert(>k_item, position as i32), + AddOp::Append => menu_bar.append(>k_item), + AddOp::Insert(position) => menu_bar.insert(>k_item, position as i32), } gtk_item.show(); } + + { + if let (menu_id, Some(menu)) = &self.gtk_menu { + let gtk_item = + item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; + match op { + AddOp::Append => menu.append(>k_item), + AddOp::Insert(position) => menu.insert(>k_item, position as i32), + } + gtk_item.show(); + } + } } match op { @@ -109,7 +127,7 @@ impl Menu { } fn add_menu_item_with_id(&self, item: &dyn crate::IsMenuItem, id: u32) -> crate::Result<()> { - return_if_predefined_item_not_supported!(item); + return_if_item_not_supported!(item); for (menu_id, menu_bar) in self.gtk_menubars.iter().filter(|m| *m.0 == id) { let gtk_item = item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; @@ -121,7 +139,7 @@ impl Menu { } fn add_menu_item_to_context_menu(&self, item: &dyn crate::IsMenuItem) -> crate::Result<()> { - return_if_predefined_item_not_supported!(item); + return_if_item_not_supported!(item); let (menu_id, menu) = &self.gtk_menu; if let Some(menu) = menu { @@ -137,12 +155,13 @@ impl Menu { self.remove_inner(item, true, None) } - pub fn remove_inner( + fn remove_inner( &mut self, item: &dyn crate::IsMenuItem, remove_from_cache: bool, id: Option, ) -> crate::Result<()> { + // get child let child = { let index = self .children @@ -156,46 +175,64 @@ impl Menu { } }; - if let MenuItemKind::Submenu(i) = item.kind() { - let gtk_menus = i.inner.borrow().gtk_menus.clone(); - - for (menu_id, _) in gtk_menus { - for item in i.items() { - i.inner - .borrow_mut() - .remove_inner(item.as_ref(), false, Some(menu_id))?; - } - } - } - for (menu_id, menu_bar) in &self.gtk_menubars { + // check if we are removing this item from all gtk_menubars + // which is usually when this is the item the user is actaully removing + // or if we are removing from a specific menu (id) + // which is when the actual item being removed is a submenu + // and we are iterating through its children and removing + // each child gtk items that are related to this submenu. if id.map(|i| i == *menu_id).unwrap_or(true) { - if let Some(items) = child - .borrow_mut() - .gtk_menu_items - .borrow_mut() - .remove(menu_id) - { - for item in items { - menu_bar.remove(&item); + // bail this is not a supported item like a close_window predefined menu item + if is_item_supported!(item) { + let mut child_ = child.borrow_mut(); + + if child_.item_type == MenuItemType::Submenu { + let menus = child_.gtk_menus.as_ref().unwrap().get(menu_id).cloned(); + if let Some(menus) = menus { + for (id, menu) in menus { + // iterate through children and only remove the gtk items + // related to this submenu + for item in child_.items() { + child_.remove_inner(item.as_ref(), false, Some(id))?; + } + unsafe { menu.destroy() }; + } + } + child_.gtk_menus.as_mut().unwrap().remove(menu_id); } + + // remove all the gtk items that are related to this gtk menubar and destroy it + if let Some(items) = child_.gtk_menu_items.borrow_mut().remove(menu_id) { + for item in items { + menu_bar.remove(&item); + if let Some(accel_group) = &child_.accel_group { + if let Some((mods, key)) = child_.gtk_accelerator { + item.remove_accelerator(accel_group, key, mods); + } + } + unsafe { item.destroy() }; + } + }; } } } + // remove from the gtk menu assigned to the context menu if remove_from_cache { - let (menu_id, menu) = &self.gtk_menu; - if let Some(menu) = menu { - if let Some(items) = child - .borrow_mut() - .gtk_menu_items - .borrow_mut() - .remove(menu_id) - { + if let (id, Some(menu)) = &self.gtk_menu { + let child_ = child.borrow_mut(); + if let Some(items) = child_.gtk_menu_items.borrow_mut().remove(id) { for item in items { menu.remove(&item); + if let Some(accel_group) = &child_.accel_group { + if let Some((mods, key)) = child_.gtk_accelerator { + item.remove_accelerator(accel_group, key, mods); + } + } + unsafe { item.destroy() }; } - } + }; } } Ok(()) @@ -365,22 +402,85 @@ pub struct MenuChild { gtk_accelerator: Option<(gdk::ModifierType, u32)>, // predefined menu item fields - predefined_item_type: PredefinedMenuItemType, + predefined_item_type: Option, // check menu item fields - checked: Rc, - is_syncing_checked_state: Rc, + checked: Option>, + is_syncing_checked_state: Option>, // icon menu item fields icon: Option, // submenu fields pub children: Option>>>, - gtk_menus: HashMap>, - gtk_menu: (u32, Option), // dedicated menu for tray or context menus + gtk_menus: Option>>, + gtk_menu: Option<(u32, Option)>, // dedicated menu for tray or context menus accel_group: Option, } +impl Drop for MenuChild { + fn drop(&mut self) { + if self.item_type == MenuItemType::Submenu { + for menus in self.gtk_menus.as_ref().unwrap().values() { + for (id, menu) in menus { + drop_children_from_menu_and_destroy(*id, menu, self.children.as_ref().unwrap()); + unsafe { menu.destroy() }; + } + } + + if let Some((id, Some(menu))) = &self.gtk_menu { + drop_children_from_menu_and_destroy(*id, menu, self.children.as_ref().unwrap()); + unsafe { menu.destroy() }; + } + } + + for items in self.gtk_menu_items.borrow().values() { + for item in items { + if let Some(accel_group) = &self.accel_group { + if let Some((mods, key)) = self.gtk_accelerator { + item.remove_accelerator(accel_group, key, mods); + } + } + unsafe { item.destroy() }; + } + } + } +} + +fn drop_children_from_menu_and_destroy( + id: u32, + menu: &impl IsA, + children: &Vec>>, +) { + for child in children { + let mut child_ = child.borrow_mut(); + { + let mut menu_items = child_.gtk_menu_items.borrow_mut(); + if let Some(items) = menu_items.remove(&id) { + for item in items { + menu.remove(&item); + if let Some(accel_group) = &child_.accel_group { + if let Some((mods, key)) = child_.gtk_accelerator { + item.remove_accelerator(accel_group, key, mods); + } + } + unsafe { item.destroy() } + } + } + } + + if child_.item_type == MenuItemType::Submenu { + if let Some(menus) = child_.gtk_menus.as_mut().unwrap().remove(&id) { + for (id, menu) in menus { + let children = child_.children.as_ref().unwrap(); + drop_children_from_menu_and_destroy(id, &menu, children); + unsafe { menu.destroy() } + } + } + } + } +} + /// Constructors impl MenuChild { pub fn new( @@ -396,7 +496,15 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), item_type: MenuItemType::MenuItem, gtk_menu_items: Rc::new(RefCell::new(HashMap::new())), - ..Default::default() + accel_group: None, + checked: None, + children: None, + gtk_accelerator: None, + gtk_menu: None, + gtk_menus: None, + icon: None, + is_syncing_checked_state: None, + predefined_item_type: None, } } @@ -407,10 +515,16 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), children: Some(Vec::new()), item_type: MenuItemType::Submenu, - gtk_menu: (COUNTER.next(), None), + gtk_menu: Some((COUNTER.next(), None)), gtk_menu_items: Rc::new(RefCell::new(HashMap::new())), - gtk_menus: HashMap::new(), - ..Default::default() + gtk_menus: Some(HashMap::new()), + accel_group: None, + gtk_accelerator: None, + icon: None, + is_syncing_checked_state: None, + predefined_item_type: None, + accelerator: None, + checked: None, } } @@ -421,9 +535,16 @@ impl MenuChild { accelerator: item_type.accelerator(), id: MenuId(COUNTER.next().to_string()), item_type: MenuItemType::Predefined, - predefined_item_type: item_type, + predefined_item_type: Some(item_type), gtk_menu_items: Rc::new(RefCell::new(HashMap::new())), - ..Default::default() + accel_group: None, + checked: None, + children: None, + gtk_accelerator: None, + gtk_menu: None, + gtk_menus: None, + icon: None, + is_syncing_checked_state: None, } } @@ -437,13 +558,19 @@ impl MenuChild { Self { text: text.to_string(), enabled, - checked: Rc::new(AtomicBool::new(checked)), + checked: Some(Rc::new(AtomicBool::new(checked))), + is_syncing_checked_state: Some(Rc::new(AtomicBool::new(false))), accelerator, id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), item_type: MenuItemType::Check, gtk_menu_items: Rc::new(RefCell::new(HashMap::new())), - is_syncing_checked_state: Rc::new(AtomicBool::new(false)), - ..Default::default() + accel_group: None, + children: None, + gtk_accelerator: None, + gtk_menu: None, + gtk_menus: None, + icon: None, + predefined_item_type: None, } } @@ -462,8 +589,14 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), item_type: MenuItemType::Icon, gtk_menu_items: Rc::new(RefCell::new(HashMap::new())), - is_syncing_checked_state: Rc::new(AtomicBool::new(false)), - ..Default::default() + accel_group: None, + checked: None, + children: None, + gtk_accelerator: None, + gtk_menu: None, + gtk_menus: None, + is_syncing_checked_state: None, + predefined_item_type: None, } } @@ -481,8 +614,15 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), item_type: MenuItemType::Icon, gtk_menu_items: Rc::new(RefCell::new(HashMap::new())), - is_syncing_checked_state: Rc::new(AtomicBool::new(false)), - ..Default::default() + accel_group: None, + checked: None, + children: None, + gtk_accelerator: None, + gtk_menu: None, + gtk_menus: None, + icon: None, + is_syncing_checked_state: None, + predefined_item_type: None, } } } @@ -591,13 +731,17 @@ impl MenuChild { .map(|e| e.map(|i| i.downcast_ref::().unwrap().is_active())) { Some(Some(checked)) => checked, - _ => self.checked.load(Ordering::Relaxed), + _ => self.checked.as_ref().unwrap().load(Ordering::Relaxed), } } pub fn set_checked(&mut self, checked: bool) { - self.checked.store(checked, Ordering::Release); - self.is_syncing_checked_state.store(true, Ordering::Release); + self.checked + .as_ref() + .unwrap() + .store(checked, Ordering::Release); + let is_syncing = self.is_syncing_checked_state.as_ref().unwrap(); + is_syncing.store(true, Ordering::Release); for items in self.gtk_menu_items.borrow().values() { for i in items { i.downcast_ref::() @@ -605,8 +749,7 @@ impl MenuChild { .set_active(checked); } } - self.is_syncing_checked_state - .store(false, Ordering::Release); + is_syncing.store(false, Ordering::Release); } } @@ -631,30 +774,29 @@ impl MenuChild { /// Submenu methods impl MenuChild { pub fn add_menu_item(&mut self, item: &dyn crate::IsMenuItem, op: AddOp) -> crate::Result<()> { - return_if_predefined_item_not_supported!(item); - - for menus in self.gtk_menus.values() { - for (menu_id, menu) in menus { - let gtk_item = - item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; - match op { - AddOp::Append => menu.append(>k_item), - AddOp::Insert(position) => menu.insert(>k_item, position as i32), + if is_item_supported!(item) { + for menus in self.gtk_menus.as_ref().unwrap().values() { + for (menu_id, menu) in menus { + let gtk_item = + item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; + match op { + AddOp::Append => menu.append(>k_item), + AddOp::Insert(position) => menu.insert(>k_item, position as i32), + } + gtk_item.show(); } - gtk_item.show(); } - } - { - let (menu_id, menu) = &self.gtk_menu; - if let Some(menu) = menu { - let gtk_item = - item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; - match op { - AddOp::Append => menu.append(>k_item), - AddOp::Insert(position) => menu.insert(>k_item, position as i32), + { + if let (menu_id, Some(menu)) = self.gtk_menu.as_ref().unwrap() { + let gtk_item = + item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; + match op { + AddOp::Append => menu.append(>k_item), + AddOp::Insert(position) => menu.insert(>k_item, position as i32), + } + gtk_item.show(); } - gtk_item.show(); } } @@ -671,9 +813,9 @@ impl MenuChild { } fn add_menu_item_with_id(&self, item: &dyn crate::IsMenuItem, id: u32) -> crate::Result<()> { - return_if_predefined_item_not_supported!(item); + return_if_item_not_supported!(item); - for menus in self.gtk_menus.values() { + for menus in self.gtk_menus.as_ref().unwrap().values() { for (menu_id, menu) in menus.iter().filter(|m| m.0 == id) { let gtk_item = item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; @@ -686,11 +828,11 @@ impl MenuChild { } fn add_menu_item_to_context_menu(&self, item: &dyn crate::IsMenuItem) -> crate::Result<()> { - return_if_predefined_item_not_supported!(item); + return_if_item_not_supported!(item); - let (menu_id, menu) = &self.gtk_menu; + let (menu_id, menu) = self.gtk_menu.as_ref().unwrap(); if let Some(menu) = menu { - let gtk_item = item.make_gtk_menu_item(*menu_id, None, true)?; + let gtk_item = item.make_gtk_menu_item(*menu_id, self.accel_group.as_ref(), true)?; menu.append(>k_item); gtk_item.show(); } @@ -708,6 +850,7 @@ impl MenuChild { remove_from_cache: bool, id: Option, ) -> crate::Result<()> { + // get child let child = { let index = self .children @@ -723,48 +866,66 @@ impl MenuChild { } }; - if let MenuItemKind::Submenu(i) = item.kind() { - let gtk_menus = i.inner.borrow().gtk_menus.clone(); - - for (menu_id, _) in gtk_menus { - for item in i.items() { - i.inner - .borrow_mut() - .remove_inner(item.as_ref(), false, Some(menu_id))?; - } - } - } - - for menus in self.gtk_menus.values() { + for menus in self.gtk_menus.as_ref().unwrap().values() { for (menu_id, menu) in menus { + // check if we are removing this item from all gtk_menus + // which is usually when this is the item the user is actaully removing + // or if we are removing from a specific menu (id) + // which is when the actual item being removed is a submenu + // and we are iterating through its children and removing + // each child gtk items that are related to this submenu. if id.map(|i| i == *menu_id).unwrap_or(true) { - if let Some(items) = child - .borrow_mut() - .gtk_menu_items - .borrow_mut() - .remove(menu_id) - { - for item in items { - menu.remove(&item); + // bail this is not a supported item like a close_window predefined menu item + if is_item_supported!(item) { + let mut child_ = child.borrow_mut(); + + if child_.item_type == MenuItemType::Submenu { + let menus = child_.gtk_menus.as_ref().unwrap().get(menu_id).cloned(); + if let Some(menus) = menus { + for (id, menu) in menus { + // iterate through children and only remove the gtk items + // related to this submenu + for item in child_.items() { + child_.remove_inner(item.as_ref(), false, Some(id))?; + } + unsafe { menu.destroy() }; + } + } + child_.gtk_menus.as_mut().unwrap().remove(menu_id); } + + // remove all the gtk items that are related to this gtk menu and destroy it + if let Some(items) = child_.gtk_menu_items.borrow_mut().remove(menu_id) { + for item in items { + menu.remove(&item); + if let Some(accel_group) = &child_.accel_group { + if let Some((mods, key)) = child_.gtk_accelerator { + item.remove_accelerator(accel_group, key, mods); + } + } + unsafe { item.destroy() }; + } + }; } } } } + // remove from the gtk menu assigned to the context menu if remove_from_cache { - let (menu_id, menu) = &self.gtk_menu; - if let Some(menu) = menu { - if let Some(items) = child - .borrow_mut() - .gtk_menu_items - .borrow_mut() - .remove(menu_id) - { + if let (id, Some(menu)) = self.gtk_menu.as_ref().unwrap() { + let child_ = child.borrow_mut(); + if let Some(items) = child_.gtk_menu_items.borrow_mut().remove(id) { for item in items { menu.remove(&item); + if let Some(accel_group) = &child_.accel_group { + if let Some((mods, key)) = child_.gtk_accelerator { + item.remove_accelerator(accel_group, key, mods); + } + } + unsafe { item.destroy() }; } - } + }; } } @@ -791,8 +952,9 @@ impl MenuChild { pub fn gtk_context_menu(&mut self) -> gtk::Menu { let mut add_items = false; { - if self.gtk_menu.1.is_none() { - self.gtk_menu.1 = Some(gtk::Menu::new()); + let gtk_menu = self.gtk_menu.as_mut().unwrap(); + if gtk_menu.1.is_none() { + gtk_menu.1 = Some(gtk::Menu::new()); add_items = true; } } @@ -803,7 +965,7 @@ impl MenuChild { } } - self.gtk_menu.1.as_ref().unwrap().clone() + self.gtk_menu.as_ref().unwrap().1.as_ref().unwrap().clone() } } @@ -860,6 +1022,8 @@ impl MenuChild { .or_insert_with(Vec::new) .push(item.clone()); self.gtk_menus + .as_mut() + .unwrap() .entry(menu_id) .or_insert_with(Vec::new) .push((id, submenu.clone())); @@ -921,7 +1085,7 @@ impl MenuChild { .as_ref() .map(parse_accelerator) .transpose()?; - let predefined_item_type = self.predefined_item_type.clone(); + let predefined_item_type = self.predefined_item_type.clone().unwrap(); let make_item = || { gtk::MenuItem::builder() @@ -1038,7 +1202,7 @@ impl MenuChild { .label(&to_gtk_mnemonic(&self.text)) .use_underline(true) .sensitive(self.enabled) - .active(self.checked.load(Ordering::Relaxed)) + .active(self.checked.as_ref().unwrap().load(Ordering::Relaxed)) .build(); self.accel_group = accel_group.cloned(); @@ -1046,8 +1210,8 @@ impl MenuChild { register_accel!(self, item, accel_group); let id = self.id.clone(); - let is_syncing_checked_state = self.is_syncing_checked_state.clone(); - let checked = self.checked.clone(); + let is_syncing_checked_state = self.is_syncing_checked_state.clone().unwrap(); + let checked = self.checked.clone().unwrap(); let store = self.gtk_menu_items.clone(); item.connect_toggled(move |i| { let should_dispatch = is_syncing_checked_state diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 160f333..c3fbcb8 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -47,17 +47,33 @@ extern "C" { #[allow(non_upper_case_globals)] const NSAboutPanelOptionCopyright: &str = "Copyright"; +#[derive(Debug, Clone)] +struct NsMenuRef(u32, id); + +impl Drop for NsMenuRef { + fn drop(&mut self) { + unsafe { + let _: () = msg_send![self.1, removeAllItems]; + let _: () = msg_send![self.1, release]; + } + } +} + #[derive(Debug)] pub struct Menu { id: MenuId, - ns_menu: id, - children: Rc>>>>, + ns_menu: NsMenuRef, + children: Vec>>, } impl Drop for Menu { fn drop(&mut self) { - unsafe { - let _: () = msg_send![self.ns_menu, release]; + for child in &self.children { + let mut child_ = child.borrow_mut(); + child_.ns_menu_items.remove(&self.ns_menu.0); + if child_.item_type == MenuItemType::Submenu { + child_.ns_menus.as_mut().unwrap().remove(&self.ns_menu.0); + } } } } @@ -66,13 +82,13 @@ impl Menu { pub fn new(id: Option) -> Self { Self { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), - ns_menu: unsafe { + ns_menu: NsMenuRef(COUNTER.next(), unsafe { let ns_menu = NSMenu::new(nil); ns_menu.setAutoenablesItems(NO); let _: () = msg_send![ns_menu, retain]; ns_menu - }, - children: Rc::new(RefCell::new(Vec::new())), + }), + children: Vec::new(), } } @@ -81,18 +97,18 @@ impl Menu { } pub fn add_menu_item(&mut self, item: &dyn crate::IsMenuItem, op: AddOp) -> crate::Result<()> { - let ns_menu_item: *mut Object = item.make_ns_item_for_menu(&self.id)?; + let ns_menu_item: id = item.make_ns_item_for_menu(self.ns_menu.0)?; let child = item.child(); unsafe { match op { AddOp::Append => { - self.ns_menu.addItem_(ns_menu_item); - self.children.borrow_mut().push(child); + self.ns_menu.1.addItem_(ns_menu_item); + self.children.push(child); } AddOp::Insert(position) => { - let () = msg_send![self.ns_menu, insertItem: ns_menu_item atIndex: position as NSInteger]; - self.children.borrow_mut().insert(position, child); + let () = msg_send![self.ns_menu.1, insertItem: ns_menu_item atIndex: position as NSInteger]; + self.children.insert(position, child); } } } @@ -100,46 +116,51 @@ impl Menu { Ok(()) } - pub fn remove(&self, item: &dyn crate::IsMenuItem) -> crate::Result<()> { - // get a list of instances of the specified `NSMenuItem` in this menu - let child = match item.kind() { - MenuItemKind::Submenu(i) => i.inner.clone(), - MenuItemKind::MenuItem(i) => i.inner.clone(), - MenuItemKind::Predefined(i) => i.inner.clone(), - MenuItemKind::Check(i) => i.inner.clone(), - MenuItemKind::Icon(i) => i.inner.clone(), + pub fn remove(&mut self, item: &dyn crate::IsMenuItem) -> crate::Result<()> { + // get child + let child = { + let index = self + .children + .iter() + .position(|e| e.borrow().id == item.id()) + .ok_or(crate::Error::NotAChildOfThisMenu)?; + self.children.remove(index) }; + let mut child_ = child.borrow_mut(); - if let Some(ns_menu_items) = child_.ns_menu_items.remove(&self.id) { - // remove each NSMenuItem from the NSMenu - unsafe { - for item in ns_menu_items { - let () = msg_send![self.ns_menu, removeItem: item]; + + if child_.item_type == MenuItemType::Submenu { + let menu_id = &self.ns_menu.0; + let menus = child_.ns_menus.as_ref().unwrap().get(menu_id).cloned(); + if let Some(menus) = menus { + for menu in menus { + for item in child_.items() { + child_.remove_inner(item.as_ref(), false, Some(menu.0))?; + } } } + child_.ns_menus.as_mut().unwrap().remove(menu_id); } - // remove the item from our internal list of children - let mut children = self.children.borrow_mut(); - let index = children - .iter() - .position(|e| e.borrow().id() == item.id()) - .ok_or(crate::Error::NotAChildOfThisMenu)?; - children.remove(index); + // remove each NSMenuItem from the NSMenu + if let Some(ns_menu_items) = child_.ns_menu_items.remove(&self.ns_menu.0) { + for item in ns_menu_items { + let () = unsafe { msg_send![self.ns_menu.1, removeItem: item] }; + } + } Ok(()) } pub fn items(&self) -> Vec { self.children - .borrow() .iter() .map(|c| c.borrow().kind(c.clone())) .collect() } pub fn init_for_nsapp(&self) { - unsafe { NSApp().setMainMenu_(self.ns_menu) } + unsafe { NSApp().setMainMenu_(self.ns_menu.1) } } pub fn remove_for_nsapp(&self) { @@ -147,16 +168,16 @@ impl Menu { } pub fn show_context_menu_for_nsview(&self, view: id, position: Option) { - show_context_menu(self.ns_menu, view, position) + show_context_menu(self.ns_menu.1, view, position) } pub fn ns_menu(&self) -> *mut std::ffi::c_void { - self.ns_menu as _ + self.ns_menu.1 as _ } } /// A generic child in a menu -#[derive(Debug)] +#[derive(Debug, Default)] pub struct MenuChild { // shared fields between submenus and menu items item_type: MenuItemType, @@ -164,13 +185,13 @@ pub struct MenuChild { text: String, enabled: bool, - ns_menu_items: HashMap>, + ns_menu_items: HashMap>, // menu item fields accelerator: Option, // predefined menu item fields - predefined_item_type: PredefinedMenuItemType, + predefined_item_type: Option, // check menu item fields checked: bool, @@ -181,37 +202,37 @@ pub struct MenuChild { // submenu fields pub children: Option>>>, - ns_menus: HashMap>, - ns_menu: NsMenuRef, + ns_menus: Option>>, + ns_menu: Option, } -#[derive(Debug)] -struct NsMenuRef(MenuId, id); - -impl Drop for NsMenuRef { +impl Drop for MenuChild { fn drop(&mut self) { - unsafe { - let _: () = msg_send![self.1, release]; - } - } -} + fn drop_children(id: u32, children: &Vec>>) { + for child in children { + let mut child_ = child.borrow_mut(); + child_.ns_menu_items.remove(&id); -impl Default for MenuChild { - fn default() -> Self { - Self { - item_type: Default::default(), - id: Default::default(), - text: Default::default(), - enabled: Default::default(), - ns_menu_items: Default::default(), - accelerator: Default::default(), - predefined_item_type: Default::default(), - checked: Default::default(), - icon: Default::default(), - native_icon: Default::default(), - children: Default::default(), - ns_menus: Default::default(), - ns_menu: NsMenuRef(MenuId(COUNTER.next().to_string()), 0 as _), + if child_.item_type == MenuItemType::Submenu { + if let Some(menus) = child_.ns_menus.as_mut().unwrap().remove(&id) { + for menu in menus { + drop_children(menu.0, child_.children.as_ref().unwrap()); + } + } + } + } + } + + if self.item_type == MenuItemType::Submenu { + for menus in self.ns_menus.as_ref().unwrap().values() { + for menu in menus { + drop_children(menu.0, self.children.as_ref().unwrap()) + } + } + + if let Some(menu) = &self.ns_menu { + drop_children(menu.0, self.children.as_ref().unwrap()); + } } } } @@ -230,7 +251,14 @@ impl MenuChild { enabled, id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), accelerator, - ..Default::default() + checked: false, + children: None, + icon: None, + native_icon: None, + ns_menu: None, + ns_menu_items: HashMap::new(), + ns_menus: None, + predefined_item_type: None, } } @@ -241,12 +269,18 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), enabled, children: Some(Vec::new()), - ns_menu: NsMenuRef(MenuId(COUNTER.next().to_string()), unsafe { + ns_menu: Some(NsMenuRef(COUNTER.next(), unsafe { let menu = NSMenu::new(nil); let _: () = msg_send![menu, retain]; menu - }), - ..Default::default() + })), + accelerator: None, + checked: false, + icon: None, + native_icon: None, + ns_menu_items: HashMap::new(), + ns_menus: Some(HashMap::new()), + predefined_item_type: None, } } @@ -279,9 +313,14 @@ impl MenuChild { enabled: true, id: MenuId(COUNTER.next().to_string()), accelerator, - predefined_item_type: item_type, - // ns_menu_item, - ..Default::default() + predefined_item_type: Some(item_type), + checked: false, + children: None, + icon: None, + native_icon: None, + ns_menu: None, + ns_menu_items: HashMap::new(), + ns_menus: None, } } @@ -299,7 +338,13 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), accelerator, checked, - ..Default::default() + children: None, + icon: None, + native_icon: None, + ns_menu: None, + ns_menu_items: HashMap::new(), + ns_menus: None, + predefined_item_type: None, } } @@ -317,7 +362,13 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), icon, accelerator, - ..Default::default() + checked: false, + children: None, + native_icon: None, + ns_menu: None, + ns_menu_items: HashMap::new(), + ns_menus: None, + predefined_item_type: None, } } @@ -335,7 +386,13 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), native_icon, accelerator, - ..Default::default() + checked: false, + children: None, + icon: None, + ns_menu: None, + ns_menu_items: HashMap::new(), + ns_menus: None, + predefined_item_type: None, } } } @@ -361,7 +418,7 @@ impl MenuChild { for ns_items in self.ns_menu_items.values() { for &ns_item in ns_items { let () = msg_send![ns_item, setTitle: title]; - let ns_submenu: *mut Object = msg_send![ns_item, submenu]; + let ns_submenu: id = msg_send![ns_item, submenu]; if ns_submenu != nil { let () = msg_send![ns_submenu, setTitle: title]; } @@ -468,28 +525,32 @@ impl MenuChild { unsafe { match op { AddOp::Append => { - for menus in self.ns_menus.values() { + for menus in self.ns_menus.as_ref().unwrap().values() { for ns_menu in menus { - let ns_menu_item: *mut Object = item.make_ns_item_for_menu(&self.id)?; - ns_menu.addItem_(ns_menu_item); + let ns_menu_item: id = + item.make_ns_item_for_menu(self.ns_menu.as_ref().unwrap().0)?; + ns_menu.1.addItem_(ns_menu_item); } } - let ns_menu_item: *mut Object = item.make_ns_item_for_menu(&self.ns_menu.0)?; - self.ns_menu.1.addItem_(ns_menu_item); + let ns_menu_item: id = + item.make_ns_item_for_menu(self.ns_menu.as_ref().unwrap().0)?; + self.ns_menu.as_ref().unwrap().1.addItem_(ns_menu_item); self.children.as_mut().unwrap().push(child); } AddOp::Insert(position) => { - for menus in self.ns_menus.values() { - for &ns_menu in menus { - let ns_menu_item: *mut Object = item.make_ns_item_for_menu(&self.id)?; - let () = msg_send![ns_menu, insertItem: ns_menu_item atIndex: position as NSInteger]; + for menus in self.ns_menus.as_ref().unwrap().values() { + for ns_menu in menus { + let ns_menu_item: id = + item.make_ns_item_for_menu(self.ns_menu.as_ref().unwrap().0)?; + let () = msg_send![ns_menu.1, insertItem: ns_menu_item atIndex: position as NSInteger]; } } - let ns_menu_item: *mut Object = item.make_ns_item_for_menu(&self.ns_menu.0)?; - let () = msg_send![ self.ns_menu.1, insertItem: ns_menu_item atIndex: position as NSInteger]; + let ns_menu_item: id = + item.make_ns_item_for_menu(self.ns_menu.as_ref().unwrap().0)?; + let () = msg_send![ self.ns_menu.as_ref().unwrap().1, insertItem: ns_menu_item atIndex: position as NSInteger]; self.children.as_mut().unwrap().insert(position, child); } @@ -500,40 +561,77 @@ impl MenuChild { } pub fn remove(&mut self, item: &dyn crate::IsMenuItem) -> crate::Result<()> { - let child = item.child(); + self.remove_inner(item, true, None) + } + pub fn remove_inner( + &mut self, + item: &dyn crate::IsMenuItem, + remove_from_cache: bool, + id: Option, + ) -> crate::Result<()> { + // get child + let child = { + let index = self + .children + .as_ref() + .unwrap() + .iter() + .position(|e| e.borrow().id == item.id()) + .ok_or(crate::Error::NotAChildOfThisMenu)?; + if remove_from_cache { + self.children.as_mut().unwrap().remove(index) + } else { + self.children.as_ref().unwrap().get(index).cloned().unwrap() + } + }; - // get a list of instances of the specified NSMenuItem in this menu - if let Some(ns_menu_items) = child.borrow_mut().ns_menu_items.remove(&self.id) { - // remove each NSMenuItem from the NSMenu - unsafe { - for item in ns_menu_items { - for menus in self.ns_menus.values() { - for &ns_menu in menus { - let () = msg_send![ns_menu, removeItem: item]; + for menus in self.ns_menus.as_ref().unwrap().values() { + for menu in menus { + // check if we are removing this item from all ns_menus + // which is usually when this is the item the user is actaully removing + // or if we are removing from a specific menu (id) + // which is when the actual item being removed is a submenu + // and we are iterating through its children and removing + // each child ns menu item that are related to this submenu. + if id.map(|i| i == menu.0).unwrap_or(true) { + let mut child_ = child.borrow_mut(); + + if child_.item_type == MenuItemType::Submenu { + let menus = child_.ns_menus.as_ref().unwrap().get(&menu.0).cloned(); + if let Some(menus) = menus { + for menu in menus { + // iterate through children and only remove the ns menu items + // related to this submenu + for item in child_.items() { + child_.remove_inner(item.as_ref(), false, Some(menu.0))?; + } + } } + child_.ns_menus.as_mut().unwrap().remove(&menu.0); } - let () = msg_send![self.ns_menu.1, removeItem: item]; + if let Some(items) = child_.ns_menu_items.remove(&menu.0) { + for item in items { + let () = unsafe { msg_send![menu.1, removeItem: item] }; + } + } } } } - if let Some(ns_menu_items) = child.borrow_mut().ns_menu_items.remove(&self.ns_menu.0) { - unsafe { + if remove_from_cache { + if let Some(ns_menu_items) = child + .borrow_mut() + .ns_menu_items + .remove(&self.ns_menu.as_ref().unwrap().0) + { for item in ns_menu_items { - let () = msg_send![self.ns_menu.1, removeItem: item]; + let () = + unsafe { msg_send![self.ns_menu.as_ref().unwrap().1, removeItem: item] }; } } } - // remove the item from our internal list of children - let children = self.children.as_mut().unwrap(); - let index = children - .iter() - .position(|e| e.borrow().id == item.id()) - .ok_or(crate::Error::NotAChildOfThisMenu)?; - children.remove(index); - Ok(()) } @@ -547,27 +645,27 @@ impl MenuChild { } pub fn show_context_menu_for_nsview(&self, view: id, position: Option) { - show_context_menu(self.ns_menu.1, view, position) + show_context_menu(self.ns_menu.as_ref().unwrap().1, view, position) } pub fn set_as_windows_menu_for_nsapp(&self) { - unsafe { NSApp().setWindowsMenu_(self.ns_menu.1) } + unsafe { NSApp().setWindowsMenu_(self.ns_menu.as_ref().unwrap().1) } } pub fn set_as_help_menu_for_nsapp(&self) { - unsafe { msg_send![NSApp(), setHelpMenu: self.ns_menu.1] } + unsafe { msg_send![NSApp(), setHelpMenu: self.ns_menu.as_ref().unwrap().1] } } pub fn ns_menu(&self) -> *mut std::ffi::c_void { - self.ns_menu.1 as _ + self.ns_menu.as_ref().unwrap().1 as _ } } /// NSMenuItem item creation methods impl MenuChild { - pub fn create_ns_item_for_submenu(&mut self, menu_id: &MenuId) -> crate::Result { - let ns_menu_item: *mut Object; - let ns_submenu: *mut Object; + pub fn create_ns_item_for_submenu(&mut self, menu_id: u32) -> crate::Result { + let ns_menu_item: id; + let ns_submenu: id; unsafe { ns_menu_item = NSMenuItem::alloc(nil).autorelease(); @@ -583,25 +681,29 @@ impl MenuChild { } } + let id = COUNTER.next(); + for item in self.children.as_ref().unwrap() { - let ns_item = item.borrow_mut().make_ns_item_for_menu(menu_id)?; + let ns_item = item.borrow_mut().make_ns_item_for_menu(id)?; unsafe { ns_submenu.addItem_(ns_item) }; } self.ns_menus - .entry(menu_id.clone()) + .as_mut() + .unwrap() + .entry(menu_id) .or_insert_with(Vec::new) - .push(ns_submenu); + .push(NsMenuRef(id, ns_submenu)); self.ns_menu_items - .entry(menu_id.clone()) + .entry(menu_id) .or_insert_with(Vec::new) .push(ns_menu_item); Ok(ns_menu_item) } - pub fn create_ns_item_for_menu_item(&mut self, menu_id: &MenuId) -> crate::Result { + pub fn create_ns_item_for_menu_item(&mut self, menu_id: u32) -> crate::Result { let ns_menu_item = create_ns_menu_item( &self.text, Some(sel!(fireMenuItemAction:)), @@ -628,11 +730,8 @@ impl MenuChild { Ok(ns_menu_item) } - pub fn create_ns_item_for_predefined_menu_item( - &mut self, - menu_id: &MenuId, - ) -> crate::Result { - let item_type = &self.predefined_item_type; + pub fn create_ns_item_for_predefined_menu_item(&mut self, menu_id: u32) -> crate::Result { + let item_type = self.predefined_item_type.as_ref().unwrap(); let ns_menu_item = match item_type { PredefinedMenuItemType::Separator => unsafe { NSMenuItem::separatorItem(nil).autorelease() @@ -640,7 +739,7 @@ impl MenuChild { _ => create_ns_menu_item(&self.text, item_type.selector(), &self.accelerator)?, }; - if let PredefinedMenuItemType::About(_) = self.predefined_item_type { + if let PredefinedMenuItemType::About(_) = item_type { unsafe { let _: () = msg_send![ns_menu_item, setTarget: ns_menu_item]; @@ -654,7 +753,7 @@ impl MenuChild { if !self.enabled { let () = msg_send![ns_menu_item, setEnabled: NO]; } - if let PredefinedMenuItemType::Services = self.predefined_item_type { + if let PredefinedMenuItemType::Services = item_type { // we have to assign an empty menu as the app's services menu, and macOS will populate it let services_menu = NSMenu::new(nil).autorelease(); let () = msg_send![NSApp(), setServicesMenu: services_menu]; @@ -670,7 +769,7 @@ impl MenuChild { Ok(ns_menu_item) } - pub fn create_ns_item_for_check_menu_item(&mut self, menu_id: &MenuId) -> crate::Result { + pub fn create_ns_item_for_check_menu_item(&mut self, menu_id: u32) -> crate::Result { let ns_menu_item = create_ns_menu_item( &self.text, Some(sel!(fireMenuItemAction:)), @@ -700,7 +799,7 @@ impl MenuChild { Ok(ns_menu_item) } - pub fn create_ns_item_for_icon_menu_item(&mut self, menu_id: &MenuId) -> crate::Result { + pub fn create_ns_item_for_icon_menu_item(&mut self, menu_id: u32) -> crate::Result { let ns_menu_item = create_ns_menu_item( &self.text, Some(sel!(fireMenuItemAction:)), @@ -733,7 +832,7 @@ impl MenuChild { Ok(ns_menu_item) } - fn make_ns_item_for_menu(&mut self, menu_id: &MenuId) -> crate::Result<*mut Object> { + fn make_ns_item_for_menu(&mut self, menu_id: u32) -> crate::Result { match self.item_type { MenuItemType::Submenu => self.create_ns_item_for_submenu(menu_id), MenuItemType::MenuItem => self.create_ns_item_for_menu_item(menu_id), @@ -771,7 +870,7 @@ impl PredefinedMenuItemType { } impl dyn IsMenuItem + '_ { - fn make_ns_item_for_menu(&self, menu_id: &MenuId) -> crate::Result<*mut Object> { + fn make_ns_item_for_menu(&self, menu_id: u32) -> crate::Result { match self.kind() { MenuItemKind::Submenu(i) => i.inner.borrow_mut().create_ns_item_for_submenu(menu_id), MenuItemKind::MenuItem(i) => i.inner.borrow_mut().create_ns_item_for_menu_item(menu_id), @@ -835,7 +934,7 @@ extern "C" fn fire_menu_item_click(this: &Object, _: Sel, _item: id) { let ptr: usize = *this.get_ivar(BLOCK_PTR); let item = ptr as *mut &mut MenuChild; - if let PredefinedMenuItemType::About(about_meta) = &(*item).predefined_item_type { + if let Some(PredefinedMenuItemType::About(about_meta)) = &(*item).predefined_item_type { match about_meta { Some(about_meta) => { unsafe fn mkstr(s: &str) -> id { @@ -923,7 +1022,7 @@ fn create_ns_menu_item( .map(|accel| accel.key_modifier_mask()) .unwrap_or_else(NSEventModifierFlags::empty); - let ns_menu_item: *mut Object = msg_send![make_menu_item_class(), alloc]; + let ns_menu_item: id = msg_send![make_menu_item_class(), alloc]; ns_menu_item.initWithTitle_action_keyEquivalent_(title, selector, key_equivalent); ns_menu_item.setKeyEquivalentModifierMask_(modifier_mask); diff --git a/src/platform_impl/windows/mod.rs b/src/platform_impl/windows/mod.rs index 108150b..e26dd8e 100644 --- a/src/platform_impl/windows/mod.rs +++ b/src/platform_impl/windows/mod.rs @@ -30,9 +30,9 @@ use windows_sys::Win32::{ Shell::{DefSubclassProc, RemoveWindowSubclass, SetWindowSubclass}, WindowsAndMessaging::{ AppendMenuW, CreateAcceleratorTableW, CreateMenu, CreatePopupMenu, - DestroyAcceleratorTable, DrawMenuBar, EnableMenuItem, GetCursorPos, GetMenu, - GetMenuItemInfoW, InsertMenuW, PostQuitMessage, RemoveMenu, SendMessageW, SetMenu, - SetMenuItemInfoW, ShowWindow, TrackPopupMenu, HACCEL, HMENU, MENUITEMINFOW, + DestroyAcceleratorTable, DestroyMenu, DrawMenuBar, EnableMenuItem, GetCursorPos, + GetMenu, GetMenuItemInfoW, InsertMenuW, PostQuitMessage, RemoveMenu, SendMessageW, + SetMenu, SetMenuItemInfoW, ShowWindow, TrackPopupMenu, HACCEL, HMENU, MENUITEMINFOW, MFS_CHECKED, MFS_DISABLED, MF_BYCOMMAND, MF_BYPOSITION, MF_CHECKED, MF_DISABLED, MF_ENABLED, MF_GRAYED, MF_POPUP, MF_SEPARATOR, MF_STRING, MF_UNCHECKED, MIIM_BITMAP, MIIM_STATE, MIIM_STRING, SW_HIDE, SW_MAXIMIZE, SW_MINIMIZE, TPM_LEFTALIGN, WM_CLOSE, @@ -61,7 +61,7 @@ macro_rules! inner_menu_child_and_flags { MenuItemKind::Predefined(i) => { let child = i.inner.clone(); let child_ = child.borrow(); - match child_.predefined_item_type { + match child_.predefined_item_type.as_ref().unwrap() { PredefinedMenuItemType::None => return Ok(()), PredefinedMenuItemType::Separator => { flags |= MF_SEPARATOR; @@ -101,6 +101,44 @@ pub(crate) struct Menu { children: Vec>>, } +impl Drop for Menu { + fn drop(&mut self) { + for hwnd in self.hwnds.clone() { + let _ = self.remove_for_hwnd(hwnd); + } + + fn remove_from_children_stores(id: &MenuId, children: &Vec>>) { + for child in children { + let mut child_ = child.borrow_mut(); + child_.root_menu_haccel_stores.remove(id); + if child_.item_type == MenuItemType::Submenu { + remove_from_children_stores(id, child_.children.as_ref().unwrap()); + } + } + } + + remove_from_children_stores(&self.id, &self.children); + + for child in &self.children { + let child_ = child.borrow(); + let id = if child_.item_type == MenuItemType::Submenu { + child_.hmenu as _ + } else { + child_.internal_id + }; + unsafe { + RemoveMenu(self.hpopupmenu, id, MF_BYCOMMAND); + RemoveMenu(self.hmenu, id, MF_BYCOMMAND); + } + } + + unsafe { + DestroyMenu(self.hmenu); + DestroyMenu(self.hpopupmenu); + } + } +} + impl Menu { pub fn new(id: Option) -> Self { Self { @@ -124,9 +162,7 @@ impl Menu { child .borrow_mut() .root_menu_haccel_stores - .as_mut() - .unwrap() - .push(self.haccel_store.clone()); + .insert(self.id.clone(), self.haccel_store.clone()); } { @@ -371,7 +407,7 @@ pub(crate) struct MenuChild { text: String, enabled: bool, parents_hemnu: Vec, - root_menu_haccel_stores: Option>>>, + root_menu_haccel_stores: HashMap>>, // menu item fields internal_id: u32, @@ -379,7 +415,7 @@ pub(crate) struct MenuChild { accelerator: Option, // predefined menu item fields - predefined_item_type: PredefinedMenuItemType, + predefined_item_type: Option, // check menu item fields checked: bool, @@ -393,6 +429,23 @@ pub(crate) struct MenuChild { pub children: Option>>>, } +impl Drop for MenuChild { + fn drop(&mut self) { + if self.item_type == MenuItemType::Submenu { + unsafe { + DestroyMenu(self.hmenu); + DestroyMenu(self.hpopupmenu); + } + } + + if self.accelerator.is_some() { + for store in self.root_menu_haccel_stores.values() { + AccelAction::remove(&mut store.borrow_mut(), self.internal_id) + } + } + } +} + /// Constructors impl MenuChild { pub fn new( @@ -410,8 +463,13 @@ impl MenuChild { internal_id, id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), accelerator, - root_menu_haccel_stores: Some(Vec::new()), - ..Default::default() + root_menu_haccel_stores: HashMap::new(), + predefined_item_type: None, + icon: None, + checked: false, + children: None, + hmenu: 0, + hpopupmenu: 0, } } @@ -427,8 +485,11 @@ impl MenuChild { internal_id, id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), hpopupmenu: unsafe { CreatePopupMenu() }, - root_menu_haccel_stores: Some(Vec::new()), - ..Default::default() + root_menu_haccel_stores: HashMap::new(), + predefined_item_type: None, + icon: None, + checked: false, + accelerator: None, } } @@ -442,9 +503,13 @@ impl MenuChild { internal_id, id: MenuId(internal_id.to_string()), accelerator: item_type.accelerator(), - predefined_item_type: item_type, - root_menu_haccel_stores: Some(Vec::new()), - ..Default::default() + predefined_item_type: Some(item_type), + root_menu_haccel_stores: HashMap::new(), + icon: None, + checked: false, + children: None, + hmenu: 0, + hpopupmenu: 0, } } @@ -465,8 +530,12 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), accelerator, checked, - root_menu_haccel_stores: Some(Vec::new()), - ..Default::default() + root_menu_haccel_stores: HashMap::new(), + predefined_item_type: None, + icon: None, + children: None, + hmenu: 0, + hpopupmenu: 0, } } @@ -487,8 +556,12 @@ impl MenuChild { id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), accelerator, icon, - root_menu_haccel_stores: Some(Vec::new()), - ..Default::default() + root_menu_haccel_stores: HashMap::new(), + predefined_item_type: None, + checked: false, + children: None, + hmenu: 0, + hpopupmenu: 0, } } @@ -508,8 +581,13 @@ impl MenuChild { internal_id, id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), accelerator, - root_menu_haccel_stores: Some(Vec::new()), - ..Default::default() + root_menu_haccel_stores: HashMap::new(), + predefined_item_type: None, + icon: None, + checked: false, + children: None, + hmenu: 0, + hpopupmenu: 0, } } } @@ -556,16 +634,17 @@ impl MenuChild { } pub fn set_text(&mut self, text: &str) { - self.text = if let Some(accelerator) = self.accelerator { - format!("{text}\t{}", accelerator) + self.text = text.to_string(); + let mut text = if let Some(accelerator) = self.accelerator { + encode_wide(format!("{text}\t{}", accelerator)) } else { - text.to_string() + encode_wide(text) }; for parent in &self.parents_hemnu { let mut info: MENUITEMINFOW = unsafe { std::mem::zeroed() }; info.cbSize = std::mem::size_of::() as _; info.fMask = MIIM_STRING; - info.dwTypeData = encode_wide(&self.text).as_mut_ptr(); + info.dwTypeData = text.as_mut_ptr(); unsafe { SetMenuItemInfoW(*parent, self.internal_id(), false.into(), &info) }; } @@ -603,8 +682,7 @@ impl MenuChild { self.accelerator = accelerator; self.set_text(&self.text.clone()); - let haccel_stores = self.root_menu_haccel_stores.as_mut().unwrap(); - for store in haccel_stores { + for store in self.root_menu_haccel_stores.values() { let mut store = store.borrow_mut(); if let Some(accelerator) = self.accelerator { AccelAction::add(&mut store, self.internal_id, &accelerator)? @@ -676,9 +754,7 @@ impl MenuChild { child .borrow_mut() .root_menu_haccel_stores - .as_mut() - .unwrap() - .extend_from_slice(self.root_menu_haccel_stores.as_ref().unwrap()); + .extend(self.root_menu_haccel_stores.clone()); } { @@ -695,7 +771,7 @@ impl MenuChild { text.push('\t'); text.push_str(&accel_str); - for root_menu in self.root_menu_haccel_stores.as_mut().unwrap() { + for root_menu in self.root_menu_haccel_stores.values() { let mut haccel = root_menu.borrow_mut(); AccelAction::add(&mut haccel, child_.internal_id(), accelerator)?; } @@ -885,30 +961,21 @@ impl AccelAction { ) -> crate::Result<()> { let accel = accelerator.to_accel(id as _)?; haccel_store.1.insert(id, Accel(accel)); - Self::update_store(haccel_store); - Ok(()) } fn remove(haccel_store: &mut RefMut, id: u32) { haccel_store.1.remove(&id); - Self::update_store(haccel_store) } fn update_store(haccel_store: &mut RefMut) { unsafe { DestroyAcceleratorTable(haccel_store.0); - haccel_store.0 = CreateAcceleratorTableW( - haccel_store - .1 - .values() - .map(|i| i.0) - .collect::>() - .as_ptr(), - haccel_store.1.len() as _, - ); + let len = haccel_store.1.len(); + let accels = haccel_store.1.values().map(|i| i.0).collect::>(); + haccel_store.0 = CreateAcceleratorTableW(accels.as_ptr(), len as _); } } } @@ -962,35 +1029,45 @@ unsafe extern "system" fn menu_subclass_proc( let checked = !item.checked; item.set_checked(checked); } - MenuItemType::Predefined => match &item.predefined_item_type { - PredefinedMenuItemType::Copy => execute_edit_command(EditCommand::Copy), - PredefinedMenuItemType::Cut => execute_edit_command(EditCommand::Cut), - PredefinedMenuItemType::Paste => execute_edit_command(EditCommand::Paste), - PredefinedMenuItemType::SelectAll => { - execute_edit_command(EditCommand::SelectAll) - } - PredefinedMenuItemType::Separator => {} - PredefinedMenuItemType::Minimize => { - ShowWindow(hwnd, SW_MINIMIZE); - } - PredefinedMenuItemType::Maximize => { - ShowWindow(hwnd, SW_MAXIMIZE); - } - PredefinedMenuItemType::Hide => { - ShowWindow(hwnd, SW_HIDE); - } - PredefinedMenuItemType::CloseWindow => { - SendMessageW(hwnd, WM_CLOSE, 0, 0); - } - PredefinedMenuItemType::Quit => { - PostQuitMessage(0); - } - PredefinedMenuItemType::About(Some(ref metadata)) => { - show_about_dialog(hwnd, metadata) - } + MenuItemType::Predefined => { + if let Some(predefined_item_type) = &item.predefined_item_type { + match predefined_item_type { + PredefinedMenuItemType::Copy => { + execute_edit_command(EditCommand::Copy) + } + PredefinedMenuItemType::Cut => { + execute_edit_command(EditCommand::Cut) + } + PredefinedMenuItemType::Paste => { + execute_edit_command(EditCommand::Paste) + } + PredefinedMenuItemType::SelectAll => { + execute_edit_command(EditCommand::SelectAll) + } + PredefinedMenuItemType::Separator => {} + PredefinedMenuItemType::Minimize => { + ShowWindow(hwnd, SW_MINIMIZE); + } + PredefinedMenuItemType::Maximize => { + ShowWindow(hwnd, SW_MAXIMIZE); + } + PredefinedMenuItemType::Hide => { + ShowWindow(hwnd, SW_HIDE); + } + PredefinedMenuItemType::CloseWindow => { + SendMessageW(hwnd, WM_CLOSE, 0, 0); + } + PredefinedMenuItemType::Quit => { + PostQuitMessage(0); + } + PredefinedMenuItemType::About(Some(ref metadata)) => { + show_about_dialog(hwnd, metadata) + } - _ => {} - }, + _ => {} + } + } + } _ => {} } }