Open Balearica opened 3 months ago
Reviewing the code, I realized that on paper a similar issue also likely impacts CffEncoding
. However, this is likely not an issue as it does not appear that CffEncoding
is actually used as the main encoding (Font.encoding
) for any font. The cmap
encoding is always used over the cff
encoding when it exists.
While this would seemingly indicate that the CffEncoding
is sometimes used, cmap
is a required table for OpenType/TrueType, and I confirmed that all fonts in the test directory use CmapEncoding
. Therefore, it looks like Font.encoding
will always be an instance of either CmapEncoding
or DefaultEncoding
.
Current vs. Expected Behavior
The method
charToGlyph
converts a literal character to a glyph. For the vast majority of fonts, usingcharToGlyph
with a character that does not exist in the font results in the.notdef
glyph being returned. This behavior is expected and should happen for all fonts. However, for fonts that useDefaultEncoding
, an error is thrown.Cause
The
charToGlyph
function works by (1) callingcharToGlyphIndex
to get the glyph index for the character, and then (2) gets the glyph at that index. This fails for fonts that useDefaultEncoding
, as theDefaultEncoding.charToGlyphIndex
function behaves differently from what is expected. WhilecharToGlyphIndex
usually defaults to0
, and the type annotations indicate it always returns anumber
,charToGlyphIndex
defaults tonull
for fonts that use the default encoding. WhencharToGlyph
attempts to look up the glyph with indexnull
, it throws an error.https://github.com/opentypejs/opentype.js/blob/e9c090ed801f74df2cedc2f1df937bd439070960/src/encoding.mjs#L200-L214
https://github.com/opentypejs/opentype.js/blob/e9c090ed801f74df2cedc2f1df937bd439070960/src/font.mjs#L201-L210
Possible Solutions
I opened a PR (#736) that resolves this issue by editing
get
to returnundefined
when the input isnull
, rather than throw an error. This resolves the problem in a minimally-invasive way that is not a breaking change and is unlikely to produce unintended consequences.However, I think editing
charToGlyphIndex
to return0
by default for fonts with default encoding should also be considered. Havingfont.charToGlyphIndex
sometimes default tonull
and sometimes default to0
depending onencoding
(something most users do not think about) is inconsistent, so unless there is a compelling reason, I think it should be standardized to return0
. This would also make the existing types correct--at present the type annotations forfont.charToGlyphIndex
indicate it always returnsnumber
, which is currently untrue as it can also returnnull
.The only potential downside that I can think of is that this would produce unintuitive results in a case where a user creates their own font where the glyph with index
0
is not.notdef
. However, that seems more like user error than a legitimate use-case, and the existing OpenType.js documentation already states that adding.notdef
is required.Steps to Reproduce (for bugs)
This can be demonstrated by creating a font using the snippet in the readme (reproduced below), which will have default encoding.
Running
font.charToGlyph('😊')
with this font results in the following error:Context
My program intermingles
Font
objects created from font files (which useCmapEncoding
) withFont
objects created with OpenType.js (which useDefaultEncoding
). Errors started being thrown with certain combinations of user-inputs and fonts, and it was unclear why.Your Environment