Improve the handling of Encoding dictionary, with Differences array, in PartialEvaluator_preEvaluateFont
#7920
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yurydelendik
approved these changes
Dec 28, 2016
| diffBuf[j] = diffEntry.toString(); | ||
| } | ||
| } | ||
| hash.update(diffBuf.join('')); |
There was a problem hiding this comment.
Change separator argument, e.g. use default or '\n' -- just to have "length" as a part of the hash.
There was a problem hiding this comment.
I've changed it to the default instead; thanks for reviewing!
…ay, in `PartialEvaluator_preEvaluateFont` I recently happened to look at the code I wrote for PR 5964, which fixed [bug 1157493](https://bugzilla.mozilla.org/show_bug.cgi?id=1157493), and I quickly realized that the solution is way too simplistic. The fact that only using the `length` of a `Differences` array worked seems more like a happy accident for a particular set of font data, but could just as easily be incorrect for other PDF files. Note that in practice, the case where the `Encoding` entry is a regular `Dict` (and not a `Ref` or `Name`) is very rare, hence I don't think that we really need to worry about having to reparse this data. Also, the performance of this code-block is quite a bit better by updating the `hash` with the data from the *entire* `Differences` array, instead of at every loop iteration.
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/9059fe585751588/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/07f4e43b645ace6/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/07f4e43b645ace6/output.txt Total script time: 25.94 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/9059fe585751588/output.txt Total script time: 26.43 mins
|
movsb
pushed a commit
to movsb/pdf.js
that referenced
this pull request
Jul 14, 2018
Improve the handling of `Encoding` dictionary, with `Differences` array, in `PartialEvaluator_preEvaluateFont`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I recently happened to look at the code I wrote for PR #5964, which fixed bug 1157493, and I quickly realized that the solution is way too simplistic.
The fact that only using the
lengthof aDifferencesarray worked seems more like a happy accident for a particular set of font data, but could just as easily be incorrect for other PDF files.Note that in practice, the case where the
Encodingentry is a regularDict(and not aReforName) is very rare, hence I don't think that we really need to worry about having to reparse this data.Also, the performance of this code-block is quite a bit better by updating the
hashwith the data from the entireDifferencesarray, instead of at every loop iteration.