fix fvar axis/instance name parsing and writing (fix #687) (#694)

* fix fvar axis/instance name parsing and writing (fix #687)

* fix unsupported tables disrupting the writing process

Author: Connum

src/tables/fvar.js

@@ -4,40 +4,16 @@
 import check from '../check.js';
 import parse from '../parse.js';
 import table from '../table.js';
-import { objectsEqual } from '../util.js';
-
-function addName(name, names) {
-    let nameID = 256;
-    for (let platform in names) {
-        for (let nameKey in names[platform]) {
-            let n = parseInt(nameKey);
-            if (!n || n < 256) {
-                continue;
-            }
-
-            if (objectsEqual(names[platform][nameKey], name)) {
-                return n;
-            }
-
-            if (nameID <= n) {
-                nameID = n + 1;
-            }
-        }
-        names[platform][nameID] = name;
-    }
-
-    return nameID;
-}
+import { getNameByID } from './name.js';
 
-function makeFvarAxis(n, axis, names) {
-    const nameID = addName(axis.name, names);
+function makeFvarAxis(n, axis) {
     return [
         {name: 'tag_' + n, type: 'TAG', value: axis.tag},
         {name: 'minValue_' + n, type: 'FIXED', value: axis.minValue << 16},
         {name: 'defaultValue_' + n, type: 'FIXED', value: axis.defaultValue << 16},
         {name: 'maxValue_' + n, type: 'FIXED', value: axis.maxValue << 16},
         {name: 'flags_' + n, type: 'USHORT', value: 0},
-        {name: 'nameID_' + n, type: 'USHORT', value: nameID}
+        {name: 'nameID_' + n, type: 'USHORT', value: axis.axisNameID}
     ];
 }
 
@@ -49,14 +25,15 @@ function parseFvarAxis(data, start, names) {
     axis.defaultValue = p.parseFixed();
     axis.maxValue = p.parseFixed();
     p.skip('uShort', 1);  // reserved for flags; no values defined
-    axis.name = (names.macintosh || names.windows || names.unicode)[p.parseUShort()] || {};
+    const axisNameID = p.parseUShort();
+    axis.axisNameID = axisNameID;
+    axis.name = getNameByID(names, axisNameID);
     return axis;
 }
 
-function makeFvarInstance(n, inst, axes, names) {
-    const nameID = addName(inst.name, names);
+function makeFvarInstance(n, inst, axes, optionalFields = {}) {
     const fields = [
-        {name: 'nameID_' + n, type: 'USHORT', value: nameID},
+        {name: 'nameID_' + n, type: 'USHORT', value: inst.subfamilyNameID},
         {name: 'flags_' + n, type: 'USHORT', value: 0}
     ];
 
@@ -69,24 +46,45 @@ function makeFvarInstance(n, inst, axes, names) {
         });
     }
 
+    if (optionalFields && optionalFields.postScriptNameID) {
+        fields.push({
+            name: 'postScriptNameID_',
+            type: 'USHORT',
+            value: inst.postScriptNameID !== undefined? inst.postScriptNameID : 0xFFFF
+        });
+    }
+
     return fields;
 }
 
-function parseFvarInstance(data, start, axes, names) {
+function parseFvarInstance(data, start, axes, names, instanceSize) {
     const inst = {};
     const p = new parse.Parser(data, start);
-    inst.name = (names.macintosh || names.windows || names.unicode)[p.parseUShort()] || {};
+    const subfamilyNameID = p.parseUShort();
+    inst.subfamilyNameID = subfamilyNameID;
+    inst.name = getNameByID(names, subfamilyNameID, [2, 17]);
     p.skip('uShort', 1);  // reserved for flags; no values defined
 
     inst.coordinates = {};
     for (let i = 0; i < axes.length; ++i) {
         inst.coordinates[axes[i].tag] = p.parseFixed();
     }
 
+    if (p.relativeOffset === instanceSize) {
+        inst.postScriptNameID = undefined;
+        inst.postScriptName = undefined;
+        return inst;
+    }
+
+    const postScriptNameID = p.parseUShort();
+    inst.postScriptNameID = postScriptNameID == 0xFFFF ? undefined : postScriptNameID;
+    inst.postScriptName = inst.postScriptNameID !== undefined ? getNameByID(names, postScriptNameID, [6]) : '';
+
     return inst;
 }
 
 function makeFvarTable(fvar, names) {
+    
     const result = new table.Table('fvar', [
         {name: 'version', type: 'ULONG', value: 0x10000},
         {name: 'offsetToData', type: 'USHORT', value: 0},
@@ -102,8 +100,25 @@ function makeFvarTable(fvar, names) {
         result.fields = result.fields.concat(makeFvarAxis(i, fvar.axes[i], names));
     }
 
+    const optionalFields = {};
+
+    // first loop over instances: find out if at least one has postScriptNameID defined
+    for (let j = 0; j < fvar.instances.length; j++) {
+        if(fvar.instances[j].postScriptNameID !== undefined) {
+            result.instanceSize += 2;
+            optionalFields.postScriptNameID = true;
+            break;
+        }
+    }
+
+    // second loop over instances: find out if at least one has postScriptNameID defined
     for (let j = 0; j < fvar.instances.length; j++) {
-        result.fields = result.fields.concat(makeFvarInstance(j, fvar.instances[j], fvar.axes, names));
+        result.fields = result.fields.concat(makeFvarInstance(
+            j,
+            fvar.instances[j],
+            fvar.axes,
+            optionalFields
+        ));
     }
 
     return result;
@@ -129,7 +144,7 @@ function parseFvarTable(data, start, names) {
     const instances = [];
     const instanceStart = start + offsetToData + axisCount * axisSize;
     for (let j = 0; j < instanceCount; j++) {
-        instances.push(parseFvarInstance(data, instanceStart + j * instanceSize, axes, names));
+        instances.push(parseFvarInstance(data, instanceStart + j * instanceSize, axes, names, instanceSize));
     }
 
     return {axes: axes, instances: instances};

src/tables/name.js

@@ -6,7 +6,7 @@ import parse from '../parse.js';
 import table from '../table.js';
 
 // NameIDs for the name table.
-const nameTableNames = [
+export const nameTableNames = [
     'copyright',              // 0
     'fontFamily',             // 1
     'fontSubfamily',          // 2
@@ -854,4 +854,23 @@ function makeNameTable(names, ltag) {
     return t;
 }
 
-export default { parse: parseNameTable, make: makeNameTable };
+export function getNameByID(names, nameID, allowedStandardIDs = []) {
+    if (nameID < 256 && nameID in nameTableNames) {
+        if (allowedStandardIDs.length && !allowedStandardIDs.includes(parseInt(nameID))) {
+            return undefined;
+        }
+        nameID = nameTableNames[nameID];
+    }
+
+    for (let platform in names) {
+        for (let nameKey in names[platform]) {
+            if(nameKey === nameID || parseInt(nameKey) === nameID) {
+                return names[platform][nameKey];
+            }
+        }
+    }
+
+    return undefined;
+}
+
+export default { parse: parseNameTable, make: makeNameTable, getNameByID };

src/tables/sfnt.js

@@ -24,6 +24,7 @@ import cpal from './cpal.js';
 import fvar from './fvar.js';
 import stat from './stat.js';
 import avar from './avar.js';
+import cvar from './cvar.js';
 import gvar from './gvar.js';
 import gasp from './gasp.js';
 
@@ -328,10 +329,6 @@ function fontToSfntTable(font) {
         names.windows.preferredSubfamily = fontNamesWindows.fontSubfamily || fontNamesUnicode.fontSubfamily || fontNamesMacintosh.fontSubfamily;
     }
 
-    // we have to handle fvar before name, because it may modify name IDs
-    const fvarTable = font.tables.fvar ? fvar.make(font.tables.fvar, font.names) : undefined;
-    const gaspTable = font.tables.gasp ? gasp.make(font.tables.gasp) : undefined;
-
     const languageTags = [];
     const nameTable = _name.make(names, languageTags);
     const ltagTable = (languageTags.length > 0 ? ltag.make(languageTags) : undefined);
@@ -363,33 +360,31 @@ function fontToSfntTable(font) {
         colr,
         stat,
         avar,
+        cvar,
+        fvar, 
         gvar,
+        gasp,
     };
 
     const optionalTableArgs = {
-        avar: [font.tables.fvar]
+        avar: [font.tables.fvar],
+        fvar: [font.names],
     };
 
-    // fvar table is already handled above
-    if (fvarTable) {
-        tables.push(fvarTable);
-    }
-
     for (let tableName in optionalTables) {
         const table = font.tables[tableName];
         if (table) {
-            tables.push(optionalTables[tableName].make.call(font, table, ...(optionalTableArgs[tableName] || [])));
+            const tableData = optionalTables[tableName].make.call(font, table, ...(optionalTableArgs[tableName] || []));
+            if (tableData) {
+                tables.push(tableData);
+            }
         }
     }
 
     if (metaTable) {
         tables.push(metaTable);
     }
 
-    if (gaspTable) {
-        tables.push(gaspTable);
-    }
-
     const sfntTable = makeSfntTable(tables);
 
     // Compute the font's checkSum and store it in head.checkSumAdjustment.

test/fonts/VARTest.ttf

@@ -1,8 +1,11 @@
 import assert from 'assert';
 import { hex, unhex } from '../testutil.js';
 import fvar from '../../src/tables/fvar.js';
+import { Font, loadSync, parse } from '../../src/opentype.js';
 
 describe('tables/fvar.js', function() {
+    const testFont = loadSync('./test/fonts/VARTest.ttf');
+
     const data =
         '00 01 00 00 00 10 00 02 00 02 00 14 00 02 00 0C ' +
         '77 67 68 74 00 64 00 00 01 90 00 00 03 84 00 00 00 00 01 01 ' +
@@ -17,40 +20,75 @@ describe('tables/fvar.js', function() {
                 minValue: 100,
                 defaultValue: 400,
                 maxValue: 900,
+                axisNameID: 257,
                 name: {en: 'Weight', ja: 'ウエイト'}
             },
             {
                 tag: 'wdth',
                 minValue: 50,
                 defaultValue: 100,
                 maxValue: 200,
+                axisNameID: 258,
                 name: {en: 'Width', ja: '幅'}
             }
         ],
         instances: [
             {
                 name: {en: 'Regular', ja: 'レギュラー'},
+                subfamilyNameID: 259,
+                postScriptName: undefined,
+                postScriptNameID: undefined,
                 coordinates: {wght: 300, wdth: 100}
             },
             {
                 name: {en: 'Condensed', ja: 'コンデンス'},
+                subfamilyNameID: 260,
+                postScriptName: undefined,
+                postScriptNameID: undefined,
                 coordinates: {wght: 300, wdth: 75}
             }
         ]
     };
 
+    const names = {
+        macintosh: {
+            257: {en: 'Weight', ja: 'ウエイト'},
+            258: {en: 'Width', ja: '幅'},
+            259: {en: 'Regular', ja: 'レギュラー'},
+            260: {en: 'Condensed', ja: 'コンデンス'}
+        }
+    };
+
     it('can parse a font variations table', function() {
-        const names = {
-            macintosh: {
-                257: {en: 'Weight', ja: 'ウエイト'},
-                258: {en: 'Width', ja: '幅'},
-                259: {en: 'Regular', ja: 'レギュラー'},
-                260: {en: 'Condensed', ja: 'コンデンス'}
-            }
-        };
         assert.deepEqual(table, fvar.parse(unhex(data), 0, names));
     });
 
+    it('parses nameIDs 2 and 17 and postScriptNameID 6 correctly', function() {
+        assert.equal(testFont.tables.fvar.instances[0].name.en, 'Regular');
+        assert.equal(testFont.tables.fvar.instances[0].subfamilyNameID, 2);
+        assert.equal(testFont.tables.fvar.instances[0].postScriptName.en, 'VARTestVF-Regular');
+        assert.equal(testFont.tables.fvar.instances[0].postScriptNameID, 6);
+        assert.equal(testFont.tables.fvar.instances[0].postScriptName.en, 'VARTestVF-Regular');
+
+        const font = new Font({
+            familyName: 'TestFont',
+            styleName: 'Medium',
+            unitsPerEm: 1000,
+            ascender: 800,
+            descender: -200,
+            glyphs: []
+        });
+        font.tables.fvar = JSON.parse(JSON.stringify(table));
+        font.names.unicode.fontSubfamily = {en: 'Font Subfamily name'};
+        font.names.unicode.preferredSubfamily = {en: 'Typographic Subfamily name'};
+        font.tables.fvar.instances[0].subfamilyNameID = 2;
+        font.tables.fvar.instances[1].subfamilyNameID = 17;
+
+        let parsedFont = parse(font.toArrayBuffer());
+        assert.deepEqual(parsedFont.tables.fvar.instances[0].name, font.names.unicode.fontSubfamily);
+        assert.deepEqual(parsedFont.tables.fvar.instances[1].name, font.names.unicode.preferredSubfamily);
+    });
+
     it('can make a font variations table', function() {
         const names = {
             macintosh: {
@@ -67,15 +105,25 @@ describe('tables/fvar.js', function() {
             }
         };
         assert.deepEqual(data, hex(fvar.make(table, names).encode()));
-        assert.deepEqual(names, {
-            macintosh: {
-                111: {en: 'Name #111'},
-                256: {en: 'Ligatures', ja: 'リガチャ'},
-                257: {en: 'Weight', ja: 'ウエイト'},
-                258: {en: 'Width', ja: '幅'},
-                259: {en: 'Regular', ja: 'レギュラー'},
-                260: {en: 'Condensed', ja: 'コンデンス'}
-            }
-        });
+    });
+
+    it('writes postScriptNameID optionally', function() {
+        let parsedFont = parse(testFont.toArrayBuffer());
+        let makeTable = fvar.make(parsedFont.tables.fvar, parsedFont.names);
+        assert.equal(parsedFont.tables.fvar.instances[0].postScriptNameID, 6);
+        assert.equal(parsedFont.tables.fvar.instances[0].postScriptName.en, 'VARTestVF-Regular');
+
+        assert.equal(makeTable.instanceSize, 10);
+
+        parsedFont.tables.fvar.instances =
+            parsedFont.tables.fvar.instances.map(i => { i.postScriptNameID = undefined; return i; });
+
+        parsedFont = parse(parsedFont.toArrayBuffer());
+        makeTable = fvar.make(parsedFont.tables.fvar, parsedFont.names);
+        
+        assert.equal(makeTable.instanceSize, 8);
+
+        assert.equal(parsedFont.tables.fvar.instances[0].postScriptNameID, undefined);
+        assert.equal(parsedFont.tables.fvar.instances[1].postScriptNameID, undefined);
     });
 });