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

XFA - Add Liberation-Sans font as a substitution for some missing fonts #13517

Merged
merged 2 commits into from
Jun 9, 2021
Merged

XFA - Add Liberation-Sans font as a substitution for some missing fonts #13517

merged 2 commits into from
Jun 9, 2021

Conversation

calixteman
Copy link
Contributor

  • Some js files contain scale factors for each glyph in order to rescale Liberation to have a final font with the correct width.
  • A lot of XFA have some containers where their dimensions are based on their text content, so using default font from browser can lead to an almost unreadable pdf.

@calixteman calixteman requested a review from brendandahl June 8, 2021 18:55
Snuffleupagus
Snuffleupagus reviewed Jun 8, 2021
View reviewed changes
src/core/calibri_bold.js Outdated Show resolved Hide resolved
src/display/display_utils.js Outdated Show resolved Hide resolved
brendandahl
brendandahl approved these changes Jun 8, 2021
View reviewed changes
Copy link
Contributor

@brendandahl brendandahl left a comment

Choose a reason for hiding this comment

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

A few refactors below. We'll probably want to do a bit more in some follow ups to avoid confusing these XFA fonts with the PDF standard 14 fonts.

src/core/standard_fonts.js Outdated Show resolved Hide resolved
src/core/standard_fonts.js Outdated Show resolved Hide resolved
src/core/standard_fonts.js Outdated Show resolved Hide resolved
@calixteman
Add the possibility to rescale each glyph in a font
fd1110a 
  - a lot of xfa files are using Myriad pro or Arial fonts without embedding them and some containers have some dimensions based on those font metrics. So not having the exact same font leads to a wrong display.
  - since it's pretty hard to find a replacement font with the exact same metrics, this patch gives the possibility to read glyf table, rescale each glyph and then write a new table.
  - so once PR mozilla#12726 is merged we could rescale for example Helvetica to replace Myriad Pro.
@calixteman calixteman force-pushed the liberation branch 2 times, most recently from 066f395 to 76bb0b1 Compare June 9, 2021 14:48
Snuffleupagus
Snuffleupagus reviewed Jun 9, 2021
View reviewed changes
src/core/document.js Outdated Show resolved Hide resolved
@calixteman
XFA - Add Liberation-Sans font as a substitution for some missing fonts
34a2fa7 
  - Some js files contain scale factors for each glyph in order to rescale Liberation to have a final font with the correct width.
  - A lot of XFA have some containers where their dimensions are based on their text content, so using default font from browser can lead to an almost unreadable pdf.
@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Linux m4)

Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Windows)

Received

Command cmd_test from @calixteman received. Current queue size: 2

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Linux m4)

Failed

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

Total script time: 26.84 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Windows)

Failed

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

Total script time: 30.25 mins

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

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

@calixteman calixteman merged commit 6d88d8c into mozilla:master Jun 9, 2021
@calixteman
Copy link
Contributor Author

/botio makeref

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Linux m4)

Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Windows)

Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Windows)

Success

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

Total script time: 27.62 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Linux m4)

Failed

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

Total script time: 60.00 mins

  • Lint: Passed

@brendandahl
Copy link
Contributor

/botio-linux makeref

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Linux m4)

Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Jun 9, 2021

From: Bot.io (Linux m4)

Success

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

Total script time: 23.44 mins

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

This was referenced Jun 9, 2021
Merged
Merged
@Snuffleupagus Snuffleupagus mentioned this pull request Jun 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Snuffleupagus Snuffleupagus Snuffleupagus left review comments

@brendandahl brendandahl brendandahl approved these changes

Assignees
No one assigned
Labels
font-conversion form-xfa
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@calixteman @pdfjsbot @brendandahl @Snuffleupagus