Add loadImage opt.dataMode argument

[pravdomil]

see diff

[zbjornson]

Hm actually... since this is meant to be the 100% browser-compatible API point, I'm not sure it should go there... @LinusU ?

[LinusU]

Hmmmm 🤔

Yeah, I'm not sure about adding it here since the goal was to be compatible with the browser. On the other hand, I'm not familiar enough with dataMode to see how it could work in the browser 🤔

Is dataMode not needed in the browser? or is it a missing feature from the browser?

[zbjornson]

Enabling mime data tracking has no benefits (only a slow down) unless you are generating a PDF.

and PDFs are unique to node-canvas...

I don't recall seeing anything similar to dataMode in any of the experimental or proposed Canvas APIs.

[LinusU]

Ah, right, dataMode is specific to PDF...

To be completely honest, I'm not sure why we have PDF-support in Canvas at all. It feels very shoehorned in and probably just added because it seemed easy.

Without having done too much thinking/research, I think that I would love to get it out from the main API. Preferably out from the package at all, but at the very least extract out the PDF stuff to a separate API, possibly a subclass of Context.

I mean, if I'm thinking correct here, it doesn't even make sense at all to specify the dataMode when loading the image. That should be specified when instering the image into the PDF document.

Pages is another thing that works really strange at the moment. That is super specific to PDFs and doesn't make sense for Canvas at all. If anything, a PDF should be a collection of Canvases, where each Canvas represents one page.

I'm very open to hearing other opinions, but I would love to get the PDF parts out from the main API, and instead have a dedicated API for it. Preferably, I would even put that in another package.

[zbjornson]

On the other hand, it's convenient to have PDF support built-in (nice to have the same Context2D API to create PDFs or PNGs with just one line of code changed), it doesn't add any more dependencies, and I don't think it's too obtrusive. That is, the only non-standard APIs that it brings are image.dataMode, createCanvas(w, h, "pdf")/new Canvas(w, h, "pdf") and canvas.addPage(), all of which you can just ignore for normal canvases.

What do you think of canvas.getContext("pdf")? Sorta seems like an unnecessary change from 1.x, but that's sort of more natural.

if I'm thinking correct here, it doesn't even make sense at all to specify the dataMode when loading the image. That should be specified when instering the image into the PDF document.

The info is used when inserting the image, yes, but the implementation needs to know what to track when it still has the handle to the file/data being loaded into the Image. You can always track both, at the cost of performance/memory.

[zbjornson]

@pravdomil sorry for the delay on this. #1402 just set us up nicely with a generic options argument for loadImage(), which resolves the above issue about browser compatibility. Do you feel like updating this PR to follow the same pattern used in #1402 please?

[pravdomil]

sorry, I cannot do that now

…

On Wed, 24 Apr 2019 at 02:22, Zach Bjornson ***@***.***> wrote: @pravdomil <https://github.com/pravdomil> sorry for the delay on this. #1402 <#1402> just set us up nicely with a generic options argument for loadImage(), which resolves the above issue about browser compatibility. Do you feel like updating this PR to follow the same pattern used in #1402 <#1402> please? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1185 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASG3HHDJPGSRW22Z73YLUDPR6Y4DANCNFSM4FGHTGIQ> .