8000 When parsing MVAR with lazy=True recordSize is wrong · Issue #2273 · fonttools/fonttools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

When parsing MVAR with lazy=True recordSize is wrong #2273

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
Closed
garretrieger opened this issue Apr 21, 2021 · 2 comments · Fixed by #2274
Closed

When parsing MVAR with lazy=True recordSize is wrong #2273

garretrieger opened this issue Apr 21, 2021 · 2 comments · Fixed by #2274

Comments

@garretrieger
Copy link
Contributor

Reproduction:

from fontTools import ttLib

import io
import sys

file_path = sys.argv[1]
fontdata = open(file_path, "rb").read()

font = ttLib.TTFont(io.BytesIO(fontdata), lazy=True)
mvar = font["MVAR"].table
print(mvar.ValueRecord.recordSize)
for rec in mvar.ValueRecord:
  print(rec.ValueTag, "->", rec.VarIdx)

Running this against the latest version of recursive gives:

16
hcrn -> 65538
sbxo -> 65536
stro -> 131072
undo -> 1
xhgt -> 0
�Ê -> 732
@ -> 1073741824
-> 0
-> 16384

Record size should be 8.
@behdad
Copy link
Member
behdad commented Apr 21, 2021

MVAR is special in that it has a stupid design which encodes ValueRecordSize. To accommodate that, MetricsValueRecord is coded this way:

        ('MetricsValueRecord', [
                ('Tag', 'ValueTag', None, None, '4-byte font-wide measure identifier'),
                ('uint32', 'VarIdx', None, None, 'Combined outer-inner variation index'),
                ('uint8', 'MoreBytes', 'ValueRecordSize', -8, 'Extra bytes.  Set to empty array.'),
        ]),

Somehow getRecordSize is returning 16 on that. Maybe it doesn't see the -8 bias? I didn't know we can even compute the recordSize for a struct like that.

@behdad
Copy link
Member
behdad commented Apr 21, 2021 •
edited
Loading

Found it. Try:

diff --git a/Lib/fontTools/ttLib/tables/otBase.py b/Lib/fontTools/ttLib/tables/otBase.py
index 3c07f9e1..cc5423fd 100644
--- a/Lib/fontTools/ttLib/tables/otBase.py
+++ b/Lib/fontTools/ttLib/tables/otBase.py
@@ -571,7 +571,7 @@ class BaseTable(object):
                        countValue = 1
                        if conv.repeat:
                                if conv.repeat in reader:
-                                       countValue = reader[conv.repeat]
+                                       countValue = reader[conv.repeat] + conv.aux
                                else:
                                        return NotImplemented
                        totalSize += size * countValue

@garretrieger garretrieger mentioned this issue Apr 21, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Include record size adjustment. googlefonts/fonttools
2 participants
@behdad @garretrieger
0