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

Allow a format and explicit Buffer as backing store #1172

Open
wants to merge 1 commit into
base: v1.x
Choose a base branch
from

Conversation

aeh
Copy link

@aeh aeh commented May 30, 2018

Most of this code comes from this PR: #678. I've just make it work with 1.x and fixed it so that it passes the tests.

Thanks for contributing!

  • Have you updated CHANGELOG.md?

slightly modified version of
Automattic#678 for canvas 1.6
@zbjornson
Copy link
Collaborator

I'm not sure that it makes sense to do this. v2.x (the active development branch) has nearly complete support for all Cairo formats via the pixelFormat API, including ImageData APIs and PNG-encoding that either work properly with all of the formats or safely throw (vs. crashing the process).

I didn't add the flush method in #935 (where I added pixelFormat stuff); that could be added to v2.x. Is there any other functionality you need that the pixelFormat API doesn't provide?

@aeh
Copy link
Author

aeh commented Jun 1, 2018

Good point. I guess I could have stripped the pull request back a bit rather than just taking the previous work by TooTallNate and getting it to work. The different backing stores are useful but they aren't the primary part of the PR we were interested in...

We are using this to convert a lot of images into a video... This uses a lot of large images (with quite a lot of repetition) so we want to cache as much as we can to make the process as fast as possible. The major issue there is that on systems like heroku the memory limit is tiny so we need to cache to disk as well. Currently canvas only imports jpeg, png, etc images that need to be decompressed which takes time, and then it needs to be copied to the canvas which on a few images doesn't add up to much but when you have enough images for a few minutes worth of video it does.

This PR allows us to pass in a specific buffer to be used as the canvas backing store. This means that we can not only get the raw data out of the canvas but also load it back in again.

fs.readFile(path.resolve(__dirname, 'image.raw'), (err, data) => {
  const canvas = new Canvas(width, height, data, Canvas.CAIRO_FORMAT_RGB24);
  // do things with the canvas
  canvas.flush();
  // save back to disk so we can use it as an input later on and/or pipe data to ffmpeg
});

This is about 7-8 times faster than loading an image, drawing onto the canvas, and getting the raw buffer data out.

If it is worth it I could probably work at stripping this PR back to just what is required for this... or if you have a suggestion as to how to get something equally performant going on canvas 2 that is also very welcome.

@zbjornson
Copy link
Collaborator

Thanks for that useful info. I'm not against adding support for a backing buffer, maybe we should just do it. #910 is a request for a zero-copy JS API (C++ API already exists). That's not exactly what you're asking for (canvas creates the buffer vs. you create the buffer), but you could fs.read into the buffer exposed by canvas to achieve the same effect. Also #909 would speed up what you're doing without adding/using a non-standard API.

With Canvas 2.x the current way looks like this:

const canvas = canvas.createCanvas(width, height, {pixelFormat: "RGB24"});
const ctx = canvas.getContext('2d')
fs.readFile(path.resolve(__dirname, 'image.raw'), (err, data) => {
  assert(!err);
  const id = canvas.createImageData(data, width, height); // zero-copy
  ctx.putImageData(id, 0, 0); // sorta slow, #909 would help
  // do things with the canvas
  fs.writeFileSync('modified.raw', canvas.toBuffer('raw')); // memcpy here
});

With #910 it might look like this (sync fs for brevity):

const fd = fs.openSync('image.raw', 'r');
const canvas = canvas.createCanvas(width, height, {pixelFormat: "RGB24"});
fs.readSync(fd, canvas.buffer, 0, width * height * 3, 0);
// do things with the canvas
fs.writeFileSync('modified.raw', canvas.buffer);
// win: you can reuse the buffer vs. using fs.readFile, which may alloc a new buffer each call

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.

2 participants