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

Remove the unused sizes and encoding properties on Font instances #11755

Merged
merged 1 commit into from
Mar 27, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Mar 26, 2020

The sizes property doesn't appear to have been used ever since the code was first split into main/worker-threads, which is so many years ago that I wasn't able to easily find exactly in which PR/commit it became unused.

The encoding property is always assigned the properties.baseEncoding value, however the PartialEvaluator doesn't actually compute/set that value any more. Again it was difficult to determine when it became unused, but it's been that way for years.

@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/8b88c55c2a98d5d/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/e156c14842eed0f/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

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

Total script time: 1.78 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/8b88c55c2a98d5d/output.txt

Total script time: 19.72 mins

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

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

The `sizes` property doesn't appear to have been used ever since the code was first split into main/worker-threads, which is so many years ago that I wasn't able to easily find exactly in which PR/commit it became unused.

The `encoding` property is always assigned the `properties.baseEncoding` value, however the `PartialEvaluator` doesn't actually compute/set that value any more. Again it was difficult to determine when it became unused, but it's been that way for years.
@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Mar 27, 2020

I've started looking at

pdf.js/src/core/fonts.js

Lines 1264 to 1265 in fa4b431

exportData: function Font_exportData() {
// TODO remove enumerating of the properties, e.g. hardcode exact names.
and have realized that there's unfortunately a large number of apparently unused properties being sent to the main-thread. However, before diving into all of this I wanted to at least remove a few properties that are guaranteed to be unused; hence this PR.

@timvandermeij timvandermeij merged commit c12ea21 into mozilla:master Mar 27, 2020
@timvandermeij
Copy link
Contributor

Thank you for cleaning this up!

@Snuffleupagus Snuffleupagus deleted the rm-fonts-sizes-encoding branch March 27, 2020 20:50
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.

3 participants