-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[api-minor] Remove the forceClamped
-functionality in the Streams (issue 14849)
#14856
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
Conversation
7f0f667
to
aef5674
Compare
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/7d4552c3466ee7a/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/ce4c00180169461/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/ce4c00180169461/output.txt Total script time: 24.46 mins
Image differences available at: http://54.241.84.105:8877/ce4c00180169461/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/7d4552c3466ee7a/output.txt Total script time: 26.83 mins
Image differences available at: http://54.193.163.58:8877/7d4552c3466ee7a/reftest-analyzer.html#web=eq.log |
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.
LGTM, thank you very much.
I'm not sure it'll induce an observable perf impact but for sure it'll help to decrease memory use.
…ssue 14849) As it turns out, most of the code-paths in the `PDFImage`-class won't actually pass the TypedArray (containing the image-data) to the `ColorSpace`-code. Hence we *generally* don't need to force the image-data to be a `Uint8ClampedArray`, and can just as well directly use a `Uint8Array` instead. In the following cases we're returning the data without any `ColorSpace`-parsing, and the exact TypedArray used shouldn't matter: - https://github.com/mozilla/pdf.js/blob/b72a4483276d65bef32cff269eb40923c1363f2d/src/core/image.js#L714 - https://github.com/mozilla/pdf.js/blob/b72a4483276d65bef32cff269eb40923c1363f2d/src/core/image.js#L751 In the following cases the image-data is only used *internally*, and again the exact TypedArray used shouldn't matter: - https://github.com/mozilla/pdf.js/blob/b72a4483276d65bef32cff269eb40923c1363f2d/src/core/image.js#L762 with the actual image-data being defined (as `Uint8ClampedArray`) further below - https://github.com/mozilla/pdf.js/blob/b72a4483276d65bef32cff269eb40923c1363f2d/src/core/image.js#L837 *Please note:* This is tagged `api-minor` because it's API-observable, given that *some* image/mask-data will now be returned as `Uint8Array` rather than using `Uint8ClampedArray` unconditionally. However, that seems like a small price to pay to (slightly) reduce memory usage during image-conversion.
aef5674
to
fbf6dee
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/bee211611b68419/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/73f6df5de7ad8b7/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/bee211611b68419/output.txt Total script time: 24.50 mins
Image differences available at: http://54.241.84.105:8877/bee211611b68419/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/73f6df5de7ad8b7/output.txt Total script time: 27.39 mins
|
As it turns out, most of the code-paths in the
PDFImage
-class won't actually pass the TypedArray (containing the image-data) to theColorSpace
-code. Hence we generally don't need to force the image-data to be aUint8ClampedArray
, and can just as well directly use aUint8Array
instead.In the following cases we're returning the data without any
ColorSpace
-parsing, and the exact TypedArray used shouldn't matter:pdf.js/src/core/image.js
Line 714 in b72a448
pdf.js/src/core/image.js
Line 751 in b72a448
In the following cases the image-data is only used internally, and again the exact TypedArray used shouldn't matter:
pdf.js/src/core/image.js
Line 762 in b72a448
Uint8ClampedArray
) further belowpdf.js/src/core/image.js
Line 837 in b72a448
Please note: This is tagged
api-minor
because it's API-observable, given that some image/mask-data will now be returned asUint8Array
rather than usingUint8ClampedArray
unconditionally. However, that seems like a small price to pay to (slightly) reduce memory usage during image-conversion.