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

Handle errors gracefully, in PartialEvaluator.translateFont, when fetching the font file (issue 9462) #12962

Merged
merged 1 commit into from
Feb 7, 2021
Merged

Handle errors gracefully, in PartialEvaluator.translateFont, when fetching the font file (issue 9462) #12962

merged 1 commit into from
Feb 7, 2021

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Feb 5, 2021 •
edited
Loading

The third page of the referenced PDF document currently fails to render completely, since one of its font files fail to load.
Since that error isn't handled, a large part of the text is thus missing which looks quite bad. By "replacing" the font data with an empty stream, we'll thus be able to fallback to rendering the text with a standard font (instead of using ErrorFont). While there's obviously no guarantee that things will look perfect, actually rendering the text at all should be an improvement in general.

Also, print a warning in PartialEvaluator.loadFont when the PartialEvaluator.translateFont method rejects, since that'd have helped debug/fix the issue faster.

Fixes #9462

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Received

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

Live output at: http://3.101.106.178:8877/66b052e858a1df1/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Linux m4)

Failed

Full output at http://54.67.70.0:8877/63f6756131ae0c4/output.txt

Total script time: 22.83 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Failed

Full output at http://3.101.106.178:8877/66b052e858a1df1/output.txt

Total script time: 29.33 mins

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

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

@Snuffleupagus Snuffleupagus marked this pull request as draft February 5, 2021 19:56
@Snuffleupagus Snuffleupagus force-pushed the issue-9462 branch 2 times, most recently from 1b9e5b4 to ada9330 Compare February 5, 2021 20:04
@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Linux m4)

Failed

Full output at http://54.67.70.0:8877/5ad5604113911c2/output.txt

Total script time: 23.02 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Failed

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

Total script time: 28.91 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Linux m4)

Failed

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

Total script time: 4.56 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Success

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

Total script time: 5.74 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Received

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

Live output at: http://3.101.106.178:8877/0ce7de403756f66/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Linux m4)

Failed

Full output at http://54.67.70.0:8877/480c3f9a11e192d/output.txt

Total script time: 4.52 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 5, 2021

From: Bot.io (Windows)

Success

Full output at http://3.101.106.178:8877/0ce7de403756f66/output.txt

Total script time: 5.80 mins

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

@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

From: Bot.io (Windows)

Received

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

Live output at: http://3.101.106.178:8877/3ba5fe37cd1f00c/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

From: Bot.io (Linux m4)

Failed

Full output at http://54.67.70.0:8877/88d823143764dd5/output.txt

Total script time: 4.58 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

From: Bot.io (Windows)

Failed

Full output at http://3.101.106.178:8877/3ba5fe37cd1f00c/output.txt

Total script time: 60.00 mins

@Snuffleupagus Snuffleupagus force-pushed the issue-9462 branch 3 times, most recently from 16e33f1 to e6fe8a7 Compare February 6, 2021 18:44
@Snuffleupagus
Handle errors gracefully, in PartialEvaluator.translateFont, when f…
e6fe8a7 
…etching the font file (issue 9462)

The *third* page of the referenced PDF document currently fails to render completely, since one of its font files fail to load.
Since that error isn't handled, a large part of the text is thus missing which looks quite bad. By "replacing" the font data with an *empty* stream, we'll thus be able to fallback to rendering the text with a standard font (instead of using `ErrorFont`). While there's obviously no guarantee that things will look perfect, actually rendering the text at all should be an improvement in general.

Also, print a warning in `PartialEvaluator.loadFont` when the `PartialEvaluator.translateFont` method rejects, since that'd have helped debug/fix the issue faster.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

From: Bot.io (Windows)

Received

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

Live output at: http://3.101.106.178:8877/90852f4260fc51f/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

From: Bot.io (Linux m4)

Failed

Full output at http://54.67.70.0:8877/8931ec781ecdfcc/output.txt

Total script time: 23.53 mins

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 6, 2021

From: Bot.io (Windows)

Failed

Full output at http://3.101.106.178:8877/90852f4260fc51f/output.txt

Total script time: 29.06 mins

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

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

@Snuffleupagus Snuffleupagus marked this pull request as ready for review February 6, 2021 21:23
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2021

From: Bot.io (Linux m4)

Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2021

From: Bot.io (Linux m4)

Success

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

Total script time: 4.31 mins

Published

@timvandermeij timvandermeij merged commit 3d33313 into mozilla:master Feb 7, 2021
@timvandermeij
Copy link
Contributor

Thank you for fixing this!

/botio makeref

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2021

From: Bot.io (Windows)

Received

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

Live output at: http://3.101.106.178:8877/632afa94079dabf/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2021

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.67.70.0:8877/4b9b8ba3e0abb5f/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2021

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/4b9b8ba3e0abb5f/output.txt

Total script time: 21.26 mins

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 7, 2021

From: Bot.io (Windows)

Success

Full output at http://3.101.106.178:8877/632afa94079dabf/output.txt

Total script time: 27.07 mins

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

@Snuffleupagus Snuffleupagus deleted the issue-9462 branch February 7, 2021 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timvandermeij timvandermeij timvandermeij approved these changes

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

Successfully merging this pull request may close these issues.

First page is mixed with the last page
3 participants
@Snuffleupagus @pdfjsbot @timvandermeij