Add support for notification gaps

[paddyw2]

Summary

Provide gap support by adding option to draw blank spaces between notifications.

Related issues:

• Margin / spacing / gaps between notifications? #1011

To support this feature, three general changes were required.

The first increases the size of the total image surface used by cairo so that extra space required by gaps does not crop notifications. To calculate this, the gap size plus size of top and bottom border frames are summed for each non top or bottom notification and added to the total height. This is because normally only the top and bottom frame widths are shown, but with gaps they are shown for each notification.

The second increments the y offset by an additional value of the gap size and extra frame widths. When gaps are enabled, each notification is treated as both first and last to reuse top and bottom frame borders draw logic.

Lastly, the click handling logic is updated to correctly associate y coordinates with the new notification positions when gaps are enabled.

To use:

[global]
    gaps = true
    gap_size = 10

Limitations/Potential Improvements

As mentioned in #1011 the gap areas are still drawn as "dead space" by cairo so any clicks on these areas do not pass down to applications below as a user may expect. Therefore this is only really suitable for small gap sizes where clicking between gaps to access applications below is not required. Additionally, like rounded corners and transparency, a compositor such as picom is required otherwise the gaps will show up as black space.

I tried adding some unit tests in test/draw.c to target the logic within calculate_dimensions and layout_render but open to feedback on better ways to do this. Same goes for a functional test/demo under test/functional-tests/test.sh that uses test/functional-tests/dunstrc.gaps where I also tried to added the option to test out click actions.

I'm new to this project so let me know if there's anything I can change or improve on :)

[fwsmit]

Thanks for putting in the work to submit this and also for creating tests to keep it functional in the future. I've added a few comments mostly to simplify the implementation. If they turn out to be more complicated, let me know and I'll take a look at it.

Could you add the new settings to docs/dunst.5.pod to include them in the man page?

Also, the tests are currently failing, because of a memory leak (might be in the test itself). Could you take a look at that as well. You can run the memleak test locally by running make test-valgrind.

[paddyw2]

Thanks for the feedback, it definitely helped clean it up. The valgrind errors are due to the tests and are caused by pangocairo internals. I had initially added pango_cairo_font_map_set_default call which appeared to resolve the issue on my distro but after the workflow ran I see it still persisted on others. I was able to recreate and (apparently) resolve also on Ubuntu by adding a call to FcFini from fontconfig (see https://github.com/paddyw2/dunst/blob/d7dbed5f3b94b8aa1e3ae6de33ca73e2d011b179/test/helpers.c#L70) but will have to see how that holds up when tested against others.

In addition to dunst.5.pod I also added some docs to dunstrc, which I'm guessing should also be updated to highlight defaults. While doing so it occurred to me that I could potentially just use a single settings value gap_size with a value of zero indicating gaps are disabled. Upside is less settings, but downside is the code may be slightly less readable (if (settings.gaps) {...} vs. if (settings.gap_size) {...} or if (settings.gap_size > 0) {...}). It would be an easy change so let me know if you have any thoughts on that or anything else.

[paddyw2]

Looks like the leak still occurs in at least debian:stretch. I was able to reproduce in a local debian stretch container but couldn't find anything else to try clean it up. My guess here is that it is maybe something to do with the fontconfig library age in each distro. If that's the case would adding a valgrind suppression be best? Something like:

{
   fontconfig-pango-leaks
   Memcheck:Leak
   fun:malloc
   ...
   fun:FcConfigSubstituteWithPat
   ...
   fun:pango_layout_get_pixel_extents
}

[fwsmit]

Looks like the issue is on debian's part, so go ahead and add it to the suppressions.

[fwsmit]

While doing so it occurred to me that I could potentially just use a single settings value gap_size with a value of zero indicating gaps are disabled.

Yeah seems like a good change.

[paddyw2]

I added a suppression entry to target both malloc and realloc leaks coming from the fontconfig via pango and it seems to work on debian, ubuntu and arch. Let me know if you have any suggestions there. I also removed the gaps boolean setting and updated the tests and docs to match.

As a side note, only the notification height is clickable and not the frame/border, which I think mirrors the previous logic in get_notification_at from src/input.c. This can be seen with the functional "gaps" test where I used an exaggerated frame border size to highlight this. However, as mouse_x is not currently used in input_handle_click, any click along the x-axis including the frame/border will still register. Not a problem, just thought I'd mention this for completeness.

[fwsmit]

As a side note, only the notification height is clickable and not the frame/border, which I think mirrors the previous logic in get_notification_at from src/input.c. This can be seen with the functional "gaps" test where I used an exaggerated frame border size to highlight this. However, as mouse_x is not currently used in input_handle_click, any click along the x-axis including the frame/border will still register. Not a problem, just thought I'd mention this for completeness.

Well spotted. The reason that frames are currently ignored is mostly because the separator lines don't really belong to either notification and frames were just an extension of that. On second thought, I think it's best to have the frames and separator lines be clickable as well. Half of the separator line can be for the top notification and half can be for the bottom. If you want you can implement that, otherwise I'll make an issue for that.

I added a suppression entry to target both malloc and realloc leaks coming from the fontconfig via pango and it seems to work on debian, ubuntu and arch. Let me know if you have any suggestions there. I also removed the gaps boolean setting and updated the tests and docs to match.

Great!

[paddyw2]

I updated the documentation and took a shot at updating the click handling logic for both gaps and separator lines, so let me know what you think. I added some tests in test/input.c but it may be easiest to see via the gaps and separator_click functional tests.

[fwsmit]

Thank you for your excellent work. Everything seems to work as intended. I'll go ahead and merge this!

[mrusme]

Any information on when this PR will make it into a release? :-)

[mrusme]

I can see a commit for 1.9.0, dated for 2022-06-21, yet no tag/release. :(

[sollymay]

I can see a commit for 1.9.0, dated for 2022-06-21, yet no tag/release. :(

Was also curious about this one myself

[fwsmit]

I was working on a release with the intention of publishing that day, but a few tests failed, so I still have to fix those. I can probably fix them today

[fwsmit]

It is now released!

[mrusme]

Just upgraded and tested, can confirm it works! Happy. Thank you!

[sollymay]

hey @fwsmit was this released for Fedora dnf yet? I updated the repos but still see 1.8.1-2 as the latest version:

Last metadata expiration check: 0:00:58 ago on Mon 27 Jun 2022 11:33:54 PM EDT.
Package dunst-1.8.1-2.fc36.x86_64 is already installed.
Dependencies resolved.
Nothing to do.
Complete!

[fwsmit]

That's up to the fedora maintainer.