[api-minor] Use a local font or fallback on an embedded one (if it exists) for non-embedded fonts (bug 1766039) #16363

calixteman · 2023-04-27T16:54:13Z

Replace FoxitSans with LiberationSans: LiberationSans is already there (for XFA) and we can use it as a good replacement of FoxitSans.
For now we just try to substitue standard fonts, the strategy is the following:
- we try to find a font locally from a hardcoded list;
- if it fails then we use Liberation as fallback (only for Helvetica for the moment);
- else we just fallback on the system serif/sansserif/monospace font.

Snuffleupagus

I'd really like us to create a new release before moving forward with this patch (or any other standard-font data related ones).

src/core/evaluator.js

src/display/font_loader.js

web/app_options.js

src/display/font_loader.js

src/core/font_substitutions.js

src/core/fonts.js

src/display/api.js

src/display/font_loader.js

Snuffleupagus · 2023-05-04T10:02:37Z

/botio test

moz-tools-bot · 2023-05-04T10:02:39Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/8975d4be50a3ce2/output.txt

moz-tools-bot · 2023-05-04T10:02:39Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/6bad623dc1f914a/output.txt

moz-tools-bot · 2023-05-04T10:29:58Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/8975d4be50a3ce2/output.txt

Total script time: 27.31 mins

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

  different ref/snapshot: 615
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/8975d4be50a3ce2/reftest-analyzer.html#web=eq.log

Snuffleupagus · 2023-05-04T10:34:26Z

I've not looked at all of the changes, however one thing that immediately jumped out at me is the movement in the following test-cases:

pdf.js/test/test_manifest.json

Lines 2895 to 2909 in 4931f29

    
           {  "id": "standard_fonts_no_system_fonts", 
        
              "file": "pdfs/standard_fonts.pdf", 
        
              "md5": "bb3a9ab3322328be983e8b4e8089843a", 
        
              "rounds": 1, 
        
              "type": "eq", 
        
              "useSystemFonts": false 
        
           }, 
        
           {  "id": "standard_fonts_main_thread_fetch", 
        
              "file": "pdfs/standard_fonts.pdf", 
        
              "md5": "bb3a9ab3322328be983e8b4e8089843a", 
        
              "rounds": 1, 
        
              "type": "eq", 
        
              "useSystemFonts": false, 
        
              "useWorkerFetch": false 
        
           },

Given that they explicitly set useSystemFonts = false, why is there movement in those cases!?

Edit: Oh, is this perhaps because we've changed from Foxit to LiberationSans fonts?

moz-tools-bot · 2023-05-04T10:36:58Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/6bad623dc1f914a/output.txt

Total script time: 34.30 mins

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

  different ref/snapshot: 182

Image differences available at: http://54.193.163.58:8877/6bad623dc1f914a/reftest-analyzer.html#web=eq.log

Snuffleupagus · 2023-05-04T10:42:04Z

Another quick find: Looking at e.g. the artofwar changes I don't think that this patch handles ArialBlack fonts correctly, since "black" fonts should have a larger font-weight than a bold font.

calixteman · 2023-05-04T11:00:08Z

Another quick find: Looking at e.g. the artofwar changes I don't think that this patch handles ArialBlack fonts correctly, since "black" fonts should have a larger font-weight than a bold font.

Interesting case.
ArialBlack is mapped onto Helvetica and the font object will have its property black set to true.
Without the patch, when setting the font in the canvas, it will result into something like 900 sansserif and we'll have a black font, but if I disable fontface we just use the embedded Helvetica which is not black at all. And we could imagine that's possible to have on OS without a black version of its sansserif font.
So I'd say that it isn't really a new bug but this patch is just highlighting it.
That said, I'll try to figure out it in this patch: it's likely a matter of adding some substitution fonts for a black version.

calixteman · 2023-05-04T14:33:18Z

I've not looked at all of the changes, however one thing that immediately jumped out at me is the movement in the following test-cases:

pdf.js/test/test_manifest.json

Lines 2895 to 2909 in 4931f29

{ "id": "standard_fonts_no_system_fonts",

"file": "pdfs/standard_fonts.pdf",

"md5": "bb3a9ab3322328be983e8b4e8089843a",

"rounds": 1,

"type": "eq",

"useSystemFonts": false

},

{ "id": "standard_fonts_main_thread_fetch",

"file": "pdfs/standard_fonts.pdf",

"md5": "bb3a9ab3322328be983e8b4e8089843a",

"rounds": 1,

"type": "eq",

"useSystemFonts": false,

"useWorkerFetch": false

},

Given that they explicitly set useSystemFonts = false, why is there movement in those cases!?

Edit: Oh, is this perhaps because we've changed from Foxit to LiberationSans fonts?

Yep exactly.

moz-tools-bot · 2023-05-10T09:37:46Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/bfdc24e68795d89/output.txt

Total script time: 34.24 mins

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

  different ref/snapshot: 58
  different first/second rendering: 1

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

Snuffleupagus · 2023-05-10T09:59:30Z

Looking at the Windows results in #16363 (comment), there seem to be a couple of regressions:

firefox-artofwar-page158 is missing a piece of italic text in the middle of the page.
firefox-standard_fonts_no_system_fonts-page7 the text now looks both italic and bold, and it should only be italic.
firefox-standard_fonts_no_system_fonts-page8 the text is no longer bold (but only italic).

calixteman · 2023-05-10T10:53:20Z

Looking at the Windows results in #16363 (comment), there seem to be a couple of regressions:
* `firefox-artofwar-page158` is missing a piece of italic text in the middle of the page.

I used Italic instead of Oblique when falling back on Helvetica for Arial Black.

* `firefox-standard_fonts_no_system_fonts-page7` the text now looks both italic _and_ bold, and it should _only_ be italic.

* `firefox-standard_fonts_no_system_fonts-page8` the text is no longer bold (but only italic).

Side effects of wrong copy/paste in standard_fonts.js: the fonts path were just permuted.

calixteman · 2023-05-10T10:53:32Z

/botio test

moz-tools-bot · 2023-05-10T10:53:34Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/beebf6b8d68adac/output.txt

moz-tools-bot · 2023-05-10T10:53:34Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/0c94ab1284955a5/output.txt

moz-tools-bot · 2023-05-10T11:21:22Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/beebf6b8d68adac/output.txt

Total script time: 27.78 mins

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

  different ref/snapshot: 608
  different first/second rendering: 1

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

moz-tools-bot · 2023-05-10T11:28:59Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/0c94ab1284955a5/output.txt

Total script time: 35.39 mins

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

  different ref/snapshot: 59
  different first/second rendering: 1

Image differences available at: http://54.193.163.58:8877/0c94ab1284955a5/reftest-analyzer.html#web=eq.log

Snuffleupagus

Nit: Please fix the typo in the commit message, i.e. replace [api-mninor] -> [api-minor]

Given the huge amount of movement on the Linux-bot I've not really looked in detail at the results, however the test results look reasonable on the Windows-bot so let's do this :-)

r=me, thank you!

calixteman · 2023-05-10T12:10:07Z

I almost looked at all the tests on linux and I didn't notice something really wrong: just few movements because the new used fonts.

…ists) for non-embedded fonts (bug 1766039) - Replace FoxitSans with LiberationSans: LiberationSans is already there (for XFA) and we can use it as a good replacement of FoxitSans. - For now we just try to substitue standard fonts, the strategy is the following: * we try to find a font locally from a hardcoded list; * if it fails then we use Liberation as fallback (only for Helvetica for the moment); * else we just fallback on the system serif/sansserif/monospace font.

calixteman · 2023-05-10T12:19:13Z

/botio makeref

moz-tools-bot · 2023-05-10T12:19:15Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/e120d724016526c/output.txt

moz-tools-bot · 2023-05-10T12:19:15Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/bda3bd24f15b2e2/output.txt

moz-tools-bot · 2023-05-10T12:43:11Z

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/bda3bd24f15b2e2/output.txt

Total script time: 23.92 mins

Lint: Passed
Make references: Passed
Check references: Passed

moz-tools-bot · 2023-05-10T12:43:15Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/e120d724016526c/output.txt

Total script time: 24.00 mins

Lint: Passed
Make references: FAILED

calixteman · 2023-05-10T12:46:55Z

/botio-windows makeref

moz-tools-bot · 2023-05-10T12:46:56Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/7ee5ee4faedd3b7/output.txt

moz-tools-bot · 2023-05-10T13:10:49Z

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/7ee5ee4faedd3b7/output.txt

Total script time: 23.86 mins

Lint: Passed
Make references: Passed
Check references: Passed

paulmenzel · 2023-07-27T08:43:12Z

This change regressed font rendering for some PDF files: https://bugzilla.mozilla.org/show_bug.cgi?id=1845551

calixteman · 2023-07-27T09:12:20Z

This change regressed font rendering for some PDF files: https://bugzilla.mozilla.org/show_bug.cgi?id=1845551

This PR is closed now and there is no need to have the same discussion in different places (bugzilla, github, ...)

calixteman requested a review from Snuffleupagus April 27, 2023 16:54

calixteman mentioned this pull request Apr 27, 2023

Use a local font when the font isn't embedded (bug 1766039) #14844

Closed

timvandermeij added the font-conversion label Apr 27, 2023

calixteman force-pushed the use_local_font branch from a7a374a to 77c91a2 Compare April 27, 2023 18:15

marco-c mentioned this pull request Apr 27, 2023

Replace most Foxit fonts with Liberation fonts #14843

Open

Snuffleupagus requested changes Apr 30, 2023

View reviewed changes

calixteman force-pushed the use_local_font branch 2 times, most recently from 264ed84 to 1a39776 Compare May 2, 2023 11:44

Snuffleupagus reviewed May 2, 2023

View reviewed changes

src/display/font_loader.js Show resolved Hide resolved

src/display/font_loader.js Outdated Show resolved Hide resolved

calixteman force-pushed the use_local_font branch from 1a39776 to 727314f Compare May 2, 2023 14:50

Snuffleupagus reviewed May 2, 2023

View reviewed changes

src/core/font_substitutions.js Outdated Show resolved Hide resolved

src/core/font_substitutions.js Outdated Show resolved Hide resolved

src/core/fonts.js Outdated Show resolved Hide resolved

src/display/api.js Outdated Show resolved Hide resolved

calixteman force-pushed the use_local_font branch from 727314f to 7179a38 Compare May 2, 2023 16:54

Snuffleupagus reviewed May 2, 2023

View reviewed changes

src/display/font_loader.js Outdated Show resolved Hide resolved

calixteman force-pushed the use_local_font branch from 7179a38 to b09a95f Compare May 3, 2023 08:15

Snuffleupagus reviewed May 3, 2023

View reviewed changes

src/display/font_loader.js Outdated Show resolved Hide resolved

calixteman force-pushed the use_local_font branch from b09a95f to f1df504 Compare May 3, 2023 09:21

calixteman requested a review from Snuffleupagus May 4, 2023 09:07

Snuffleupagus mentioned this pull request May 4, 2023

Fix PDF.js 3.5.141 post-release steps #16245

Closed

Snuffleupagus changed the title ~~Use a local font or fallback on an embedded one (if it exists) for non-embedded fonts (bug 1766039)~~ [api-minor] Use a local font or fallback on an embedded one (if it exists) for non-embedded fonts (bug 1766039) May 4, 2023

calixteman force-pushed the use_local_font branch 2 times, most recently from ee9da13 to dffca91 Compare May 4, 2023 14:52

calixteman force-pushed the use_local_font branch from b38bf2f to a75e220 Compare May 10, 2023 10:51

Snuffleupagus approved these changes May 10, 2023

View reviewed changes

calixteman force-pushed the use_local_font branch from a75e220 to 53134c0 Compare May 10, 2023 12:11

calixteman merged commit 2d2f7b3 into mozilla:master May 10, 2023

marco-c linked an issue May 10, 2023 that may be closed by this pull request

Replace most Foxit fonts with Liberation fonts #14843

Open

This was referenced May 12, 2023

Multiple issues for an NVIDIA pdf #10878

Closed

Fonts/text are rendered inconsistently in Santa Cruz County COVID-19 shelter-in-place PDF #11840

Closed

Overlapping glyphs in Times New Roman substitute on Ubuntu 18.04 LTS #13176

Closed

marco-c linked an issue May 12, 2023 that may be closed by this pull request

Overlapping glyphs in Times New Roman substitute on Ubuntu 18.04 LTS #13176

Closed

Snuffleupagus mentioned this pull request Jul 20, 2023

Regression introduced in pdf.js 3.7: Some fonts aren't being rendered correctly. #16711

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[api-minor] Use a local font or fallback on an embedded one (if it exists) for non-embedded fonts (bug 1766039) #16363

[api-minor] Use a local font or fallback on an embedded one (if it exists) for non-embedded fonts (bug 1766039) #16363

calixteman commented Apr 27, 2023 •

edited

Loading

Snuffleupagus left a comment

Snuffleupagus commented May 4, 2023

moz-tools-bot commented May 4, 2023

moz-tools-bot commented May 4, 2023

moz-tools-bot commented May 4, 2023

Snuffleupagus commented May 4, 2023 •

edited

Loading

moz-tools-bot commented May 4, 2023

Snuffleupagus commented May 4, 2023

calixteman commented May 4, 2023

calixteman commented May 4, 2023

moz-tools-bot commented May 10, 2023

Snuffleupagus commented May 10, 2023

calixteman commented May 10, 2023

calixteman commented May 10, 2023

moz-tools-bot commented May 10, 2023

moz-tools-bot commented May 10, 2023

moz-tools-bot commented May 10, 2023

moz-tools-bot commented May 10, 2023

Snuffleupagus left a comment

calixteman commented May 10, 2023

calixteman commented May 10, 2023

moz-tools-bot commented May 10, 2023

moz-tools-bot commented May 10, 2023

moz-tools-bot commented May 10, 2023

moz-tools-bot commented May 10, 2023

calixteman commented May 10, 2023

moz-tools-bot commented May 10, 2023

moz-tools-bot commented May 10, 2023

paulmenzel commented Jul 27, 2023

calixteman commented Jul 27, 2023

[api-minor] Use a local font or fallback on an embedded one (if it exists) for non-embedded fonts (bug 1766039) #16363

[api-minor] Use a local font or fallback on an embedded one (if it exists) for non-embedded fonts (bug 1766039) #16363

Conversation

calixteman commented Apr 27, 2023 • edited Loading

Snuffleupagus left a comment

Choose a reason for hiding this comment

Snuffleupagus commented May 4, 2023

moz-tools-bot commented May 4, 2023

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 4, 2023

From: Bot.io (Windows)

Received

moz-tools-bot commented May 4, 2023

From: Bot.io (Linux m4)

Failed

Snuffleupagus commented May 4, 2023 • edited Loading

moz-tools-bot commented May 4, 2023

From: Bot.io (Windows)

Failed

Snuffleupagus commented May 4, 2023

calixteman commented May 4, 2023

calixteman commented May 4, 2023

moz-tools-bot commented May 10, 2023

From: Bot.io (Windows)

Failed

Snuffleupagus commented May 10, 2023

calixteman commented May 10, 2023

calixteman commented May 10, 2023

moz-tools-bot commented May 10, 2023

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 10, 2023

From: Bot.io (Windows)

Received

moz-tools-bot commented May 10, 2023

From: Bot.io (Linux m4)

Failed

moz-tools-bot commented May 10, 2023

From: Bot.io (Windows)

Failed

Snuffleupagus left a comment

Choose a reason for hiding this comment

calixteman commented May 10, 2023

calixteman commented May 10, 2023

moz-tools-bot commented May 10, 2023

From: Bot.io (Windows)

Received

moz-tools-bot commented May 10, 2023

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 10, 2023

From: Bot.io (Linux m4)

Success

moz-tools-bot commented May 10, 2023

From: Bot.io (Windows)

Failed

calixteman commented May 10, 2023

moz-tools-bot commented May 10, 2023

From: Bot.io (Windows)

Received

moz-tools-bot commented May 10, 2023

From: Bot.io (Windows)

Success

paulmenzel commented Jul 27, 2023

calixteman commented Jul 27, 2023

calixteman commented Apr 27, 2023 •

edited

Loading

Snuffleupagus commented May 4, 2023 •

edited

Loading