charToGlyph throws error for missing glyph for fonts with default encoding

[Balearica]

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.

opentype.js/src/encoding.mjs

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;
	};

opentype.js/src/font.mjs

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:

[Balearica]

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.

opentype.js/src/tables/cff.mjs

Lines 1341 to 1342 in e9c090e

	// Prefer the CMAP encoding to the CFF encoding.
	font.encoding = font.encoding || font.cffEncoding;

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.