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

Put the string name of the glyph in the charset array. #10604

Merged
merged 1 commit into from
Mar 2, 2019

Conversation

brendandahl
Copy link
Contributor

Also, only warn once per font when missing a glyph name.

@brendandahl
Copy link
Contributor Author

@Snuffleupagus Can you review?
/botio test

@pdfjsbot
Copy link

pdfjsbot commented Mar 1, 2019

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/2e6bf79814870c4/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 1, 2019

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/d65b00a29621abd/output.txt

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to revert

return this.strings.push(value) + NUM_STANDARD_CFF_STRINGS - 1;
to just this.strings.push(value) again, since nothing seems to be using that any more?

Thanks for the quick fix, looks good to me!

@@ -3335,14 +3335,14 @@ var Type1Font = (function Type1FontClosure() {
cff.globalSubrIndex = new CFFIndex();

var count = glyphs.length;
var charsetArray = [0];
var charsetArray = ['.notdef'];
var i, ii;
for (i = 0; i < count; i++) {
var index = CFFStandardStrings.indexOf(charstrings[i].glyphName);
Copy link
Collaborator

@Snuffleupagus Snuffleupagus Mar 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Would it make (any) sense to define let glyphName = charstrings[i].glyphName; once at the top of the loop, rather than re-getting it potentially three times per iteration?

@pdfjsbot
Copy link

pdfjsbot commented Mar 1, 2019

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/2e6bf79814870c4/output.txt

Total script time: 17.97 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Mar 1, 2019

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/d65b00a29621abd/output.txt

Total script time: 25.53 mins

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

Also, only warn once per font when missing a glyph name.
@brendandahl
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Mar 2, 2019

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/1d7929abe67ba3b/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 2, 2019

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/f4edbdd0724b73f/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 2, 2019

From: Bot.io (Linux m4)


Success

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

Total script time: 18.04 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Mar 2, 2019

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/1d7929abe67ba3b/output.txt

Total script time: 25.46 mins

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

@timvandermeij timvandermeij merged commit 4f13eb0 into mozilla:master Mar 2, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 7, 2019
Summary:
Includes pdf.js pull requests:
mozilla/pdf.js#10591
mozilla/pdf.js#10604

Reviewers: yury

Reviewed By: yury

Bug #: 1530881

Differential Revision: https://phabricator.services.mozilla.com/D22131

--HG--
extra : rebase_source : a325a575ce4511bfa37c899411ece4fac17d6900
extra : histedit_source : d3026260dd6701c4def4c0e74ad84516c138b292
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Mar 7, 2019
Summary:
Includes pdf.js pull requests:
mozilla/pdf.js#10591
mozilla/pdf.js#10604

Reviewers: yury

Reviewed By: yury

Bug #: 1530881

Differential Revision: https://phabricator.services.mozilla.com/D22131
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.

4 participants