Fix/Hyprland/Workspaces: Window Rewrite on multiple non-overlapping bars #2817

Syndelis · 2024-01-08T21:29:08Z

About the Problem

An issue exists when using the Window Rewrite feature on Hyprland/Workspaces. The issue manifests only when the user has all_outputs = false on their configuration and moves a window from one of monitor A's workspaces to one of monitor B's workspaces. When that happens, the window's icon vanishes, and can only be seen again if the bar is restarted.

This happens because on this setup, not all workspaces exist accross all bar instances. The way that window moving works is that it "cuts" the icon from workspace 1 and "pastes" it to workspace 2, avoiding having to re-process the window's icon. However, since the origin workspace might not exist on one of the bar's instances, the window is rendered empty.

About the Solution

This PR introduces a new map Workspacess::m_orphanWindowMap which will now hold every window that couldn't be assigned to a workspace. This is then used when moving windows around if the origin workspace is not present on that bar's instance.

Related PRs

Although #2809 also fixes the same issue, it does so by fetching the clients data on every call to Workspaces::update(). The solution being introduced here should hopefully be enough to avoid what might be a resource-intensive call.

CC: @zjeffer @khaneliman

this attribute will keep every window that doesn't have an associated workspace in the current bar

this avoids the window arriving with the wrong icon when its eventually able to be created

khaneliman · 2024-01-08T21:39:33Z

building it now to test it out

zjeffer · 2024-01-08T22:17:11Z

src/modules/hyprland/workspaces.cpp

@@ -625,10 +670,13 @@ void Workspaces::init() {

  for (Json::Value workspaceJson : workspacesJson) {
    std::string workspaceName = workspaceJson["name"].asString();
+    spdlog::info("initing workpsace {}:{}", workspaceJson["id"], workspaceJson["name"]);


Suggested change

spdlog::info("initing workpsace {}:{}", workspaceJson["id"], workspaceJson["name"]);

spdlog::debug("initing workspace {}:{}", workspaceJson["id"], workspaceJson["name"]);

Oh, honestly that's a leftover; didn't intend to log anything haha. Good catch

Addressed in 4339030

khaneliman

looks like it fixed both issues for me. thanks!

khaneliman · 2024-01-08T22:24:55Z

Err... maybe not. Old behavior on reboot. Gonna have to investigate again.. was bad timing on the catch2 update in this project... nixpkgs doesn't have the latest version so i'm doing some stupid overlay stuff to get this program to build.

zjeffer · 2024-01-08T22:29:56Z

Err... maybe not. Old behavior on reboot. Gonna have to investigate again.. was bad timing on the catch2 update in this project... nixpkgs doesn't have the latest version so i'm doing some stupid overlay stuff to get this program to build.

You should be able to change the minimum catch2 version in meson.build. I had the same issue with the arch package.

khaneliman · 2024-01-08T22:31:24Z

Err... maybe not. Old behavior on reboot. Gonna have to investigate again.. was bad timing on the catch2 update in this project... nixpkgs doesn't have the latest version so i'm doing some stupid overlay stuff to get this program to build.

You should be able to change the minimum catch2 version in meson.build. I had the same issue with the arch package.

Yeah, i'm creating a branch that just lowers that version again just for testing until I can spend a little bit of time on the proper way of passing the nixpkgs-master catch2_3 package as the dependency to use.

khaneliman · 2024-01-08T22:34:34Z

@Syndelis so after rebooting a couple times. It looks like there's still something weird going on for the initial boot behavior. The icons might not render but I can drag them to another monitor and then back and see the icons. I'll try and create some logs later.

Syndelis · 2024-01-08T22:50:47Z

As a side note, we should definitely remove catch2 as an actual build/runtime dependency, and use it only for test-runner builds. ~~Also, it probably should be a git submodule, rather than a system lib dependency~~ it's already sort of a submodule, my bad.

@Syndelis so after rebooting a couple times. It looks like there's still something weird going on for the initial boot behavior. The icons might not render but I can drag them to another monitor and then back and see the icons.

I've just installed the newly built version with ninja -C build install and rebooted my system; everything seems fine. Does launching new programs not get their icons rendered in your bar? Or have you got some programs starting before Waybar does?

Edit: You know what? Swapping workspaces between monitors actually make the windows disappear, so maybe you have a setup doing something similar? I'll try to fix that.
Edit 2: Fixed on 4339030

khaneliman · 2024-01-08T23:07:37Z

I have two bars, one on each monitor. Workspace 1 is assigned to one monitor and the rest on the other. Waybar runs as a systemd service on login and other applications are launched via hyprland on login. The other applications started via hyprland on login are what don’t get rendered. With this PR they at least show after I drag them to the other monitor and render again dragging back to their workspace.

Syndelis · 2024-01-08T23:20:04Z

The other applications started via hyprland on login are what don’t get rendered. With this PR they at least show after I drag them to the other monitor and render again dragging back to their workspace.

I'd guess the applications are getting ophaned on both bars, but I'm unsure of what could be causing that. Could you verify that with this patch?

diff --git a/src/modules/hyprland/workspaces.cpp b/src/modules/hyprland/workspaces.cpp
index 3f85b975..b15680bd 100644
--- a/src/modules/hyprland/workspaces.cpp
+++ b/src/modules/hyprland/workspaces.cpp
@@ -222,6 +222,7 @@ void Workspaces::doUpdate() {
       if (windowPayload.incrementTimeSpentUncreated() < WINDOW_CREATION_TIMEOUT) {
         notCreated.push_back(windowPayload);
       } else {
+        spdlog::info("Orphaning window {}/{} on bar {}", windowPayload.getAddress(), windowPayload.repr(*this), m_monitorId);
         registerOrphanWindow(windowPayload);
       }
     }

khaneliman · 2024-01-09T04:38:38Z

The other applications started via hyprland on login are what don’t get rendered. With this PR they at least show after I drag them to the other monitor and render again dragging back to their workspace.

I'd guess the applications are getting ophaned on both bars, but I'm unsure of what could be causing that. Could you verify that with this patch?

Yeah, I see a bunch of orphaning window stuff on boot.

Jan 08 22:36:28 khanelinix systemd[3178]: Started Highly customizable Wayland bar for Sway and Wlroots based compositors..
Jan 08 22:36:28 khanelinix waybar[3396]: [2024-01-08 22:36:28.890] [info] Using configuration file /home/khaneliman/.config/waybar/config
Jan 08 22:36:28 khanelinix waybar[3396]: [2024-01-08 22:36:28.893] [info] Discovered appearance 'dark'
Jan 08 22:36:28 khanelinix waybar[3396]: [2024-01-08 22:36:28.893] [info] Using CSS file /home/khaneliman/.config/waybar/style.css
Jan 08 22:36:28 khanelinix waybar[3396]: [2024-01-08 22:36:28.900] [info] Hyprland IPC starting
Jan 08 22:36:28 khanelinix waybar[3396]: [2024-01-08 22:36:28.902] [info] Registering for Hyprland's 'windowtitle' events because a user-defined window rewrite rule uses the 'title' field.
Jan 08 22:36:29 khanelinix .waybar-wrapped[3396]: ../gobject/gsignal.c:2088: type 'GtkWindow' is already overridden for signal id '73'
Jan 08 22:36:29 khanelinix .waybar-wrapped[3396]: ../gobject/gsignal.c:2088: type 'GtkWindow' is already overridden for signal id '72'
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.010] [info] Registering for Hyprland's 'windowtitle' events because a user-defined window rewrite rule uses the 'title' field.
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.285] [info] Bar configured (width: 1880, height: 64) for output: DP-3
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.289] [info] Bar configured (width: 5080, height: 64) for output: DP-1
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.296] [info] Bar configured (width: 1880, height: 64) for output: DP-3
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.416] [info] Orphaning window 3391520/<U+F08B9> on bar 1
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.416] [info] Orphaning window 3391520/<U+F08B9> on bar 0
Jan 08 22:36:29 khanelinix .waybar-wrapped[3396]: Unable to replace properties on 0: Error getting properties for ID
Jan 08 22:36:29 khanelinix .waybar-wrapped[3396]: Unable to replace properties on 0: Error getting properties for ID
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.792] [info] Orphaning window 339c7d0/<U+F066F> on bar 1
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.792] [info] Orphaning window 339c7d0/<U+F066F> on bar 0
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.793] [info] Orphaning window 33a2220/<U+F066F> on bar 1
Jan 08 22:36:29 khanelinix waybar[3396]: [2024-01-08 22:36:29.794] [info] Orphaning window 33a2220/<U+F066F> on bar 0
Jan 08 22:36:30 khanelinix waybar[3396]: [2024-01-08 22:36:30.190] [info] Orphaning window 33a8e30/<U+EB1C> on bar 1
Jan 08 22:36:30 khanelinix waybar[3396]: [2024-01-08 22:36:30.418] [info] Orphaning window 33b5c60/<U+F269> on bar 1
Jan 08 22:36:30 khanelinix waybar[3396]: [2024-01-08 22:36:30.471] [info] Orphaning window 33a0c80/<U+F059> on bar 1
Jan 08 22:36:31 khanelinix waybar[3396]: [2024-01-08 22:36:31.010] [info] Orphaning window 33e4750/<U+F269> on bar 1
Jan 08 22:36:37 khanelinix waybar[3396]: [2024-01-08 22:36:37.499] [info] Orphaning window 33b5c60/<U+F144> on bar 0
Jan 08 22:36:40 khanelinix waybar[3396]: [2024-01-08 22:36:40.340] [info] Orphaning window 33cf640/<U+E795> on bar 1
Jan 08 22:36:40 khanelinix waybar[3396]: [2024-01-08 22:36:40.450] [info] Orphaning window 33d9820/<U+E795> on bar 1
Jan 08 22:36:59 khanelinix .waybar-wrapped[3396]: Status Notifier Item with bus name ':1.74' and object path '/org/ayatana/NotificationItem/steam' is already registered
Jan 08 22:37:17 khanelinix waybar[3396]: [2024-01-08 22:37:17.885] [info] Orphaning window 33ce150/<U+F1B6> on bar 1
Jan 08 22:37:17 khanelinix waybar[3396]: [2024-01-08 22:37:17.887] [info] Orphaning window 33ce150/<U+F1B6> on bar 0

Syndelis · 2024-01-09T13:38:49Z

Yeah, I see a bunch of orphaning window stuff on boot.

Could you check if this patch does it for you, @khaneliman ?

diff --git a/include/modules/hyprland/workspaces.hpp b/include/modules/hyprland/workspaces.hpp
index d2006fcc..ae936bdf 100644
--- a/include/modules/hyprland/workspaces.hpp
+++ b/include/modules/hyprland/workspaces.hpp
@@ -28,6 +28,7 @@ class Workspaces;
 
 class WindowCreationPayload {
  public:
+  WindowCreationPayload() = default;
   WindowCreationPayload(std::string workspace_name, WindowAddress window_address,
                         std::string window_repr);
   WindowCreationPayload(std::string workspace_name, WindowAddress window_address,
@@ -174,7 +175,7 @@ class Workspaces : public AModule, public EventHandler {
   // Map for windows stored in workspaces not present in the current bar.
   // This happens when the user has multiple monitors (hence, multiple bars)
   // and doesn't share windows accross bars (a.k.a `all-outputs` = false)
-  std::map<WindowAddress, std::string> m_orphanWindowMap;
+  std::map<WindowAddress, WindowCreationPayload> m_orphanWindowMap;
 
   enum class SortMethod { ID, NAME, NUMBER, DEFAULT };
   util::EnumParser<SortMethod> m_enumParser;
diff --git a/src/modules/hyprland/workspaces.cpp b/src/modules/hyprland/workspaces.cpp
index 3f85b975..2c54a8b0 100644
--- a/src/modules/hyprland/workspaces.cpp
+++ b/src/modules/hyprland/workspaces.cpp
@@ -130,7 +130,7 @@ auto Workspaces::parseConfig(const Json::Value &config) -> void {
 
 void Workspaces::registerOrphanWindow(WindowCreationPayload create_window_paylod) {
   if (!create_window_paylod.isEmpty(*this)) {
-    m_orphanWindowMap[create_window_paylod.getAddress()] = create_window_paylod.repr(*this);
+    m_orphanWindowMap[create_window_paylod.getAddress()] = create_window_paylod;
   }
 }
 
@@ -222,6 +222,10 @@ void Workspaces::doUpdate() {
       if (windowPayload.incrementTimeSpentUncreated() < WINDOW_CREATION_TIMEOUT) {
         notCreated.push_back(windowPayload);
       } else {
+        spdlog::info("Orphaning window {}/{} on bar {}. No workspace {} on that bar. Workspaces here:", windowPayload.getAddress(), windowPayload.repr(*this), m_monitorId, windowPayload.getWorkspaceName());
+        for (const auto& workspace : m_workspaces) {
+          spdlog::info("\t {}:{}", workspace->id(), workspace->name());
+        }
         registerOrphanWindow(windowPayload);
       }
     }
@@ -316,6 +320,15 @@ void Workspaces::onWorkspaceCreated(std::string const &workspaceName,
       }
     }
   }
+
+  for (const auto& [orphanWindowId, windowPayload] : m_orphanWindowMap) {
+    if (windowPayload.getWorkspaceName() == workspaceName) {
+      spdlog::info("De-orphaning window {} because of Workspace {} created on bar {}", orphanWindowId, workspaceName, m_monitorId);
+      m_windowsToCreate.push_back(windowPayload);
+    }
+  }
+
+  std::erase_if(m_orphanWindowMap, [workspaceName](auto pair) { return pair.second.getWorkspaceName() == workspaceName; });
 }
 
 void Workspaces::onWorkspaceMoved(std::string const &payload) {
@@ -376,6 +389,10 @@ void Workspaces::onWindowClosed(std::string const &addr) {
       break;
     }
   }
+
+  if (m_orphanWindowMap.contains(addr)) {
+    m_orphanWindowMap.erase(addr);
+  }
 }
 
 void Workspaces::onWindowMoved(std::string const &payload) {
@@ -407,7 +424,8 @@ void Workspaces::onWindowMoved(std::string const &payload) {
 
   // ...if it was empty, check if the window is an orphan...
   if (windowRepr.empty() && m_orphanWindowMap.contains(windowAddress)) {
-    windowRepr = m_orphanWindowMap[windowAddress];
+    windowRepr = m_orphanWindowMap[windowAddress].repr(*this);
+    m_orphanWindowMap.erase(windowAddress);
   }
 
   // ...and then add it to the new workspace

This sould de-orphan windows as soon as their workspaces are created

khaneliman · 2024-01-09T18:53:12Z

Yeah, I see a bunch of orphaning window stuff on boot.

Could you check if this patch does it for you, @khaneliman ?

This sould de-orphan windows as soon as their workspaces are created

Didn't seem to change behavior.
https://gist.github.com/khaneliman/0787f1f898c3c9d4b2d641416bc8b3ae

EDIT: reading the output... is hyprland reporting the workspace with the silent command as the name? Does the getWorkspaceName just need to clean up that extra information?

Syndelis · 2024-01-09T19:06:57Z

Yeah, I see a bunch of orphaning window stuff on boot.

Could you check if this patch does it for you, @khaneliman ?
This sould de-orphan windows as soon as their workspaces are created

Didn't seem to change behavior. https://gist.github.com/khaneliman/0787f1f898c3c9d4b2d641416bc8b3ae

EDIT: reading the output... is hyprland reporting the workspace with the silent command as the name? Does the getWorkspaceName just need to clean up that extra information?

Definitely seems like it. However I also got some other bugs where windows may not show and/or never disappear. I guess it's time to refactor out that m_workspacesToCreate/m_workspacesToDestroy duo and actually create/destroy them when required; it's just becoming a huge burden to develop features around.

khaneliman · 2024-01-09T19:09:28Z

This solved it for me. I just threw together something real quick to test the theory.

diff --git a/include/modules/hyprland/workspaces.hpp b/include/modules/hyprland/workspaces.hpp
index ae936bd..52fb41d 100644
--- a/include/modules/hyprland/workspaces.hpp
+++ b/include/modules/hyprland/workspaces.hpp
@@ -40,7 +40,15 @@ class WindowCreationPayload {
   bool reprIsReady() const { return std::holds_alternative<Repr>(m_window); }
   std::string repr(Workspaces& workspace_manager);
 
-  std::string getWorkspaceName() const { return m_workspaceName; }
+  std::string getWorkspaceName() const {
+    std::string workspaceName = m_workspaceName;
+    std::string searchString = " silent";
+    std::size_t found = workspaceName.find(searchString);
+    if (found != std::string::npos) {
+      workspaceName.erase(found, searchString.length());
+    }
+    return workspaceName;
+}
   WindowAddress getAddress() const { return m_windowAddress; }
 
   void moveToWorksace(std::string& new_workspace_name);

Syndelis · 2024-01-10T05:26:08Z

This solved it for me. I just threw together something real quick to test the theory.

Can you check if 9e08512 yields the same results?

khaneliman · 2024-01-10T14:37:10Z

This solved it for me. I just threw together something real quick to test the theory.

Can you check if 9e08512 yields the same results?

Yep, still works after removing my test commits and using your latest commit! Thanks!

khaneliman

@Alexays should be good imo

Alexays · 2024-01-12T08:44:01Z

Nice work guys!

Syndelis added 2 commits January 8, 2024 18:30

feat: add orphan windows attribute to workspaces

512c6fb

this attribute will keep every window that doesn't have an associated workspace in the current bar

fix: rename windows while queued for creation

bc7acbd

this avoids the window arriving with the wrong icon when its eventually able to be created

Syndelis force-pushed the fix/window-rewrite-multiple-bars-no-overlap branch from 703479f to bc7acbd Compare January 8, 2024 21:31

Syndelis mentioned this pull request Jan 8, 2024

fix: hyprlands window-rewrite #2809

Closed

Syndelis mentioned this pull request Jan 8, 2024

hyprland/workspaces window icons sometimes disappear #2802

Open

zjeffer reviewed Jan 8, 2024

View reviewed changes

khaneliman approved these changes Jan 8, 2024

View reviewed changes

feat: fetch clients data when moving workspaces accross monitors

4339030

chore: update Hyprland's featureset in readme

c69a6dd

feat: strip workspace qualifiers when creating windows

9e08512

khaneliman approved these changes Jan 10, 2024

View reviewed changes

zjeffer approved these changes Jan 10, 2024

View reviewed changes

Alexays merged commit fa3ce14 into Alexays:master Jan 12, 2024
9 checks passed

aegagros mentioned this pull request Jan 15, 2024

Window-rewrite rules are ignored when moving windows across monitors #2780

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/Hyprland/Workspaces: Window Rewrite on multiple non-overlapping bars #2817

Fix/Hyprland/Workspaces: Window Rewrite on multiple non-overlapping bars #2817

Syndelis commented Jan 8, 2024

khaneliman commented Jan 8, 2024

zjeffer Jan 8, 2024

Syndelis Jan 8, 2024

Syndelis Jan 8, 2024

khaneliman left a comment

khaneliman commented Jan 8, 2024 •

edited

Loading

zjeffer commented Jan 8, 2024

khaneliman commented Jan 8, 2024

khaneliman commented Jan 8, 2024 •

edited

Loading

Syndelis commented Jan 8, 2024 •

edited

Loading

khaneliman commented Jan 8, 2024

Syndelis commented Jan 8, 2024

khaneliman commented Jan 9, 2024

Syndelis commented Jan 9, 2024

khaneliman commented Jan 9, 2024 •

edited

Loading

Syndelis commented Jan 9, 2024

khaneliman commented Jan 9, 2024 •

edited

Loading

Syndelis commented Jan 10, 2024

khaneliman commented Jan 10, 2024

khaneliman left a comment

Alexays commented Jan 12, 2024

	spdlog::info("initing workpsace {}:{}", workspaceJson["id"], workspaceJson["name"]);
	spdlog::debug("initing workspace {}:{}", workspaceJson["id"], workspaceJson["name"]);

Fix/Hyprland/Workspaces: Window Rewrite on multiple non-overlapping bars #2817

Fix/Hyprland/Workspaces: Window Rewrite on multiple non-overlapping bars #2817

Conversation

Syndelis commented Jan 8, 2024

About the Problem

About the Solution

Related PRs

khaneliman commented Jan 8, 2024

zjeffer Jan 8, 2024

Choose a reason for hiding this comment

Syndelis Jan 8, 2024

Choose a reason for hiding this comment

Syndelis Jan 8, 2024

Choose a reason for hiding this comment

khaneliman left a comment

Choose a reason for hiding this comment

khaneliman commented Jan 8, 2024 • edited Loading

zjeffer commented Jan 8, 2024

khaneliman commented Jan 8, 2024

khaneliman commented Jan 8, 2024 • edited Loading

Syndelis commented Jan 8, 2024 • edited Loading

khaneliman commented Jan 8, 2024

Syndelis commented Jan 8, 2024

khaneliman commented Jan 9, 2024

Syndelis commented Jan 9, 2024

khaneliman commented Jan 9, 2024 • edited Loading

Syndelis commented Jan 9, 2024

khaneliman commented Jan 9, 2024 • edited Loading

Syndelis commented Jan 10, 2024

khaneliman commented Jan 10, 2024

khaneliman left a comment

Choose a reason for hiding this comment

Alexays commented Jan 12, 2024

khaneliman commented Jan 8, 2024 •

edited

Loading

khaneliman commented Jan 8, 2024 •

edited

Loading

Syndelis commented Jan 8, 2024 •

edited

Loading

khaneliman commented Jan 9, 2024 •

edited

Loading

khaneliman commented Jan 9, 2024 •

edited

Loading