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

Use the Font Loading API in MOZCENTRAL builds, and GENERIC builds for Firefox version 63 and above (issue 9945, bug 1088663) #9982

Merged
merged 6 commits into from
Oct 1, 2018

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Aug 17, 2018

I was feeling bored, so this is an attempt at fixing issue #9945.

This PR refactor/modernize/clean-up the font loading code, and then adds MOZCENTRAL support for the Font Loading API on top. /cc @brendandahl

Note: For easier reviewing, I'd suggest looking at one commit at a time and also using the ?w=1 URL flag to reduce the size of the larger diffs.

Fixes #9945.

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

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

Total script time: 30.12 mins

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/62508676d6db318/output.txt

Total script time: 35.88 mins

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/2a188b35bf07193/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

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

Total script time: 7.30 mins

Published

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/83d65b3cdfa74ed/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/83d65b3cdfa74ed/output.txt

Total script time: 7.05 mins

Published

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/77b00c5d5431f46/output.txt

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.215.176.217:8877/77b00c5d5431f46/output.txt

Total script time: 31.22 mins

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

Image differences available at: http://54.215.176.217:8877/77b00c5d5431f46/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/34fa113572492dc/output.txt

Total script time: 36.37 mins

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

return null;
}
createNativeFontFace() {
if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('MOZCENTRAL')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this need to be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

D'uh, this is such a rookie mistake on my part; thank you for spotting this!

@Snuffleupagus Snuffleupagus force-pushed the mozcentral-FontLoadingAPI branch 3 times, most recently from 5cc1895 to 20a46f9 Compare August 23, 2018 09:17
@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/3888a4b8650bb73/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/3888a4b8650bb73/output.txt

Total script time: 24.49 mins

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/1dbb624d51433b0/output.txt

Total script time: 57.65 mins

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

@Snuffleupagus
Copy link
Collaborator Author

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/12e74d2a4615639/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/12e74d2a4615639/output.txt

Total script time: 4.31 mins

Published

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/9f3266d8ef21648/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.215.176.217:8877/9f3266d8ef21648/output.txt

Total script time: 24.72 mins

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

Image differences available at: http://54.215.176.217:8877/9f3266d8ef21648/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/73db6c45a3dcfae/output.txt

Total script time: 36.22 mins

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

@Snuffleupagus Snuffleupagus changed the title Use the Font Loading API in MOZCENTRAL builds, and GENERIC builds for Firefox version 63 and above (issue 9945) Use the Font Loading API in MOZCENTRAL builds, and GENERIC builds for Firefox version 63 and above (issue 9945, bug 1088663) Sep 10, 2018
@mark-i-m
Copy link

Any progress on this? It would be nice to have the printing issues fixed. Thanks!

@brendandahl
Copy link
Contributor

I want to hold off on this until the end of the week so #9340 has more time to bake in Firefox Nightly. It was a pretty large change and it will be easier to see if any regression come from it or this change if they are spread out a bit.

Also changes `var` to `let`/`const` in code already touched in the patch, and makes use of template strings in a few spots.
….missingFile` property

The `Font.loading` property is only ever used *once* in the code, whereas `Font.missingFile` is more widely used. Furthermore the name `loading` feels, at least to me, slight less clear than `missingFile`. Finally, note that these two properties are the inverse of each other.
The `started` timestamp is completely usused, and the `end` timestamp is currently[1] being used essentially like a boolean value.
Hence this code can be simplified to use an actual boolean value instead, which avoids potentially hundreds (or even thousands) of unnecessary `Date.now()` calls.

---
[1] Looking briefly at the history of this code, I cannot tell if the timestamps themselves were ever used for anything (except for tracking "boolean" state).
Also changes `var` to `let`/`const` in code already touched in the patch, and makes use of template strings in a few spots.
… for Firefox version 63 and above (issue 9945)
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

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

Total script time: 19.64 mins

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

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

Total script time: 24.05 mins

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

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

@brendandahl
Copy link
Contributor

/botio-windows test

@pdfjsbot
Copy link

pdfjsbot commented Oct 1, 2018

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/52b4e3802f0ad74/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Oct 1, 2018

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/52b4e3802f0ad74/output.txt

Total script time: 24.03 mins

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

@brendandahl
Copy link
Contributor

/botio-windows test

@pdfjsbot
Copy link

pdfjsbot commented Oct 1, 2018

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/6ad32510d2e1809/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Oct 1, 2018

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/6ad32510d2e1809/output.txt

Total script time: 23.81 mins

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

@brendandahl brendandahl merged commit 25446db into mozilla:master Oct 1, 2018
@Snuffleupagus Snuffleupagus deleted the mozcentral-FontLoadingAPI branch October 2, 2018 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use JS font loading API on Firefox
5 participants