Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wlr/taskbar on multi-monitor: icons not displayed for apps launched before waybar #3366

Closed
abc-nix opened this issue Jun 17, 2024 · 4 comments
Labels
bug Something isn't working sway

Comments

@abc-nix
Copy link

abc-nix commented Jun 17, 2024

Wayland compositor: sway (v 1.9) or Hyprland (0.41.1) [error happens with both]

Waybar version: 0.10.3

I have 2 monitors (output LVDS-1 and HDMI-A-1), and the exact same waybar configuration is applied to both. Specifically for wlr/taskbar:

wlr/taskbar config
    "wlr/taskbar": {
      "format": "{icon}",
      "icon-size": 24,
      "on-click": "activate",
      "on-click-middle": "close",
    },

Observations:

  1. If waybar has already started, all new apps that I open will display the correct icon on any of the waybar taskbars on both monitors.
  2. If waybar starts later, with an app already open, the icon is only displayed correctly in one of the taskbars (the one in HDMI-A-1), and not on the other (there is an empty space with no icon, though it works when clicked and tooltips are displayed).

Logs:

By using log-level trace, I see that icons are found and "Loaded" for all open applications, but only are "now visible" on the HDMI-A-1 output. When the icon is "loaded" for the application on LVDS-1, it isn't "now visible". See examples below:

Example on HDMI-A-1:

Task (4) overwriting app_id 'unknown' with 'firefox-esr'
Task (4) Mozilla Firefox Private Browsing [firefox-esr] <amif> Loaded icon 'firefox-esr'
Task (4) Mozilla Firefox Private Browsing [firefox-esr] <amif> entered output 0x5d274d223ef0
Task (4) Mozilla Firefox Private Browsing [firefox-esr] <amif> now visible on HDMI-A-1
Task (4) Mozilla Firefox Private Browsing [firefox-esr] <amif> changed

Example on LVDS-1:

Task (7) overwriting app_id 'unknown' with 'thunar'
Task (7) waybar - Thunar [thunar] <amif> Loaded icon 'org.xfce.thunar'
Task (7) waybar - Thunar [thunar] <amif> entered output 0x5d274d2bc930
Task (7) waybar - Thunar [thunar] <amif> changed

As you can see, on LVDS-1, the line "now visible on LVDS-1" is missing. I think this could help find the issue.

I have attached the trace logs in case it is helpful.

Operations performed:

  1. Run sway with 2 apps open on each monitor (4 apps in total, foot and thunar on LVDS-1, firefox and geany on HDMI-A-1).
  2. Start waybar.
  3. Move the cursor to the LVDS-1 monitor (on thunar).
  4. Open a new foot terminal (icon is correctly rendered for this app on LVDS-1).
  5. Kill waybar.

I will provide any information required to help understand and fix this bug.

waybar.log

@RobertMueller2
Copy link
Contributor

RobertMueller2 commented Jun 30, 2024

I think Task (7) is not the correct task we should be looking at.

My (limited) understanding of wlr/taskbar is that all apps have a Task object representation in the module regardless of whether the app is currently on the module's output or not. There is a module instance for each output. So there are actually two representations for thunar, one is a Task object in the taskbar module on LVDS-1, and one is a Task object in the taskbar module on HDMI-A-1.

Task (7) is thunar's representation on HDMI-A-1's wlr/taskbar module. Task (3) is thunar's representation on LVDS-1.

When the module is rendered (or when apps are added, moved or removed), the handle_output_enter/handle_output_leave run in each module. But using tbar_->show_output(output) the method checks if the app's output matches the module's output and skips the rest if not. THAT explains why you don't see a "now visible on" line. Looking at Task (3):

[2024-06-16 16:30:47.551] [debug] Task (3) setting app_id to unknown
[2024-06-16 16:30:47.553] [debug] Task (3) overwriting app_id 'unknown' with 'thunar'
[2024-06-16 16:30:47.553] [debug] Couldn't find icon for thunar
[2024-06-16 16:30:47.553] [debug] Task (3) waybar - Thunar [thunar] <amif> entered output 0x5d274d2bc930
[2024-06-16 16:30:47.553] [debug] Task (3) waybar - Thunar [thunar] <amif> now visible on LVDS-1
[2024-06-16 16:30:47.553] [debug] Task (3) waybar - Thunar [thunar] <amif> changed
[2024-06-16 16:30:48.770] [debug] Task (3) waybar - Thunar [thunar] <Amif> changed
[2024-06-16 16:30:50.426] [debug] Task (3) waybar - Thunar [thunar] <Amif> changed
[2024-06-16 16:30:50.471] [debug] Task (3) waybar - Thunar [thunar] <amif> changed

I think [2024-06-16 16:30:47.553] [debug] Couldn't find icon for thunar is the interesting part. The module will never try to load an icon again for a Task where the app id is already set.

Why that again is happening, I'm not sure at this point.

I can reproduce the issue, though.

@RobertMueller2
Copy link
Contributor

Apparently, the icon theme can be empty, so the for loop in handle_app_id isn't entered. This means that the constructor for Taskbar has not run, but the Tasks already get their app_ids/icons. Possibly something around the static methods, but with my limited knowledge of C++ that might be complete nonsense. The following seems to work around the issue:

diff --git a/src/modules/wlr/taskbar.cpp b/src/modules/wlr/taskbar.cpp
index 41d46748..bf634192 100644
--- a/src/modules/wlr/taskbar.cpp
+++ b/src/modules/wlr/taskbar.cpp
@@ -434,10 +434,16 @@ void Task::handle_app_id(const char *app_id) {
 
   int icon_size = config_["icon-size"].isInt() ? config_["icon-size"].asInt() : 16;
   bool found = false;
-  for (auto &icon_theme : tbar_->icon_themes()) {
-    if (image_load_icon(icon_, icon_theme, app_info_, icon_size)) {
+  if (tbar_->icon_themes().size() == 0) {
+    if (image_load_icon(icon_, Gtk::IconTheme::get_default(), app_info_, icon_size)) {
       found = true;
-      break;
+    }
+  } else {
+    for (auto &icon_theme : tbar_->icon_themes()) {
+      if (image_load_icon(icon_, icon_theme, app_info_, icon_size)) {
+        found = true;
+        break;
+      }
     }
   }

But of course it has the disadvantage that any icon themes specified in the config would be ignored on that particular screen, for existing tasks. That shouldn't be the solution. :P

@RobertMueller2
Copy link
Contributor

Workaround (a different one) is now in the master branch via PR #3393 . @abc-nix Any chance you can check if it solves the issue for you?

@abc-nix
Copy link
Author

abc-nix commented Jul 1, 2024

Fantastic, @RobertMueller2. It works. I can only test on sway right now, but it works and fixes the issue on multi-monitors (at least on my system).

Many thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sway
Projects
None yet
Development

No branches or pull requests

2 participants