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

Adds built in CMaps and unifies the glyph mapping. #4259

Merged
merged 6 commits into from
Mar 13, 2014
Merged

Adds built in CMaps and unifies the glyph mapping. #4259

merged 6 commits into from
Mar 13, 2014

Conversation

brendandahl
Copy link
Contributor

Changes

  • Use adobe provided CMaps instead of relying on text decoder.
  • Attempts to follow section 9.6.6 (text encoding) as much as possible for mapping glyphs.
  • Builds a unicode map based of 9.10 which is then used adjust the cmap we build.
  • All cmap building goes through one function now.
  • All fonts build a toFontChar so charToGlyphs doesn't have multiple paths.
  • Tries to standardize on names of various common things:
    • fontCharCode - the character code that will be in the final font e.g. what we use in canvasCtx.fillText
    • charCode - the character code that comes from the pdf e.g. what is in a pdf show text command
    • glyphId - the glyph index
    • cmap - true type cmap
    • cMap - character map

Expected Failures:
thuluthfont-pdf - minor - junk changes to different junk
issue919 - minor - change, some glyphs now don't end up in private use area
issue1912 - minor - movement not sure why, some glyphs are now mapped to correct unicode location
issue3405 - good - notdef shows in acrobat and preview
mao.pdf - good - looks more like acrobat
p020121130574743273239 - good - looks more like acrobat

Fixes:
#2884
#3323
#4061
#3120 - though this also seem to be fixed from a FF patch, but fixed in Aurora too
#2931
#2882
#1691
#2375

@brendandahl
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/e375b3ae779ef3f/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/727ad7886171746/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Linux)

Failed

Full output at http://107.21.233.14:8877/e375b3ae779ef3f/output.txt

Total script time: 24.76 mins

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

Image differences available at: http://107.21.233.14:8877/e375b3ae779ef3f/reftest-analyzer.xhtml#web=eq.log

@Snuffleupagus Snuffleupagus mentioned this pull request Feb 7, 2014
Closed
@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/727ad7886171746/output.txt

Total script time: 38.75 mins

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

Image differences available at: http://107.22.172.223:8877/727ad7886171746/reftest-analyzer.xhtml#web=eq.log

@Snuffleupagus Snuffleupagus mentioned this pull request Feb 7, 2014
Closed
@Snuffleupagus
Copy link
Collaborator

Really nice patch!

This PR also fixes:
#4304.
#4377.
https://bugzilla.mozilla.org/show_bug.cgi?id=961098
https://bugzilla.mozilla.org/show_bug.cgi?id=894572 (as far as I can tell, I don't speak Tamil)
https://bugzilla.mozilla.org/show_bug.cgi?id=893730
https://bugzilla.mozilla.org/show_bug.cgi?id=892457

@timvandermeij
Copy link
Contributor

I'm really impressed; this must have been a lot of work!

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/bae462589388042/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/bae462589388042/output.txt

Total script time: 0.32 mins

Published

@brendandahl
Copy link
Contributor Author

/botio-windows test

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/5f90e6e942eabc2/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2014

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/5f90e6e942eabc2/output.txt

Total script time: 38.94 mins

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

Image differences available at: http://107.22.172.223:8877/5f90e6e942eabc2/reftest-analyzer.xhtml#web=eq.log

@Snuffleupagus Snuffleupagus mentioned this pull request Feb 8, 2014
Closed
@yurydelendik
Copy link
Contributor

@brendandahl Could you split external files addition into separate commit?

@yurydelendik
Copy link
Contributor

Also, could you add them in binary form (as is), so we will not deal with crlf thing, e.g. add ".gitattributes" file with * binary to the "external/cmaps/" folder

@brendandahl
Copy link
Contributor Author

/botio-linux fonttest

@pdfjsbot
Copy link

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/cfa130d46434aaa/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/cfa130d46434aaa/output.txt

Total script time: 0.36 mins

  • Font Tests: Passed

@brendandahl
Copy link
Contributor Author

Looks like I broke something on linux fips test pdf. I'll fix that before merge. Most of the other ref failures are minor text movement which seem to vary from looking better to worse.

@brendandahl
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/e9f1c67531fba0c/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/55c54787d86f1b1/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)

Failed

Full output at http://107.21.233.14:8877/e9f1c67531fba0c/output.txt

Total script time: 23.74 mins

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

Image differences available at: http://107.21.233.14:8877/e9f1c67531fba0c/reftest-analyzer.xhtml#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/55c54787d86f1b1/output.txt

Total script time: 37.42 mins

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

Image differences available at: http://107.22.172.223:8877/55c54787d86f1b1/reftest-analyzer.xhtml#web=eq.log

@Snuffleupagus
Copy link
Collaborator

The symbol font issues seems to be fixed, but there are still missing hyphens in fips197 (page 3 and 11).
From a cursory look into the file, it appears that this could be explained by note 5 here: http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#page=664

This was referenced May 7, 2014
Closed
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request May 24, 2014
Closed
This was referenced Jun 14, 2014
Merged
Merged
This was referenced Jul 5, 2014
Closed
Merged
@sylvestre sylvestre mentioned this pull request Jul 7, 2014
Closed
This was referenced Jul 25, 2014
Merged
Merged
fkaelberer pushed a commit to fkaelberer/pdf.js that referenced this pull request Aug 1, 2014
@Snuffleupagus @fkaelberer
Stop including cidmaps.js
ec26931 
In mozilla@b5b94a4, i.e. PR mozilla#4259, we stopped using cidmaps.js. Despite that, it's still included when PDF.js is built. At almost 0.5 MB (and approx. 7000 lines), this is currently the single largest file in the codebase.
Including such a large file in the builds, when it is not actually used, seems extremely wasteful; hence this patch.
@Snuffleupagus Snuffleupagus mentioned this pull request Aug 22, 2014
Merged
This was referenced Aug 31, 2014
Closed
Merged
@Snuffleupagus Snuffleupagus mentioned this pull request Sep 7, 2014
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request Nov 26, 2014
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request Jan 4, 2015
Closed
This was referenced Jan 13, 2015
Closed
Closed
@vivacewith vivacewith mentioned this pull request Jan 19, 2015
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request Jan 27, 2015
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request Feb 6, 2015
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request Mar 3, 2015
Merged
@Snuffleupagus Snuffleupagus mentioned this pull request Jun 11, 2015
Merged
@Snuffleupagus Snuffleupagus mentioned this pull request Aug 13, 2015
Closed
This was referenced Aug 28, 2015
Closed
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request Jan 18, 2016
Closed
@Snuffleupagus Snuffleupagus mentioned this pull request Feb 15, 2017
Merged
@Snuffleupagus Snuffleupagus mentioned this pull request Jan 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@brendandahl @pdfjsbot @Snuffleupagus @timvandermeij @yurydelendik @bthorben @vyv03354