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

The number of lines changes when moving between monitors of different DPI #4114

Closed
page-down opened this issue Oct 12, 2021 · 22 comments
Closed
Labels

Comments

@page-down
Copy link
Contributor

Describe the bug
Under macOS, moving windows between displays of different DPI works perfectly, except that the number of lines changes (increase or decrease) , even if the size of the window does not change.

The following is the case of integer scaling (2x), and fractional scaling is not tested.

To Reproduce

With the default font size of 11.0, the blank area increases a few pixel gradually as the window size increases (by option initial_window_height, not resizing window) after moving to a HiDPI monitor. When the window height is 24c, there is exactly one extra line.

Steps to reproduce the behavior:

  1. Run kitty on a 1080p display, using the default font size of 11.0 and a height of 24c.
kitty --config=NONE -o font_size=11.0  -o initial_window_height=24c -o initial_window_width=80c -o remember_window_size=no -o "shell zsh -il"
  1. Drag the window to the HiDPI display and an extra blank line will appear.
  2. Press Enter and this empty line can render the text, for a total of 25c height.
  3. Drag the window back to the low DPI display, which returns to 24c height.

When using 13.0 font size, JetBrains Mono font, 40c height, lines are missing after dragging the window to the HiDPI monitor.

Steps to reproduce the behavior:

  1. Run kitty on a 1080p display.
kitty --config=NONE -o "font_family JetBrains Mono" -o font_size=13.0 -o initial_window_height=40c -o initial_window_width=80c -o remember_window_size=no -o "shell zsh -il" 
  1. Dragging the window to the HiDPI display, when calculating the number of available lines, kitty thinks there is not enough space to display a full line, so blank area appears. Two lines are missing at this point. Only 38c height remains.
  2. After dragging the window back to the low DPI display, the 40c height is restored. However, there are two more empty lines and they are always present in the scrollback and can be scrolled.

Screenshots

default font size 11.0, 24c height

  1. 1080p display
    kitty_height_24c_1080p

  2. move to 4K HiDPI display

kitty_moved_to_4k_same_window_size_25c

  1. press Enter

kitty_4k_empty_line_25

  1. move back to 1080p display
    kitty_move_back_to_1080p_restored_24c

font JetBrains Mono, size 13.0, 40c height

  1. 1080p display
    kitty_font_size_13_height_40c_1080p

  2. move to 4K HiDPI display

kitty_font_size_13_4k_only_38c

  1. move back to 1080p display
    kitty_font_size_13_move_back_to_1080p_40c_extra_empty_lines

Environment details

  • macOS 11.6
  • kitty from master branch
  • display 1: 1920 * 1080
  • display 2: 3840 * 2160 @ 2x

Additional context
The above is only tested under macOS, not sure if similar problem exists under Linux.

When dragging between different DPI monitors, the resulting number of lines should remain the same when calculating the available rendering space.

@page-down page-down added the bug label Oct 12, 2021
@kovidgoyal
Copy link
Owner

cell size depends on font size and DPI, there is no guarantee that cell
size will remains the same when dpi changes. Since both font size and
DPI are floating point, and cell size in integer because of rounding,
cell sizes may change. Not to mention that other metrics in the window
such as border thickness and margins/paddings can also be dpi dependent.

@page-down
Copy link
Contributor Author

Ok. The cell inside the window cannot be scaled because the font size has to be correct.

I would like to remove the blank margin, which has been mentioned in lots of previous issues, the only correct way to do this is to resize the window to a multiple of the size of the internal cell. However it is not practical to adjust by hand, is it possible to adjust in kitty so that the window size is the same as the multiplier of cell width and height?

It should be the window manager's responsibility to resize the window, but macOS doesn't have a good way to handle it. Is it possible to get the exact pixel size of the cell after the DPI change?

The misalignment of the window size with the cell is killing me. I'll see if I can use the kitten keyboard shortcut to realign the cell columns and rows after dragging the window.

@kovidgoyal
Copy link
Owner

@page-down
Copy link
Contributor Author

https://sw.kovidgoyal.net/kitty/conf/#opt-kitty.resize_in_steps

Thank you, this configuration option is very useful.

However, I found that when used with window_resize_step_lines 1, every time the window size is changed on HiDPI displays, it changes by two lines per step.

kitty --config=NONE -o remember_window_size=no -o resize_in_steps=yes -o window_resize_step_cells=1 -o window_resize_step_lines=1

kitty_resize_in_steps

This option works fine on low DPI displays, adjusting one line per step.

@kovidgoyal
Copy link
Owner

window_resize_step_lines refers to resizing using the resize kitten. kitty has no control over how the window is resized when using the mouse.

@kovidgoyal
Copy link
Owner

Oh and if you want to resize OS windows accurately use the kitty remote control command for it https://sw.kovidgoyal.net/kitty/remote-control/#kitty-resize-os-window which you can run either via a keybaord shortcut or in the kitty shell or actually using remote control

@kovidgoyal
Copy link
Owner

Also for the resizing by two lines problem check in os_window_update_size_increments() in glfw.c it might be that on macOS the value needs to be scaled by the window scale.

@page-down
Copy link
Contributor Author

Oh and if you want to resize OS windows accurately use the kitty remote control command for it ...

Yes, thanks for the reminder, I currently have a shortcut set to reset the OS window size.

It would be nice if kitten could be executed automatically via hook, so that it could be called in case of DPI changes, etc.

However, I don't think it makes much sense to do hook specifically for this purpose.

Also for the resizing by two lines problem check in os_window_update_size_increments() in glfw.c it might be that on macOS the value needs to be scaled by the window scale.

Yes, after scaling the values, changing the window size will be resized according to the single cell size.

diff --git a/kitty/glfw.c b/kitty/glfw.c
index 3a12411e..fe391026 100644
--- a/kitty/glfw.c
+++ b/kitty/glfw.c
@@ -807,8 +807,15 @@ create_os_window(PyObject UNUSED *self, PyObject *args, PyObject *kw) {
 void
 os_window_update_size_increments(OSWindow *window) {
     if (OPT(resize_in_steps)) {
-        if (window->handle && window->fonts_data) glfwSetWindowSizeIncrements(
-                window->handle, window->fonts_data->cell_width, window->fonts_data->cell_height);
+        if (window->handle && window->fonts_data) {
+            int cw = window->fonts_data->cell_width;
+            int ch = window->fonts_data->cell_height;
+#ifdef __APPLE__
+            if (window->viewport_x_ratio > 0) cw = (int)ceil(cw / window->viewport_x_ratio);
+            if (window->viewport_y_ratio > 0) ch = (int)ceil(ch / window->viewport_y_ratio);
+#endif
+            glfwSetWindowSizeIncrements(window->handle, cw, ch);
+        }
     } else {
         if (window->handle) glfwSetWindowSizeIncrements(
                 window->handle, GLFW_DONT_CARE, GLFW_DONT_CARE);

I can't test on Linux at the moment, but if there is no problem under Linux, is there a difference in the font size calculation?

@page-down
Copy link
Contributor Author

is there a difference in the font size calculation?

No, it is not related to font size calculation, it should be related to glfw platform window resize.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Oct 12, 2021 via email

@kovidgoyal
Copy link
Owner

I have added a macOS specific fix.

@page-down
Copy link
Contributor Author

I have added a macOS specific fix.

Confirm that resize_in_steps is normal in HiDPI displays.

I found another problem.

For resize-os-window, under HiDPI monitor, after resizing by unit cells, the height is missing a line. The number of lines is correct in low DPI displays.

kitty --config=NONE -o "map f1 remote_control resize-os-window --width=80 --height=24 --unit=cells"

There are only 23 lines after pressing F1. There is no blank margin in the os window, just missing one line.

It would be nice if kitten could be executed automatically via hook, so that it could be called in case of DPI changes, etc.
However, I don't think it makes much sense to do hook specifically for this purpose.

There already is ... and in current master you can specify a watcher script in kitty.conf to watch all windows.

Thank you. DPI changes can be watched after adding the on_dpi_change related code to the following file.

  • kitty/launch.py
  • kitty/window.py
kitty -o watcher=/path/to/watcher.py
# watcher.py
def on_dpi_change(boss: BossType, window: 'Window', data: Dict[str, Any]) -> None:
    boss.resize_os_window(window.os_window_id, width=80, height=24, unit='cells')

However, this doesn't make much sense, I can't think of any usage scenarios for on_dpi_change.
Also, since the kitty window is watched instead of the os window, the above code may be executed multiple times.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Oct 12, 2021 via email

@page-down
Copy link
Contributor Author

That's probably because of titlebar height. Try hiding the titlebar and see if it works.

kitty --config=NONE -o hide_window_decorations=yes -o "map f1 remote_control resize-os-window --width=80 --height=24 --unit=cells"

On the HiDPI display, press F1 and exactly 23 lines are displayed.

@kovidgoyal
Copy link
Owner

Fixed by: 8057c42

@page-down
Copy link
Contributor Author

Fixed by: 8057c42

Thanks, after updating to the latest commit, everything works fine with the resize-os-window remote control.

To conclude this issue

  • The number of screen pixels is integer.
  • After the font size converted to HiDPI corresponding pts, although without the fractional part, but the pixels are more fine. This means that in the case of 2x scale up, there may be a 1 pixel difference.
    • For example, the Menlo font at 11.0 pts requires only 25px cell height at 2x, while at 1x it requires 13px.
  • Since the pixel scale is not exactly equal to the 2x integer scaling ( actually fractional scaling), it can happen that the window is either insufficient or excessive in height after 2x.

What I'm curious about is why on proprietary platforms like Windows and macOS, many native apps don't change their content position at all when dragged across different DPI displays. It looks like they didn't do such a fine control over the rendering of the content like kitty did, but just rendered at the highest DPI and then scaled down to the rest.

@page-down
Copy link
Contributor Author

For resize-os-window, under HiDPI monitor, after resizing by unit cells, the height is missing a line. The number of lines is correct in low DPI displays.

Fixed by: 8057c42

I found that resize_in_steps causes a similar problem, with one line missing after restoring from full screen, on the same monitor. I have noticed this when using specific font with font size.

kitty --config=NONE -o remember_window_size=no -o initial_window_width=80c -o initial_window_height=24c -o resize_in_steps=yes -o font_family=Menlo -o font_size=17.0 bash -c 'for i in {1..23};do echo $i;done;printf 24;read'

A few pixels of height are missing after restoring the window.

This is only reproduced under macOS, everything works fine with Linux X11. Even with resize_in_steps enabled, the window size is exactly the same after restoring from full screen on Linux.

@kovidgoyal
Copy link
Owner

resize_in_steps is implemented by the OS not kitty. You can check the values are correct by adding a print in _glfwPlayformSetWindowSizeIncrements in cocoa_window.m

@page-down
Copy link
Contributor Author

resize_in_steps is implemented by the OS not kitty. You can check the values are correct by adding a print in _glfwPlayformSetWindowSizeIncrements in cocoa_window.m

Unsurprisingly, there's another Apple glitch.

The following patch attempts to handle this cocoa issue. The window can be restored to its pre-fullscreen size.

diff --git a/glfw/cocoa_window.m b/glfw/cocoa_window.m
index b7e41e15..3383528e 100644
--- a/glfw/cocoa_window.m
+++ b/glfw/cocoa_window.m
@@ -1601,7 +1601,11 @@ - (void)toggleFullScreen:(nullable id)sender
 {
     if (glfw_window && glfw_window->ns.toggleFullscreenCallback && glfw_window->ns.toggleFullscreenCallback((GLFWwindow*)glfw_window) == 1)
             return;
+    // When resizeIncrements is set, Cocoa cannot restore the original window size after returning from fullscreen.
+    const NSSize resizeIncrements = [super resizeIncrements];
+    [super setResizeIncrements:NSMakeSize(1.0, 1.0)];
     [super toggleFullScreen:sender];
+    [super setResizeIncrements:resizeIncrements];
 }
 
 @end

@kovidgoyal
Copy link
Owner

seems harmless enough, I have merged

@page-down
Copy link
Contributor Author

seems harmless enough, I have merged

Thanks.

Found another cocoa window size and fullscreen issue.

When in full screen, the cocoa os window size can still be changed. In the default mode, the window blank area is the background color. In traditional fullscreen mode, the contents of the screen behind the window can be seen.

Can be reproduced by using the remote control command resize-os-window while in full screen.

Linux works fine and does not respond to window size changes in fullscreen mode.

This patch fixes this issue, please check if it is safe.

diff --git a/glfw/cocoa_window.m b/glfw/cocoa_window.m
index b8296f1c..b1c0b2b1 100644
--- a/glfw/cocoa_window.m
+++ b/glfw/cocoa_window.m
@@ -1845,6 +1845,8 @@ void _glfwPlatformSetWindowSize(_GLFWwindow* window, int width, int height)
     }
     else
     {
+        // Disable window resizing in fullscreen.
+        if ([window->ns.object styleMask] & NSWindowStyleMaskFullScreen || window->ns.in_traditional_fullscreen) return;
         NSRect contentRect =
             [window->ns.object contentRectForFrameRect:[window->ns.object frame]];
         contentRect.origin.y += contentRect.size.height - height;

@kovidgoyal
Copy link
Owner

Also should be OK, merged.

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

No branches or pull requests

2 participants