Merge pull request #470 from JakeStanger/fix/tray

More tray fixes

docs/modules/Tray.md

@@ -7,9 +7,10 @@ Displays a fully interactive icon tray using the KDE `libappindicator` protocol.
 > Type: `tray`
 
 
-| Name        | Type     | Default                                                         | Description                                                                                                              |
+| Name        | Type      | Default                                                         | Description                                                                                                              |
-|-------------|----------|-----------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------|
+|-------------|-----------|-----------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------|
-| `direction` | `string` | `left_to_right` if bar is horizontal, `top_to_bottom` otherwise | Direction to display the tray items. Possible values: `top_to_bottom`, `bottom_to_top`, `left_to_right`, `right_to_left` |
+| `direction` | `string`  | `left_to_right` if bar is horizontal, `top_to_bottom` otherwise | Direction to display the tray items. Possible values: `top_to_bottom`, `bottom_to_top`, `left_to_right`, `right_to_left` |
+| `icon_size` | `integer` | `16`                                                            | Size in pixels to display tray icons as                                                                                  |
 
 <details>
 <summary>JSON</summary>

src/image/provider.rs

@@ -172,7 +172,7 @@ impl<'a> ImageProvider<'a> {
                     );
 
                     // Different error types makes this a bit awkward
-                    match pixbuf.map(|pixbuf| Self::create_and_load_surface(&pixbuf, &image, scale))
+                    match pixbuf.map(|pixbuf| Self::create_and_load_surface(&pixbuf, &image))
                     {
                         Ok(Err(err)) => error!("{err:?}"),
                         Err(err) => error!("{err:?}"),
@@ -203,7 +203,7 @@ impl<'a> ImageProvider<'a> {
             _ => unreachable!(), // handled above
         }?;
 
-        Self::create_and_load_surface(&pixbuf, image, scale)
+        Self::create_and_load_surface(&pixbuf, image)
     }
 
     /// Attempts to create a Cairo surface from the provided `Pixbuf`,
@@ -211,10 +211,13 @@ impl<'a> ImageProvider<'a> {
     /// The surface is then loaded into the provided image.
     ///
     /// This is necessary for HiDPI since `Pixbuf`s are always treated as scale factor 1.
-    fn create_and_load_surface(pixbuf: &Pixbuf, image: &gtk::Image, scale: i32) -> Result<()> {
+    pub fn create_and_load_surface(pixbuf: &Pixbuf, image: &gtk::Image) -> Result<()> {
         let surface = unsafe {
-            let ptr =
+            let ptr = gdk_cairo_surface_create_from_pixbuf(
-                gdk_cairo_surface_create_from_pixbuf(pixbuf.as_ptr(), scale, std::ptr::null_mut());
+                pixbuf.as_ptr(),
+                image.scale_factor(),
+                std::ptr::null_mut(),
+            );
             Surface::from_raw_full(ptr)
         }?;
 

src/modules/tray/icon.rs

@@ -1,3 +1,5 @@
+use crate::image::ImageProvider;
+use color_eyre::{Report, Result};
 use glib::ffi::g_strfreev;
 use glib::translate::ToGlibPtr;
 use gtk::ffi::gtk_icon_theme_get_search_path;
@@ -36,38 +38,49 @@ fn get_icon_theme_search_paths(icon_theme: &IconTheme) -> HashSet<String> {
     paths
 }
 
+pub fn get_image(item: &StatusNotifierItem, icon_theme: &IconTheme, size: u32) -> Result<Image> {
+    get_image_from_icon_name(item, icon_theme, size).or_else(|_| get_image_from_pixmap(item, size))
+}
+
 /// Attempts to get a GTK `Image` component
 /// for the status notifier item's icon.
-pub(crate) fn get_image_from_icon_name(
+fn get_image_from_icon_name(
     item: &StatusNotifierItem,
     icon_theme: &IconTheme,
-) -> Option<Image> {
+    size: u32,
+) -> Result<Image> {
     if let Some(path) = item.icon_theme_path.as_ref() {
         if !path.is_empty() && !get_icon_theme_search_paths(icon_theme).contains(path) {
             icon_theme.append_search_path(path);
         }
     }
 
-    item.icon_name.as_ref().and_then(|icon_name| {
+    let icon_info = item.icon_name.as_ref().and_then(|icon_name| {
-        let icon_info = icon_theme.lookup_icon(icon_name, 16, IconLookupFlags::empty());
+        icon_theme.lookup_icon(icon_name, size as i32, IconLookupFlags::empty())
-        icon_info.map(|icon_info| Image::from_pixbuf(icon_info.load_icon().ok().as_ref()))
+    });
-    })
+
+    let pixbuf = icon_info.unwrap().load_icon()?;
+
+    let image = Image::new();
+    ImageProvider::create_and_load_surface(&pixbuf, &image)?;
+    Ok(image)
 }
 
 /// Attempts to get an image from the item pixmap.
 ///
 /// The pixmap is supplied in ARGB32 format,
 /// which has 8 bits per sample and a bit stride of `4*width`.
-pub(crate) fn get_image_from_pixmap(item: &StatusNotifierItem) -> Option<Image> {
+fn get_image_from_pixmap(item: &StatusNotifierItem, size: u32) -> Result<Image> {
     const BITS_PER_SAMPLE: i32 = 8;
 
     let pixmap = item
         .icon_pixmap
         .as_ref()
-        .and_then(|pixmap| pixmap.first())?;
+        .and_then(|pixmap| pixmap.first())
+        .ok_or_else(|| Report::msg("Failed to get pixmap from tray icon"))?;
 
     let bytes = glib::Bytes::from(&pixmap.pixels);
-    let row_stride = pixmap.width * 4; //
+    let row_stride = pixmap.width * 4;
 
     let pixbuf = Pixbuf::from_bytes(
         &bytes,
@@ -80,7 +93,10 @@ pub(crate) fn get_image_from_pixmap(item: &StatusNotifierItem) -> Option<Image>
     );
 
     let pixbuf = pixbuf
-        .scale_simple(16, 16, InterpType::Bilinear)
+        .scale_simple(size as i32, size as i32, InterpType::Bilinear)
         .unwrap_or(pixbuf);
-    Some(Image::from_pixbuf(Some(&pixbuf)))
+
+    let image = Image::new();
+    ImageProvider::create_and_load_surface(&pixbuf, &image)?;
+    Ok(image)
 }

src/modules/tray/interface.rs

@@ -188,8 +188,23 @@ enum TrayMenuWidget {
 
 impl TrayMenuItem {
     fn new(info: &MenuItemInfo, tx: mpsc::Sender<i32>) -> Self {
+        let mut submenu = HashMap::new();
         let menu = Menu::new();
 
+        macro_rules! add_submenu {
+            ($menu:expr, $widget:expr) => {
+                if !info.submenu.is_empty() {
+                    for sub_item in &info.submenu {
+                        let sub_item = TrayMenuItem::new(sub_item, tx.clone());
+                        call!($menu, add, sub_item.widget);
+                        submenu.insert(sub_item.id, sub_item);
+                    }
+
+                    $widget.set_submenu(Some(&menu));
+                }
+            };
+        }
+
         let widget = match (info.menu_type, info.toggle_type) {
             (MenuType::Separator, _) => TrayMenuWidget::Separator(SeparatorMenuItem::new()),
             (MenuType::Standard, ToggleType::Checkmark) => {
@@ -200,6 +215,8 @@ impl TrayMenuItem {
                     .active(info.toggle_state == ToggleState::On)
                     .build();
 
+                add_submenu!(menu, widget);
+
                 {
                     let tx = tx.clone();
                     let id = info.id;
@@ -212,12 +229,13 @@ impl TrayMenuItem {
                 TrayMenuWidget::Checkbox(widget)
             }
             (MenuType::Standard, _) => {
-                let builder = MenuItem::builder()
+                let widget = MenuItem::builder()
                     .label(&info.label)
                     .visible(info.visible)
-                    .sensitive(info.enabled);
+                    .sensitive(info.enabled)
+                    .build();
 
-                let widget = builder.build();
+                add_submenu!(menu, widget);
 
                 {
                     let tx = tx.clone();
@@ -236,7 +254,7 @@ impl TrayMenuItem {
             id: info.id,
             widget,
             menu_widget: menu,
-            submenu: HashMap::new(),
+            submenu,
             tx,
         }
     }

src/modules/tray/mod.rs

@@ -18,12 +18,20 @@ use tokio::sync::mpsc;
 
 #[derive(Debug, Deserialize, Clone)]
 pub struct TrayModule {
+    #[serde(default = "default_icon_size")]
+    icon_size: u32,
+
     #[serde(default, deserialize_with = "deserialize_orientation")]
-    pub direction: Option<PackDirection>,
+    direction: Option<PackDirection>,
+
     #[serde(flatten)]
     pub common: Option<CommonConfig>,
 }
 
+const fn default_icon_size() -> u32 {
+    16
+}
+
 fn deserialize_orientation<'de, D>(deserializer: D) -> Result<Option<PackDirection>, D::Error>
 where
     D: serde::Deserializer<'de>,
@@ -106,7 +114,7 @@ impl Module<MenuBar> for TrayModule {
 
             // listen for UI updates
             glib_recv!(context.subscribe(), update =>
-                on_update(update, &container, &mut menus, &icon_theme, &context.controller_tx)
+                on_update(update, &container, &mut menus, &icon_theme, self.icon_size, &context.controller_tx)
             );
         };
 
@@ -124,6 +132,7 @@ fn on_update(
     container: &MenuBar,
     menus: &mut HashMap<Box<str>, TrayMenu>,
     icon_theme: &IconTheme,
+    icon_size: u32,
     tx: &mpsc::Sender<NotifierItemCommand>,
 ) {
     match update {
@@ -148,11 +157,9 @@ fn on_update(
                 }
 
                 if item.icon_name.as_ref() != menu_item.icon_name() {
-                    match icon::get_image_from_icon_name(&item, icon_theme)
+                    match icon::get_image(&item, icon_theme, icon_size) {
-                        .or_else(|| icon::get_image_from_pixmap(&item))
+                        Ok(image) => menu_item.set_image(&image),
-                    {
+                        Err(_) => menu_item.set_label(label),
-                        Some(image) => menu_item.set_image(&image),
-                        None => menu_item.set_label(label),
                     };
                 }