wez / wezterm

A GPU-accelerated cross-platform terminal emulator and multiplexer written by @wez and implemented in Rust
https://wezfurlong.org/wezterm/
Other
17.85k stars 800 forks source link

font_size != 12 vs initial_cols + initial_rows vs external monitor #4851

Open stefanwascoding opened 9 months ago

stefanwascoding commented 9 months ago

What Operating System(s) are you seeing this problem on?

macOS

Which Wayland compositor or X11 Window manager(s) are you using?

n/a

WezTerm version

20240124-220416-642bfbb6

Did you try the latest nightly build to see if the issue is better (or worse!) than your current version?

Yes, and I updated the version box above to show the version of the nightly that I tried

Describe the bug

Using the latest (as of today) nightly build of WezTerm, I'm trying to get a window with a (roughly) certain number of rows & columns. While this works without issue on an internal (Retina) Display on a MacBook Pro, it fails for font_size values other than 12.

External monitor, font_size=12, everything as exected. fs12--as-expected

However when going to font_size=13, on the external monitor we get a window that as roughly double the expected numbers of rows and columns: fs13--wtf-more-than-double

Going up one step to 14 gives even more rows & columns than expected: fs14--nope fs14-yes-it-actually-has-that-many-lines

Going down to font_size=8 nearly works (except for one row): fs8--nearly-works-one-line-missing

If you try to fudge the expected rowXcolumn size, by adjusting the input numbers, it works ... but then if you open a window on an internal retina screen, you of course get the input numbers, so the window is too small. fs14--fudging-size

To Reproduce

No response

Configuration

config.font_size = 14 -- this is variable in testing
config.initial_cols = 137  -- width
config.initial_rows = 42   -- height

Expected Behavior

a window with the configured number of rows & columns, no matter the display

Logs

No response

Anything else?

I have played around with dpi_by_screen ... did not really help, just made the rendering larger, and the font_sizes have been pretty good without that for me on both internal & external.

Issue might be related to or duplicate of https://github.com/wez/wezterm/issues/4285.

wez commented 9 months ago

Likely a duplicate of:

stefanwascoding commented 9 months ago

Likely a duplicate of:

* [Initial font size is wrong when launched on external display #3900](https://github.com/wez/wezterm/issues/3900)

You will have a better idea what's behind #3900, so this might well be a duplicate. But to me the font-size itself is not an issue, that looks good to me.

When I play around with dpi_by_screen the font size changes, but the calculation for rows & columns remains the same, which actually leads to a broken "view" of the window, where $LINES is 86, but only about e.g. 43 actually fit into the window size. I can't even scroll to the end of the output of for i in $(seq 1 $LINES); do echo $i; done. Only when I resize the window does this seem to recompute, and I can scroll to the prompt again.

wez commented 9 months ago

Please try the latest nightly build and let me know if this is still an issue!

stefanwascoding commented 9 months ago

@wez I just tested again with:

wezterm version: 20240212-074107-22f9f8d2 aarch64-apple-darwin
Window Environment: macOS 14.3.1 (23D60)
Lua Version: Lua 5.4
OpenGL: Apple M1 Pro 4.1 Metal - 88

and settings:

config.font_size = 14
config.initial_cols = 137  -- width
config.initial_rows = 42   -- height

On a non-retina screen I get a window with:

% printf '%sx%s\n' $(tput cols) $(tput lines)
293x84
stefanwascoding commented 9 months ago

I'm currently using the following code (in a separate module file) as a workaround:

-- window size really gets messed up when using different monitors and non-default font_size (i.e. != 12) (at least on macOS)
--   https://github.com/wez/wezterm/issues/2753
--   https://github.com/wez/wezterm/issues/3575
--   https://github.com/wez/wezterm/issues/3900
--   https://github.com/wez/wezterm/issues/4285
--   https://github.com/wez/wezterm/issues/4447

local module = {}

local wezterm = require 'wezterm'

local COLS_NON_RETINA = 80
local ROWS_NON_RETINA = 21

function module.apply_to_config(config)
    -- config.dpi_by_screen = {
    --   -- https://github.com/wez/wezterm/issues/4096
    --   ['U32E850'] = 100,
    --   ['LG Ultra HD'] = 100,
    -- }

    -- config that applies to retina screens
    config.font_size = 14
    config.initial_cols = 137  -- width
    config.initial_rows = 42   -- height
end

-- https://coralpink.github.io/commentary/wezterm/dpi-detection.html
-- wezterm.on('window-focus-changed', function(window, pane)
--   local overrides = window:get_config_overrides() or {}
--   overrides.font_size = 10
--
--   window:set_config_overrides(overrides)
-- end)

-- make sure additional windows (after the initial one) are created correctly, on non-retina screens
wezterm.on('window-config-reloaded', function(window)
    if wezterm.gui.screens().active.name == 'Built-in Retina Display' then
        return
    end

    local overrides = window:get_config_overrides() or {}
    overrides.initial_cols = COLS_NON_RETINA
    overrides.initial_rows = ROWS_NON_RETINA
    window:set_config_overrides(overrides)

end)

-- make sure the initial window is created correctly, on non-retina screens
wezterm.on('gui-startup', function(cmd)
    if wezterm.gui.screens().active.name == 'Built-in Retina Display' then
        return
    end
    -- https://wezfurlong.org/wezterm/config/lua/wezterm.mux/spawn_window.html#width-and-height
    local tab, pane, window = wezterm.mux.spawn_window { width = COLS_NON_RETINA, height = ROWS_NON_RETINA }
end)

-- return our module table
return module

You can see that it is a bit of a collection of trial & error code-pieces to get WezTerm to behave roughly how I want it to.

Maybe it helps in testing ... or someone else who has this issue.

I had this module removed before testing with the new version ( https://github.com/wez/wezterm/issues/4851#issuecomment-1945682588 ). 🙂

jimwins commented 6 months ago

I have been trying to track this down, but it's been tricky to keep straight where the DPI in some calculations is coming from. I can see clearly in trace logs that it is getting the correct DPI (72) for the screen that it is creating the window on, but then somewhere in the process of creating the window it gets confused and thinks the DPI is 144 (the appropriate value for the built-in display) so it blows up the window to twice what it should be, and then recognizes that the DPI is 72 again but leaves the window doubled(ish) so the number of rows and columns is now wrong.

If I populate config.dpi_by_screen with the correct values for the two displays, it still works okay on the built-in 144dpi display, but then the window on the secondary screen is half the height it should be and nothing actually gets drawn within the window borders.

I've attached a trace-level log that may help illuminate things. There are some WARN lines that contain 'JW' which are a few extra things I added in trying to figure things out, like dumping screens in default_dpi. wezterm.log

jimwins commented 6 months ago

Some more spelunking: if I change dpi_for_window_screen to always return the effective_dpi from the screen info, I at least get the same behavior as when I populate config.dpi_by_screen with the correct values. That is, the window on my 72-dpi screen is the correct width but wrong height.

(I have questions about this function. It tries to look up the DPI in config.dpi_by_screen by calling crate::os::macos::connection::nsscreen_to_screen_info to get the screen name, when that method will itself also look up the DPI from config.dpi_by_screen but from the global config instead of a local one? I'm not sure when that would happen, but I can imagine it being very confusing.)

What made me suspicious is that at least one place where it is figuring the wrong DPI seems to be in extern "C" fn did_resize and I wonder if the NSView it is using to calculate frame and backing_frame is the right one, or it should be creating those with inner.window?

It also feels like things go awry in some font metric calculations too but I haven't tracked down where it is getting the DPI to use for those.

There seem to be a number of places where a couple of different methods are called to get the DPI and then there is further calculation if those methods return None, and then possibly scaling that. I wonder if there is some divergence in how those are being calculated.

jimwins commented 6 months ago

I was able to hack away at did_resize and get it to derive the frame and backing_frame from inner.window, and it results in the terminal window being created at the correct size and the DPI apparently being calculated correctly. But then the contents get drawn at 1/2 size so it only fills a corner of the window. (No problem on the 144 dpi screen.)

It must still be using the wrong DPI when deciding on the font metrics or something like that.

The rendering gets corrected if the window is resized.