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

PuTTY + Inconsolata 2.000+ #35

Open
cprivitere opened this issue Oct 20, 2019 · 9 comments
Open

PuTTY + Inconsolata 2.000+ #35

cprivitere opened this issue Oct 20, 2019 · 9 comments

Comments

@cprivitere
Copy link

Long story short, the version 2.000+ versions of Inconsolata have 2-3 times the vertical spacing in PuTTY versus the 1.009 version. It works fine in other applications like Visual Studio Code. I'm around folks who use this font for Sublime, Visual Code, and PuTTY. So It'd be nice for us if it worked the same in all three. Is there a way to get a more programmer friendly version of the font to be part of future releases that doesn't have the same vertical spacing issues?

@raphlinus
Copy link
Collaborator

Ok, I'm digging into this now. Unfortunately, any possible solution to this issue is a compromise. Also see #9 and #26.

Historically, the usWinAscent and usWinDescent values control both the line spacing and clipping of glyphs. In order to avoid clipping, these values must be large enough to hold the highest and lowest point of any glyph in the font. While 1.0 was carefully designed to fit into a restrained bounding box, 2.0 added Vietnamese support, which has many tall glyphs. Therefore, it is impossible to simultaneously satisfy all three of the following requirements (it's basically "choose two"):

  • Work correctly in apps that use old GDI-based font rendering (including Internet Explorer pre-9).
  • Have tighter line spacing.
  • Support Vietnamese.

In order to address this problem, OpenType 1.5 added a "use typo metrics" bit, which specifies that the sTypoAscender, sTypoDescender, and sTypoLineGap metrics should be used in place of usWin. I believe this bit is set in all 2.000+ versions of Inconsolata (I verified it from the zipfile downloaded from Google Fonts). However, older apps, such as PuTTY, apparently do not respect this setting. I would consider this a bug in PuTTY.

I'm searching for guidance. The font bakery checker will complain if the usWin metrics are too small. The Font Development Best Practices document doesn't give direct advice, but suggests avoiding clipping, giving an out (in the paragraph beginning, "There is one situation") if the line spacing is too loose.

The Google Fonts docs are perhaps the most prescriptive. Rule 4 is fairly clear: "Win Ascent and Win Descent values must be the same as the family's tallest/deepest yMin and yMax bbox values". They cite a bug in another font (Overpass) where clipping occurred.

So, while I understand the frustration, I believe the correct resolution is to leave the larger usWin metrics as they are. Perhaps PuTTY could be fixed to respect the "use typo metrics" flag. Failing that, I might consider making a patched version of the font available, to address this problem specifically.

@cprivitere
Copy link
Author

cprivitere commented Dec 8, 2019

What about releasing a version of the font without the Vietnamese (and also any future tall) glyphs in it so that English users can use the font in PuTTy without the issue? It can just be marked as for older programs or terminal compatibility, maybe?

raphlinus added a commit that referenced this issue Dec 11, 2019
This is a candidate release of 3.000. It updates the generated fonts (all
made with fontmake) and also the README.

There's a bit of unfinished work, in particular the Ligconsolata variants
are not generated yet. I also want to do a subset excluding Vietnamese,
for use in terminals (see #35).

Closes #32, though there will be some followon work.
@cprivitere
Copy link
Author

cprivitere commented Dec 12, 2019

Ok, I went to the PuTTY team and got a response.

You say that "PuTTY doesn't respect" this bit, and Raph Levien's comment on that github page says he considers it a bug in PuTTY. But PuTTY does not read font files itself: it depends on the Windows text rendering API to do that. So we're not deliberately choosing to use any particular set of font metrics from your OpenType file: we're just asking Windows what the metrics are for a given systemwide-installed font, and trusting it to tell us the right answer.

(That is, assuming you're talking about Windows PuTTY and not GTK, which looks likely from the github discussion, but I didn't see it stated in your email.)

In other words, if this is a bug, then it's a bug somewhere in the combined system of PuTTY's code and the Windows text rendering libraries. But on which side of the boundary? Are we failing to make some critical API call, or pass a particular configuration flag, for example? Or is this a bug that Windows itself ought to be fixing in their graphics libraries?

I wasn't able to find the answer in a quick search of MSDN. You mentioned that VS Code gets this right, so I checked in the VS Code sources too (and the Electron sources, which VS Code is apparently based on), but I didn't find the answer in five minutes of searching those, either.

But I'd guess that among people who work with fonts at a detailed enough level to add this kind of advanced feature, the right ways to use operating system rendering APIs to get the features respected are probably common knowledge. So, perhaps you or Raph Levien might be able to find out how to get PuTTY to do this?

Any idea what we could pass back to them to help them fix it? If you also don't know, don't worry about it, I just honestly don't know who else to ask. Maybe I can spend some time in 2020 learning low level font interpretation. :)

@raphlinus
Copy link
Collaborator

I'll take a look and see what I can find. No doubt the problem is that they're using older Windows API's for this, in particular GDI+ rather than DirectWrite. It's not surprising that they didn't find the answer after searching Electron sources for five minutes - I've spent literally weeks understanding how Chromium (and Firefox) hooks into platform font code, especially around font fallback and related issues.

@raphlinus
Copy link
Collaborator

Ok, I did some digging, and can offer a plan. I've been experimenting with the PuTTY code, but don't feel quite confident enough to prepare a patch - it's been a while since I've done serious work in C.

Basically, applications will need to read the relevant font tables themselves. Most of the code I looked at is based on a powerful graphics library such as Cairo. However, it's not a huge amount of code to do this from scratch. At a high level, the plan is to read font tables from the font, and, if the use_typo_metrics flag is set, use the typo metrics instead of the tm metrics. There also needs to be an adjustment to the y position where the text is drawn.

The first step is to use GetFontData to read the relevant tables. One is the OS/2 table, which has tag 0x322F534F, and the other is head (tag 0x64616568). A good reference for bit-bashing these tables btw is sfntly; here's the code for OS/2 and head. Both are quite small, OS/2 is typically 96 bytes and head is 54 bytes, so the buffers can be stack allocated. I recommend first calling with out a buffer to get a size, then calling a second time with min(sizeof(stack buffer), size reported). If the size reported from head is below 54 or the size of OS/2 is below 74, then bail (the sizes can vary somewhat based on table version).

From the head table you'll need unitsPerEm (big-endian uint16 at offset 18), and from OS/2 you'll need fsSelection (uint16 at 62), sTypoAscender, sTypoDescender, and sTypoLineGap (int16 at 68, 70, and 72). The logic goes like this. If bit 7 is set (fsSelection & 0x80 != 0), then compute ascent and descent from the typo metrics instead. I'd recommend combining line gap and ascender, as this will minimize clipping of accented characters. The math goes like this:

round_up(units, font_height, unitsPerEm) = floor((units * font_height + units - 1) / unitsPerEm)

new_ascent = round_up(sTypoAscender + sTypoLineGap, font_height, unitsPerEm)
new_descent = round_up(-sTypoDescender, font_height, unitsPerEm)

The sum of the two is your new height, to be used in place of tm.tmHeight. You will also need to subtract tm.tmAscent - new_ascent from your y coordinate when drawing text (ExtTextOut) to avoid the baseline being too low, as these drawing commands use the "top" as a reference point, and the GDI understanding of that point is the point above the Vietnamese accented characters, which is quite high.

Let me know if there are any questions. I hope this is enough detail, but I could make a patch if that would be significantly more helpful.

@cprivitere
Copy link
Author

Wow. Thank you for this. I’ve forwarded it on to the PuTTY wishlist request email. Hopefully it’s what Simon needs. I imagine he’ll be disappointed it’s not just a matter of using a newer API call.

Sent with GitHawk

@dumblob
Copy link

dumblob commented Apr 18, 2020

I hope I'm not too late to the party, but could we also extend the amount of excluded things to all ligatures? The reason is the very same as for exclusion of Vietnamese chars etc. as described above. This time it seems ligatures don't work with the Xft library which has a bug, but won't be updated any more.

The easiest solution seems to simply exclude ligatures when building Inconsolata, but I could not find any easy way to do that. Would you @raphlinus have any pointers?

For discussion on the ligatures problem see #42 (Emacs is actually fine as it can be built with Cairo instead of Xft, but e.g. xterm doesn't have and will probably never have a different option than Xft).

smemsh added a commit to smemsh/.fonts that referenced this issue Nov 9, 2022
reverts commit d95f070

there is a bug in later Inconsolata (v3+?) where it has extra wide
spacing.  this is apparently due to a bug in Xft, which will never be
fixed (?)

follow googlefonts/Inconsolata#42 for when
this will work.  the mentioned patch for 'st' does work btw, but urxvt
for example still fails to load, we don't know what else will be
affected by this, so we will just revert until this is either fixed in
Xft, or fixed by a release of Inconsolata without ligatures, as
described in googlefonts/Inconsolata#35
@dumblob
Copy link

dumblob commented Nov 11, 2022

@cprivitere are you in contact with Simon and/or PuTTY team? I see https://www.chiark.greenend.org.uk/~sgtatham/putty/wishlist/typographical-metrics.html is still in an undecided state 😢.

@cprivitere
Copy link
Author

I am not, no.

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

No branches or pull requests

3 participants