[otBase/packer] Allow sharing tables reached by different offset sizes

This makes us match hb-subset now.

Fixes #3236

Lib/fontTools/ttLib/tables/otBase.py

@@ -376,6 +376,20 @@ def __contains__(self, name):
         return self.localState and name in self.localState
 
 
+class OffsetToWriter(object):
+    def __init__(self, subWriter, offsetSize):
+        self.subWriter = subWriter
+        self.offsetSize = offsetSize
+
+    def __eq__(self, other):
+        if type(self) != type(other):
+            return NotImplemented
+        return self.subWriter == other.subWriter and self.offsetSize == other.offsetSize
+
+    def __hash__(self):
+        # only works after self._doneWriting() has been called
+        return hash((self.subWriter, self.offsetSize))
+
 class OTTableWriter(object):
 
     """Helper class to gather and assemble data for OpenType tables."""
@@ -388,16 +402,6 @@ def __init__(self, localState=None, tableTag=None, offsetSize=2):
         self.offsetSize = offsetSize
         self.parent = None
 
-    # DEPRECATED: 'longOffset' is kept as a property for backward compat with old code.
-    # You should use 'offsetSize' instead (2, 3 or 4 bytes).
-    @property
-    def longOffset(self):
-        return self.offsetSize == 4
-
-    @longOffset.setter
-    def longOffset(self, value):
-        self.offsetSize = 4 if value else 2
-
     def __setitem__(self, name, value):
         state = self.localState.copy() if self.localState else dict()
         state[name] = value
@@ -417,7 +421,7 @@ def getDataLength(self):
         for item in self.items:
             if hasattr(item, "getCountData"):
                 l += item.size
-            elif hasattr(item, "getData"):
+            elif hasattr(item, "subWriter"):
                 l += item.offsetSize
             else:
                 l = l + len(item)
@@ -431,19 +435,19 @@ def getData(self):
         for i in range(numItems):
             item = items[i]
 
-            if hasattr(item, "getData"):
+            if hasattr(item, "subWriter"):
                 if item.offsetSize == 4:
-                    items[i] = packULong(item.pos - pos)
+                    items[i] = packULong(item.subWriter.pos - pos)
                 elif item.offsetSize == 2:
                     try:
-                        items[i] = packUShort(item.pos - pos)
+                        items[i] = packUShort(item.subWriter.pos - pos)
                     except struct.error:
                         # provide data to fix overflow problem.
                         overflowErrorRecord = self.getOverflowErrorRecord(item)
 
                         raise OTLOffsetOverflowError(overflowErrorRecord)
                 elif item.offsetSize == 3:
-                    items[i] = packUInt24(item.pos - pos)
+                    items[i] = packUInt24(item.subWriter.pos - pos)
                 else:
                     raise ValueError(item.offsetSize)
 
@@ -454,7 +458,7 @@ def getDataForHarfbuzz(self):
         items = list(self.items)
         packFuncs = {2: packUShort, 3: packUInt24, 4: packULong}
         for i, item in enumerate(items):
-            if hasattr(item, "getData"):
+            if hasattr(item, "subWriter"):
                 # Offset value is not needed in harfbuzz repacker, so setting offset to 0 to avoid overflow here
                 if item.offsetSize in packFuncs:
                     items[i] = packFuncs[item.offsetSize](0)
@@ -474,7 +478,7 @@ def __ne__(self, other):
     def __eq__(self, other):
         if type(self) != type(other):
             return NotImplemented
-        return self.offsetSize == other.offsetSize and self.items == other.items
+        return self.items == other.items
 
     def _doneWriting(self, internedTables, shareExtension=False):
         # Convert CountData references to data string items
@@ -500,16 +504,18 @@ def _doneWriting(self, internedTables, shareExtension=False):
             item = items[i]
             if hasattr(item, "getCountData"):
                 items[i] = item.getCountData()
-            elif hasattr(item, "getData"):
-                item._doneWriting(internedTables, shareExtension=shareExtension)
+            elif hasattr(item, "subWriter"):
+                item.subWriter._doneWriting(
+                    internedTables, shareExtension=shareExtension
+                )
                 # At this point, all subwriters are hashable based on their items.
                 # (See hash and comparison magic methods above.) So the ``setdefault``
                 # call here will return the first writer object we've seen with
                 # equal content, or store it in the dictionary if it's not been
                 # seen yet. We therefore replace the subwriter object with an equivalent
                 # object, which deduplicates the tree.
                 if not dontShare:
-                    items[i] = item = internedTables.setdefault(item, item)
+                    items[i].subWriter = internedTables.setdefault(item.subWriter, item.subWriter)
         self.items = tuple(items)
 
     def _gatherTables(self, tables, extTables, done):
@@ -543,30 +549,30 @@ def _gatherTables(self, tables, extTables, done):
             # Find coverage table
             for i in range(numItems):
                 item = self.items[i]
-                if getattr(item, "name", None) == "Coverage":
+                if hasattr(item, "subWriter") and getattr(item.subWriter, "name", None) == "Coverage":
                     sortCoverageLast = True
                     break
-            if id(item) not in done:
-                item._gatherTables(tables, extTables, done)
+            if id(item.subWriter) not in done:
+                item.subWriter._gatherTables(tables, extTables, done)
             else:
                 # We're a new parent of item
                 pass
 
         for i in iRange:
             item = self.items[i]
-            if not hasattr(item, "getData"):
+            if not hasattr(item, "subWriter"):
                 continue
 
             if (
                 sortCoverageLast
                 and (i == 1)
-                and getattr(item, "name", None) == "Coverage"
+                and getattr(item.subWriter, "name", None) == "Coverage"
             ):
                 # we've already 'gathered' it above
                 continue
 
-            if id(item) not in done:
-                item._gatherTables(tables, extTables, done)
+            if id(item.subWriter) not in done:
+                item.subWriter._gatherTables(tables, extTables, done)
             else:
                 # Item is already written out by other parent
                 pass
@@ -601,7 +607,7 @@ def _gatherGraphForHarfbuzz(self, tables, obj_list, done, objidx, virtual_edges)
         child_idx = 0
         offset_pos = 0
         for i, item in enumerate(self.items):
-            if hasattr(item, "getData"):
+            if hasattr(item, "subWriter"):
                 pos = offset_pos
             elif hasattr(item, "getCountData"):
                 offset_pos += item.size
@@ -610,12 +616,12 @@ def _gatherGraphForHarfbuzz(self, tables, obj_list, done, objidx, virtual_edges)
                 offset_pos = offset_pos + len(item)
                 continue
 
-            if id(item) not in done:
-                child_idx = item_idx = item._gatherGraphForHarfbuzz(
+            if id(item.subWriter) not in done:
+                child_idx = item_idx = item.subWriter._gatherGraphForHarfbuzz(
                     tables, obj_list, done, item_idx, virtual_edges
                 )
             else:
-                child_idx = done[id(item)]
+                child_idx = done[id(item.subWriter)]
 
             real_edge = (pos, item.offsetSize, child_idx)
             real_links.append(real_edge)
@@ -774,7 +780,8 @@ def writeTag(self, tag):
         self.items.append(tag)
 
     def writeSubTable(self, subWriter):
-        self.items.append(subWriter)
+        self.items.append(OffsetToWriter(subWriter, subWriter.offsetSize))
+        del subWriter.offsetSize
 
     def writeCountReference(self, table, name, size=2, value=None):
         ref = CountReference(table, name, size=size, value=value)
@@ -1376,7 +1383,6 @@ def writeValueRecord(self, writer, font, valueRecord):
 
 
 class ValueRecord(object):
-
     # see ValueRecordFactory
 
     def __init__(self, valueFormat=None, src=None):