-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Bugfix/diacritics in mono #394
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[why] For some reason reading a font into fontforge looses data on glyphs. As a workaround the `font-patcher` script has the switch `--mono` to ascertain an equal width of all glyphs. We want to (and sometimes need to) use that mode of the script to get a new font that is detected as monospaced by Windows. Sometimes that workaround fails. For example it breaks ligature glyphs in some circumstances; they seem to be moved sideways. [how] The workaround is done in two steps: First each glyph is checked for negative bearings. If these are found they are set to fixed zero. Afterwards the glyph width is set to the font global width. The ligature glyphs have already the correct width, they just have negative bearings that they indeed need. There is no reason to change the (correct) bearings to get a monospaced font. In strict monospaced fonts that would be impossible, but the ligatures are only used in applications that could work with negative bearings and so they should be there. Strict monospaced usage does ignore the bearings anyhow. Signed-off-by: Fini Jastrow <[email protected]>
[why] Some glyphs are just used as overlays for 'real' glyphs. These can be for example U+0300 .. U+036F (the 'COMBINING ...' diactritics) like U+0300, gravecomb, COMBINING GRAVE ACCENT U+0301, acutecomb, COMBINING ACUTE ACCENT U+0308, uni0308, COMBINING DIAERESIS They are never used on their own, at least they are overlayed over a blanc (U+0020, space). For the font rendering engine they need to have the correct negative bearings, so they are shifted to take no space by themselves. The font-patcher script does not allow negative bearings in monospaced fonts. This makes sense if every glyph is in itself a 'letter' that should not reach beyond it's allotted (monospaced) space. [how] In the font-patcher script we do not touch the bearings of such overlay glyphs. They can be identified by their width of zero. For Windows to detect this font as 'monospaced' we need to change the width to the standard width, though. Signed-off-by: Fini Jastrow <[email protected]>
Showing the bug again, without Windows: Need to use the latest Cascadio Code for this to happen.
Install font and use Writer (note the broken ligatures). |
This is great! Sorry I haven't had much time to take a look.. yet. |
adam7
added a commit
to adam7/delugia-code
that referenced
this pull request
Dec 28, 2019
Since [this pull request](ryanoasis/nerd-fonts#394) was merged we no longer need to patch font patcher so let's not.
LNKLEO
pushed a commit
to LNKLEO/Nerd
that referenced
this pull request
Nov 24, 2023
Bugfix/diacritics in mono
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
For some reason reading a font into
fontforge
looses data on glyphs. Asa workaround the
font-patcher
script has the switch--mono
toascertain an equal width of all glyphs. An example is Cascadia Code, that is detected as monospaced by Windows. Reading in and just generating creates a font that is not detected as monospaced by Windows anymore.
But the
--mono
switch has issues, that this PR shall fix.The
--mono
switch works in two steps: First each glyph is checked fornegative bearings. If these are found they are set to hard fixed zero.
Afterwards the glyph width is set to the font global width.
The ligature glyphs of previously monospaced fonts have already the correct width,
they just have negative bearings that they indeed need.
here is no reason to change the (correct) bearings to get a monospaced font.
In strict monospaced fonts that would be impossible, but the ligatures are only used in
applications that could work with negative bearings and so they should
be there. Strict monospaced usage does ignore the bearings anyhow.
Change: Do not change bearings if width is already ok
Some glyphs are just used as overlays for 'real' glyphs. These can be
for example U+0300 .. U+036F (the 'COMBINING ...' diactritics) like
They are never used on their own, at least they are overlayed over a
blanc (U+0020, space).
For the font rendering engine they need to have the correct negative
bearings, so they are shifted to take no space by themselves.
The font-patcher script does not allow negative bearings in monospaced
fonts. This makes sense if every glyph is in itself a 'letter' that
should not reach beyond it's allotted (monospaced) space.
Overlay glyphs can be identified by the width of zero.
Change: Do not change bearings of overlay glyphs
Requirements / Checklist
./font-patcher Inconsolata.otf --fontawesome --octicons --pomicons
./gotta-patch-em-all-font-patcher\!.sh Hermit
What does this Pull Request (PR) do?
Change the
font-patcher
's--mono
mode such that:How should this be manually tested?
Patch some fonts like Inconsolata and Cascadia Code with
--mono
.Check that Windows detects them as monospaced.
Check Ä (not on the console but in a real font rendering application)
Check ligatures (e.g.
!=
(Cascadia Code has them) in for example Visual Code with ligature mode enabled.Any background context you can provide?
This came up when working on a Nerd Font patched version of Cascadia Code here. This is the relevant PR over there.
What are the relevant tickets (if any)?
PR #374
Screenshots (if appropriate or helpful)
Shifted (too far right) ligatures of
--mono
patched Cascadia Code in Visual Studio:which are
::
!=
++
==
in this snippet.Wrong placement of diacritics:
(Note glyphs in bottom 4 rows.)
Be careful, in strictly monospaced usage the font looks ok (see below), the bug is visible only in a propotional-font-able application. The same font (that has the issues shown above) works fine on the console:
(Note
// Äther
comment and correct non-ligature!=
etc.)