Fixes CanvasPixelArray set polyfill (#4974) #4997
Conversation
| if (window.CanvasPixelArray) { | ||
| if (typeof window.CanvasPixelArray.prototype.set !== 'function') { | ||
| window.CanvasPixelArray.prototype.set = function(arr) { | ||
| var ctx = document.createElement('canvas').getContext('2d'); |
There was a problem hiding this comment.
I'm just wondering if we can avoid to creating a canvas element for browsers that has no such problem to reduce startup time
There was a problem hiding this comment.
unfortunately there is no easy way to know what type the data array will have without creating it. What we could do is assume that browsers who support Uint8ClampedArray will use that for backing the canvas. But I do not know if that holds true.
There was a problem hiding this comment.
Let use userAgent string to not bother to check for Firefox, Opera, Safari and newer versions of Chrome and IE
There was a problem hiding this comment.
Could you also time it? Just for our information
There was a problem hiding this comment.
would be possible, but for which versions?
basically all which not use Uint8ClampedArray right?
http://caniuse.com/typedarrays doesnt really help.
There was a problem hiding this comment.
good question, so far we had problem with ie and chrome (probably androids browser) only
There was a problem hiding this comment.
just tested IE. <11 doesn't have set method.
i tested a few chrome verions (glad there is chromium portable(, and support for the set method was added in chrome 21.
Ee can do a user agent check and polyfill it with like i proposed.
There was a problem hiding this comment.
i am downloading a few old chrome versions to test. Maybe you could check firefox?
document.createElement('canvas').getContext('2d').createImageData(1,1).data.set
should not return undefined :)
There was a problem hiding this comment.
its undefined in firefox < 4 - I will add a polyfill for that as well, tell me if I do not need to do that :)
There was a problem hiding this comment.
I don't think pdf.js works or tested for firefox < ~17
|
@yurydelendik i added now the polyfill based on user agents for IE < 11 and chrom(e|ium) < 21. Not really beautiful but the only way to avoid creating an instance. |
| // feature-detectable, we have to rely on user agents: | ||
| // IE<11 contains MSIE - starting from 11 it no longer does | ||
| var polyfill = false; | ||
| if (navigator.userAgent.indexOf('MSIE')) { |
There was a problem hiding this comment.
did you mean .indexOf(...) >= 0 ?
There was a problem hiding this comment.
well result is the same, but i can add it.
|
Look fine to me after comments address. Can somebody else review this as well? |
|
ouch - i just thought: hey why is it working in IE10 with the old polyfill? |
| @@ -492,6 +492,25 @@ if (typeof PDFJS === 'undefined') { | |||
| } | |||
| }; | |||
| } | |||
| } else { | |||
| // Chrome < 21 uses an inaccessible CanvasPixelArray prototype. | |||
| // be cause we cannot feature detect it, we rely on user agent. | |||
There was a problem hiding this comment.
A small nit: Please use upper-case letters at the beginning of sentences; it makes the text in the comment much easier to read! :-)
Edit: Also be cause should be one word -> Because.
There was a problem hiding this comment.
this PR is horrible, so many amends, and I cannot hide them :-(
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @yurydelendik received. Current queue size: 1 Live output at: http://107.21.233.14:8877/c1e36e06a178552/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/c1e36e06a178552/output.txt Total script time: 0.73 mins Published
|
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/0d3850a29c5408c/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/0d3850a29c5408c/output.txt Total script time: 0.73 mins Published
|
Fixes CanvasPixelArray set polyfill (#4974)
|
Thank you |
what this PR does to fix #4974 is to inspect the actual image data returned from the canvas context.
If the browser supports Uint8ClampedArray and the image data is of that type we are all set and can return.
if not, we need to check if there might be a set function available. if that is not the case, we wrap the createImageData function and add a set function to it.
As far as I could figure out it is not possible to add it to the CanvasPixelArray prototype used by Chrome 17.