Merge pull request #2398 from fonttools/vid

Clean up virtual GID handling

Lib/fontTools/misc/testTools.py

@@ -57,11 +57,16 @@ class FakeFont:
     def getGlyphID(self, name):
         return self.reverseGlyphOrderDict_[name]
 
+    def getGlyphIDMany(self, lst):
+        return [self.getGlyphID(gid) for gid in lst]
+
     def getGlyphName(self, glyphID):
         if glyphID < len(self.glyphOrder_):
             return self.glyphOrder_[glyphID]
         else:
             return "glyph%.5d" % glyphID
+    def getGlyphNameMany(self, lst):
+        return [self.getGlyphName(gid) for gid in lst]
 
     def getGlyphOrder(self):
         return self.glyphOrder_
@@ -136,7 +141,7 @@ class MockFont(object):
         self._reverseGlyphOrder = AllocatingDict({'.notdef': 0})
         self.lazy = False
 
-    def getGlyphID(self, glyph, requireReal=None):
+    def getGlyphID(self, glyph):
         gid = self._reverseGlyphOrder[glyph]
         return gid
 

Lib/fontTools/subset/__init__.py

@@ -2864,7 +2864,6 @@ class Subsetter(object):
 				glyphOrder = [g for g in glyphOrder if font.getGlyphID(g) <= self.last_retained_order]
 
 			font.setGlyphOrder(glyphOrder)
-			font._buildReverseGlyphOrderDict()
 
 
 	def _prune_post_subset(self, font):
@@ -2913,13 +2912,11 @@ class Subsetter(object):
 @timer("load font")
 def load_font(fontFile,
 	      options,
-	      allowVID=False,
 	      checkChecksums=0,
 	      dontLoadGlyphNames=False,
 	      lazy=True):
 
 	font = ttLib.TTFont(fontFile,
-			    allowVID=allowVID,
 			    checkChecksums=checkChecksums,
 			    recalcBBoxes=options.recalc_bounds,
 			    recalcTimestamp=options.recalc_timestamp,

Lib/fontTools/ttLib/tables/_c_m_a_p.py

@@ -14,15 +14,11 @@ log = logging.getLogger(__name__)
 
 def _make_map(font, chars, gids):
 	assert len(chars) == len(gids)
+	glyphNames = font.getGlyphNameMany(gids)
 	cmap = {}
-	glyphOrder = font.getGlyphOrder()
-	for char,gid in zip(chars,gids):
+	for char,gid,name in zip(chars,gids,glyphNames):
 		if gid == 0:
 			continue
-		try:
-			name = glyphOrder[gid]
-		except IndexError:
-			name = font.getGlyphName(gid)
 		cmap[char] = name
 	return cmap
 

Lib/fontTools/ttLib/tables/otConverters.py

@@ -347,24 +347,11 @@ class GlyphID(SimpleValue):
 	staticSize = 2
 	typecode = "H"
 	def readArray(self, reader, font, tableDict, count):
-		glyphOrder = font.getGlyphOrder()
-		gids = reader.readArray(self.typecode, self.staticSize, count)
-		try:
-			l = [glyphOrder[gid] for gid in gids]
-		except IndexError:
-			# Slower, but will not throw an IndexError on an invalid glyph id.
-			l = [font.getGlyphName(gid) for gid in gids]
-		return l
+		return font.getGlyphNameMany(reader.readArray(self.typecode, self.staticSize, count))
 	def read(self, reader, font, tableDict):
 		return font.getGlyphName(reader.readValue(self.typecode, self.staticSize))
 	def writeArray(self, writer, font, tableDict, values):
-		glyphMap = font.getReverseGlyphMap()
-		try:
-			values = [glyphMap[glyphname] for glyphname in values]
-		except KeyError:
-			# Slower, but will not throw a KeyError on an out-of-range glyph name.
-			values = [font.getGlyphID(glyphname) for glyphname in values]
-		writer.writeArray(self.typecode, values)
+		writer.writeArray(self.typecode, font.getGlyphIDMany(values))
 	def write(self, writer, font, tableDict, value, repeatIndex=None):
 		writer.writeValue(self.typecode, font.getGlyphID(value))
 
@@ -1222,8 +1209,7 @@ class STXHeader(BaseConverter):
 	def _readLigatures(self, reader, font):
 		limit = len(reader.data)
 		numLigatureGlyphs = (limit - reader.pos) // 2
-		return [font.getGlyphName(g)
-		        for g in reader.readUShortArray(numLigatureGlyphs)]
+		return font.getGlyphNameMany(reader.readUShortArray(numLigatureGlyphs))
 
 	def _countPerGlyphLookups(self, table):
 		# Somewhat annoyingly, the morx table does not encode

Lib/fontTools/ttLib/tables/otTables.py

@@ -425,8 +425,7 @@ class InsertionMorphAction(AATAction):
 			return []
 		reader = actionReader.getSubReader(
 			actionReader.pos + index * 2)
-		return [font.getGlyphName(glyphID)
-		        for glyphID in reader.readUShortArray(count)]
+		return font.getGlyphNameMany(reader.readUShortArray(count))
 
 	def toXML(self, xmlWriter, font, attrs, name):
 		xmlWriter.begintag(name, **attrs)
@@ -521,12 +520,10 @@ class Coverage(FormatSwitchingBaseTable):
 
 	def postRead(self, rawTable, font):
 		if self.Format == 1:
-			# TODO only allow glyphs that are valid?
 			self.glyphs = rawTable["GlyphArray"]
 		elif self.Format == 2:
 			glyphs = self.glyphs = []
 			ranges = rawTable["RangeRecord"]
-			glyphOrder = font.getGlyphOrder()
 			# Some SIL fonts have coverage entries that don't have sorted
 			# StartCoverageIndex.  If it is so, fixup and warn.  We undo
 			# this when writing font out.
@@ -536,25 +533,11 @@ class Coverage(FormatSwitchingBaseTable):
 				ranges = sorted_ranges
 			del sorted_ranges
 			for r in ranges:
-				assert r.StartCoverageIndex == len(glyphs), \
-					(r.StartCoverageIndex, len(glyphs))
 				start = r.Start
 				end = r.End
-				try:
-					startID = font.getGlyphID(start, requireReal=True)
-				except KeyError:
-					log.warning("Coverage table has start glyph ID out of range: %s.", start)
-					continue
-				try:
-					endID = font.getGlyphID(end, requireReal=True) + 1
-				except KeyError:
-					# Apparently some tools use 65535 to "match all" the range
-					if end != 'glyph65535':
-						log.warning("Coverage table has end glyph ID out of range: %s.", end)
-					# NOTE: We clobber out-of-range things here.  There are legit uses for those,
-					# but none that we have seen in the wild.
-					endID = len(glyphOrder)
-				glyphs.extend(glyphOrder[glyphID] for glyphID in range(startID, endID))
+				startID = font.getGlyphID(start)
+				endID = font.getGlyphID(end) + 1
+				glyphs.extend(font.getGlyphNameMany(range(startID, endID)))
 		else:
 			self.glyphs = []
 			log.warning("Unknown Coverage format: %s", self.Format)
@@ -566,10 +549,9 @@ class Coverage(FormatSwitchingBaseTable):
 			glyphs = self.glyphs = []
 		format = 1
 		rawTable = {"GlyphArray": glyphs}
-		getGlyphID = font.getGlyphID
 		if glyphs:
 			# find out whether Format 2 is more compact or not
-			glyphIDs = [getGlyphID(glyphName) for glyphName in glyphs ]
+			glyphIDs = font.getGlyphIDMany(glyphs)
 			brokenOrder = sorted(glyphIDs) != glyphIDs
 
 			last = glyphIDs[0]
@@ -768,9 +750,9 @@ class SingleSubst(FormatSwitchingBaseTable):
 		input = _getGlyphsFromCoverageTable(rawTable["Coverage"])
 		if self.Format == 1:
 			delta = rawTable["DeltaGlyphID"]
-			inputGIDS =  [ font.getGlyphID(name) for name in input ]
+			inputGIDS = font.getGlyphIDMany(input)
 			outGIDS = [ (glyphID + delta) % 65536 for glyphID in inputGIDS ]
-			outNames = [ font.getGlyphName(glyphID) for glyphID in outGIDS ]
+			outNames = font.getGlyphNameMany(outGIDS)
 			for inp, out in zip(input, outNames):
 				mapping[inp] = out
 		elif self.Format == 2:
@@ -924,51 +906,30 @@ class ClassDef(FormatSwitchingBaseTable):
 
 	def postRead(self, rawTable, font):
 		classDefs = {}
-		glyphOrder = font.getGlyphOrder()
 
 		if self.Format == 1:
 			start = rawTable["StartGlyph"]
 			classList = rawTable["ClassValueArray"]
-			try:
-				startID = font.getGlyphID(start, requireReal=True)
-			except KeyError:
-				log.warning("ClassDef table has start glyph ID out of range: %s.", start)
-				startID = len(glyphOrder)
+			startID = font.getGlyphID(start)
 			endID = startID + len(classList)
-			if endID > len(glyphOrder):
-				log.warning("ClassDef table has entries for out of range glyph IDs: %s,%s.",
-					start, len(classList))
-				# NOTE: We clobber out-of-range things here.  There are legit uses for those,
-				# but none that we have seen in the wild.
-				endID = len(glyphOrder)
-
-			for glyphID, cls in zip(range(startID, endID), classList):
+			glyphNames = font.getGlyphNameMany(range(startID, endID))
+			for glyphName, cls in zip(glyphNames, classList):
 				if cls:
-					classDefs[glyphOrder[glyphID]] = cls
+					classDefs[glyphName] = cls
 
 		elif self.Format == 2:
 			records = rawTable["ClassRangeRecord"]
 			for rec in records:
+				cls = rec.Class
+				if not cls:
+					continue
 				start = rec.Start
 				end = rec.End
-				cls = rec.Class
-				try:
-					startID = font.getGlyphID(start, requireReal=True)
-				except KeyError:
-					log.warning("ClassDef table has start glyph ID out of range: %s.", start)
-					continue
-				try:
-					endID = font.getGlyphID(end, requireReal=True) + 1
-				except KeyError:
-					# Apparently some tools use 65535 to "match all" the range
-					if end != 'glyph65535':
-						log.warning("ClassDef table has end glyph ID out of range: %s.", end)
-					# NOTE: We clobber out-of-range things here.  There are legit uses for those,
-					# but none that we have seen in the wild.
-					endID = len(glyphOrder)
-				for glyphID in range(startID, endID):
-					if cls:
-						classDefs[glyphOrder[glyphID]] = cls
+				startID = font.getGlyphID(start)
+				endID = font.getGlyphID(end) + 1
+				glyphNames = font.getGlyphNameMany(range(startID, endID))
+				for glyphName in glyphNames:
+					classDefs[glyphName] = cls
 		else:
 			log.warning("Unknown ClassDef format: %s", self.Format)
 		self.classDefs = classDefs

Lib/fontTools/ttLib/ttFont.py

@@ -20,7 +20,7 @@ class TTFont(object):
 
 	def __init__(self, file=None, res_name_or_index=None,
 			sfntVersion="\000\001\000\000", flavor=None, checkChecksums=0,
-			verbose=None, recalcBBoxes=True, allowVID=False, ignoreDecompileErrors=False,
+			verbose=None, recalcBBoxes=True, allowVID=NotImplemented, ignoreDecompileErrors=False,
 			recalcTimestamp=True, fontNumber=-1, lazy=None, quiet=None,
 			_tableCache=None):
 
@@ -61,16 +61,6 @@ class TTFont(object):
 		If the recalcTimestamp argument is false, the modified timestamp in the
 		'head' table will *not* be recalculated upon save/compile.
 
-		If the allowVID argument is set to true, then virtual GID's are
-		supported. Asking for a glyph ID with a glyph name or GID that is not in
-		the font will return a virtual GID.   This is valid for GSUB and cmap
-		tables. For SING glyphlets, the cmap table is used to specify Unicode
-		values for virtual GI's used in GSUB/GPOS rules. If the gid N is requested
-		and does not exist in the font, or the glyphname has the form glyphN
-		and does not exist in the font, then N is used as the virtual GID.
-		Else, the first virtual GID is assigned as 0x1000 -1; for subsequent new
-		virtual GIDs, the next is one less than the previous.
-
 		If ignoreDecompileErrors is set to True, exceptions raised in
 		individual tables during decompilation will be ignored, falling
 		back to the DefaultTable implementation, which simply keeps the
@@ -92,12 +82,6 @@ class TTFont(object):
 		self.recalcTimestamp = recalcTimestamp
 		self.tables = {}
 		self.reader = None
-
-		# Permit the user to reference glyphs that are not int the font.
-		self.last_vid = 0xFFFE # Can't make it be 0xFFFF, as the world is full unsigned short integer counters that get incremented after the last seen GID value.
-		self.reverseVIDDict = {}
-		self.VIDDict = {}
-		self.allowVID = allowVID
 		self.ignoreDecompileErrors = ignoreDecompileErrors
 
 		if not file:
@@ -429,6 +413,8 @@ class TTFont(object):
 
 	def setGlyphOrder(self, glyphOrder):
 		self.glyphOrder = glyphOrder
+		if hasattr(self, '_reverseGlyphOrderDict'):
+			del self._reverseGlyphOrderDict
 
 	def getGlyphOrder(self):
 		try:
@@ -544,67 +530,35 @@ class TTFont(object):
 		from fontTools.misc import textTools
 		return textTools.caselessSort(self.getGlyphOrder())
 
-	def getGlyphName(self, glyphID, requireReal=False):
+	def getGlyphName(self, glyphID):
 		try:
 			return self.getGlyphOrder()[glyphID]
 		except IndexError:
-			if requireReal or not self.allowVID:
-				# XXX The ??.W8.otf font that ships with OSX uses higher glyphIDs in
-				# the cmap table than there are glyphs. I don't think it's legal...
-				return "glyph%.5d" % glyphID
-			else:
-				# user intends virtual GID support
+			return "glyph%.5d" % glyphID
+
+	def getGlyphNameMany(self, lst):
+		glyphOrder = self.getGlyphOrder();
+		cnt = len(glyphOrder)
+		return [glyphOrder[gid] if gid < cnt else "glyph%.5d" % gid
+			for gid in lst]
+
+	def getGlyphID(self, glyphName):
+		try:
+			return self.getReverseGlyphMap()[glyphName]
+		except KeyError:
+			if glyphName[:5] == "glyph":
 				try:
-					glyphName = self.VIDDict[glyphID]
-				except KeyError:
-					glyphName  ="glyph%.5d" % glyphID
-					self.last_vid = min(glyphID, self.last_vid )
-					self.reverseVIDDict[glyphName] = glyphID
-					self.VIDDict[glyphID] = glyphName
-				return glyphName
-
-	def getGlyphID(self, glyphName, requireReal=False):
-		if not hasattr(self, "_reverseGlyphOrderDict"):
-			self._buildReverseGlyphOrderDict()
-		glyphOrder = self.getGlyphOrder()
-		d = self._reverseGlyphOrderDict
-		if glyphName not in d:
-			if glyphName in glyphOrder:
-				self._buildReverseGlyphOrderDict()
-				return self.getGlyphID(glyphName)
-			else:
-				if requireReal:
+					return int(glyphName[5:])
+				except (NameError, ValueError):
 					raise KeyError(glyphName)
-				elif not self.allowVID:
-					# Handle glyphXXX only
-					if glyphName[:5] == "glyph":
-						try:
-							return int(glyphName[5:])
-						except (NameError, ValueError):
-							raise KeyError(glyphName)
-				else:
-					# user intends virtual GID support
-					try:
-						glyphID = self.reverseVIDDict[glyphName]
-					except KeyError:
-						# if name is in glyphXXX format, use the specified name.
-						if glyphName[:5] == "glyph":
-							try:
-								glyphID = int(glyphName[5:])
-							except (NameError, ValueError):
-								glyphID = None
-						if glyphID is None:
-							glyphID = self.last_vid -1
-							self.last_vid = glyphID
-						self.reverseVIDDict[glyphName] = glyphID
-						self.VIDDict[glyphID] = glyphName
-					return glyphID
 
-		glyphID = d[glyphName]
-		if glyphName != glyphOrder[glyphID]:
-			self._buildReverseGlyphOrderDict()
-			return self.getGlyphID(glyphName)
-		return glyphID
+	def getGlyphIDMany(self, lst):
+		d = self.getReverseGlyphMap()
+		try:
+			return [d[glyphName] for glyphName in lst]
+		except KeyError:
+			getGlyphID = self.getGlyphID
+			return [getGlyphID(glyphName) for glyphName in lst]
 
 	def getReverseGlyphMap(self, rebuild=False):
 		if rebuild or not hasattr(self, "_reverseGlyphOrderDict"):
@@ -613,9 +567,9 @@ class TTFont(object):
 
 	def _buildReverseGlyphOrderDict(self):
 		self._reverseGlyphOrderDict = d = {}
-		glyphOrder = self.getGlyphOrder()
-		for glyphID in range(len(glyphOrder)):
-			d[glyphOrder[glyphID]] = glyphID
+		for glyphID,glyphName in enumerate(self.getGlyphOrder()):
+			d[glyphName] = glyphID
+		return d
 
 	def _writeTable(self, tag, writer, done, tableCache=None):
 		"""Internal helper function for self.save(). Keeps track of
@@ -820,9 +774,9 @@ class GlyphOrder(object):
 	def fromXML(self, name, attrs, content, ttFont):
 		if not hasattr(self, "glyphOrder"):
 			self.glyphOrder = []
-			ttFont.setGlyphOrder(self.glyphOrder)
 		if name == "GlyphID":
 			self.glyphOrder.append(attrs["name"])
+		ttFont.setGlyphOrder(self.glyphOrder)
 
 
 def getTableModule(tag):

Lib/fontTools/ttx.py

@@ -118,7 +118,6 @@ class Options(object):
 	disassembleInstructions = True
 	mergeFile = None
 	recalcBBoxes = True
-	allowVID = False
 	ignoreDecompileErrors = True
 	bitmapGlyphDataFormat = 'raw'
 	unicodedata = None
@@ -184,8 +183,6 @@ class Options(object):
 				self.mergeFile = value
 			elif option == "-b":
 				self.recalcBBoxes = False
-			elif option == "-a":
-				self.allowVID = True
 			elif option == "-e":
 				self.ignoreDecompileErrors = False
 			elif option == "--unicodedata":
@@ -258,7 +255,7 @@ def ttDump(input, output, options):
 	log.info('Dumping "%s" to "%s"...', input, output)
 	if options.unicodedata:
 		setUnicodeData(options.unicodedata)
-	ttf = TTFont(input, 0, allowVID=options.allowVID,
+	ttf = TTFont(input, 0,
 			ignoreDecompileErrors=options.ignoreDecompileErrors,
 			fontNumber=options.fontNumber)
 	ttf.saveXML(output,
@@ -280,8 +277,7 @@ def ttCompile(input, output, options):
 		sfnt.USE_ZOPFLI = True
 	ttf = TTFont(options.mergeFile, flavor=options.flavor,
 			recalcBBoxes=options.recalcBBoxes,
-			recalcTimestamp=options.recalcTimestamp,
-			allowVID=options.allowVID)
+			recalcTimestamp=options.recalcTimestamp)
 	ttf.importXML(input)
 
 	if options.recalcTimestamp is None and 'head' in ttf:

Tests/ttLib/ttFont_test.py

@@ -1,7 +1,11 @@
 import io
+import os
+import re
 from fontTools.ttLib import TTFont, newTable, registerCustomTableClass, unregisterCustomTableClass
 from fontTools.ttLib.tables.DefaultTable import DefaultTable
 
+DATA_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), "data")
+
 
 class CustomTableClass(DefaultTable):
 
@@ -20,6 +24,13 @@ table_C_U_S_T_ = CustomTableClass  # alias for testing
 TABLETAG = "CUST"
 
 
+def normalize_TTX(string):
+    string = re.sub(' ttLibVersion=".*"', "", string)
+    string = re.sub('checkSumAdjustment value=".*"', "", string)
+    string = re.sub('modified value=".*"', "", string)
+    return string
+
+
 def test_registerCustomTableClass():
     font = TTFont()
     font[TABLETAG] = newTable(TABLETAG)
@@ -78,3 +89,32 @@ def test_sfntVersionFromTTX():
     # Font is not "empty", sfntVersion in TTX file will be ignored
     font.importXML(ttx)
     assert font.sfntVersion == "OTTO"
+
+
+def test_virtualGlyphId():
+    otfpath = os.path.join(DATA_DIR, "TestVGID-Regular.otf")
+    ttxpath = os.path.join(DATA_DIR, "TestVGID-Regular.ttx")
+
+    otf = TTFont(otfpath)
+
+    ttx = TTFont()
+    ttx.importXML(ttxpath)
+
+    with open(ttxpath, encoding="utf-8") as fp:
+        xml = normalize_TTX(fp.read()).splitlines()
+
+    for font in (otf, ttx):
+        GSUB = font["GSUB"].table
+        assert GSUB.LookupList.LookupCount == 37
+        lookup = GSUB.LookupList.Lookup[32]
+        assert lookup.LookupType == 8
+        subtable = lookup.SubTable[0]
+        assert subtable.LookAheadGlyphCount == 1
+        lookahead = subtable.LookAheadCoverage[0]
+        assert len(lookahead.glyphs) == 46
+        assert "glyph00453" in lookahead.glyphs
+
+        out = io.StringIO()
+        font.saveXML(out)
+        outxml = normalize_TTX(out.getvalue()).splitlines()
+        assert xml == outxml

Tests/ttx/ttx_test.py

@@ -436,12 +436,6 @@ def test_options_b():
     tto = ttx.Options([("-b", "")], 1)
     assert tto.recalcBBoxes is False
 
-
-def test_options_a():
-    tto = ttx.Options([("-a", "")], 1)
-    assert tto.allowVID is True
-
-
 def test_options_e():
     tto = ttx.Options([("-e", "")], 1)
     assert tto.ignoreDecompileErrors is False