-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Improve image mask handling again #4406
Improve image mask handling again #4406
Conversation
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/e8904701fbfbbad/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/56ce53a7a85efba/output.txt |
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/e8904701fbfbbad/output.txt Total script time: 24.48 mins
Image differences available at: http://107.21.233.14:8877/e8904701fbfbbad/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/56ce53a7a85efba/output.txt Total script time: 35.55 mins
Image differences available at: http://107.22.172.223:8877/56ce53a7a85efba/reftest-analyzer.html#web=eq.log |
// Invert if necessary. | ||
if (inverseDecode) { | ||
for (var i = 0; i < actualLength; i++) { | ||
data[i] = ~data[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we corrupting the original PDF data here? Does it make sense to do inversion on copy only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we haven't made a copy, that's because we've decided it's safe to transfer, which means we've effectively taken possession of the array. So modifying it should be ok.
I can explain this better in the comment, if you like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, yes, comment would be nice. Thanks
/botio-windows preview |
From: Bot.io (Windows)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/3de895a63740db3/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/3de895a63740db3/output.txt Total script time: 0.32 mins Published
|
Fixes #3411. Thanks! |
@@ -146,10 +146,12 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { | |||
var bitStrideLength = (width + 7) >> 3; | |||
var imgArray = image.getBytes(bitStrideLength * height); | |||
var decode = dict.get('Decode', 'D'); | |||
var canTransfer = image instanceof DecodeStream; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jshint needs definition
I updated the globals list for jshint, and expanded the comment. |
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/ac270f8ae425c1f/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/ce6e2906cb57ab7/output.txt |
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/ce6e2906cb57ab7/output.txt Total script time: 25.31 mins
Image differences available at: http://107.21.233.14:8877/ce6e2906cb57ab7/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/ac270f8ae425c1f/output.txt Total script time: 35.41 mins
Image differences available at: http://107.22.172.223:8877/ac270f8ae425c1f/reftest-analyzer.html#web=eq.log |
I don't know why bug921760 and issue1796 are failing. I only changed comments in my updated patches, and the earlier versions passed. Those tests pass for me locally with the updated patches. Do the failures have some other cause? |
You can safely ignore those failures! They happened because we forgot to generate new reference images in PR #4318, but that has been fixed now. Let me re-run the tests, just to make sure. |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/8e488ff52243ce0/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/4445b4930d770e1/output.txt |
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/8e488ff52243ce0/output.txt Total script time: 25.31 mins
Image differences available at: http://107.21.233.14:8877/8e488ff52243ce0/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/4445b4930d770e1/output.txt Total script time: 35.50 mins
Image differences available at: http://107.22.172.223:8877/4445b4930d770e1/reftest-analyzer.html#web=eq.log |
So now the only "failure" is with issue2984, which is expected. Thanks for re-triggering the tests. |
/botio makeref |
From: Bot.io (Linux)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://107.21.233.14:8877/81d6863e1a7b884/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://107.22.172.223:8877/21ff2fa69035fe6/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/81d6863e1a7b884/output.txt Total script time: 25.42 mins
|
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/21ff2fa69035fe6/output.txt Total script time: 35.33 mins
|
Improve image mask handling again
The first patch fixes a what I believe is a bug in the handling of image masks when the amount of data falls short of height*width. This causes the test for issue2984.pdf to fail, but that's because the previous rendering was incorrect, featuring lots of thin horizontal lines at the tops of many letters. This patch causes those lines to disappear, which is the same result given by evince and Mac Preview.
The second patch optimizates mask handling by transferring image mask arrays where possible, instead of copying. I have two test files involving image masks, and the reduction in peak RSS was as follows: