canvas-to-buffer icon indicating copy to clipboard operation
canvas-to-buffer copied to clipboard

Some suggestions

Open jimmywarting opened this issue 4 years ago • 4 comments

  1. Drop support for IE
    • MS will end the support soon in Agust...
    • MS office 365 have already dropped support for IE
    • Global marketshare tells us that it's down at < 1% (0.7% in fact)
  2. Switch to using canvas.toBlob() instead.
    • Even doe it's async it much faster since it don't have to encode the image data to base64 only to be decoded again by converting it to binary data again when you convert it into a Uint8Array. then you can do blob.arrayBuffer().then(do_something) afterwards
    • Using toBlob would mean that it would become async only - so drop all the sync code?
  3. typedarray-to-buffer isn't really necessary. you can just safely do Buffer.from(arrayBuffer) or Buffer.from(uint8.buffer)
    • The uint8 array you produce don't have any byteOffsets or anything like that.
    • typedarray-to-buffer is such a small package that it can be inlined.
  4. Really don't think you should be returning a Node Buffer, I think a Uint8Array is way better. It's more lightweight and don't depend on any nodejs stuff

jimmywarting avatar May 24 '21 12:05 jimmywarting

Great feedback @jimmywarting - busy family father here - care to submit a PR??

binarykitchen avatar May 25 '21 03:05 binarykitchen

Do you actually feel like canvas-to-buffer actually bring something useful into the mix? if so then i could maybe submit a PR. but it would likely be a breaking change then. i would for instance not use node buffer anymore cuz it would bloat the browser bundle even further. and it would then also probably also be async and not sync then.

and i don't think there is any value into getting it as a Uint8Array. i honestly think returning a Blob would turn out to be just as fine as any. you could submit this blob to any api without the need for any typed array. and you can also attach this blob to formdata (unlike a uint8array that's just stringified to [Object object] when doing formdata.append(file, uint8array) appending a blob work much better formdata.append(file, blob, 'filename')

with that said, i think that returning a blob asynchronously is better / faster.

i remember a long time ago when i did a perf test on toDataURL vs toBlob and even tough toBlob was async it performed much faster than toDataURL b/c it didn't have to encode the image binary data to a long data url that would take up ~33% more memory.

using the toBlob function gives me the raw data/image quicker then having to rely on some unnecessary encoding/decoding process (that this library currently deal with right now...) canvas binary -> dataurl then dataurl -> node:buffer (just a unnecessary overhead back and forth between strings)

with toBlob it's just essentially: canvas binary -> blob

i honestly think that doing

canvas.toBlob(blob => {
  blob.arrayBuffer().then(ab => {
    const uint8 = new Uint8Array(ab)
  })
}, 'image/webp', 1)

...is faster then doing the hole encoding/decoding dance from binary to dataurl and dataurl back to binary.

jimmywarting avatar May 30 '23 21:05 jimmywarting

would it not only be better to suggest to folk to use canvas.toBlob directly instead?

jimmywarting avatar May 30 '23 21:05 jimmywarting

(also the jsperf link in your readme is dead...)

jimmywarting avatar May 30 '23 21:05 jimmywarting