refactor: rewrite gtk backend from scratch (#24)

examples/tao.rs

@@ -13,7 +13,7 @@ use tao::platform::windows::{EventLoopBuilderExtWindows, WindowExtWindows};
 use tao::{
     event::{ElementState, Event, MouseButton, WindowEvent},
     event_loop::{ControlFlow, EventLoopBuilder},
-    window::WindowBuilder,
+    window::{Window, WindowBuilder},
 };
 
 fn main() {
@@ -148,7 +148,7 @@ fn main() {
                 window_id,
                 ..
             } => {
-                if window_id == window.id() {
+                if window_id == window2.id() {
                     x = position.x;
                     y = position.y;
                 }
@@ -164,12 +164,7 @@ fn main() {
                 ..
             } => {
                 if window_id == window2.id() {
-                    #[cfg(target_os = "windows")]
+                    show_context_menu(&window2, &file_m, x, y);
-                    window_m.show_context_menu_for_hwnd(window2.hwnd() as _, x, y);
-                    #[cfg(target_os = "linux")]
-                    window_m.show_context_menu_for_gtk_window(window2.gtk_window(), x, y);
-                    #[cfg(target_os = "macos")]
-                    menu_bar.show_context_menu_for_nsview(window2.ns_view() as _, x, y);
                 }
             }
             Event::MainEventsCleared => {
@@ -180,9 +175,18 @@ fn main() {
 
         if let Ok(event) = menu_channel.try_recv() {
             if event.id == custom_i_1.id() {
-                file_m.insert(&MenuItem::new("New Menu Item", false, None), 2);
+                file_m.insert(&MenuItem::new("New Menu Item", true, None), 2);
             }
             println!("{:?}", event);
         }
     })
 }
+
+fn show_context_menu(window: &Window, menu: &dyn ContextMenu, x: f64, y: f64) {
+    #[cfg(target_os = "windows")]
+    menu.show_context_menu_for_hwnd(window.hwnd() as _, x, y);
+    #[cfg(target_os = "linux")]
+    menu.show_context_menu_for_gtk_window(window.gtk_window(), x, y);
+    #[cfg(target_os = "macos")]
+    menu.show_context_menu_for_nsview(window.ns_view() as _, x, y);
+}

examples/winit.rs

@@ -11,7 +11,7 @@ use winit::platform::windows::{EventLoopBuilderExtWindows, WindowExtWindows};
 use winit::{
     event::{ElementState, Event, MouseButton, WindowEvent},
     event_loop::{ControlFlow, EventLoopBuilder},
-    window::WindowBuilder,
+    window::{Window, WindowBuilder},
 };
 
 fn main() {
@@ -143,7 +143,7 @@ fn main() {
                 window_id,
                 ..
             } => {
-                if window_id == window.id() {
+                if window_id == window2.id() {
                     x = position.x;
                     y = position.y;
                 }
@@ -159,10 +159,7 @@ fn main() {
                 ..
             } => {
                 if window_id == window2.id() {
-                    #[cfg(target_os = "windows")]
+                    show_context_menu(&window2, &window_m, x, y);
-                    window_m.show_context_menu_for_hwnd(window2.hwnd(), x, y);
-                    #[cfg(target_os = "macos")]
-                    menu_bar.show_context_menu_for_nsview(window2.ns_view() as _, x, y);
                 }
             }
             Event::MainEventsCleared => {
@@ -179,3 +176,10 @@ fn main() {
         }
     })
 }
+
+fn show_context_menu(window: &Window, menu: &dyn ContextMenu, x: f64, y: f64) {
+    #[cfg(target_os = "windows")]
+    menu.show_context_menu_for_hwnd(window.hwnd() as _, x, y);
+    #[cfg(target_os = "macos")]
+    menu.show_context_menu_for_nsview(window.ns_view() as _, x, y);
+}

src/error.rs

@@ -12,6 +12,12 @@ pub enum Error {
     #[cfg(target_os = "linux")]
     #[error("This menu has not been initialized for this gtk window`")]
     NotInitialized,
+    #[cfg(windows)]
+    #[error("This menu has already been initialized for this hwnd`")]
+    AlreadyInitialized,
+    #[cfg(target_os = "linux")]
+    #[error("This menu has already been initialized for this gtk window`")]
+    AlreadyInitialized,
     #[error("{0}")]
     AcceleratorParseError(String),
     #[error("Cannot map {0} to gdk key")]

src/lib.rs

@@ -126,8 +126,8 @@ pub use submenu::Submenu;
 pub enum MenuItemType {
     Submenu,
     Normal,
-    Check,
     Predefined,
+    Check,
 }
 
 impl Default for MenuItemType {

src/menu.rs

@@ -1,4 +1,4 @@
-use crate::{ContextMenu, MenuItemExt};
+use crate::{util::AddOp, ContextMenu, MenuItemExt};
 
 /// A root menu that can be added to a Window on Windows and Linux
 /// and used as the app global menu on macOS.
@@ -11,48 +11,6 @@ impl Default for Menu {
     }
 }
 
-impl ContextMenu for Menu {
-    #[cfg(target_os = "windows")]
-    fn hpopupmenu(&self) -> windows_sys::Win32::UI::WindowsAndMessaging::HMENU {
-        self.0.hpopupmenu()
-    }
-
-    #[cfg(target_os = "windows")]
-    fn show_context_menu_for_hwnd(&self, hwnd: isize, x: f64, y: f64) {
-        self.0.show_context_menu_for_hwnd(hwnd, x, y)
-    }
-
-    #[cfg(target_os = "windows")]
-    fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) {
-        self.0.attach_menu_subclass_for_hwnd(hwnd)
-    }
-
-    #[cfg(target_os = "windows")]
-    fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) {
-        self.0.detach_menu_subclass_from_hwnd(hwnd)
-    }
-
-    #[cfg(target_os = "linux")]
-    fn show_context_menu_for_gtk_window(&self, w: &gtk::ApplicationWindow, x: f64, y: f64) {
-        self.0.show_context_menu_for_gtk_window(w, x, y)
-    }
-
-    #[cfg(target_os = "linux")]
-    fn gtk_context_menu(&self) -> gtk::Menu {
-        self.0.gtk_context_menu()
-    }
-
-    #[cfg(target_os = "macos")]
-    fn show_context_menu_for_nsview(&self, view: cocoa::base::id, x: f64, y: f64) {
-        self.0.show_context_menu_for_nsview(view, x, y)
-    }
-
-    #[cfg(target_os = "macos")]
-    fn ns_menu(&self) -> *mut std::ffi::c_void {
-        self.0.ns_menu()
-    }
-}
-
 impl Menu {
     /// Creates a new menu.
     pub fn new() -> Self {
@@ -74,7 +32,7 @@ impl Menu {
     ///
     /// [`Submenu`]: crate::Submenu
     pub fn append(&self, item: &dyn MenuItemExt) {
-        self.0.append(item)
+        self.0.add_menu_item(item, AddOp::Append)
     }
 
     /// Add menu items to the end of this menu. It calls [`Menu::append`] in a loop internally.
@@ -98,7 +56,7 @@ impl Menu {
     ///
     /// [`Submenu`]: crate::Submenu
     pub fn prepend(&self, item: &dyn MenuItemExt) {
-        self.0.prepend(item)
+        self.0.add_menu_item(item, AddOp::Insert(0))
     }
 
     /// Add menu items to the beginning of this menu. It calls [`Menu::insert_items`] with position of `0` internally.
@@ -120,7 +78,7 @@ impl Menu {
     ///
     /// [`Submenu`]: crate::Submenu
     pub fn insert(&self, item: &dyn MenuItemExt, position: usize) {
-        self.0.insert(item, position)
+        self.0.add_menu_item(item, AddOp::Insert(position))
     }
 
     /// Insert menu items at the specified `postion` in the menu.
@@ -160,7 +118,7 @@ impl Menu {
     ///
     /// Panics if the gtk event loop hasn't been initialized on the thread.
     #[cfg(target_os = "linux")]
-    pub fn init_for_gtk_window<W>(&self, w: &W) -> std::rc::Rc<gtk::Box>
+    pub fn init_for_gtk_window<W>(&self, w: &W) -> crate::Result<gtk::Box>
     where
         W: gtk::prelude::IsA<gtk::ApplicationWindow>,
         W: gtk::prelude::IsA<gtk::Container>,
@@ -194,7 +152,7 @@ impl Menu {
     /// }
     /// ```
     #[cfg(target_os = "windows")]
-    pub fn init_for_hwnd(&self, hwnd: isize) {
+    pub fn init_for_hwnd(&self, hwnd: isize) -> crate::Result<()> {
         self.0.init_for_hwnd(hwnd)
     }
 
@@ -264,3 +222,45 @@ impl Menu {
         self.0.remove_for_nsapp()
     }
 }
+
+impl ContextMenu for Menu {
+    #[cfg(target_os = "windows")]
+    fn hpopupmenu(&self) -> windows_sys::Win32::UI::WindowsAndMessaging::HMENU {
+        self.0.hpopupmenu()
+    }
+
+    #[cfg(target_os = "windows")]
+    fn show_context_menu_for_hwnd(&self, hwnd: isize, x: f64, y: f64) {
+        self.0.show_context_menu_for_hwnd(hwnd, x, y)
+    }
+
+    #[cfg(target_os = "windows")]
+    fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) {
+        self.0.attach_menu_subclass_for_hwnd(hwnd)
+    }
+
+    #[cfg(target_os = "windows")]
+    fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) {
+        self.0.detach_menu_subclass_from_hwnd(hwnd)
+    }
+
+    #[cfg(target_os = "linux")]
+    fn show_context_menu_for_gtk_window(&self, w: &gtk::ApplicationWindow, x: f64, y: f64) {
+        self.0.show_context_menu_for_gtk_window(w, x, y)
+    }
+
+    #[cfg(target_os = "linux")]
+    fn gtk_context_menu(&self) -> gtk::Menu {
+        self.0.gtk_context_menu()
+    }
+
+    #[cfg(target_os = "macos")]
+    fn show_context_menu_for_nsview(&self, view: cocoa::base::id, x: f64, y: f64) {
+        self.0.show_context_menu_for_nsview(view, x, y)
+    }
+
+    #[cfg(target_os = "macos")]
+    fn ns_menu(&self) -> *mut std::ffi::c_void {
+        self.0.ns_menu()
+    }
+}

src/platform_impl/macos/mod.rs

@@ -147,19 +147,7 @@ impl Menu {
         }
     }
 
-    pub fn append(&self, item: &dyn crate::MenuItemExt) {
+    pub fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
-        self.add_menu_item(item, AddOp::Append)
-    }
-
-    pub fn prepend(&self, item: &dyn crate::MenuItemExt) {
-        self.add_menu_item(item, AddOp::Insert(0))
-    }
-
-    pub fn insert(&self, item: &dyn crate::MenuItemExt, position: usize) {
-        self.add_menu_item(item, AddOp::Insert(position))
-    }
-
-    fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
         let ns_menu_item: *mut Object = item.make_ns_item_for_menu(self.id);
         let child: Rc<RefCell<MenuChild>> = item.get_child();
 
@@ -333,19 +321,7 @@ impl Submenu {
         ns_menu_item
     }
 
-    pub fn append(&self, item: &dyn crate::MenuItemExt) {
+    pub fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
-        self.add_menu_item(item, AddOp::Append)
-    }
-
-    pub fn prepend(&self, item: &dyn crate::MenuItemExt) {
-        self.add_menu_item(item, AddOp::Insert(0))
-    }
-
-    pub fn insert(&self, item: &dyn crate::MenuItemExt, position: usize) {
-        self.add_menu_item(item, AddOp::Insert(position))
-    }
-
-    fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
         let mut self_ = self.0.borrow_mut();
 
         let item_child: Rc<RefCell<MenuChild>> = item.get_child();

src/platform_impl/windows/mod.rs

@@ -182,19 +182,7 @@ impl Menu {
         }
     }
 
-    pub fn append(&self, item: &dyn crate::MenuItemExt) {
+    pub fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
-        self.add_menu_item(item, AddOp::Append)
-    }
-
-    pub fn prepend(&self, item: &dyn crate::MenuItemExt) {
-        self.add_menu_item(item, AddOp::Insert(0))
-    }
-
-    pub fn insert(&self, item: &dyn crate::MenuItemExt, position: usize) {
-        self.add_menu_item(item, AddOp::Insert(position))
-    }
-
-    fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
         let mut flags = 0;
         let child = match item.type_() {
             MenuItemType::Submenu => {
@@ -422,7 +410,11 @@ impl Menu {
         self.hpopupmenu
     }
 
-    pub fn init_for_hwnd(&self, hwnd: isize) {
+    pub fn init_for_hwnd(&self, hwnd: isize) -> crate::Result<()> {
+        if self.hwnds.borrow().iter().any(|h| *h == hwnd) {
+            return Err(crate::Error::AlreadyInitialized);
+        }
+
         self.hwnds.borrow_mut().push(hwnd);
         unsafe {
             SetMenu(hwnd, self.hmenu);
@@ -434,6 +426,8 @@ impl Menu {
             );
             DrawMenuBar(hwnd);
         };
+
+        Ok(())
     }
 
     pub fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) {
@@ -528,19 +522,7 @@ impl Submenu {
         self.0.borrow().hpopupmenu
     }
 
-    pub fn append(&self, item: &dyn crate::MenuItemExt) {
+    pub fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
-        self.add_menu_item(item, AddOp::Append)
-    }
-
-    pub fn prepend(&self, item: &dyn crate::MenuItemExt) {
-        self.add_menu_item(item, AddOp::Insert(0))
-    }
-
-    pub fn insert(&self, item: &dyn crate::MenuItemExt, position: usize) {
-        self.add_menu_item(item, AddOp::Insert(position))
-    }
-
-    fn add_menu_item(&self, item: &dyn crate::MenuItemExt, op: AddOp) {
         let mut flags = 0;
         let child = match item.type_() {
             MenuItemType::Submenu => {

src/submenu.rs

@@ -1,4 +1,4 @@
-use crate::{ContextMenu, MenuItemExt, MenuItemType};
+use crate::{util::AddOp, ContextMenu, MenuItemExt, MenuItemType};
 
 /// A menu that can be added to a [`Menu`] or another [`Submenu`].
 ///
@@ -19,48 +19,6 @@ unsafe impl MenuItemExt for Submenu {
     }
 }
 
-impl ContextMenu for Submenu {
-    #[cfg(target_os = "windows")]
-    fn hpopupmenu(&self) -> windows_sys::Win32::UI::WindowsAndMessaging::HMENU {
-        self.0.hpopupmenu()
-    }
-
-    #[cfg(target_os = "windows")]
-    fn show_context_menu_for_hwnd(&self, hwnd: isize, x: f64, y: f64) {
-        self.0.show_context_menu_for_hwnd(hwnd, x, y)
-    }
-
-    #[cfg(target_os = "windows")]
-    fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) {
-        self.0.attach_menu_subclass_for_hwnd(hwnd)
-    }
-
-    #[cfg(target_os = "windows")]
-    fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) {
-        self.0.detach_menu_subclass_from_hwnd(hwnd)
-    }
-
-    #[cfg(target_os = "linux")]
-    fn show_context_menu_for_gtk_window(&self, w: &gtk::ApplicationWindow, x: f64, y: f64) {
-        self.0.show_context_menu_for_gtk_window(w, x, y)
-    }
-
-    #[cfg(target_os = "linux")]
-    fn gtk_context_menu(&self) -> gtk::Menu {
-        self.0.gtk_context_menu()
-    }
-
-    #[cfg(target_os = "macos")]
-    fn show_context_menu_for_nsview(&self, view: cocoa::base::id, x: f64, y: f64) {
-        self.0.show_context_menu_for_nsview(view, x, y)
-    }
-
-    #[cfg(target_os = "macos")]
-    fn ns_menu(&self) -> *mut std::ffi::c_void {
-        self.0.ns_menu()
-    }
-}
-
 impl Submenu {
     /// Create a new submenu.
     ///
@@ -84,7 +42,7 @@ impl Submenu {
 
     /// Add a menu item to the end of this menu.
     pub fn append(&self, item: &dyn MenuItemExt) {
-        self.0.append(item)
+        self.0.add_menu_item(item, AddOp::Append)
     }
 
     /// Add menu items to the end of this submenu. It calls [`Submenu::append`] in a loop.
@@ -96,7 +54,7 @@ impl Submenu {
 
     /// Add a menu item to the beginning of this submenu.
     pub fn prepend(&self, item: &dyn MenuItemExt) {
-        self.0.prepend(item)
+        self.0.add_menu_item(item, AddOp::Insert(0))
     }
 
     /// Add menu items to the beginning of this submenu.
@@ -109,7 +67,7 @@ impl Submenu {
 
     /// Insert a menu item at the specified `postion` in the submenu.
     pub fn insert(&self, item: &dyn MenuItemExt, position: usize) {
-        self.0.insert(item, position)
+        self.0.add_menu_item(item, AddOp::Insert(position))
     }
 
     /// Insert menu items at the specified `postion` in the submenu.
@@ -171,3 +129,45 @@ impl Submenu {
         self.0.set_help_menu_for_nsapp()
     }
 }
+
+impl ContextMenu for Submenu {
+    #[cfg(target_os = "windows")]
+    fn hpopupmenu(&self) -> windows_sys::Win32::UI::WindowsAndMessaging::HMENU {
+        self.0.hpopupmenu()
+    }
+
+    #[cfg(target_os = "windows")]
+    fn show_context_menu_for_hwnd(&self, hwnd: isize, x: f64, y: f64) {
+        self.0.show_context_menu_for_hwnd(hwnd, x, y)
+    }
+
+    #[cfg(target_os = "windows")]
+    fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) {
+        self.0.attach_menu_subclass_for_hwnd(hwnd)
+    }
+
+    #[cfg(target_os = "windows")]
+    fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) {
+        self.0.detach_menu_subclass_from_hwnd(hwnd)
+    }
+
+    #[cfg(target_os = "linux")]
+    fn show_context_menu_for_gtk_window(&self, w: &gtk::ApplicationWindow, x: f64, y: f64) {
+        self.0.show_context_menu_for_gtk_window(w, x, y)
+    }
+
+    #[cfg(target_os = "linux")]
+    fn gtk_context_menu(&self) -> gtk::Menu {
+        self.0.gtk_context_menu()
+    }
+
+    #[cfg(target_os = "macos")]
+    fn show_context_menu_for_nsview(&self, view: cocoa::base::id, x: f64, y: f64) {
+        self.0.show_context_menu_for_nsview(view, x, y)
+    }
+
+    #[cfg(target_os = "macos")]
+    fn ns_menu(&self) -> *mut std::ffi::c_void {
+        self.0.ns_menu()
+    }
+}