Refactor draw code to use cairo_surface_set_device_scale

[bynect]

This fixes something that has been bugging me for a while.
Basically now dunst is scaling itself when drawing every single shape and text (with cairo and pango).
With this patch we render everything at a uniform scaling factor of 1 and scale everything afterwards using cairo_surface_set_device_scale.

This should have some advantages (apart from conciseness), like making scaling on wayland possible (like I discussed with @alebastr under #1316)

NOTE: Everything works on Xorg, however I can't test it on Wayland and my display is not Hi-Dpi, so I am not sure if the pango dpi scaling is being applied correctly. If someone can test it it would be great 👍🏻

[bynect]

I think that all the get_icon_width and get_*scale*icon function aren't needed anymore

[bynect]

I have no clue about why the test are failing 😢

[bynect]

I am trying to debug the use of uninitialized variables but even with libAsan I can't find it

[bynect]

before wayland didn't scale the user input

[bynect]

Finally figured it out. Basically I forgot that the test didn't call draw_setup. Since I moved the initialization of the PangoContext in draw_setup it was undefined and thus buggy, but only in tests.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 57.14286% with 48 lines in your changes missing coverage. Please review.

Project coverage is 64.93%. Comparing base (e1e772c) to head (7edbebd).

Files with missing lines	Patch %	Lines
src/draw.c	59.61%	21 Missing ⚠️
src/wayland/wl.c	0.00%	14 Missing ⚠️
src/wayland/wl_seat.c	0.00%	4 Missing ⚠️
src/x11/x.c	0.00%	4 Missing ⚠️
test/draw.c	82.35%	3 Missing ⚠️
src/utils.c	0.00%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1337      +/-   ##
==========================================
- Coverage   65.12%   64.93%   -0.20%     
==========================================
  Files          50       50              
  Lines        8654     8658       +4     
  Branches     1023     1023              
==========================================
- Hits         5636     5622      -14     
- Misses       3018     3036      +18     

Flag	Coverage Δ
unittests	64.93% <57.14%> (-0.20%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bynect]

Everything things to work. I think it is ready for merge.

Things we may want to consider

• remove the icon scaling code (since we don't use it anymore). Alternatively we can render icons separately over the cairo surface to not downscale->upscale them. But this can be done in another pr.
• integrate the new scaling method with wayland for per-monitor-dpi

[bynect]

@alebastr Where do you think should we place scale code so that the wayland backend can correctly get the scale needed? (to solve #1316)

Note that the pr is still not quite ready

[matejdro]

If someone can test it it would be great 👍🏻

I can test it on Wayland if desired, just give me instructions on what to look for

[bynect]

If someone can test it it would be great 👍🏻

I can test it on Wayland if desired, just give me instructions on what to look for

it would be great if you could check if the input works correctly (mouse clicks for closing etc) and if you notice any graphical discrepancy let me know.
The only difference I noticed is that text is a bit less sharp but maybe it's just me?

[matejdro]

Only issue I have noticed is that, with scaling enabled, dunst window has bigger horizontal offset from the edge of the screen and the window is smaller. All below screenshots extend to the top right corner of the monitor.

This branch

Scaled monitor:

Interestingly enough, after I take a screenshot and Satty opens fullscreen, dunst window will become taller (even before Satty makes second notification):

Non-scaled monitor (still suffering from #1316):

After I turn off scaling on the other monitor, text is clearer for the non-scaled monitor and position bug disappears:

Dunst release

Scaled monitor:

Non-scaled monitor:

After I turn off scaling on the other monitor, text is clearer:

Mouse clicks seem to work fine. I also don't see any discrepancies regarding text quality between this branch and latest dunst release (but you can judge yourself from screenshots). Both monitors are 2560x1440, with 2x scaling on one of the monitors (where specified).

Compositor:

Hyprland, built from branch  at commit 918d8340afd652b011b937d29d5eea0be08467f5  (flake.lock: update).
Date: Tue Jun 25 12:06:02 2024
Tag: v0.41.2, commits: 4886

[bynect]

@matejdro so is the resolution still wrong even with this branch?

Also I can look into the offset problem, probably something was not scaled appropriately

[matejdro]

Above screenshots are from Hyprland.

On Sway, text look as bad on this branch as with the release:

But interestingly enough, on Hyprland, issue appears to be different. As you can see on the screenshots from the previous post, text is still worse when scaling is enabled, but it's not as jagged as on Sway. Maybe Hyprland applies some sort of font smoothing by default, not sure. This is also the case with release version of dunst, so no changes here.

[bynect]

Thanks very much for the feedback. I will investigate this further and make some changes 👍

[bynect]

@matejdro just to let you know. the problem with wayland scaling is that we check for the screen before drawing but on wayland the protocol does not allow that with follow mode. so further changes are needed to this pr for it to work on wayland. maybe it should be done in a seperate pr

[bynect]

I am not sure if this needs more work. I would probably do the scale detection refactor in another pr since it requires wayland backend changes.

Also the graphical effect of cairo_surface_set_device_scale is not as good as I thought due to antialiasing...

[fwsmit]

Can you show what is the visual difference between the two? I don't really understand why the old method would not work on Wayland. Not that using pango device scale is bad, but I'm not sure this should solve a scaling issue on Wayland.
It seems like the problem on Wayland is related to not detecting the scale factor correctly which can be solved by changing the Wayland backend as you mentioned. I thought of some hacky way a while ago where you first draw the surface and then find on which display the surface was drawn and get the right display scale factor. After you might need to redraw. But there might be a better way possible now with protocol updates.

[bynect]

Can you show what is the visual difference between the two? I don't really understand why the old method would not work on Wayland. Not that using pango device scale is bad, but I'm not sure this should solve a scaling issue on Wayland. It seems like the problem on Wayland is related to not detecting the scale factor correctly which can be solved by changing the Wayland backend as you mentioned. I thought of some hacky way a while ago where you first draw the surface and then find on which display the surface was drawn and get the right display scale factor. After you might need to redraw. But there might be a better way possible now with protocol updates.

the problem is the layer protocol like you mentioned. As far I understand it doesn't allow you to know beforehand where you are drawing. The possible solution using this device scale is that you first render at 1x on the surface image (like we already do) and then you scale this surface afterwards.
This in general should be better than scaling everything but somehow the result I get is worse.
Probably skill issue on my part.
Anyway I don't see an easy way to deal with the wayland issue without thorough refactoring

[fwsmit]

the problem is the layer protocol like you mentioned. As far I understand it doesn't allow you to know beforehand where you are drawing.

If you set the output to some value, then you know the surface will be drawn to this output. The thing is that if follow=mouse or follow=keyboard is set, we set the output to NULL, indicating to the compositor it can decide where to place the notification. This means you don't know where the notification will end up until after it's shown. When a single output it set in the configuration, it is always possible to determine beforehand on which output the notification will end up and there should be no scaling issues.

So the only case where there should be scaling issues is when follow=keyboard/mouse AND there are multiple outputs with different scales. We could mention this shortcoming in the documentation or work around it in a hacky way.

This PR will not solve the mentioned issue, it only makes the code a bit cleaner.

[fwsmit]

if the offset problem, as mentioned in the testing above is fixed, I think this can be merged.

[b0ch3nski]

scaling issues is when follow=keyboard/mouse AND there are multiple outputs with different scales.

This is exactly what is happening in my case 🙁