mintty / wsltty

Mintty as a terminal for Bash on Ubuntu on Windows / WSL
Other
3.1k stars 103 forks source link

Descenders are cut off with some fonts #257

Closed rj667 closed 3 years ago

rj667 commented 4 years ago

I notice that the descender (the portion of a letter that extends below the baseline of a font) of the letters g, j, q, p, y is cut off with some fonts and font sizes. I attached screenshot of Cascadia Mono (regular) 12pt in WSLtty, Putty and Windows Terminal to demonstrate the problem in WSLtty. cascadia12pt-wsltty cascadia12pt-putty cascadia12pt-windowsterminal Cut off descenders also occur in various other sizes of this font, but not in 11pt. It also occurs at large sizes (20+) of Dejavu Sans Mono and Source Code Pro, but not with Courier New or Lucida Console.

Is this something I can fix with local settings? I would like to use Cascadia Mono, the default font of Windows Terminal, in WSLtty because of the much better UTF-8 support.

mintty commented 4 years ago

Did you configure a RowSpacing value? You could try one. What's the value of Leading reported in Options - Text - Font?

rj667 commented 4 years ago

I don't see where RowSpacing can be configured. When choosing a font I can only select font, style and size. The value of Leading is -2. I'm running wsltty 3.3.0 [windows 19041]

mintty commented 4 years ago

The Leading value explains the effect. However, there is no explanation for the leading value; I get 0 for that font. RowSpacing can only be configured in a config file. You could find one in %APPDATA%/wsltty/config or %APPDATA%/mintty/config. Does it help to set RowSpacing=0 explicitly there?

rj667 commented 4 years ago

I found %APPDATA%/wsltty/config. %APPDATA%/mintty/config does not exist. Setting RowSpacing=0 does not seem to change anything: descenders still cut off, the value of Leading is still -2. But setting RowSpacing=2 does fix my problem: descenders are fully visible now and the value of Leading is now 0. The font in WSLtty now looks exactly like the Putty screenshot above and their terminal sizes are now the same. Thank you for this fix!

mintty commented 4 years ago

Thanks; I'd still be curious to find out why the Leading value is different for you. Did you install the font yourself? What's its version?

rj667 commented 4 years ago

The version of the font is 2008.025. I didn't install it myself but I think it came with Windows Terminal. I first had installed a beta version of it, later the released version.

aaronbell commented 3 years ago

I recently received a bug report on Cascadia Code’s github related to this issue. Out of interest, how does mintty calculate the leading value?

antoineco commented 3 years ago

@aaronbell it's using GetTextMetrics, which returns a TEXTMETRIC struct (relevant code location: src/wintext.c#L542).

Example (not from Cascadia Code):

The "leading" value is calculated here using the value of tmInternalLeading: src/wintext.c#L625-L626

mintty commented 3 years ago

Thanks; it also uses some heuristics for tuning, in function row_padding().

aaronbell commented 3 years ago

OK! I have some thoughts on this :). I'm not super familiar with the back-end code processes, but I think I can make some assumptions that should give clarity here.

My understanding is that: tmAscent is based on winAscent tmDescent is based on winDescent

I believe that tmInternalLeading is derived based on the difference between sCapHeight and winAscent.

So! In most coding fonts, the winAscent value and sCapHeight values are very similar (if not identical). However, Cascadia Code is different. It was developed for Windows Terminal which is build on DirectWrite (instead of GDI) and has a flag set to ignore the winAscent and winDescent values to achieve better line spacing.

In Cascadia Code: winAscent = 2226 winDescent = 480 sCapHeight = 1420

So, for Cascadia Code, the large difference between sCapHeight and winAscent has resulted in a large tm.tmInternalLeading value. That way, when row_padding runs, int i is large, then exc is large (even with the exc = i - 3), and so int adj = e - exc results in a negative adj value.

So if that's accurate then that would explain why the leading value is what it is.

A couple questions: 1) The system that you've implemented for leading is a bit 'fuzzy' in that you multiply tmInternalLeading by the screen resolution. So that could result in rounding errors in the leading, either slightly larger or smaller. When rendered, that could lead to a pixel being clipped incorrectly and might explain why fonts may work correctly at some sizes but not others.

1) Is there ever a situation where negative leading makes sense? In most fonts, any extra space is at the top of the glyphs, so negative leading will result in clipping the bottom of descender values. I'd probably suggest always setting leading to 0. That way you're never going to have clipping on the bottom.

mintty commented 3 years ago

I reproduce the issue now (don't know why I didn't before) and sure it should be fixed. Problems are:

aaronbell commented 3 years ago

I took a look at Monaco and it has very similar properties to Cascadia Code in that the winAscent metrics have been set to account for Vietnamese support:

winAscent:2504 sCapHeight:1552

I can totally understand the desire not to mess with things that has taken so long to get working, but here's my suggestion:

As you know, GDI's TEXTMETRIC values are dpi-dependent, which is part of the reason why you're seeing issues pop up at certain sizes of certain fonts. In reading through these threads, I am particularly concerned about the fact that issues pop up at larger font sizes, and commonly on high-dpi devices. That indicates to me that, as resolutions increase, the exc = i - 3 code will become less and less accurate and will need constant adjusting to correct. What you really need is a proper solution that is resolution independent.

Unfortunately, GDI does not offer very much information in TEXTMETRIC. If you were to switch to DWRITE, more font metrics are available to you (which also accounts for hasTypographicMetrics, a flag set in Cascadia for improved metrics) but I understand that is a fairly significant undertaking :).

So I see two options:

1) I think you can estimate the font sCapHeight value by subtracting tmInternalLeading from tmAscent. That might be an avenue to explore as a mechanism to reduce the line spacing depending on the font metrics. For fonts where tmInternalLeading is close to 0, then this will have minimal impact. On fonts where tmInternalLeading is large, then it will have a more significant impact. It may be necessary to additionally vertically shift the position of text to avoid clipping, but that would have to be tested.

2) Set leading value to always be 0 and tell users to customize it per their preference via RowSpacing. TBH, it is really hard to predict how fonts are designed / metrics are set. In the case of Monaco and Cascadia Code, the ascent values are set to include Vietnamese support (which tends to be quite tall), whereas few other coding typefaces offer that language coverage, so have more 'normal' metrics. This should avoid clipping entirely, but puts more work on users to customize for their particular font choice.

mintty commented 3 years ago

Released 3.4.7.

litesam commented 3 years ago

I have the same issue but for characters like arrow (Font used is Cascadia Mono) image I am at mintty 3.5.0 version.

antoineco commented 3 years ago

@litesam that's a different issue, see https://github.com/mintty/mintty/issues/1104