Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wayland: text is garbled on non-scaled monitors #1316

Open
matejdro opened this issue Mar 13, 2024 · 16 comments
Open

Wayland: text is garbled on non-scaled monitors #1316

matejdro opened this issue Mar 13, 2024 · 16 comments

Comments

@matejdro
Copy link
Contributor

Issue description

When using dunst with Sway and using multiple monitors, where at least one monitor is using scaling, dunst's text will be garbled on non-scaled monitors:

image

On scaled monitors it appears fine:

image

Installation info

  • Version: Dunst - A customizable and lightweight notification-daemon 1.10.0 (2024-02-19)
  • Install type: package
  • Window manager / Desktop environment: sway version 1.9
  • Distro: Arch
Minimal dunstrc default + `follow` option enabled to allow displaying on multiple monitors

Setting force_xwayland to true fixes the issue, but it also breaks follow, which makes it an useless workaround.

@matejdro matejdro changed the title Wayland: text is garbled on non-scalled monitors Wayland: text is garbled on non-scaled monitors Mar 13, 2024
@bynect
Copy link
Member

bynect commented Mar 13, 2024

did you enable per_monitor_dpi?

@matejdro
Copy link
Contributor Author

Yes, with no effect. From what I can see, this settings is X11-only.

@bynect
Copy link
Member

bynect commented Mar 13, 2024

Yes, with no effect. From what I can see, this settings is X11-only.

I am checking the code and it is indeed an x11 only option.

But does the error occur only when the scaled screen is connected?

@matejdro
Copy link
Contributor Author

Yes, only when the scaled screen is connected. Otherwise, it works fine.

@bynect
Copy link
Member

bynect commented Mar 13, 2024

are you able to compile dunst ? I can add some logging in the wl_get_scale function. Unfortunately I can't test it myself because I don't have such wayland environment

@matejdro
Copy link
Contributor Author

Yes, I can compile it. Feel free to point me to a branch with extra logs.

@bynect
Copy link
Member

bynect commented Mar 13, 2024

I have made that small modification in my fork (https://github.com/bynect/dunst/tree/log-scale). If you could try it maybe we can get a better understanding of what's going on

@matejdro
Copy link
Contributor Author

matejdro commented Mar 20, 2024

Sorry this took a while.

Here is log from notification on non-scaled monitor:

WARNING: No icon found in path: 'dialog-information'
DEBUG: [         wake_up:0067] Waking up
DEBUG: [             run:0077] RUN, reason 1
INFO: Idle status queried: 0
DEBUG: [calculate_dimensions:0306] Notification dimensions 300x91
DEBUG: [            draw:0982] Window dimensions 309x97
DEBUG: [  render_content:0748] Layout height 75
DEBUG: [wl_display_surface:0800] Buffer size (scaled) 618x194
DEBUG: [             run:0103] Sleeping for 59992 ms

Here is log from notification on scaled monitor:

WARNING: No icon found in path: 'dialog-information'
DEBUG: [         wake_up:0067] Waking up
DEBUG: [             run:0077] RUN, reason 1
INFO: Idle status queried: 0
DEBUG: [calculate_dimensions:0306] Notification dimensions 300x91
DEBUG: [            draw:0982] Window dimensions 309x97
DEBUG: [  render_content:0748] Layout height 75
DEBUG: [wl_display_surface:0800] Buffer size (scaled) 618x194
DEBUG: [             run:0103] Sleeping for 59991 ms

Scaled monitor = 3840x2160 with 2x scaling
Unscaled monitor = 1920x1080 with no scaling
(so effectively they are the same size)

@bynect
Copy link
Member

bynect commented Mar 20, 2024

@matejdro I am very sorry I just noticed that the branch I pointed you was not actually synced with the changes I made.

I added some logging calls in the wl_get_scale function. This time the changes are actually up on github at the aforementioned link, but I will send also the diff

diff --git a/src/wayland/wl.c b/src/wayland/wl.c
index 18a2173..7c792bd 100644
--- a/src/wayland/wl.c
+++ b/src/wayland/wl.c
@@ -896,13 +896,20 @@ double wl_get_scale(void) {
         struct dunst_output *output = get_configured_output();
         if (output) {
                 scale = output->scale;
+
+                LOG_I("Output found, global name = %u, name = %s, scale = %u", output->global_name, output->name, output->scale);
         } else {
+                LOG_I("Output not found!");
+
                 // return the largest scale
                 struct dunst_output *output;
                 wl_list_for_each(output, &ctx.outputs, link) {
                         scale = MAX(output->scale, scale);
+                        LOG_I("Checking output %s (%u) with scale %u", output->name, output->global_name, output->scale);
                 }
         }
+
+        LOG_I("Scale is %d", scale);
         if (scale <= 0)
                 scale = 1;
         return scale;

Could you please test with these changes so I can understand if the monitor are correctly detected

@matejdro
Copy link
Contributor Author

matejdro commented Mar 22, 2024

Both monitors print:

INFO: Output not found!
INFO: Checking output (null) (49) with scale 2
INFO: Checking output (null) (48) with scale 1
INFO: Scale is 2

So, detected scale is 2 at both. If I hardcode that method to return 1, then text is not garbled anymore on the non-scaled monitor, but it is blurry on the scaled one.

from what I can see, the problem seems to be the get_configured_output method? It always returns null when FOLLOW_MOUSE or FOLLOW_KEYBOARD is enabled. There is no actual detection of the currently focused monitor.

@bynect
Copy link
Member

bynect commented Mar 22, 2024

Both monitors print:

INFO: Output not found!
INFO: Checking output (null) (49) with scale 2
INFO: Checking output (null) (48) with scale 1
INFO: Scale is 2

So, detected scale is 2 at both. If I hardcode that method to return 1, then text is not garbled anymore on the non-scaled monitor, but it is blurry on the scaled one.

from what I can see, the problem seems to be the get_configured_output method? It always returns null when FOLLOW_MOUSE or FOLLOW_KEYBOARD is enabled. There is no actual detection of the currently focused monitor.

Right, I figured it probably was the monitor detection. I am not an expert in wayland but I'll try looking into it.

Maybe returning the scale of the monitor where the window currently is positioned is a better choice? I don't know the rationale behind the way follow is implemented in wayland

@bynect
Copy link
Member

bynect commented Apr 6, 2024

@alebastr sorry to disturb you, but you seem to be one of the people that know the wayland backend the most. Do you know why the get_configured_output function doesn't return the focused monitor if the follow mode is not none? Is there a profound reason related to the protocol or was it just a past miscalculation ?

For reference

        switch (settings.f_mode){
                case FOLLOW_NONE: ; // this semicolon is neccesary
                        if (!configured_output) {
                                LOG_W("Monitor %s doesn't exist, using focused monitor", settings.monitor);
                        }
                        return configured_output;
/// why? VVVV
                case FOLLOW_MOUSE:
                        // fallthrough
                case FOLLOW_KEYBOARD:
                        // fallthrough
                default:
                        return NULL;
        }

@alebastr
Copy link
Contributor

alebastr commented Apr 7, 2024

I'm afraid that is a correct behavior. Wayland does not offer info about the currently focused output, partly because it is considered a sensitive information, partly because it doesn't make much sense with multi-seat support and multiple independent pointers. So if you don't have an output that is explicitly set in the configuration and exists in runtime, there's nothing to return.
The NULL result lets the compositor assign the output, but that'll happen with the initial layer_surface.configure. A bit past the wl_display_surface and the buffer creation.

The fallback code in wl_get_scale (largest scale factor of all the available outputs) is not optimal though, and there's a few more things that could be improved here. I'm looking into that.

Edit: a bit tricky to get it right; the event order is following (starting from draw.c:draw): wl_get_scale - render a scaled cairo buffer - wl_display_surface - create the surface on current output (send_frame) - get actual scale factor - commit already rendered buffer. At no point before send_frame we can reliably tell the scale factor for an automatically selected output.

Should be fine to move the surface recreation a bit ahead. It could be nice to reduce the amount of scaled calculations with something like cairo_surface_set_device_scale and to be able to redraw the cairo surface when we have the updated scale factor.

I'll try to get the Wayland parts done next week and will look into the rest.

@bynect
Copy link
Member

bynect commented Apr 7, 2024

I'm afraid that is a correct behavior. Wayland does not offer info about the currently focused output, partly because it is considered a sensitive information, partly because it doesn't make much sense with multi-seat support and multiple independent pointers. So if you don't have an output that is explicitly set in the configuration and exists in runtime, there's nothing to return.
The NULL result lets the compositor assign the output, but that'll happen with the initial layer_surface.configure. A bit past the wl_display_surface and the buffer creation.

The fallback code in wl_get_scale (largest scale factor of all the available outputs) is not optimal though, and there's a few more things that could be improved here. I'm looking into that.

Edit: a bit tricky to get it right; the event order is following (starting from draw.c:draw): wl_get_scale - render a scaled cairo buffer - wl_display_surface - create the surface on current output (send_frame) - get actual scale factor - commit already rendered buffer. At no point before send_frame we can reliably tell the scale factor for an automatically selected output.

Should be fine to move the surface recreation a bit ahead. It could be nice to reduce the amount of scaled calculations with something like cairo_surface_set_device_scale and to be able to redraw the cairo surface when we have the updated scale factor.

I'll try to get the Wayland parts done next week and will look into the rest.

Thank you very much.

I was thinking of using cairo device scaling but I have yet to code anything.
Would reworking the draw code to use it remove the need to know the scaling factor beforehand? (As far as I understand we can use digital pixel and then put a scaling factor for device pixels afterwards)

Does wayland not allow to know the output where the window currently is? Is this a limitation of the layer protocol we are using?

@alebastr
Copy link
Contributor

alebastr commented Apr 7, 2024

I was thinking of using cairo device scaling but I have yet to code anything. Would reworking the draw code to use it remove the need to know the scaling factor beforehand? (As far as I understand we can use digital pixel and then put a scaling factor for device pixels afterwards)

cairo_image_surface_create still takes the size in device pixels. But it would limit the use of scale to cairo surface creation and icon loading.

Does wayland not allow to know the output where the window currently is? Is this a limitation of the layer protocol we are using?

It is known. But send_frame may decide to recreate a surface on another output, invalidating this knowledge.

@matejdro
Copy link
Contributor Author

Just want to copy mention this for record keeping:

#1337 (comment)

It appears that this jaggedness happens on Sway. On Hyprland, text is still worse when scaling is enabled, but not as bad as on Sway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants