Skip to content
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

expose ImageData constructor #569

Merged
merged 2 commits into from
Oct 26, 2015
Merged

Conversation

targos
Copy link
Contributor

@targos targos commented Jun 23, 2015

I need the ImageData constructor and I currently have to get it with require('canvas/lib/bindings').ImageData

Example use: convert a pixel array to dataURL

function toDataURL(width, height, pixels) {
    var imgData = new ImageData(pixels, width, height);
    var canvas = new Canvas(width, height);
    var ctx = canvas.getContext('2d');
    ctx.putImageData(imgData, 0, 0);
    return canvas.toDataURL();
}

@kangax
Copy link
Collaborator

kangax commented Jun 23, 2015

+1 from me, but we should add it to documentation too then.

@targos
Copy link
Contributor Author

targos commented Jun 24, 2015

I will investigate further because it only works with the internal CanvasPixelArray objects.
new ImageData(new Uint8ClampedArray(4), 1, 1) fails

@eqyiel
Copy link

eqyiel commented Oct 22, 2015

I also need the ImageData constructor.

The line @targos posted not fail since #604 (now that ImageData is using Uint8ClampedArray). Can we get this merged?

@LinusU
Copy link
Collaborator

LinusU commented Oct 22, 2015

@targos Could you rebase on master? Also, adding documentation and tests will greatly increase the chances of this getting merged.

@targos
Copy link
Contributor Author

targos commented Oct 22, 2015

I just found that the constructor does not behave correctly. For example, new ImageData(2, 2) creates an array with 4 elements when it should be 16 (width * height * 4)
I will try to fix that and add tests.

@zbjornson
Copy link
Collaborator

@targos my mistake (never tested those since they weren't exposed). Let me know if you want help or want me to fix it. I'm going to change that area soon for #637.

@targos
Copy link
Contributor Author

targos commented Oct 24, 2015

I pushed an update to ImageData.cc + tests. PTAL
For the error messages, I copied the output from Chrome.

@zbjornson
Copy link
Collaborator

Looks good!

if (width == 0) {
Nan::ThrowRangeError("The source width is zero.");
return;
} else if (size % width != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need else here, please change to

}
if (size % width != 0) {

@LinusU
Copy link
Collaborator

LinusU commented Oct 25, 2015

One small nit and if you could please rebase on master again (sorry), we have updated the travis config and it will ensure that your tests runs with our new config. Thank you!

@targos
Copy link
Contributor Author

targos commented Oct 26, 2015

@LinusU done :)

@LinusU
Copy link
Collaborator

LinusU commented Oct 26, 2015

Cool! Merging now 👍

LinusU added a commit that referenced this pull request Oct 26, 2015
@LinusU LinusU merged commit cf5e5c4 into Automattic:master Oct 26, 2015
@LinusU
Copy link
Collaborator

LinusU commented Oct 26, 2015

Released as 1.3.0 \o/

@targos targos deleted the expose-imagedata branch October 26, 2015 08:04
@targos
Copy link
Contributor Author

targos commented Oct 26, 2015

Thank you!

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.

5 participants