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

feat: sway service #246

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat: sway service #246

wants to merge 16 commits into from

Conversation

ozwaldorf
Copy link

@ozwaldorf ozwaldorf commented Jan 8, 2024

Adds a sway service with usage similar to the hyprland service

  • initial sway ipc @kotontrion
  • update service implementation patterns
  • Sway.msg utility function
  • add example widget
  • consider unifying active types
  • examine getWorkspace(id) usage, sway uses workspace.name (ids are random), hyprland uses workspace.id (ids are fixed)
  • explore unified workspace service
  • fix ipc socket writer closing on its own
  • cleanup how workspace.nodes are maintained

Example

import Widget from "resource:///com/github/Aylur/ags/widget.js";
import Sway from "resource:///com/github/Aylur/ags/service/sway.js";

const Workspaces = () => {
  return Widget.Box({
    children: Array.from({ length: 20 }, (_, i) => {
      i += 1;
      return Widget.Button({
        setup: (btn) => {
          btn.hook(Sway, (btn) => {
            const ws = Sway.getWorkspace(`${i}`);
            btn.visible = ws !== undefined;
            btn.toggleClassName("occupied", ws?.nodes.length + ws?.floating_nodes.length > 0);
          }, 'notify::workspaces');

          btn.hook(Sway.active.workspace, (btn) => {
            btn.toggleClassName("active", Sway.active.workspace.name == i);
          });
        },
        on_clicked: () => Sway.msg(`workspace ${i}`),
        child: Widget.Label({
          label: `${i}`,
          class_name: "indicator",
          vpack: "center",
        }),
      })
    }),
  });
};

export default () =>
  Widget.EventBox({
    class_name: "workspaces panel-button",
    child: Widget.Box({
      child: Widget.EventBox({
        on_scroll_up: () => Sway.msg("workspace next"),
        on_scroll_down: () => Sway.msg("workspace prev"),
        class_name: "eventbox",
        child: Workspaces(),
      }),
    }),
  });

@ozwaldorf ozwaldorf force-pushed the feature/sway branch 3 times, most recently from 3e39670 to ebe8ce0 Compare January 8, 2024 20:11
@ozwaldorf
Copy link
Author

ozwaldorf commented Jan 9, 2024

@Aylur I think maybe we could have a services/workspace.js that just contains a shared definition of the active gobjects that the compositor services can reuse, and eventually maybe a unified workspace service that supports all compositor services

@Aylur
Copy link
Owner

Aylur commented Jan 9, 2024

A unified service that would automatically pick up the compositor sounds awesome, but I am not sure how extensive that could be as I am not familiar with the Sway IPC, and I assume there is no standard way to implement it, so later on if we support more compositors there could be some complications.
Just as you mentioned that Sway uses a name and Hyprland using an id alone would break this unified service, so I think we are good without it for now

@ozwaldorf ozwaldorf marked this pull request as ready for review January 15, 2024 00:26
@ozwaldorf ozwaldorf force-pushed the feature/sway branch 2 times, most recently from 30347ef to d676774 Compare January 25, 2024 01:24
@Aylur
Copy link
Owner

Aylur commented Jan 27, 2024

@ozwaldorf How ready do you think this PR is? I thought I would set up Sway just to test it and play around with it, but I changed my mind, I don't really care for Sway. So I will just trust you that this is tested and the best solution for managing the sway socket.

@ozwaldorf
Copy link
Author

ozwaldorf commented Jan 27, 2024

@Aylur currently there's one outstanding bug in keeping sway.workspaces[*].nodes up to date consistently. Since the client node type doesn't contain information about the workspace it's part of, these child fields on the workspaces are important to keep up to date since they're useful for implementing a dock and showing/hiding the dock based on the active workspace's state. Unfortunately, the client events don't contain enough information on their own to manage the workspace nodes. The current workaround to solve this is to basically refresh the entire tree, but this does have a caveat that every signal will fire as the tree is re-populated (monitors, workspaces, clients). One solution is performing deep object comparison, to only update the necessary nodes and send the correct signals, but it might be expensive to do so.

The basic information needed to implement workspace indicators are working fine though, specifically the sway.active.* sub-services.

@Aylur
Copy link
Owner

Aylur commented Jan 27, 2024

There was a similar issue in the Hyprland one. There were race conditions based on what signal the user connected to, I solved it by moving the signals from the Active objects, to the event handler callback.
Basically the idea is that we move every signal emitting after the state logic is handled, its then possible to partially check the previous state with the new one and then signal based on the difference

@dlasky
Copy link

dlasky commented Mar 14, 2024

can i help push this along at all? i have a bit of expy with sway IPC

@Aylur
Copy link
Owner

Aylur commented Mar 14, 2024

I haven't payed much attention to this PR as I got kind of busy
I merged upstream, you should be able to test it out a bit
I won't be able to play around with Sway for a while, but the code lgtm, so if you report back that it works as expected, I'll merge it

@dlasky
Copy link

dlasky commented Mar 14, 2024

No worries very happy to test! Will report back when I've done so.

Aside: Brand new to this project and thanks for your work, I was looking at doing something similar with a gtk - js node bindings but it was super wonky.

@dlasky
Copy link

dlasky commented Mar 15, 2024

question regarding functionality before I can call this out as a bug or not (and/or take a swing at fixing)

if i have workspaces 1,3,5 i jump to 2, it appends it to the end of the list, giving me 1,3,5,2 I suspect that's what @ozwaldorf was referring to?

would you expect that to be managed by the service itself, or the consuming (config) code to order correctly?

edit: yeah its a bug, when i add a window to the workspace it jumps back to the correct order. I'll look into it, I feel like I ran into something similar on https://github.com/dlasky/away-bar

@ozwaldorf
Copy link
Author

ozwaldorf commented Mar 16, 2024

Sorry I haven't had much time to spend on this pr, but hopefully will be able to take a look at the outstanding refactor needed on the socket handling and event triggers.

if i have workspaces 1,3,5 i jump to 2, it appends it to the end of the list, giving me 1,3,5,2 I suspect that's what @ozwaldorf was referring to?

I suppose this is a bug, although I guess it was unclear on the requirement for ordering of the data in the service. Doing a solution where you create a static number of workspaces and dynamically show or hide them if they exist in the data doesn't do things out of order, as demonstrated in the example widget.

The biggest issue atm is that on any workspace but the first, there's a race condition where events are fired before all the previous tree data is repopulated, as outlined #246 (comment)

But a simpler explanation of what happens is:
0. Given workspace 1 and 2 with a client in each

  1. Additional window is created in workspace 2
  2. ipc sends client update notification, dont know workspace info from the event to update tree here though
  3. we request the complete updated tree
  4. state is cleared for loading in the new tree
  5. recursively add to state:
    a. workspace 1 is added
    b. workspace event is fired <- HERE the other workspaces are missing
    c. workspace 2 is added
    d. workspace event is fired

@dlasky
Copy link

dlasky commented Mar 16, 2024

I suppose this is a bug, although I guess it was unclear on the requirement for ordering of the data in the service. Doing a solution where you create a static number of workspaces and dynamically show or hide them if they exist in the data doesn't do things out of order, as demonstrated in the example widget.

yeah i wasn't sure either on this. FWIW an easy way to do it based on my hacking around on this is just dump the workspaces array directly to your service. This has the side effect of ignoring the bag of holding workspace though which may or may not be a good thing depending on your use cases (for me its ideal).

The biggest issue atm is that on any workspace but the first, there's a race condition where events are fired before all the previous tree data is repopulated, as outlined #246 (comment)

ah, that wasn't really what I was experiencing, but I understand the issue I think.

The way I did a 'per workspace set of clients' in away-bar was effectively having a map of clientId's per workspace and pulling the tree (as you are doing) on changes.

The issue you are describing though is likely caused by emitting on line 378.

you could change the structure of the _handleTreeMessage so that it actually returns a list of changed entity types, and do the emit after the recursive walk is completed:

_handleTreeMessage(node) {
  const deltas = _walkTreeMessage(node).flatMap()
  ["monitors","workspaces","clients"].forEach(predicate => {
  if (deltas.indexOf(predicate) !== 0) {
  this.notify(predicate)
  } 
  })
}

depending on how fancy you want to get you could check the maps as well, but diffing might be overly expensive there.

@ozwaldorf
Copy link
Author

you could change the structure of the _handleTreeMessage so that it actually returns a list of changed entity types, and do the emit after the recursive walk is completed

Yeah this is the initial solution i was thinking of, it makes sense.

depending on how fancy you want to get you could check the maps as well, but diffing might be overly expensive there.

Yeah, the most perfect solution would be to do deep object comparison and build a list of change events and fire at once at the end, but I agree this can get really expensive and wouldn't scale for a user with a very complex nested session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants