Skip to content

Conversation

@Pantura
Copy link
Contributor

@Pantura Pantura commented Mar 24, 2021

This PR adds support for RGBA buffer input. This is the format that is output by DOM canvas getImageData but could be generated elsewhere.

Comment on lines +67 to +68
var rgbData = this.__addimage__.arrayBufferToBinaryString(rgbOut);
var alphaData = this.__addimage__.arrayBufferToBinaryString(alphaOut);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed these accepted ArrayBuffer, not Uint8Array given the name...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not give enough thought to these before but the other code paths call it the same way.
If it is a buffer, make it an array, if it is an array, run it through arrayBufferToBinaryString.
https://github.com/MrRio/jsPDF/blob/master/src/modules/jpeg_support.js#L71-L83

All usage should be checked and name changed if that is uniform across all usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming of the arrayBufferToBinaryString should imo be changed on a separate PR.

Copy link
Collaborator

@HackbrettXXX HackbrettXXX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR :)

Comment on lines +64 to +82
it("with alpha", () => {
const c = document.createElement("canvas");
const ctx = c.getContext("2d");
ctx.fillStyle = "#FFFFFF";
ctx.fillRect(0, 0, 150, 60);
ctx.fillStyle = "#AA00FF77";
ctx.fillRect(10, 10, 130, 40);
const dataFromCanvas = ctx.getImageData(0, 0, 150, 60);

const doc = new jsPDF({
orientation: "p",
unit: "px",
format: "a4",
floatPrecision: 2
});
doc.addImage(dataFromCanvas, 10, 10, 150, 60);

comparePdf(doc.output(), "rgba_alpha.pdf", "addimage");
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this test case produces slightly different results in IE. Could you investigate why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it would take some time to setup a windows test environment myself, I'm going throw a question: is IE11 really something that needs to be supported still? Microsoft itself tries to migrate everyone to Edge as fast as possible with all the IE problems and security issues being abundant.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. IE is probably not that important anymore. I think it's OK, if we drop IE support for new features. So let's just skip this test case in IE.

# Conflicts:
#	types/jspdf-tests.ts
@HackbrettXXX HackbrettXXX merged commit b6ed102 into parallax:master Sep 9, 2021
@Pantura
Copy link
Contributor Author

Pantura commented Sep 9, 2021

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants