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

Tweak adjustToUnicode to allow extending a built-in /ToUnicode map #13393

Conversation

Snuffleupagus
Copy link
Collaborator

This is somewhat similiar to the recent changes, in PR #13277, for fonts with an /Encoding entry.

Currently we're completely ignoring the builtInEncoding, from the font data itself, for fonts which have a built-in /ToUnicode map.
While it (obviously) doesn't seem like a good idea in general to simply overwrite existing built-in /ToUnicode entries, it should however not hurt to use the builtInEncoding to supplement missing /ToUnicode entires.

@Snuffleupagus Snuffleupagus force-pushed the adjustToUnicode-hasIncludedToUnicodeMap branch 5 times, most recently from b7d7b16 to 8b99730 Compare May 26, 2021 07:33
@Snuffleupagus Snuffleupagus force-pushed the adjustToUnicode-hasIncludedToUnicodeMap branch 3 times, most recently from 48c4e38 to bc27e66 Compare June 5, 2021 06:23
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 6, 2021
@Snuffleupagus Snuffleupagus force-pushed the adjustToUnicode-hasIncludedToUnicodeMap branch 3 times, most recently from 6c2948e to 7337987 Compare June 9, 2021 19:19
@Snuffleupagus Snuffleupagus force-pushed the adjustToUnicode-hasIncludedToUnicodeMap branch from 7337987 to 9b75271 Compare June 9, 2021 19:41
@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/d64def1b46a8c14/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://3.101.106.178:8877/c68e82fc1793cfd/output.txt

@mozilla mozilla deleted a comment from pdfjsbot Jun 10, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 10, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 10, 2021
@mozilla mozilla deleted a comment from pdfjsbot Jun 10, 2021
@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/d64def1b46a8c14/output.txt

Total script time: 26.41 mins

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

Image differences available at: http://54.67.70.0:8877/d64def1b46a8c14/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/c68e82fc1793cfd/output.txt

Total script time: 29.82 mins

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

Image differences available at: http://3.101.106.178:8877/c68e82fc1793cfd/reftest-analyzer.html#web=eq.log

@Snuffleupagus Snuffleupagus force-pushed the adjustToUnicode-hasIncludedToUnicodeMap branch from 9b75271 to 3c2c5fd Compare June 13, 2021 09:15
*This is somewhat similiar to the recent changes, in PR 13277, for fonts with an /Encoding entry.*

Currently we're *completely* ignoring the `builtInEncoding`, from the font data itself, for fonts which have a built-in /ToUnicode map.
While it (obviously) doesn't seem like a good idea in general to simply overwrite existing built-in /ToUnicode entries, it should however not hurt to use the `builtInEncoding` to supplement *missing* /ToUnicode entires.
This removes the need to *manually* wrap all return values in a Promise.
…ToGlyphId` for TrueType fonts

Note that all standard Encodings have the same length (i.e. `256` elements) and that missing entries are always represented by empty strings, hence why a separate exists-check isn't necessary in the `baseEncoding` case.
Rather than having to create and check a *separate* `ToUnicodeMap` to handle these cases, we can simply use the `fallbackToUnicode`-data (when it exists) to directly supplement *missing* /ToUnicode entires in the regular `ToUnicodeMap` instead.
@Snuffleupagus Snuffleupagus force-pushed the adjustToUnicode-hasIncludedToUnicodeMap branch from 3c2c5fd to 229a49b Compare June 14, 2021 13:05
@brendandahl
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/09d22e2fbb9ef7c/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://3.101.106.178:8877/1d0c5fda3eec8be/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/09d22e2fbb9ef7c/output.txt

Total script time: 27.39 mins

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

Image differences available at: http://54.67.70.0:8877/09d22e2fbb9ef7c/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/1d0c5fda3eec8be/output.txt

Total script time: 31.32 mins

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

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

@Snuffleupagus Snuffleupagus merged commit 7fa61c0 into mozilla:master Jun 16, 2021
@Snuffleupagus Snuffleupagus deleted the adjustToUnicode-hasIncludedToUnicodeMap branch June 16, 2021 15:13
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.

None yet

4 participants