-
Notifications
You must be signed in to change notification settings - Fork 514
Description
Current vs. Expected Behavior
The method charToGlyph
converts a literal character to a glyph. For the vast majority of fonts, using charToGlyph
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 use DefaultEncoding
, an error is thrown.
Cause
The charToGlyph
function works by (1) calling charToGlyphIndex
to get the glyph index for the character, and then (2) gets the glyph at that index. This fails for fonts that use DefaultEncoding
, as the DefaultEncoding.charToGlyphIndex
function behaves differently from what is expected. While charToGlyphIndex
usually defaults to 0
, and the type annotations indicate it always returns a number
, charToGlyphIndex
defaults to null
for fonts that use the default encoding. When charToGlyph
attempts to look up the glyph with index null
, it throws an error.
Lines 200 to 214 in e9c090e
DefaultEncoding.prototype.charToGlyphIndex = function(c) { | |
const code = c.codePointAt(0); | |
const glyphs = this.font.glyphs; | |
if (glyphs) { | |
for (let i = 0; i < glyphs.length; i += 1) { | |
const glyph = glyphs.get(i); | |
for (let j = 0; j < glyph.unicodes.length; j += 1) { | |
if (glyph.unicodes[j] === code) { | |
return i; | |
} | |
} | |
} | |
} | |
return null; | |
}; |
Lines 201 to 210 in e9c090e
Font.prototype.charToGlyph = function(c) { | |
const glyphIndex = this.charToGlyphIndex(c); | |
let glyph = this.glyphs.get(glyphIndex); | |
if (!glyph) { | |
// .notdef | |
glyph = this.glyphs.get(0); | |
} | |
return glyph; | |
}; |
Possible Solutions
I opened a PR (#736) that resolves this issue by editing get
to return undefined
when the input is null
, 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 return 0
by default for fonts with default encoding should also be considered. Having font.charToGlyphIndex
sometimes default to null
and sometimes default to 0
depending on encoding
(something most users do not think about) is inconsistent, so unless there is a compelling reason, I think it should be standardized to return 0
. This would also make the existing types correct--at present the type annotations for font.charToGlyphIndex
indicate it always returns number
, which is currently untrue as it can also return null
.
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.
// this .notdef glyph is required.
const notdefGlyph = new opentype.Glyph({
name: '.notdef',
advanceWidth: 650,
path: new opentype.Path()
});
const aPath = new opentype.Path();
aPath.moveTo(100, 0);
aPath.lineTo(100, 700);
// more drawing instructions...
const aGlyph = new opentype.Glyph({
name: 'A',
unicode: 65,
advanceWidth: 650,
path: aPath
});
const font = new opentype.Font({
familyName: 'OpenTypeSans',
styleName: 'Medium',
unitsPerEm: 1000,
ascender: 800,
descender: -200,
glyphs: [notdefGlyph, aGlyph]});
Running font.charToGlyph('π')
with this font results in the following error:
glyphset.js:51 Uncaught TypeError: this.font._push is not a function
at GlyphSet.get (glyphset.js:51:19)
at Font.charToGlyph (font.js:142:29)
at <anonymous>:1:7
Context
My program intermingles Font
objects created from font files (which use CmapEncoding
) with Font
objects created with OpenType.js (which use DefaultEncoding
). Errors started being thrown with certain combinations of user-inputs and fonts, and it was unclear why.
Your Environment
- Version used: e9c090e
- Font used:
- Browser Name and version:
- Operating System and version (desktop or mobile):
- Link to your project: