Add wayland support #781

fwsmit · 2020-11-22T16:17:59Z

Don't merge this in master yet, it's probably best to put this on a separate branch first.

Thanks to a lot of code from the old wayland branch and from mako, I improved dunst's wayland support.
It now handles mouse input well. This is done in a hacky way by making the sleep in the main loop shorter. I noticed that otherwise the input was only handled when the sleep was over. I'm not sure what the right way to do this is.
The second improvement is the positioning of the notification. Instead of putting the notification at the edge of the screen, it's positioned using zwlr_layer_surface_v1_set_anchor and using some margin. This will probably not work on non-wayland compositors, but I haven't tested that yet.
And thirdly, the Makefile now builds, so that's good. I also made sure to autogenerate the protocol headers with wayland-scanner, so they always stay up-to-date.

These are the things that have to be done, before this can be considered to be merged in master:

progandy · 2020-11-22T16:46:02Z

I think the proper way to fix the wayland mouse interactions would be to add a wayland source to the mainloop (just like x_win_reg_source) and let it handle all wayland events asynchronously. libgwater includes an implementation if you do not want to write your own.

https://github.com/sardemff7/libgwater/blob/master/wayland/libgwater-wayland.c

fwsmit · 2020-11-22T17:15:37Z

Thanks, that looks like a good solution! I'm not sure how the maintainers feel about adding this as a dependency. It won't be too hard to implement with this as a reference either.

fwsmit · 2020-11-24T12:32:10Z

I think the proper way to fix the wayland mouse interactions would be to add a wayland source to the mainloop (just like x_win_reg_source) and let it handle all wayland events asynchronously. libgwater includes an implementation if you do not want to write your own.

https://github.com/sardemff7/libgwater/blob/master/wayland/libgwater-wayland.c

I implemented it. Works well.

tsipinakis · 2020-11-24T23:13:23Z

Looking good!

Don't merge this in master yet, it's probably best to put this on a separate branch first.

Since you're going with the 'one big PR in one go' strategy feel free to work on your branch until you feel you've reached a sufficient milestone to merge this into master, I'll do a full review then (perhaps consider marking the PR as draft until then).

I'm not sure how the maintainers feel about adding this as a dependency.

It looks like that library isn't packaged in Debian and I'd assume other distros so for that alone I'd hesitate adding it as a dependency, given that it's only a ~200 line file, I'm ok with embedding it.

fwsmit · 2020-11-26T11:07:52Z

Since you're going with the 'one big PR in one go' strategy feel free to work on your branch until you feel you've reached a sufficient milestone to merge this into master, I'll do a full review then (perhaps consider marking the PR as draft until then).

I've converted it to a draft.

It looks like that library isn't packaged in Debian and I'd assume other distros so for that alone I'd hesitate adding it as a dependency, given that it's only a ~200 line file, I'm ok with embedding it.

In the linked repository they recommend to use it as a git submodule, but I think it's easier to embed it as well.

codecov-io · 2020-11-28T14:10:58Z

Codecov Report

Merging #781 (3822f09) into master (0e6997b) will decrease coverage by 5.52%.
The diff coverage is 4.14%.

@@            Coverage Diff             @@
##           master     #781      +/-   ##
==========================================
- Coverage   65.01%   59.48%   -5.53%     
==========================================
  Files          30       36       +6     
  Lines        5276     5798     +522     
==========================================
+ Hits         3430     3449      +19     
- Misses       1846     2349     +503

Flag	Coverage Δ
unittests	`59.48% <4.14%> (-5.53%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/draw.c	`0.00% <0.00%> (ø)`
src/dunst.c	`10.18% <0.00%> (-0.20%)`	⬇️
src/input.c	`0.00% <0.00%> (ø)`
src/output.c	`0.00% <0.00%> (-37.50%)`	⬇️
src/wayland/libgwater-wayland.c	`0.00% <0.00%> (ø)`
src/wayland/pool-buffer.c	`0.00% <0.00%> (ø)`
src/wayland/protocols/idle-client-header.h	`0.00% <0.00%> (ø)`
...tocols/wlr-layer-shell-unstable-v1-client-header.h	`0.00% <0.00%> (ø)`
src/wayland/wl.c	`0.00% <0.00%> (ø)`
src/x11/x.c	`2.29% <0.00%> (+0.08%)`	⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e6997b...3822f09. Read the comment docs.

fwsmit · 2020-12-04T18:18:47Z

This PR is now ready for review. The wayland output has almost all of the features of the X11 output. One notable exception is that the wayland version cannot detect fullscreen, thus cannot show only some notifications in fullscreen.
You can however let notifications get on top of fullscreen applications by selecting the layer overlay in the settings.

Here is a summary of the changes in other places:

I extracted the input handling from x.c and put it in its own file.
I added the wayland dependencies to the makefile. You should autogenerate the wayland protocol files with wayland-scanner, but I didn't make that the default for compatibility. Thus the generated files had to be included in the repo (see src/wayland/protocols)
Embedded libgwater in the repo

Edit: There is still one bug that I know of. The input region isn't updated when a notification is clicked away. Only when you move your mouse it's updated again. According to the docs, the input region should resize with the surface. I also tried manually updating the input region (mako does that as well), but that didn't work either. It's not a very important bug, so it can be solved later.
Edit two: I discovered two more bugs.
The first is that notifications with geometry '..x..-0-0' don't appear at the bottom of the screen, but instead just above the status bar (assuming the status bar is at the bottom.
Bug two is that somewhere along the way Xwayland has become buggy. Notifications don't disappear when clicking them away. The window will instead flicker. It's probably worth to fix this one, so I'll look at it soon.

fwsmit · 2020-12-09T13:09:28Z

Bug two is that somewhere along the way Xwayland has become buggy. Notifications don't disappear when clicking them away. The window will instead flicker. It's probably worth to fix this one, so I'll look at it soon.

I bisected it, and the bug was introduced by ed1e3a8 which applies a fix from master. I'm not sure why it works on wayland and X11, but not on Xwayland.

tsipinakis · 2020-12-13T20:15:52Z

I bisected it, and the bug was introduced by ed1e3a8 which applies a fix from master. I'm not sure why it works on wayland and X11, but not on Xwayland.

Try out https://github.com/tsipinakis/dunst/tree/action-fix, a better fix for the same bug. Is the issue still present there?

fwsmit · 2020-12-13T21:32:15Z

Try out https://github.com/tsipinakis/dunst/tree/action-fix, a better fix for the same bug. Is the issue still present there?

I tried it out on wayland, and I didn't see any issues. I'll rebase onto that branch when it is in master.

tsipinakis · 2020-12-17T19:51:57Z

I've pushed the updated fix to master, please rebase when possible.

fwsmit · 2020-12-17T20:23:39Z

I've pushed the updated fix to master, please rebase when possible.

Thanks, I've rebased ( or rather merged) onto master. This doesn't fix the issue on xwayland right away (flickering when clicking on notification and notifications not disapearing).
I remembered that I forgot idle skipping on xwayland (as there is no way to detect if the user is idle) thinking it would fix the second issue. It fixed both issues however, so that's great. Do you have any reason how wrong idle detection make notifications not disappear by clicking on them?

fwsmit · 2021-01-09T12:36:22Z

I've used this branch now on my main desktop. Previously I dunst master with xwayland.

It works, but I'm not yet convinced, this is everything to properly support wayland. Still feels a bit clumsy during daily usage.

Could you elaborate on that? I have compared the wayland and Xwayland versions and the wayland version for me seems way better. Try replacing a long notification with a short one. I believe on Xwayland, you will resize artifacts, while on wayland it's perfect.
I'm curious what you have noticed that's worse on wayland compared to xwayland

bebehei · 2021-01-09T17:24:32Z

Could you elaborate on that?

Well, I used dunst in combination with xwayland for about a month. It wasn't a nice experience, but it worked some way.

Now using it with native wayland, there didn't stick out any big improvements. I still feel pain points one thing, that the window opens on the wrong output. It doesn't open anymore on the same monitor, but it's not well aligned.

But please don't count on me here. I'm a sway user for one month now. I do not understand all concepts of wayland yet and maybe these settings have to be done in sway somewhere.

fwsmit · 2021-01-09T18:00:41Z

Could you elaborate on that?

Well, I used dunst in combination with xwayland for about a month. It wasn't a nice experience, but it worked some way.

Now using it with native wayland, there didn't stick out any big improvements. I still feel pain points one thing, that the window opens on the wrong output. It doesn't open anymore on the same monitor, but it's not well aligned.

But please don't count on me here. I'm a sway user for one month now. I do not understand all concepts of wayland yet and maybe these settings have to be done in sway somewhere.

So if I understand correctly, the dunst window opens on the wrong output and is positioned incorrectly on that output? Looking at your dotfiles it seems like you have

geometry = "300x5-30-30"
follow = keyboard

Note that on wayland the bottom is somehow counted from the top of your status bar (if your status bar is at the bottom). So if you want the notification to appear at the same place as on X11, you might have to subtract the status bar height from that. Now that I think of it, it's probably good to document that.

The follow part I don't understand why it's not working. You will have to give more info about that.

fwsmit · 2021-01-09T18:10:19Z

Another thing to note, I recently discovered there is a wayland protocol for detecting if there is a fullscreen window. I will hopefully implement that protocol for dunst soon, but that will mean that the layer setting will be almost useless. Maybe it's wise to move that setting to the experimental section, making it easy to immediately deprecated it.

Also added some more documentation.

Because idle detection on xwayland is not possible, just assume that the user is not idle. This also somehow fixes notifications not disapearing when clicking on them.

This can be changed in config.mk or by using the command make WAYLAND=0 Also removed using_xwayland function definition as it isn't defined anymore

fwsmit · 2021-01-16T16:26:30Z

@tsipinakis Do you have time to look at this PR?

tsipinakis

Well, I used dunst in combination with xwayland for about a month. It wasn't a nice experience, but it worked some way.

Now using it with native wayland, there didn't stick out any big improvements. I still feel pain points one thing, that the window opens on the wrong output. It doesn't open anymore on the same monitor, but it's not well aligned.

Native support is always better than going through the translation layer. Especially since it looks like most distros are moving away from X11.
Now, some pain points might still be there but they can be fixed with later iterations.

Sorry for the delay in this, I've been a bit busier than I expected lately. This is ready for a merge. Just a tiny log nitpick I noticed when scanning the code for a final pass ;)

src/dunst.c

All comments resolved

fwsmit · 2021-01-18T16:52:58Z

Sorry for the delay in this, I've been a bit busier than I expected lately. This is ready for a merge. Just a tiny log nitpick I noticed when scanning the code for a final pass ;)

No problem. Thanks for reviewing the code. It should be good to go now.

tsipinakis · 2021-01-18T18:28:12Z

Thanks again @fwsmit, that's an insane amount of work you did here! Merged!

Add wayland support

While mako has the more straight-forward config and better out-of-the-box looks, it's missing some killer features: 1. easy silent toggling - mako's mode solution is subpar IMHO and does not allow for easy toggle compared to dunstctl's `set-paused toggle` 2. no easy counting of "waiting" notifications, need to manually parse JSON history compared to `dunstctl count waiting` 3. worst of all, it does not show silenced messages after unsetting silent mode and can only show latest hidden message via `makoctl restore` (though iteratively). dunst just unhides ALL previously paused notifications! Dunst supports Wayland since dunst-project/dunst#781, so no need to sick with mako or even swaync.

While mako has the more straight-forward config and better out-of-the-box looks, it's missing some killer features: 1. easy silent toggling - mako's mode solution is subpar IMHO and does not allow for easy toggle compared to dunstctl's `set-paused toggle` 2. no easy counting of "waiting" notifications, need to manually parse JSON history compared to `dunstctl count waiting` 3. worst of all, it does not show silenced messages after unsetting silent mode and can only show latest hidden message via `makoctl restore` (though iteratively). dunst just unhides ALL previously paused notifications! Dunst supports Wayland since dunst-project/dunst#781, so no need to stick with mako or even swaync.

fwsmit force-pushed the wayland-2 branch from a9a4c65 to 13bbbc2 Compare November 22, 2020 17:19

fwsmit force-pushed the wayland-2 branch from 9d96943 to 6c09134 Compare November 24, 2020 13:43

fwsmit mentioned this pull request Nov 24, 2020

Wayland support, e.g. for sway wm #264

Closed

fwsmit force-pushed the wayland-2 branch from 6c09134 to 56556f8 Compare November 24, 2020 14:32

fwsmit marked this pull request as draft November 26, 2020 11:04

fwsmit changed the title ~~Wayland: handle input and improve positioning of notifications~~ Add wayland support Nov 27, 2020

fwsmit mentioned this pull request Nov 27, 2020

Add missing wayland packages dunst-project/docker-images#3

Merged

fwsmit force-pushed the wayland-2 branch 3 times, most recently from 1775cfb to a605259 Compare November 28, 2020 14:09

fwsmit force-pushed the wayland-2 branch 2 times, most recently from 8cc135b to 7747de0 Compare December 3, 2020 23:24

fwsmit marked this pull request as ready for review December 4, 2020 17:03

fwsmit force-pushed the wayland-2 branch 2 times, most recently from 288c0e6 to 5f1c7e4 Compare December 4, 2020 18:08

fwsmit force-pushed the wayland-2 branch from 5f1c7e4 to 7c2f56f Compare December 5, 2020 21:46

colemickens mentioned this pull request Dec 5, 2020

package request: dunst nix-community/nixpkgs-wayland#250

Closed

tsipinakis mentioned this pull request Dec 13, 2020

Feature Request: Add ability to put certain parts of notification on a specific side of that notification. #744

Closed

fwsmit added 13 commits January 9, 2021 21:48

Added option for forcing X11 on wayland.

25e1d0c

Also added some more documentation.

Free idle handler and remove unnecesary variables

b094074

improve error messages

c40e797

Simplify idle handling

20eebb8

Update input.c

7b26b4b

Update dunstrc with Wayland settings

ae38efe

Fix notifications not disapearing on xwayland

756f822

Because idle detection on xwayland is not possible, just assume that the user is not idle. This also somehow fixes notifications not disapearing when clicking on them.

Change default layer to overlay, add note about pushback

3441f4c

Fix indent

af10f6f

Implement xwayland idle fix differently

5a20d46

Make compiling for wayland optional

bb12727

This can be changed in config.mk or by using the command make WAYLAND=0 Also removed using_xwayland function definition as it isn't defined anymore

Remove unnecesary code. Improve log messages

6a6cd66

Move reason_string debug log to dbus.c

3ba614b

fwsmit force-pushed the wayland-2 branch from ad03d50 to cfacf9c Compare January 9, 2021 20:55

tsipinakis approved these changes Jan 18, 2021

View reviewed changes

src/dunst.c Outdated Show resolved Hide resolved

Minor fixes

3822f09

fwsmit force-pushed the wayland-2 branch from cfacf9c to 3822f09 Compare January 18, 2021 16:52

tsipinakis merged commit 77bfbc4 into dunst-project:master Jan 18, 2021

fwsmit pushed a commit to fwsmit/dunst that referenced this pull request Jan 19, 2021

Merge pull request dunst-project#781 from fwSmit/wayland-2

d3dd26a

Add wayland support

fwsmit deleted the wayland-2 branch October 17, 2021 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wayland support #781

Add wayland support #781

fwsmit commented Nov 22, 2020 •

edited by bebehei

Loading

progandy commented Nov 22, 2020 •

edited

Loading

fwsmit commented Nov 22, 2020

fwsmit commented Nov 24, 2020 •

edited

Loading

tsipinakis commented Nov 24, 2020

fwsmit commented Nov 26, 2020

codecov-io commented Nov 28, 2020 •

edited

Loading

fwsmit commented Dec 4, 2020 •

edited

Loading

fwsmit commented Dec 9, 2020

tsipinakis commented Dec 13, 2020

fwsmit commented Dec 13, 2020

tsipinakis commented Dec 17, 2020

fwsmit commented Dec 17, 2020

fwsmit commented Jan 9, 2021

bebehei commented Jan 9, 2021 •

edited

Loading

fwsmit commented Jan 9, 2021

fwsmit commented Jan 9, 2021

fwsmit commented Jan 16, 2021

tsipinakis left a comment •

edited

Loading

fwsmit commented Jan 18, 2021

tsipinakis commented Jan 18, 2021

Add wayland support #781

Add wayland support #781

Conversation

fwsmit commented Nov 22, 2020 • edited by bebehei Loading

progandy commented Nov 22, 2020 • edited Loading

fwsmit commented Nov 22, 2020

fwsmit commented Nov 24, 2020 • edited Loading

tsipinakis commented Nov 24, 2020

fwsmit commented Nov 26, 2020

codecov-io commented Nov 28, 2020 • edited Loading

Codecov Report

fwsmit commented Dec 4, 2020 • edited Loading

fwsmit commented Dec 9, 2020

tsipinakis commented Dec 13, 2020

fwsmit commented Dec 13, 2020

tsipinakis commented Dec 17, 2020

fwsmit commented Dec 17, 2020

fwsmit commented Jan 9, 2021

bebehei commented Jan 9, 2021 • edited Loading

fwsmit commented Jan 9, 2021

fwsmit commented Jan 9, 2021

fwsmit commented Jan 16, 2021

tsipinakis left a comment • edited Loading

Choose a reason for hiding this comment

fwsmit commented Jan 18, 2021

tsipinakis commented Jan 18, 2021

fwsmit commented Nov 22, 2020 •

edited by bebehei

Loading

progandy commented Nov 22, 2020 •

edited

Loading

fwsmit commented Nov 24, 2020 •

edited

Loading

codecov-io commented Nov 28, 2020 •

edited

Loading

fwsmit commented Dec 4, 2020 •

edited

Loading

bebehei commented Jan 9, 2021 •

edited

Loading

tsipinakis left a comment •

edited

Loading