Line Height Issue on latest master

Describe the bug
I noticed that when building the latest master of macvim, font line-heights get weirdly messed up.
I am on macOS Mojave - 10.14.6

Normally I run off the stateful-render branch, but was trying to rebase master into it when I came across the bug. So I tried building master manually to see if it reproed, and it did indeed.

Edit Notes:
I build via github, via this lil custom build script:

#!/bin/bash
make clean
make distclean
./configure --with-features=huge --enable-terminal --enable-rubyinterp --enable-python3interp --enable-perlinterp --enable-cscope --enable-rubyinterp --with-ruby-command=/usr/bin/ruby
make
rm src/MacVim/build/Release/MacVim.app/Contents/Resources/MacVim.icns
cp ./MacVim.icns src/MacVim/build/Release/MacVim.app/Contents/Resources/
open src/MacVim/build/Release

Output from reading org.vim.MacVim:

    MMAutosaveColumns = 142;
    MMAutosaveRows = 63;
    MMCurrentPreferencePane = Advanced;
    MMFullScreenFadeTime = 0;
    MMNativeFullScreen = 0;
    MMNoTitleBarWindow = 0;
    MMRenderer = 1;
    MMShowAddTabButton = 0;
    MMTitlebarAppearsTransparent = 1;
    MMTopLeftPoint = "{1279, 1417}";
    SUEnableAutomaticChecks = 0;
    SUHasLaunchedBefore = 1;
    SULastCheckTime = "2019-01-28 18:28:47 +0000";
    SUSendProfileInfo = 0;

@amadeus

@amadeus thank you for an issue

You missed a few important steps of creating bug-report, so please, fill them out.

Could you tell us more about your system?

From source did you installed MacVim and which version? GitHub/Homebrew/MacPorts/built from source (if the latest, please specify commit/tag you used)
Try to run with mvim --clean (gvim --clean) to eliminate influence of all plugins you have
Provide output of following command in Terminal: defaults read org.vim.MacVim | grep 'MM\|SU'

What font are you using? There was some line height issue that I think was fixed in 159 so curious if it’s related.

@ychin

@ychin my guifont settings are:

  guifont=InputMono-Regular:h15

https://input.fontbureau.com/

@eirnym

@eirnym I edited my original post with the missing things you requested, however I don't think there's any way this is plugin related, unless a plugin is editing linespace which it's not - since checking the value yields 0, which my older working build had as well.

@ychin I can also confirm downloading the latest release from the releases tab in github also replicates this bug with my font settings, so could be related. Also I don't think stateful-render PR branch has 159 updates yet either, so that seems like a really good candidate.

@ychin

@ychin it seems to be same as with Menlo. Spacing between letters are higher. Fira Code Light have the same issues, but less visible

@amadeus

@amadeus Looking at the commits, I think the stateful-render branch actually contains a temporary bug that was introduced and fixed in between snapshot-157 and snapshot-158 (see #928). (Edit: Interestingly #928 was also complaining about the InputMono font)

Now, it's arguable whether it's a bug or not. Basically MacVim has never respected the font's line heights (try snapshot 157/158 if you don't believe me) and there was a PR which started reading font's line heights, which while sort of correct, introduced a behavior that's different from before. I fixed the issue before releasing 158 to keep it consistent with before.

If we want to support line heights, we could, but it would need to be a new option to keep it consistent with years of existing behavior.

@amadeus feel free to try older versions to see if they behave similar to snapshot-159/160.

Interesting! I’ll experiment more tomorrow.

Out of curiosity, how do we calculate line height now? Is it simply the font size?

Ideally you guys could expose a guilineheight option to override. It looks like I could customize Input Mono to download it with a 1x line-height - but the font ends up looking VERY tight that way, so the breathing room in stateful-renderer (which i believe is using a 1.2x line height) is good.

Out of curiosity, how do we calculate line height now? Is it simply the font size?

I believe that's the case but I actually haven't looked closely enough. I just fixed it up to make sure we didn't have a regression or incompatible change but I didn't have time to dive into the specifics of how the line height is calculated now.

Ideally you guys could expose a guilineheight option to override. It looks like I could customize Input Mono to download it with a 1x line-height - but the font ends up looking VERY tight that way, so the breathing room in stateful-renderer (which i believe is using a 1.2x line height) is good.

Yeah I agree we could expose an option. It's just that in true Vim fashion we should keep the old behavior which has been around for a while the default.

@s4y

Maybe we can convince @s4y to make it part of the stateful-render branch 😏

If I was to spec the feature, I would say - guilineheight should default to whatever it does now (for backwards compatibility), but then it could also take a floating point value to override the existing behavior which would then be a multiplier for the existing lineheight. So if the default is 1, then I could specify 1.2 which would be 1.2 * guifontsize == 1.2 * 15 == 18

@s4y

Maybe we can convince @s4y to make it part of the stateful-render branch 😏

No please don't. Let's just try to merge that first before tacking on small features. I need to review the latest changes in that PR and hopefully this will get in soon.

If I was to spec the feature, I would say - guilineheight should default to whatever it does now (for backwards compatibility), but then it could also take a floating point value to override the existing behavior which would then be a multiplier for the existing lineheight. So if the default is 1, then I could specify 1.2 which would be 1.2 * guifontsize == 1.2 * 15 == 18

Can't you just use linespace for that? I would imagine we just need some way to tell MacVim to respect the font's native line height. It could either be set as part of guifont, a new Vim settings, or just a preference set in the GUI preference panel.

Fair enough - I always assumed linespace was to create a gap between lines, not actually change the line height.

@ychin

@ychin wrote

If we want to support line heights, we could, but it would need to be a new option to keep it consistent with years of existing behavior.

I will never understand this line of thinking. It's an editor that has always been broken. Fix it. It's not a legacy bank transaction system. It's an editor. No one is going to lose anything if their editor suddenly starts behaving more rationally. Just fix it, for goodness sake. Why not?

Please?

Wish I knew enough to help more practically. Here's some more info that may help.

This bug never used to happen -- like for years, until a couple or three months ago or however long it's been.

Hopefully the following will at least show a way to reliably duplicate the issue on the dev's system.

MacVim's start-up behaviour is different from any other time a font size or family is changed. At start-up, the lines are too short and font is clipped. After making any change to font however, the issues corrects itself. It's so easy to fix / work around (see below) that I just figured the bug would "go away of its own accord." (Wouldn't that be cool? :p) However, it's been months now. So, I guess the bug's presentation is not as prevalent as I have assumed.

Examples ...

% mvim --clean
:option<Cr>

screen snippet

Increase font size +1 then immediately -1 (or any change in font, by any manner)

<cmd +><cmd ->

new screen snippet -- not that lines are "correct" (more appropriate) height now.

Plugins were disabled for this data. Confirming evidence; I usually work on a dark background.

iMac 27" macOS 10.15.4 (19E287)
Bug also present and identical on MacBook Pro, fresh install, 10.15.4 (19E287)

:version
VIM - Vi IMproved 8.1 (2018 May 18, compiled Feb  4 2019 08:13:44)
macOS version
Included patches: 1-873
Compiled by travis@Traviss-Mac-6.local
Huge version with MacVim GUI.  Features included (+) or not (-):
+acl               +cmdline_info      +farsi             +langmap           +mouse_sgr         +profile           -tag_any_white     +visual
+arabic            +comments          +file_in_path      +libcall           -mouse_sysmouse    +python/dyn        -tcl               +visualextra
+autocmd           +conceal           +find_in_path      +linebreak         +mouse_urxvt       +python3/dyn       +termguicolors     +viminfo
+autochdir         +cryptv            +float             +lispindent        +mouse_xterm       +quickfix          +terminal          +vreplace
-autoservername    +cscope            +folding           +listcmds          +multi_byte        +reltime           +terminfo          +wildignore
+balloon_eval      +cursorbind        -footer            +localmap          +multi_lang        +rightleft         +termresponse      +wildmenu
+balloon_eval_term +cursorshape       +fork()            +lua/dyn           -mzscheme          +ruby/dyn          +textobjects       +windows
+browse            +dialog_con_gui    +fullscreen        +menu              +netbeans_intg     +scrollbind        +textprop          +writebackup
++builtin_terms    +diff              -gettext           +mksession         +num64             +signs             +timers            -X11
+byte_offset       +digraphs          -hangul_input      +modify_fname      +odbeditor         +smartindent       +title             -xfontset
+channel           +dnd               +iconv             +mouse             +packages          +startuptime       +toolbar           +xim
+cindent           -ebcdic            +insert_expand     +mouseshape        +path_extra        +statusline        +transparency      -xpm
+clientserver      +emacs_tags        +job               +mouse_dec         +perl/dyn          -sun_workshop      +user_commands     -xsmp
+clipboard         +eval              +jumplist          -mouse_gpm         +persistent_undo   +syntax            +vartabs           -xterm_clipboard
+cmdline_compl     +ex_extra          +keymap            -mouse_jsbterm     +postscript        +tag_binary        +vertsplit         -xterm_save
+cmdline_hist      +extra_search      +lambda            +mouse_netterm     +printer           +tag_old_static    +virtualedit
   system vimrc file: "$VIM/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
  system gvimrc file: "$VIM/gvimrc"
    user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "~/.vim/gvimrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
    system menu file: "$VIMRUNTIME/menu.vim"
  fall-back for $VIM: "/Applications/MacVim.app/Contents/Resources/vim"
Compilation: clang -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_MACVIM -Wall -Wno-unknown-pragmas -pipe  -DMACOS_X -DMACOS_X_DARWIN  -g -O2 -U_FORTIFY_SOURCE -D_FORTIFY_
SOURCE=1
Linking: clang   -L. -fstack-protector-strong -L/usr/local/lib -L/usr/local/opt/libyaml/lib -L/usr/local/opt/openssl/lib -L/usr/local/opt/readline/lib -L. -fstack-pro
tector-strong -L/usr/local/lib -L/usr/local/opt/libyaml/lib -L/usr/local/opt/openssl/lib -L/usr/local/opt/readline/lib  -L/usr/local/lib -o Vim -framework Cocoa -fram
ework Carbon       -lm  -lncurses -liconv -framework AppKit   -fstack-protector  -L/System/Library/Perl/5.18/darwin-thread-multi-2level/CORE

@gruvin

@gruvin Does this happen to you in the stateful-render branch?

@s4y

@s4y I'm not set up to compile the code myself, sorry

This issue appears to have been fixed in MacVim Version 8.2.539 (163) which came through the automatic updatimathing, for me at around 2020-05-05 10:00 GMT.

Thanks. :-)

Fix issue with font line spacing support and add option for it

Navigation Menu

Repository navigation

Line Height Issue on latest master #977

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Line Height Issue on latest master #977

Description

Activity

eirnym commented on Oct 28, 2019

ychin commented on Oct 28, 2019

amadeus commented on Oct 28, 2019

amadeus commented on Oct 28, 2019

eirnym commented on Oct 28, 2019

ychin commented on Oct 29, 2019

amadeus commented on Oct 29, 2019

ychin commented on Oct 30, 2019

amadeus commented on Oct 30, 2019

ychin commented on Oct 30, 2019

amadeus commented on Oct 30, 2019

gruvin commented on May 3, 2020

gruvin commented on May 3, 2020

s4y commented on May 3, 2020

gruvin commented on May 3, 2020

gruvin commented on May 6, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions