Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
Aug 4, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b629e5
[otBase/packer] Allow sharing tables reached by different offset sizes
behdad Aug 3, 2023
286e646
[otBase/packer] Remove offsetSize from OTTableWriter
behdad Aug 3, 2023
d2fd4df
C_O_L_R_test: add test for paint pointed to by both 3- and 4-byte off…
anthrotype Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
90 changes: 49 additions & 41 deletions Lib/fontTools/ttLib/tables/otBase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,28 +376,32 @@
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

Check warning on line 386 in Lib/fontTools/ttLib/tables/otBase.py

View check run for this annotation

Codecov / codecov/patch

Lib/fontTools/ttLib/tables/otBase.py#L386 

Added line #L386 was not covered by tests
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."""

def __init__(self, localState=None, tableTag=None, offsetSize=2):
def __init__(self, localState=None, tableTag=None):
self.items = []
self.pos = None
self.localState = localState
self.tableTag = tableTag
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
Expand All @@ -417,7 +421,7 @@
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)
Expand All @@ -431,19 +435,19 @@
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)

Expand All @@ -454,7 +458,7 @@
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)
Expand All @@ -474,7 +478,7 @@
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
Expand All @@ -500,16 +504,20 @@
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):
Expand Down Expand Up @@ -543,30 +551,33 @@
# 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
Expand Down Expand Up @@ -601,7 +612,7 @@
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
Expand All @@ -610,12 +621,12 @@
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)
Expand Down Expand Up @@ -698,10 +709,8 @@

# interface for gathering data, as used by table.compile()

def getSubWriter(self, offsetSize=2):
subwriter = self.__class__(
self.localState, self.tableTag, offsetSize=offsetSize
)
def getSubWriter(self):
subwriter = self.__class__(self.localState, self.tableTag)
subwriter.parent = (
self # because some subtables have idential values, we discard
)
Expand Down Expand Up @@ -773,8 +782,8 @@
assert len(tag) == 4, tag
self.items.append(tag)

def writeSubTable(self, subWriter):
self.items.append(subWriter)
def writeSubTable(self, subWriter, offsetSize):
self.items.append(OffsetToWriter(subWriter, offsetSize))

def writeCountReference(self, table, name, size=2, value=None):
ref = CountReference(table, name, size=size, value=value)
Expand Down Expand Up @@ -1365,7 +1374,7 @@
if isDevice:
if value:
subWriter = writer.getSubWriter()
writer.writeSubTable(subWriter)
writer.writeSubTable(subWriter, offsetSize=2)
value.compile(subWriter, font)
else:
writer.writeUShort(0)
Expand All @@ -1376,7 +1385,6 @@


class ValueRecord(object):

# see ValueRecordFactory

def __init__(self, valueFormat=None, src=None):
Expand Down
19 changes: 8 additions & 11 deletions Lib/fontTools/ttLib/tables/otConverters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,6 @@ def write(self, writer, font, tableDict, value, repeatIndex=None):


class Table(Struct):

staticSize = 2

def readOffset(self, reader):
Expand All @@ -746,16 +745,15 @@ def write(self, writer, font, tableDict, value, repeatIndex=None):
if value is None:
self.writeNullOffset(writer)
else:
subWriter = writer.getSubWriter(offsetSize=self.staticSize)
subWriter = writer.getSubWriter()
subWriter.name = self.name
if repeatIndex is not None:
subWriter.repeatIndex = repeatIndex
writer.writeSubTable(subWriter)
writer.writeSubTable(subWriter, offsetSize=self.staticSize)
value.compile(subWriter, font)


class LTable(Table):

staticSize = 4

def readOffset(self, reader):
Expand All @@ -767,7 +765,6 @@ def writeNullOffset(self, writer):

# Table pointed to by a 24-bit, 3-byte long offset
class Table24(Table):

staticSize = 3

def readOffset(self, reader):
Expand Down Expand Up @@ -1147,13 +1144,13 @@ def write(self, writer, font, tableDict, value, repeatIndex=None):
offsetByGlyph[glyph] = offset
# For calculating the offsets to our AATLookup and data table,
# we can use the regular OTTableWriter infrastructure.
lookupWriter = writer.getSubWriter(offsetSize=4)
lookupWriter = writer.getSubWriter()
lookup = AATLookup("DataOffsets", None, None, UShort)
lookup.write(lookupWriter, font, tableDict, offsetByGlyph, None)

dataWriter = writer.getSubWriter(offsetSize=4)
writer.writeSubTable(lookupWriter)
writer.writeSubTable(dataWriter)
dataWriter = writer.getSubWriter()
writer.writeSubTable(lookupWriter, offsetSize=4)
writer.writeSubTable(dataWriter, offsetSize=4)
for d in compiledData:
dataWriter.writeData(d)

Expand Down Expand Up @@ -1483,9 +1480,9 @@ def _compilePerGlyphLookups(self, table, font):
)
writer = OTTableWriter()
for lookup in table.PerGlyphLookups:
lookupWriter = writer.getSubWriter(offsetSize=4)
lookupWriter = writer.getSubWriter()
self.perGlyphLookup.write(lookupWriter, font, {}, lookup, None)
writer.writeSubTable(lookupWriter)
writer.writeSubTable(lookupWriter, offsetSize=4)
return writer.getAllData()

def _compileLigComponents(self, table, font):
Expand Down
35 changes: 23 additions & 12 deletions Tests/ttLib/tables/C_O_L_R_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test_round_trip_xml(self, font):
(b"\x00\x03", "LayerRecordCount (3)"),
(b"\x00\x00\x00\x34", "Offset to BaseGlyphList from beginning of table (52)"),
(b"\x00\x00\x00\x9f", "Offset to LayerList from beginning of table (159)"),
(b"\x00\x00\x01\x62", "Offset to ClipList (354)"),
(b"\x00\x00\x01\x66", "Offset to ClipList (358)"),
(b"\x00\x00\x00\x00", "Offset to DeltaSetIndexMap (NULL)"),
(b"\x00\x00\x00\x00", "Offset to VarStore (NULL)"),
(b"\x00\x06", "BaseGlyphRecord[0].BaseGlyph (6)"),
Expand Down Expand Up @@ -187,22 +187,26 @@ def test_round_trip_xml(self, font):
(b"\x00\x05", "ColorLine.ColorStop[1].PaletteIndex (5)"),
(b"@\x00", "ColorLine.ColorStop[1].Alpha (1.0)"),
# LayerList
(b"\x00\x00\x00\x04", "LayerList.LayerCount (4)"),
(b"\x00\x00\x00\x05", "LayerList.LayerCount (5)"),
(
b"\x00\x00\x00\x14",
"First Offset to Paint table from beginning of LayerList (20)",
b"\x00\x00\x00\x18",
"First Offset to Paint table from beginning of LayerList (24)",
),
(
b"\x00\x00\x00\x23",
"Second Offset to Paint table from beginning of LayerList (35)",
b"\x00\x00\x00\x27",
"Second Offset to Paint table from beginning of LayerList (39)",
),
(
b"\x00\x00\x00\x4e",
"Third Offset to Paint table from beginning of LayerList (78)",
b"\x00\x00\x00\x52",
"Third Offset to Paint table from beginning of LayerList (82)",
),
(
b"\x00\x00\x00\x9e",
"Fourth Offset to Paint table from beginning of LayerList (158)",
b"\x00\x00\x00\xa2",
"Fourth Offset to Paint table from beginning of LayerList (162)",
),
(
b"\x00\x00\x00\xbc",
"Fifth Offset to Paint table from beginning of LayerList (188)",
),
# BaseGlyphPaintRecord[2]
(b"\x0a", "BaseGlyphPaintRecord[2].Paint.Format (10)"),
Expand Down Expand Up @@ -296,7 +300,7 @@ def test_round_trip_xml(self, font):
),
(b"\xfc\x17", "xSkewAngle (-0.0611)"),
(b"\x01\xc7", "ySkewAngle (0.0278)"),
# PaintGlyph
# PaintGlyph glyph00011 (pointed to by both PaintSkew above and by LayerList[4] offset)
(b"\x0a", "LayerList.Paint[3].Paint.Paint.Paint.Format (10)"),
(b"\x00\x00\x06", "Offset to Paint subtable from beginning of PaintGlyph (6)"),
(b"\x00\x0b", "LayerList.Paint[2].Glyph (11)"),
Expand Down Expand Up @@ -413,7 +417,7 @@ def test_round_trip_xml(self, font):
" </BaseGlyphPaintRecord>",
"</BaseGlyphList>",
"<LayerList>",
" <!-- LayerCount=4 -->",
" <!-- LayerCount=5 -->",
' <Paint index="0" Format="10"><!-- PaintGlyph -->',
' <Paint Format="3"><!-- PaintVarSolid -->',
' <PaletteIndex value="2"/>',
Expand Down Expand Up @@ -510,6 +514,13 @@ def test_round_trip_xml(self, font):
' <dx value="257"/>',
' <dy value="258"/>',
" </Paint>",
' <Paint index="4" Format="10"><!-- PaintGlyph -->',
' <Paint Format="2"><!-- PaintSolid -->',
' <PaletteIndex value="2"/>',
' <Alpha value="0.5"/>',
" </Paint>",
' <Glyph value="glyph00011"/>',
" </Paint>",
"</LayerList>",
'<ClipList Format="1">',
" <Clip>",
Expand Down