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

Support canvas.getBuffer('raw') #819

Merged
merged 6 commits into from Oct 16, 2016
Merged

Conversation

adamhooper
Copy link
Contributor

This should help interface with custom image libraries like LodePNG or
WebP.

refs #306. Also, I think users could handle #562 themselves, using https://github.com/lovell/sharp plus some bit-shifts. (I haven't seen a Node library that does bulk bit-shifts on Buffer data, but it'd be straightforward to write one.)

This should help interface with custom image libraries like LodePNG or
WebP.
@adamhooper adamhooper mentioned this pull request Oct 7, 2016
@adamhooper
Copy link
Contributor Author

Along with https://github.com/huffpostdata/node-lightnpng, I've achieved a 50% speedup in PNG compression.

Before: canvas.toBuffer(undefined, 3, canvas.PNG_FILTER_SUB)

After: lightnpng.native_argb32_to_png(canvas.toBuffer('raw'), canvas.width, canvas.height, canvas.stride)

@adamhooper
Copy link
Contributor Author

Also, it works a treat with node-jpeg-turbo:

const buf = canvas.toBuffer('raw')
return jpeg.compressSync(buf, {
  format: os.endianness() == 'LE' ? jpeg.FORMAT_BGRA : jpeg.FORMAT_ARGB,
  width: canvas.width,
  height: canvas.height,
  quality: 85
})

That's faster than node-canvas's JPEG encoding, and it's actually synchronous.

@LinusU
Copy link
Collaborator

LinusU commented Oct 9, 2016

This is great!! I'm hoping to review this very soon.

If you're interested, I also maintain some lodepng bindings here: https://github.com/LinusU/node-lodepng

Copy link
Collaborator

@LinusU LinusU left a comment

Choose a reason for hiding this comment

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

A few small nits, but this looks great! I agree that toBuffer never should return PNG data, but as said that will be for the next major version.

I actually want to move all non-standard APIs away from the Canvas and Context and keep them as close to the spec as possible

var buf = canvas.toBuffer('raw');
var stride = canvas.stride;

var isBE = (function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be replaced with os.endianness()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

os.endianness() was introduced in Node v0.9.4, but Travis runs the tests on Node v0.8.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch

// Buffer doesn't have readUInt32(): it only has readUInt32LE() and
// readUInt32BE().
var px = buf.readUInt32BE(y * stride + x * 4);
if (isBE) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about something like this?

var postfix = os.endianness()
var px = buf['readUInt32' + postfix](y * stride + x * 4)

@@ -248,6 +257,16 @@ NAN_METHOD(Canvas::ToBuffer) {
return;
}

if (info.Length() == 1 && info[0]->StrictEquals(Nan::New<String>("raw").ToLocalChecked())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about info.Length() >= 1 to support .toBuffer('raw', undefined, undefined)? (might be used if someone wraps .toBuffer)

@adamhooper
Copy link
Contributor Author

Good advice! Tweaked.

@LinusU
Copy link
Collaborator

LinusU commented Oct 16, 2016

Thank you for this!

@LinusU LinusU merged commit f677469 into Automattic:master Oct 16, 2016
@paramaggarwal
Copy link

paramaggarwal commented Dec 20, 2017

I am curious to understand how is this different from ctx.getImageData() which also returns the raw pixel data?

EDIT: Found the answer here: #306 (comment)

@adamhooper
Copy link
Contributor Author

adamhooper commented Dec 20, 2017 via email

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.

None yet

3 participants