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

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

Open
stefanwascoding opened this issue Jan 25, 2024 · 8 comments
Open
Labels
bug Something isn't working macOS Issue applies to Apple macOS

Comments

@stefanwascoding
Copy link

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 #4285.

@stefanwascoding stefanwascoding added the bug Something isn't working label Jan 25, 2024
@wez
Copy link
Owner

wez commented Jan 25, 2024

@wez wez added the macOS Issue applies to Apple macOS label Jan 25, 2024
@stefanwascoding
Copy link
Author

stefanwascoding commented Jan 25, 2024

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 added a commit that referenced this issue Feb 7, 2024
There are a number of open issues that relate to getting the dpi
wrong when spawning a window. In theory it shouldn't matter because
we will immediately realize the difference and synthesize the correct
information, but evidence shows this isn't quite true.

What this commit does is:

* Override Connection::default_dpi() on macOS to return the
  effective_dpi from the active screen instead of the default
  non-retina dpi
* Adjust the Config::initial_size() method to accept an optional
  cell pixel dimension
* Add a helper function to wezterm-gui to compute the cell pixel
  dimensions given the config and the (usually default) dpi, and
  feed that through to Config::initial_size
* in the macos window impl, scale the computed geometry based on
  the ratio of the Connection::default_dpi and the default non-retina
  dpi.

This helps to avoid needing to do a fixup in the
#4966 case, and may help with
the various other macos quirky issues.

refs: #2958
refs: #3575
refs: #3900
refs: #4250
refs: #4285
refs: #4447
refs: #4851
refs: #4966
@wez
Copy link
Owner

wez commented Feb 8, 2024

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

@wez wez added the waiting-on-op Waiting for more information from the original poster label Feb 8, 2024
@stefanwascoding
Copy link
Author

stefanwascoding commented Feb 15, 2024

@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

@github-actions github-actions bot removed the waiting-on-op Waiting for more information from the original poster label Feb 15, 2024
@stefanwascoding
Copy link
Author

stefanwascoding commented Feb 15, 2024

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 ( #4851 (comment) ). 🙂

@jimwins
Copy link

jimwins commented Apr 24, 2024

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
Copy link

jimwins commented Apr 24, 2024

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
Copy link

jimwins commented Apr 25, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working macOS Issue applies to Apple macOS
Projects
None yet
Development

No branches or pull requests

3 participants