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

Fix two cmap related issues. #8747

Merged
merged 1 commit into from
Aug 4, 2017
Merged

Conversation

brendandahl
Copy link
Contributor

@brendandahl brendandahl commented Aug 4, 2017

In issue #8707, there's a char code mapped to a non-
existing glyph which shouldn't be drawn. However, we
saw it was missing and tried to then use the post table and
end up mapping it incorrectly.

This illuminated a problem with issue #5704 and bug
893730 where glyphs disappeared after above fix. This was
from the cmap returning the wrong glyph id. Which in turn was
caused because the font had multiple of the same type of cmap
table and we were choosing the last one. Now, we instead
default to the first one. I'm unsure if we should instead be
merging the multiple cmaps, but using only the first one works.

Fixes #8707

In issue mozilla#8707, there's a char code mapped to a non-
existing glyph which shouldn't be drawn. However, we
saw it was missing and tried to then use the post table and
end up mapping it incorrectly.

This illuminated a problem with issue mozilla#5704 and bug
893730 where glyphs disappeared after above fix.  This was
from the cmap returning the wrong glyph id. Which in turn was
caused because the font had multiple of the same type of cmap
table and we were choosing the last one. Now, we instead
default to the first one. I'm unsure if we should instead be
merging the multiple cmaps, but using only the first one works.
@brendandahl
Copy link
Contributor Author

Fixes #8707

@brendandahl
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/385d6434fe2c885/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Linux m4)


Failed

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

Total script time: 16.65 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/385d6434fe2c885/output.txt

Total script time: 29.33 mins

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

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.

Looks good, thank you!

@Snuffleupagus
Copy link
Collaborator

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/5d727f6dfb20000/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Linux m4)


Failed

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

Total script time: 16.62 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/5d727f6dfb20000/output.txt

Total script time: 29.18 mins

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

@Snuffleupagus
Copy link
Collaborator

/botio makeref

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Linux m4)


Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/7b673f1c712a3dd/output.txt

@Snuffleupagus Snuffleupagus merged commit ace9de6 into mozilla:master Aug 4, 2017
@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Linux m4)


Success

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

Total script time: 15.55 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Aug 4, 2017

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/7b673f1c712a3dd/output.txt

Total script time: 27.29 mins

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

@mikelweb
Copy link

This didn't solve my problem: Spaces are rendered as glyphs (big dots)

@brendandahl
Copy link
Contributor Author

brendandahl commented Aug 19, 2017

@mikelweb please file a new issue with your pdf.

movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018
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.

Spaces are render as dots
4 participants