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.

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

[merge] OS/2 table Mixed Versions Crash #1865

Closed
ClintGoss opened this issue Mar 29, 2020 · 6 comments
Closed

[merge] OS/2 table Mixed Versions Crash #1865

ClintGoss opened this issue Mar 29, 2020 · 6 comments
Assignees

Comments

@ClintGoss
Copy link

Got a crash in pyftmerge when merging several fonts with OS/2 tables that have mixed versions.

If a font with a v5 OS/2 table follows font(s) with v4 OS/2 tables, I get:

File "C:\App\Python37\lib\site-packages\fontTools\misc\loggingTools.py", line 375, in wrapper
return func(*args, **kwds)
File "D:\Kurinto\Bin\Bin_CG\kmerge.py", line 1196, in main
font.save(outfile)
File "C:\App\Python37\lib\site-packages\fontTools\ttLib\ttFont.py", line 173, in save
writer_reordersTables = self._save(tmp)
File "C:\App\Python37\lib\site-packages\fontTools\ttLib\ttFont.py", line 212, in _save
self._writeTable(tag, writer, done, tableCache)
File "C:\App\Python37\lib\site-packages\fontTools\ttLib\ttFont.py", line 633, in _writeTable
tabledata = self.getTableData(tag)
File "C:\App\Python37\lib\site-packages\fontTools\ttLib\ttFont.py", line 651, in getTableData
return self.tables[tag].compile(self)
File "C:\App\Python37\lib\site-packages\fontTools\ttLib\tables\O_S_2f_2.py", line 152, in compile
d['usLowerOpticalPointSize'] = round(self.usLowerOpticalPointSize * 20)
AttributeError: 'table_O_S_2f_2' object has no attribute 'usLowerOpticalPointSize'

Confession: I have not done any serious debugging here. I am hypothesizing the cause of the failure ... but it does seem reasonable given that, when I hacked my one font with a v5 OS/2 table to v4, the issue went away.

@ClintGoss
Copy link
Author

Hit this again trying to merge fonts A and B, with A and B having OS/2 table versions of 1 and 3, respectively.

@anthrotype anthrotype changed the title OS/2 table Mixed Versions Crash [merge] OS/2 table Mixed Versions Crash May 13, 2020
@anthrotype
Copy link
Member

I see that OS/2 v5 has a # TODO in fontTools.merge

# TODO version 5

we just need to handle the new attributes.

@anthrotype
Copy link
Member

from what I can tell in https://docs.microsoft.com/en-us/typography/opentype/spec/OS2#lps, it should be enough that we take the min() of all the fonts' usLowerOpticalPointSize and the max() of the fonts' usUpperOpticalPointSize.

@anthrotype anthrotype changed the title [merge] OS/2 table Mixed Versions Crash [merge] OS/2 table Versions 5 Crash May 13, 2020
@anthrotype anthrotype changed the title [merge] OS/2 table Versions 5 Crash [merge] OS/2 table Versions Mixed Version Crash May 13, 2020
@anthrotype anthrotype changed the title [merge] OS/2 table Versions Mixed Version Crash [merge] OS/2 table Mixed Version Crash May 13, 2020
@anthrotype anthrotype changed the title [merge] OS/2 table Mixed Version Crash [merge] OS/2 table Mixed Versions Crash May 13, 2020
@anthrotype
Copy link
Member

Hit this again trying to merge fonts A and B, with A and B having OS/2 table versions of 1 and 3, respectively.

@ClintGoss that's strange, v1 and v3 are supported and the merger should already take the maximum between the OS/2 versions. Can you provide a test font, the exact command you run and the error you got?

@anthrotype
Copy link
Member

Ah I see. The merger is not handling attributes like sxHeight, sCapHeight, usDefaultChar, usBreakChar that were only added in version 2 and are missing from version 1, so when merging v1 with v > 1 you get the AttributeError. I'll fix this

@anthrotype anthrotype self-assigned this May 13, 2020
anthrotype added a commit to anthrotype/fonttools that referenced this issue May 13, 2020
@ClintGoss
Copy link
Author

Glad you see the issue ... because it is actually difficult for me to isolate a demonstration case. I've got merge and pyftsubset running hundreds of times in a rather arcane and massive pipeline / build process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants