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

Add support for TrueType format 12 cmaps (issue 14881) #14882

Merged
merged 1 commit into from
May 7, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

This is, as far as I can tell, the first case we've seen of a format 12 cmap.
Please see https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6cmap.html

This is, as far as I can tell, the first case we've seen of a format 12 `cmap`.
Please see https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6cmap.html
@Snuffleupagus Snuffleupagus linked an issue May 6, 2022 that may be closed by this pull request
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented May 6, 2022

From: Bot.io (Linux m4)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/1c5dfd3a61d10d9/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 6, 2022

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/e2c6d4c4ba1e278/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 6, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/1c5dfd3a61d10d9/output.txt

Total script time: 26.14 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 8

Image differences available at: http://54.241.84.105:8877/1c5dfd3a61d10d9/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

pdfjsbot commented May 6, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/e2c6d4c4ba1e278/output.txt

Total script time: 31.15 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented May 7, 2022

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/6784e6ab3f85701/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 7, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/6784e6ab3f85701/output.txt

Total script time: 2.67 mins

Published

@timvandermeij timvandermeij merged commit f8838eb into mozilla:master May 7, 2022
@timvandermeij
Copy link
Contributor

Thank you for implementing this!

/botio makeref

@pdfjsbot
Copy link

pdfjsbot commented May 7, 2022

From: Bot.io (Linux m4)


Received

Command cmd_makeref from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/b91499d816fb2cf/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 7, 2022

From: Bot.io (Windows)


Received

Command cmd_makeref from @timvandermeij received. Current queue size: 1

Live output at: http://54.193.163.58:8877/697a5a7af381a44/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 7, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/b91499d816fb2cf/output.txt

Total script time: 23.30 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@pdfjsbot
Copy link

pdfjsbot commented May 7, 2022

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/697a5a7af381a44/output.txt

Total script time: 24.73 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@Snuffleupagus Snuffleupagus deleted the issue-14881 branch May 7, 2022 10:14
@Snuffleupagus
Copy link
Collaborator Author

@timvandermeij Thanks a lot for reviewing/merging my patches!


A possible follow-up here could be to add support for format 13 cmaps as well, since according to the specification those are nothing more than a special-case of format 12; see https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6cmap.html

Based on the specification text it seems to me that format 13 might be quite rare, and I'd not be able to supply a test-case, but do you think that it'd make sense to add support for those cmaps as well to prevent a possible future bug report?

@timvandermeij
Copy link
Contributor

Given the "[...] used internally by Apple for its last resort font and by Unicode's last resort font. It would, in general, not be appropriate for any font other than a last resort font" part of the specification, I don't really think we'll see it in practice. Moreover, I think it's good to only implement this if we actually have a file that uses it, otherwise it will be hard to verify the correctness of the implementation and we won't have a regression test. For now I'd say we don't implement this until we actually find/make a file with format 13 cmaps.

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

Successfully merging this pull request may close these issues.

Text is not rendered
3 participants