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

Per-screen dpi override #4096

Open
jesseleite opened this issue Aug 5, 2023 · 16 comments
Open

Per-screen dpi override #4096

jesseleite opened this issue Aug 5, 2023 · 16 comments
Labels
enhancement New feature or request

Comments

@jesseleite
Copy link

What are your thoughts on per-screen config overrides for people to fix font rendering issues on screens with non-standard densities, maybe something like...

local wezterm = require('wezterm')

local config = {
  term = 'wezterm',
  dpi = 144,
  font = wezterm.font('JetBrains Mono'),
  font_size = 16.5,
  line_height = 1.2,
  screens = {
    ['LG HDR WQHD'] = {
      dpi = 109,
      font_size = 11,
      line_height: 1.8,
    },
  },
}

return config

At the moment, setting a custom dpi affects all screens, which is undesirable when using WezTerm on multiple monitors.

Setting a custom dpi also affects font scaling, which is why I'm suggesting the ability to configure font_size per screen along with dpi.

Maybe the user also wants to set more spacious line_height on a larger screen. This would nicely allow for that as well.

Note: Ideally, this wouldn't only allow for per-screen dpi settings, but it would also change the window's dpi when dragging between screens, without the need to reboot WezTerm. At the moment, it seems setting a custom dpi doesn't take effect until you reboot WezTerm.

The reason for this feature request...

Like #3575 #3900 etc, I'm experiencing random font rendering oddities on my external LG display. From what I can tell, this is due to WezTerm's default dpi not lining up with my external monitor's physical ppi (pixels per inch).

For example, this is what I get with my external monitor at WezTerm's default dpi...

CleanShot 2023-08-04 at 20 57 29

And this is what I get when I explicitly set my config to dpi = 109 to match my monitor's physical ppi...

CleanShot 2023-08-04 at 20 59 16

As you can see, the font rendering is much smoother.

However, there are currently some pretty big cons to configuring an explicit dpi setting at the moment...

  1. When I set an explicit dpi in my config, I then have to reconfigure my font_size because the scaling will be off.

  2. When I switch to my macOS's built in retina screen, it's all wrong again. So I need some kind of conditional handling to only set custom dpi and font_size when booting WezTerm on my external monitor. I'm close with this implementation, but you can see in that discussion that there are hot reloading issues because dpi cannot be changed at runtime.

  3. As mentioned above, setting custom dpi affects all screens, so it makes for a worse experience when using WezTerm on multiple monitors / dragging between multiple monitors.

For these reasons, I think per-screen config overrides as outlined above could be a pretty elegant solution to font rendering problems like this. Curious on your thoughts?

PS. Thank you @wez for all you do on this rad piece of software 💘

@jesseleite jesseleite added the enhancement New feature or request label Aug 5, 2023
@wez
Copy link
Owner

wez commented Aug 6, 2023

I think setting dpi overrides per-screen makes sense, but the other options are problematic; that would require introducing something akin to Diamond Inheritance in the configuration resolution, which is a huge headache.

@jesseleite
Copy link
Author

That's fair! If WezTerm did implement per-screen dpi overrides (🙏), could it also automatically handle font scaling so that font size at least looks consistent between screens, like it already does between macOS' 72 dpi for low density and 144 dpi for retina? The way you implemented font scaling between those two macOS dpi defaults is perfect, aside from the fact that idiots like me have non-standard 109 ppi monitors 😁

wez added a commit that referenced this issue Aug 6, 2023
Allows specifying the precise dpi to use on a per-screen basis:

```lua
return {
  dpi_by_screen = {
    ["Built-in Retina Display"] = 144,
  },
}
```

The screen names are the same as those returned from
`wezterm.gui.screens()`.

Changing either `dpi` or `dpi_by_screen` in the config will now cause
the window to be immediately resized/adjusted to the changed dpi
override.

Ultimately, I'd like to deprecate `dpi` in favor of `dpi_by_screen`,
but can't do that until this functionality is ported to windows, x11
and wayland.

refs: #4096
@wez
Copy link
Owner

wez commented Aug 6, 2023

I took a crack at adding a per-screen dpi override on macos; I'll work on porting this to the other environments later.
You'd use it like this:

local wezterm = require('wezterm')

local config = {
  term = 'wezterm',
  dpi_by_screen = {
    ['Built-in Retina Display'] = 144, -- You can omit this if you don't need to change it
    ['LG HDR WQHD'] = 109,
  },
  font = wezterm.font('JetBrains Mono'),
  font_size = 16.5,
  line_height = 1.2,
}

return config

Those changes will show up in a nightly build within about an hour from now. Please try it out and let me know how it goes!

could it also automatically handle font scaling so that font size at least looks consistent between screens, like it already does between macOS' 72 dpi for low density and 144 dpi for retina?

It should work the same; the reason wezterm cares about dpi and uses a font_size expressed in points is expressly so that the size can be consistent across different devices. The size in pixels is calculated based on the size in points and the dpi.

macOS, unfortunately, does not expose the underlying dpi for its displays, instead using an abstract scaling value that is either 1 or 2 depending on whether it is "standard" or "retina" density. The scaling is applied to the macOS concept of the standard dpi for a display.

The dpi_by_screen override should allow you to dial in the perfect dpi for your screen if it lies somewhere between those values.

wez added a commit that referenced this issue Aug 6, 2023
Maintain a cache of the positions of the various named screens,
and use that to resolve the screen of the current window, and
from there we can resolve the correct dpi_by_screen screen.

Make dpi and dpi_by_screen config changes generate a resize
event with the updated dpi.

refs: #4096
@wez wez changed the title Per-screen config overrides? Per-screen dpi override Aug 6, 2023
@jesseleite
Copy link
Author

Just pulled nightly and dpi_by_screen itself works awesome! 🎉🎉🎉 There's something up with the font scaling still though. The font on my LG looks huge, and if I adjust for my LG, the font on my retina is super small. I'll post some pics of my screens side-by-side when I get home later this afternoon.

Also it seems that when I run window:set_config_overrides({ font_size = ...}) in an event, it applies that font size to all windows on both of my screens, not just the current window on the current screen. Is that intentional?

PS. What do you think of adding dpi output to wez.gui.screens() so that we could see what dpi each screen is set with in the debug overlay, whether explicitly overriden w/ dpi_by_screen, or automagically when WezTerm detects low density (72) or built in retina (144)?

@wez
Copy link
Owner

wez commented Aug 6, 2023

What do you think of adding dpi output to wez.gui.screens() so that we could see what dpi each screen is set with in the debug overlay, whether explicitly overriden w/ dpi_by_screen, or automagically when WezTerm detects low density (72) or built in retina (144)?

Ultimately, I will, but until the Wayland and Windows flavors of dpi_by_screen are done, I can't make that change.

@wez
Copy link
Owner

wez commented Aug 6, 2023

Also it seems that when I run window:set_config_overrides({ font_size = ...}) in an event, it applies that font size to all windows on both of my screens, not just the current window on the current screen. Is that intentional?

What event? Please share the configuration you are using

wez added a commit that referenced this issue Aug 6, 2023
This is a baby step towards handling dpi_by_screen.
I don't want to do the actualy per-screen stuff here;
it touches stuff around the edges of SCTK and there is a pending,
significant, rewrite of that code needed to upgrade to a more
recent version of SCTK + wayland-protocols, and I don't want to waste
my effort on the intermediate state.
#3996 (comment)

refs: #4096
wez added a commit that referenced this issue Aug 6, 2023
wez added a commit that referenced this issue Aug 6, 2023
The overrides are now applied at the window layer, which is (mostly!)
aware of per-screen overrides.

refs: #4096
wez added a commit that referenced this issue Aug 6, 2023
wez added a commit that referenced this issue Aug 6, 2023
Note that this also does not respect dpi_by_screen; this is for
consistency in behavior and reported values.

Once we can produce the correct overridden value in
dispatch_pending_event, we can update these functions to return
the same data.

refs: #4096
wez added a commit that referenced this issue Aug 6, 2023
@jesseleite
Copy link
Author

^ Rad! 😍

What event? Please share the configuration you are using

wezterm.on('window-config-reloaded', function(window)
  if wezterm.gui.screens().active.name == 'LG HDR WQHD' then
    window:set_config_overrides({
      font_size = 11
    })
  end
end)

...But I realize I was wrong about set_config_overrides() affecting all windows, because when I hook onto the window-resized event for example, I see it only applying to the current window 👌

Also I realize my event logic there is faulty, because I'm setting the font size based on screens().active rather than setting based on the screen where the current window resides. If my goal is to set a varying font_size per screen, is there a way to detect which screen the current window resides on?

Anyway, the reason I'm even looking into this font size per screen thing is because of the font scaling issue I mentioned earlier...

There's something up with the font scaling still though. The font on my LG looks huge, and if I adjust for my LG, the font on my retina is super small. I'll post some pics of my screens side-by-side when I get home later this afternoon.

Not sure screenshots alone would do justice to show what happens between multiple physical monitors, so here's a video demo (I apologize in advance for the low quality).

^ You can see there's some kind of font scaling thing happening between the inferred 144 vs 72 dpi defaults on macOS, that's behaving differently when setting dpi_by_screen explicitly.

Curious on your thoughts? ❤️

@wez
Copy link
Owner

wez commented Aug 6, 2023

Thanks for sharing the video; the audio was a bit low, but I appreciate hearing the explanation and seeing the effects.

If setting the dpi to 109 is too big, can you try experimentally dialing it back down until you find a value that makes 16.5 points look equivalent between the screens? I'm curious to discover what the ratio is between the 109 and the actual value that looks the right size.

You shouldn't need to vary the font size per-screen; that's the intended purpose of the dpi. So we should focus on what's happening with that.

Could you also share the output from wezterm.gui.screens() on a more recent nightly (run wezterm -n to skip loading your config overrides and return the raw/default information)? That has the effective dpi and also the scale factor from macos and may provide some more clues.

@jesseleite
Copy link
Author

Absolutely will try those things! Also, if you feel a live pair 🍐 session might be easier (to give me things to try, with you being able to see results in realtime), I'd also be happy to do that (feel free to email me, but no pressure!). Either way, I'll dig into this more tonight hopefully and post my findings.

@wez
Copy link
Owner

wez commented Aug 6, 2023

Something interesting to note: macOS considers the default DPI for a normal resolution screen to be 72, but every other OS considers it to be 96. I wonder if the discrepancy you're seeing is the ratio of those numbers ($96/72$) and if the dpi that will look better is:

$$\huge {109 * 72 \over 96} = 81.75$$

@jesseleite
Copy link
Author

jesseleite commented Aug 7, 2023

You shouldn't need to vary the font size per-screen; that's the intended purpose of the dpi. So we should focus on what's happening with that.

Gotcha, thank you for clarifying that!

If setting the dpi to 109 is too big, can you try experimentally dialing it back down until you find a value that makes 16.5 points look equivalent between the screens? I'm curious to discover what the ratio is between the 109 and the actual value that looks the right size.

Really, any dpi higher than ['LG HDR WQHD'] = 72 in my config makes the font look bigger on my LG than it does on my retina... So although that 16.5 pt @ 81.75 dpi suggestion looks less different than @ 109 dpi, it is still noticeably larger than what my retina is rendering (compared to when dragging other apps like mac's default Terminal app between screens, which seem to render a very consistent looking font size between screens) 🤔

Could you also share the output from wezterm.gui.screens() on a more recent nightly (run wezterm -n to skip loading your config overrides and return the raw/default information)? That has the effective dpi and also the scale factor from macos and may provide some more clues.

CleanShot 2023-08-06 at 20 26 47

@wez
Copy link
Owner

wez commented Aug 7, 2023

Ok, so 72 dpi, which is the default for that display, sounds like it is approx the right dpi to use.

Zooming back out to the problem that you were trying to solve, I think the problem statement could be phrased as:

  • Fonts looked weird at 72 dpi and 16.5 points
  • Increasing the dpi to 109 and decreasing the font size to 11 made it look better.

The pixel height calculation in wezterm is:

$$\huge PixelSize = {{FontSize * Dpi} \over \huge 72}$$

So the two scenarios produced similar pixel sizes:

  • $16.5 * 72 / 72 = 16.5$
  • $11 * 109 / 72 = 16.65277777777778$

I'd suggest rearranging the math to compute the "perfect" dpi setting to use

$$\huge Dpi = {{72 * PixelSize} \over FontSize}$$ $$\huge Dpi = {{72 * 16.65277777777778} \over 16.5} = 72.66666666666667$$

So I'd suggest using:

  dpi_by_screen = {
    ['LG HDR WQHD'] = 72.66666666666667,
  }

@jesseleite
Copy link
Author

Oh my, this is clever! I'll roll with this for a bit and see how it goes.

@jesseleite
Copy link
Author

Darn, been seeing quite a few font rendering oddities @ 72.66666666666667...

CleanShot 2023-08-08 at 23 15 48

There's something about the 11 pt @ 109 dpi combo that still seems much more stable (not perfect, but way harder to reproduce) 🤔

@syhol
Copy link

syhol commented Oct 3, 2024

It's such an glaring eye-sore when you see it 🤮

I also found that setting font_size to 11 and then adjusting the dpi_by_screen for my individual displays gave me the best results for this problem.

@jesseleite
Copy link
Author

Yep, this snippet has been the only real solution for me for me. It's pretty rare that I see it, but it still happens to the odd character 😥

-- This doesn't work well on a dual screen setup, and is hopefully a temporary solution to the font rendering oddities
-- shown in https://github.com/wez/wezterm/issues/4096. Ideally I'll switch to using `dpi_by_screen` at some point,
-- but for now 11pt @ 109dpi seems to be the most stable for font rendering on my 38" ultrawide LG monitor here.
wezterm.on('window-config-reloaded', function(window)
  if wezterm.gui.screens().active.name == 'LG HDR WQHD' then
    window:set_config_overrides({
      dpi = 109,
      font_size = 11,
    })
  end
end)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants