Skip to content

wayland hidpi support#854

Merged
tsipinakis merged 5 commits intodunst-project:masterfrom
fwsmit:wayland-hidpi
May 26, 2021
Merged

wayland hidpi support#854
tsipinakis merged 5 commits intodunst-project:masterfrom
fwsmit:wayland-hidpi

Conversation

@fwsmit
Copy link
Member

@fwsmit fwsmit commented Apr 19, 2021

Fixes #836
@bebehei This PR currently only implements scale detection on wayland. I'm making this PR already so you know I'm working on it.
To test it on sway, it's really easy:
swaymsg output "*" scale 1.5

Sway will message a scale of 2 to dunst and it will scale it down to 1.5 afterwards.

@fwsmit fwsmit changed the title wayland: implement get_scale function for hidpi support wayland hidpi support Apr 19, 2021
@fwsmit
Copy link
Member Author

fwsmit commented Apr 21, 2021

@adamcstephens @bebehei The wayland HiDPI support is implemented! Could you test it? With an integer scaling factor everything should look razor sharp, and with a non-integer scaling factor it is rendered in the nearest scaling above that and scaled down from there. Everything should work pretty well. The only thing I've noticed is that some icons are smaller than when redered with a scale of 1, but that can probably be solved with min_icon_size.
(I'll fix the tests later)

@fwsmit fwsmit force-pushed the wayland-hidpi branch 2 times, most recently from e2e1541 to 34ea6eb Compare April 22, 2021 11:01
@fwsmit fwsmit marked this pull request as ready for review April 22, 2021 11:07
This commit implements support for HiDPI rendering for wayland. X11
should be unaffected by this.
It is implemented by scaling everything that's rendered by a scale
factor that's obtained from the wayland protocol. All sizes before
rendering remain the same, so the same settings should provide the same
output for different scaling factors, only scaled by that factor.
@fwsmit
Copy link
Member Author

fwsmit commented Apr 22, 2021

The tests are now fixed. Icons are now also scaling properly, even when they're low resolution. The only bug remaining, which I probably won't fix is that the icons are not immediately updated on scaling. Only new notification have the bigger icons.

@adamcstephens
Copy link

I'm not seeing a difference with this patch. Let me know if there's any info I can provide to help, and I'm also on IRC.

@fwsmit
Copy link
Member Author

fwsmit commented Apr 27, 2021

What compositor are you using? This is tested on sway. Debug output of Dunst would help as well

@adamcstephens
Copy link

I'm using sway 1.5.1 on Debian Bullseye.

Here's the debug output:

$ ~/bin/dunst -verbosity debug
INFO: Using Wayland output
INFO: New output found - id 0
DEBUG: Ignoring wake up
DEBUG: RUN
DEBUG: Fullscreen queried: 0
INFO: Idle status queried: 0
INFO: Wayland: Hiding window
WARNING: No icon found in path: 'dialog-information'
DEBUG: Waking up
DEBUG: RUN
DEBUG: Fullscreen queried: 0
INFO: Idle status queried: 0
DEBUG: Buffer size (scaled) 300x60
DEBUG: Sleeping for 59959 ms
INFO: Pointer handle button 272: 1
INFO: Pointer handle button 272: 0
DEBUG: Waking up
DEBUG: RUN
DEBUG: Fullscreen queried: 0
INFO: Idle status queried: 0
DEBUG: Queues: Closing notification for reason: user
INFO: Wayland: Hiding window

@fwsmit
Copy link
Member Author

fwsmit commented Apr 27, 2021

What scale are you using? A non-integer scale will not look perfectly sharp. Try a scale of 2 and see if it looks better with this PR than before

@paretje
Copy link

paretje commented Apr 28, 2021

@fwsmit Judging by mako's pango init, I suspect you need to set the pango scale attribute for better non-integer scaling of text.

@fwsmit
Copy link
Member Author

fwsmit commented Apr 28, 2021

@fwsmit Judging by mako's pango init, I suspect you need to set the pango scale attribute for better non-integer scaling of text.

No unfortunately the wayland protocol only allows for integer scaling to be passed to an application (probably with good reason). The way sway allow for non-integer scaling is by passing the next integer above your scaling and scale it down afterwards. Most of the times it shouldn't matter, because you'll use it when you have a 4k screen which will work perfectly with 2x scaling.

This is what it looks like on my pc (2x scale):
20210428_22h27m26s_grim
Without HiDPI support

20210428_22h27m57s_grim
With HiDPI support

The text is definitely improved compared to before, but the icon might be worse, because it's blurry. I haven't looked into it yet, but I suspect the scaling method we use with gdk is different from the scaling method used by sway.

Another notification (2x scale):
20210428_22h42m13s_grim
Without HiDPI support

20210428_22h41m59s_grim
With HiDPI support

The line height seems to be slightly higher with this PR, so I'll look into if the pango scale function might help with that. Have you tried this PR by the way @paretje?

@paretje
Copy link

paretje commented Apr 28, 2021

Ah, sorry, it was the most obvious difference when I had a quick look at the codebases. I didn't notice scale was an int ...

On my personal laptop, I have a 14'" QHD screen, and I apply a scaling factor of 1.25. Since I switched to sway, I have been using mako as my notification daemon, which works fine, but I'm missing some features. I knew dunst had Wayland support now, and wanted to try it. I ended up trying your PR, but I noticed the text to be very blurry. I don't have this issue with mako, which was why I ended having a look at the code.

dunst:
dunst

mako:
mako

@fwsmit
Copy link
Member Author

fwsmit commented Apr 29, 2021

Okay from your screenshot it looks like it can definitely be done better. I'll have to experiment a bit and maybe copy some code. Does a scaling factor of 2 not have any issues on your machine?

@paretje
Copy link

paretje commented Apr 29, 2021

Hmmm, yes, it doesn't really work with scaling factor 2 either for me.

2021-04-29_194709

@fwsmit
Copy link
Member Author

fwsmit commented Apr 29, 2021

Can you do a picture with the latest Dunst release as well?

@paretje
Copy link

paretje commented Apr 29, 2021

2021-04-30_001420

@fwsmit
Copy link
Member Author

fwsmit commented Apr 30, 2021

It seems like they are exactly the same. I think there must be something wrong in the scale detection then. I've added a debug print to see what scale you are using. Could you see if it even receives the right scale from the compositor?
EDIT: and could you also check you are using the same font for mako as for dunst? A bitmap font obviously doesn't scale very well.

@codecov-commenter
Copy link

codecov-commenter commented Apr 30, 2021

Codecov Report

Merging #854 (ea531ca) into master (3acffdb) will decrease coverage by 0.44%.
The diff coverage is 17.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #854      +/-   ##
==========================================
- Coverage   58.54%   58.09%   -0.45%     
==========================================
  Files          37       37              
  Lines        5987     6040      +53     
==========================================
+ Hits         3505     3509       +4     
- Misses       2482     2531      +49     
Flag Coverage Δ
unittests 58.09% <17.60%> (-0.45%) ⬇️

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

Impacted Files Coverage Δ
src/dunst.c 9.82% <0.00%> (-0.37%) ⬇️
src/output.c 0.00% <ø> (ø)
src/wayland/wl.c 0.00% <0.00%> (ø)
src/x11/x.c 2.28% <0.00%> (-0.02%) ⬇️
src/draw.c 0.89% <6.34%> (+0.89%) ⬆️
src/icon.c 68.45% <66.66%> (-1.67%) ⬇️
test/icon.c 95.26% <88.88%> (ø)
src/notification.c 59.68% <100.00%> (ø)

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 3acffdb...ea531ca. Read the comment docs.

@fwsmit
Copy link
Member Author

fwsmit commented Apr 30, 2021

After zooming into those images I've found at least one difference that has nothing to do with the scaling. Mako apparently uses subpixel rendering for their fonts. Still, the 1.25x result from dunst doesn't look great.

Below are zoomed in versions of you screenshots:

Mako 1.25x
20210430_14h56m40s_grim

Dunst 1.25x
20210430_14h56m53s_grim

Dunst 2x
20210430_14h57m04s_grim

@paretje
Copy link

paretje commented Apr 30, 2021

Yes, I had noticed it and tried it, but it didn't really help. But, I now applied this additional diff:

diff --git a/src/draw.c b/src/draw.c
index f3e3a6a..5df7298 100644
--- a/src/draw.c
+++ b/src/draw.c
@@ -142,6 +142,8 @@ static struct color layout_get_sepcolor(struct colored_layout *cl,
 static void layout_setup_pango(PangoLayout *layout, int width)
 {
         int scale = output->get_scale();
+        LOG_D("Setup pango with scale %i", scale);
+
         pango_layout_set_wrap(layout, PANGO_WRAP_WORD_CHAR);
         pango_layout_set_width(layout, width * scale * PANGO_SCALE);
         pango_layout_set_font_description(layout, pango_fdesc);

And now I get (using 1.25):

MESSAGE: 'allow_markup' is deprecated, please use 'markup' instead.
INFO: Max width was set but max_icon_size is unlimited. Limiting icons to 150 pixels
MESSAGE: The option 'icon_folders' is deprecated, please use 'icon_path' instead.
MESSAGE: The frame section is deprecated, width has been renamed to frame_width and moved to the global section.
MESSAGE: The frame section is deprecated, color has been renamed to frame_color and moved to the global section.
INFO: Using Wayland output
INFO: New output found - id 0
DEBUG: Output scale changed to 2
DEBUG: Ignoring wake up
DEBUG: RUN
DEBUG: Fullscreen queried: 0
INFO: Idle status queried: 0
INFO: Wayland: Hiding window
DEBUG: Waking up
DEBUG: RUN
DEBUG: Fullscreen queried: 0
INFO: Idle status queried: 0
DEBUG: Setup pango with scale 1
DEBUG: Buffer size (scaled) 300x52
DEBUG: Sleeping for 9967 ms

So, while the output scale is updated to 2, which sounds right given it's an integer, when we actually setup pango, output->get_scale() returns 1.

@paretje
Copy link

paretje commented Apr 30, 2021

Oh, now I see why. wl_get_scale always returns 1 unless you use a fixed monitor? I can confirm that setting follow to none fixes the scaling for me.

int wl_get_scale(void) {
        struct dunst_output *output = get_configured_output();
        if (output)
                return output->scale;
        else
                return 1;
}

@fwsmit
Copy link
Member Author

fwsmit commented Apr 30, 2021

Thanks for debugging. The get_scale function may indeed return the default of 1 when it cannot detect what the current output is. This happens when follow is something else than none. We then leave it up to the compositor to decide on what output to place the dunst window, making it harder (but not impossible) for us to know what output the window is currently on. On my system when I do this, the text becomes way bigger than it's supposed to be
It should now work for the case where there is no output though, because it now returns that output instead of NULL (let the compositor decide). Do you have multiple monitors, or is the follow value just there doing nothing?
I'll try out if taking the largest scale factor among the monitors and see if the compositor scales the buffer down if needed.

@fwsmit
Copy link
Member Author

fwsmit commented Apr 30, 2021

It should use the right scale in any monitor configuration. It works fine to use a larger scale if the current output cannot be found.
I'll still have to look if the text rendering can be more crisp, but it doesn't necessarily have to be in this PR if the text looks good enough.

@paretje
Copy link

paretje commented Apr 30, 2021

No, I don't use multiple monitors, it probably was just part of the default config at the time. When I sit at my desk, I use an external monitor, but my laptop monitor is disabled.

dunst does crash when I switch between monitors though. I don't think (dis)connecting monitors is actually handled at the moment? The immediate cause is the fact that the initial value of scale is 0, so if for some reason there is no output, scale 0 is used, resulting in a division by zero in get_text_size. I suggest to use 1 for robustness.

@fwsmit
Copy link
Member Author

fwsmit commented May 1, 2021

Okay thanks for mentioning. That will be an easy fix. Have you compared the text between Dunst and mako now? Is there still a lot of difference or only the subpixel rendering?

@paretje
Copy link

paretje commented May 1, 2021

No, dunst looks fine now, thank you.

@fwsmit
Copy link
Member Author

fwsmit commented May 1, 2021

No, dunst looks fine now, thank you.

Cool! The divide by 0 issue is fixed as well. I'll see if I'll implement subpixel rendering as well. It should be only a few lines of code.

@adamcstephens
Copy link

I can confirm that this PR looks good now, and you've resolved the blurriness for me. It works for multiple monitors with different scales.

Tested on:

  • single screen at scale of 2.2
  • dual screen, scales of 1.32 and 2.2

@fwsmit
Copy link
Member Author

fwsmit commented May 1, 2021

I can confirm that this PR looks good now, and you've resolved the blurriness for me. It works for multiple monitors with different scales.

Tested on:

* single screen at scale of 2.2

* dual screen, scales of 1.32 and 2.2

Did you try it with follow=none or something else? When follow is mouse or keyboard, it'll be rendered at 2.2x res and scaled down to 1.32 if needed.

@adamcstephens
Copy link

Did you try it with follow=none or something else? When follow is mouse or keyboard, it'll be rendered at 2.2x res and scaled down to 1.32 if needed.

I just tested both follow=none and follow=mouse, with either display as primary, and it looks good in every scenario I tested.

fwsmit added 2 commits May 2, 2021 13:21
When there's only one output, just return that output.
When follow mode != none dunst cannot detect what output is being used
to display the notification (yet). With this commit dunst falls back to
using the largest scale from any ouput. The compositor should scale the
buffer down again if the scale of the output is smaller that the used
scale.
@fwsmit
Copy link
Member Author

fwsmit commented May 2, 2021

It sounds like you tested it pretty well. I don't see any issues either, except the icons not resizing immediately upon changing the scale. Subpixel font rendering will have to wait a bit as well, since it's not as easy as I thought. This PR is finished, though. We'll just have to wait for someone to merge it (might take a while, since the maintainers are busy).

@tsipinakis
Copy link
Member

Sorry for the delay in merging this. I can't do much to test it as I'm not on wayland (yet) but I'll trust you saying it looks good. 👍

@tsipinakis tsipinakis merged commit b77f76f into dunst-project:master May 26, 2021
@fwsmit fwsmit deleted the wayland-hidpi branch February 16, 2022 09:31
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.

Wayland HiDPI Font Scaling Blurriness

5 participants