Tray D-Bus Menu

[FlexW]

This pull request implements D-Bus menus for the tray icons. @ianyfan @nmschulte It seems like #5161 is dead, and I would like to take over. Please tell me if this is not the case, then I will close this pull request.

This pull request is based on #5161 and is a draft for now. Please don't test it yet. When the pull request is ready for testing, I will say so.

@ianyfan @nmschulte I created this early pull request to let you know I'm working on it. Just in case that we do not double the work. Maybe you could add some other points that should be addressed before this pull request can be merged. I would say keyboard navigation and touch support can be done in a second pr.

[emersion]

@ianyfan @nmschulte It seems like #5161 is dead

Yes, this PR has been looking for a new owner for a while. Thanks for taking over!

[nmschulte]

@FlexW Thank you for doing this. I would like to support you however I can.

Please see my branch from #5161 work; https://github.com/nmschulte/sway/tree/fix-tray-updates. It has improved message handling to resolve some the standing issues of #5161; it is based upon current master/master, and I rebase it locally about once a week to continue using it. Since 1.6 / wlroots 1.2/1.3, something has broken for me w/re: non-1.0 DPI scaling and popups, such that most apps no longer work to show menus in this scenario.

This is all I have for now; I will be following closely. Later I will check your list and my notes and share any new items I've thought about to incorporate.

[FlexW]

@FlexW Thank you for doing this. I would like to support you however I can.

Please see my branch from #5161 work; https://github.com/nmschulte/sway/tree/fix-tray-updates. It has improved message handling to resolve some the standing issues of #5161; it is based upon current master/master, and I rebase it locally about once a week to continue using it. Since 1.6 / wlroots 1.2/1.3, something has broken for me w/re: non-1.0 DPI scaling and popups, such that most apps no longer work to show menus in this scenario.

This is all I have for now; I will be following closely. Later I will check your list and my notes and share any new items I've thought about to incorporate.

Thank you for your support. I'll check your branch out. I think I had the same scaling issue that you described, and I think that issue is resolved in my branch. There was an issue with not specifying the correct size of the buffer when scaling is activated. If you want you can check out my branch and see if the scaling issue is gone for you too.

[nmschulte]

👍 I hope to dive through the diff later this week.

On Sun, May 2, 2021, 1:41 PM Felix Weilbach @.***> wrote:

@FlexW https://github.com/FlexW Thank you for doing this. I would like to support you however I can.

Please see my branch from #5161 https://github.com/swaywm/sway/pull/5161 work; https://github.com/nmschulte/sway/tree/fix-tray-updates. It has improved message handling to resolve some the standing issues of #5161 https://github.com/swaywm/sway/pull/5161; it is based upon current master/master, and I rebase it locally about once a week to continue using it. Since 1.6 / wlroots 1.2/1.3, something has broken for me w/re: non-1.0 DPI scaling and popups, such that most apps no longer work to show menus in this scenario.

This is all I have for now; I will be following closely. Later I will check your list and my notes and share any new items I've thought about to incorporate.

Thank you for your support. I'll check your branch out. I think I had the same scaling issue that you described, and I think that issue is resolved in my branch. There was an issue with not specifying the correct size of the buffer when scaling is activated. If you want you can check out my branch and see if the scaling issue is gone for you too.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/swaywm/sway/pull/6249#issuecomment-830852893, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBFAT6TYWVL7JLF63C5XS3TLWMENANCNFSM437SF2HQ .

[Xyene]

something has broken for me w/re: non-1.0 DPI scaling and popups, such that most apps no longer work to show menus in this scenario.

Does https://github.com/swaywm/sway/issues/6147 match what you're describing? If so and you're experiencing it with other apps, would be worth leaving a note in that thread.

[nmschulte]

Does #6147 match what you're describing? If so and you're experiencing it with other apps, would be worth leaving a note in that thread.

@Xyene I don't think #6147 describes the issue, but I can try with external displays; experiencing this on a laptop with 4K display, no external monitors.

Zoom does strange things with its menus under Wayland such that the menus popup on the wrong output or don't show at all, depending which output they're parented, on a top-bottom-plus-(rotated-)straddling-sides 4-output DPI==1 stationary desktop setup though. Also I see #6148 with this same setup.

[nmschulte]

Since 1.6 / wlroots 1.2/1.3, something has broken for me w/re: non-1.0 DPI scaling and popups, such that most apps no longer work to show menus in this scenario.

😃 All of the popups seem to be working on this branch (flexw/tray-dbus-menu). swaybar crashed during my first smoke test, however. More to come.

[FlexW]

Since 1.6 / wlroots 1.2/1.3, something has broken for me w/re: non-1.0 DPI scaling and popups, such that most apps no longer work to show menus in this scenario.

All of the popups seem to be working on this branch (flexw/tray-dbus-menu). swaybar crashed during my first smoke test, however. More to come.

Thats great to hear. I'm aware of that crash, it's because I don't handle the hotspots correct fow now. I'll let you know when I fixed it.

[FlexW]

@nmschulte The crash that you experienced is gone now. You may experience other crashes or bugs. It's WIP. Just wanted to let you know :)

[nmschulte]

@FlexW after merging master and dealing with wayland-protocol version issues, I've managed to build this branch and use it for about a week. I haven't yet had time to review the code changes, however. I do see a few bugs and swaybar crashes still, related to sub-menu interactions. I'll investigate and follow up when I can.

[TimB87]

uneducated question: is there a possibility that this might work with basu as well?

[emersion]

It should, yes.

[FlexW]

@nmschulte Finally I found some time to work on it. Should now work better. So far I couldn't crash it anymore and it works as I would expect it with all my tray icons (nmapplet, nextcloud, udiskie, blueman, gammastep-indicator). I would appreciate it if you could do some tests too (I rebased against the latest master). Maybe you also know some more applications with which I can test.

[FlexW]

@ianyfan @emersion One more question: It seems like the wl_pointer_button, wl_pointer_motion event and dbus events etc run in parallel. How would you advise avoiding race conditions while operating on the swaybar_dbusmenu structure? Mutexes?

[emersion]

They don't run "in parallel", there's only one thread. The event loop will dispatch D-Bus events and Wayland events one after the other. No need for any kind of locking or race condition prevention.

[FlexW]

@emersion Thank you for clarifying. Will a call to wl_display_roundtrip() lead to dispatching events? So if I call it in a function I will after wl_display_roundtrip() returns have code running in parallel?

EDIT: Probably not, because there is only one thread.

[nmschulte]

I finally managed to build this (caved and built libdrm, wayland-protocols); sorry my report is shallow.

I managed to crash swaybar playing with udiskie again; I only managed it once and have no logs.

Both udiskie and nm-applet menus seem to flicker more than I'd like to see as a user.

Sub-menus are placed a bit off for me (2x scale), and there's an odd looking "bar"/line/border at the bottom of the sub-menus.

Finally, and I'm not confident in my sway-term/understanding here, there's still something odd with focus, popups, and xwayland vs xdg_shell (backends?). I was unable to use my key binding to launch dmenu; gimp's menubar would not... popup, even though the interaction feedback seemed to work. Just prior, I was using screenshot.sh (dmenu) to take a shot of the open popup(s) from swaybar (which works great/is possible as long as I don't have to click), which uses mako to notify the success (popup?). In the end, I was able to rectify things after mashing the keyboard, switching workspaces, clicking / changing focus, etc. without restarting sway. :shrug: I've spoke about similar things in the original PR.

[FlexW]

Thank you for your feedback @nmschulte. I would like to get this pr reviewed, because I think it works well enough and provides everything to do basic work with tray menus. There are some things that could be better, but I think it is usable. I can not crash it anymore, udiskie does just sometimes blink for me one time when I open the menu. Submenus work also fine.

Maybe one of the maintainers can help with getting the last flickering and focus problems away.

@emersion

[nmschulte]

I started reviewing this last night, FYI. I've found some quirks but they're minor. It seems like this still includes Ian's (and perhaps some of my) work; I wish you hadn't squashed all of the history is all.

I'm trying to carry all of my effort/discovery forward through your changeset before I'm happy, but so far the diff looks less involved than I had thought. More to come...

[emersion]

Please use Co-authored-by to credit the appropriate authors when squashing multiple commits together. Squashing commits to have a nice history is good, but it's also important to retain authorship of the work.

@nmschulte Reviews definitely welcome! I'll try to have a look at some point too.

[emersion]

It seems like this needs a rebase against master.

[FlexW]

Thank you for reviewing this. I will add the authors. The reason why this is only one commit is because I started a new branch. I wasn't able to transition from @ianyfan approach to load everything as lazy as possible (which made it difficult to make submenus work) to my approach that loads all submenus upfront. A lot of the dbus message handling and a big part of the drawing was a copypaste from @ianyfan PR. I will note that in the commit message.

[nmschulte]

@FlexW, try this in the commit message:

Co-authored-by: Ian Fan <[email protected]>
Co-authored-by: Nathan Schulte <[email protected]>

[nmschulte]

I haven't yet chased everything from my end but here's a video showing a few quirks.

1. the already noted flicker/re-painting quirk; I hope I can mitigate this by chasing my end of things.
2. cannot interact with other tray items after opening a the menu for an item and never giving it focus; you must first give it focus, or close it by clicking outside the menu and the bar.
   1. if a menu updates/repaints, its state changes as though it's been interacted with and is "closable." this is a good clue I guess
   2. a user should be able to open a different tray item menu by simply doing so (right click/Activate on it); no extra steps; any open menu(s) automatically close; should not have to interact with a menu in order for it to be "closable."
3. sub menus remain open when cursoring past to siblings; I suspect this is related to the previous bullet. the capture cut off the good example, but you can see it briefly around [0m:14s]
4. scrolling (vertically, with my touchpad]) on the open menu of nm-applet is a sure-fire way to crash swaybar. it's not all menus but seems specific to ones that are dynamic/update, and it seems I must be hovering the menu while scrolling. 00:00:07.671 [swaybar/input.c:306] wl_pointer_axis:axis with no active output Aborted
   1. scrolling on the menu causes my workspaces to change: just like scrolling on the bar itself. I don't think this should occur.

https://user-images.githubusercontent.com/8540239/133378687-13c7fb2a-9726-45ef-baf3-7b61e94de5c7.mp4

[nmschulte]

d3768e4 and 44f947a seem effective as they claim; I haven't caused a crash yet.

there's a new symptom now though: scrolling events/signals seem to just queue up and now take effect when the cursor enter/leaves (I think that's technically accurate).

[FlexW]

d3768e4 and 44f947a seem effective as they claim; I haven't caused a crash yet.

there's a new symptom now though: scrolling events/signals seem to just queue up and now take effect when the cursor enter/leaves (I think that's technically accurate).

fixed :) thanks for all your testing.

[nmschulte]

fixed

:+1: indeed so

---

a few more quirks:

• an action causing an open menu to close should still take the action (e.g. like Firefox and others do, see below)
• an (e.g. scroll; non-"activation" class?) action on the same item as an open menu should not close the menu (see next para; this I suspect is due to repainting issue)
• I'm not sure which popup/window wins, but whatever is going on w/re: this is rather crude (menu vs notify)

I'm guessing these won't be fixed (and therefor, won't be fixed... ever), but I think they're defects:

• the state of the menu is lost during repaint: hovered items no longer appear hovered (and there-in: open sub-menus do not re-open)
• flicker due to repaint (probably this and last are one-and-the-same)

[Seltyk]

What's the quickest way for me to run this on my own machine? Doesn't sound like this is getting merged any time soon but I'd like to give it a whirl without risking my current setup. Trying to build fails because it requires wlroots 0.15+ but that isn't a version that exists. I'm new to Wayland; please forgive the stupid question

[nmschulte]

What's the quickest way for me to run this on my own machine?

I have a script that builds libseat, wlroots, and sway, with wlroots and libseat as "subprojects." I sometimes have to also build wayland-protocols and deal with some other lower-level hurdles, but those are infrequent and tend to resolve as my distro "catches up" upstream. I'll share it here for you, but note it is raw and won't work out of the box, and I won't be able to support it.

https://gist.github.com/nmschulte/66a0b798dded3947695ade6bbd8189f5

[FlexW]

@bl4ckb0ne Thanks for reviewing. It might make sense to change the .clang-format file to account for this style because the formating in this pr was all done by clang-format automatically.