Skip to content

allow a Cairo format to be specified, and explicit Buffer to be used as Canvas backing store #678

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TooTallNate
Copy link
Contributor

Also adds new flush() function, and format getter.

This allows a specific Buffer instance to be the "backing store" for a Canvas instance. This is especially useful when interfacing directly with a Linux framebuffer, as it allows for faster rendering and the format can be specified to match what the framebuffer is expecting.

For example, the PiTFT screen expects 16-bits per pixel to be written to /dev/fb1, so we can specify that the CAIRO_FORMAT_RGB16_565 format be used for the Canvas instance.

Overall this API could be bikeshedded a bit perhaps, but it's functional at least. Some Canvas features that assume a 32-byte backing store will segfault, like getting the PixelData array, but the main functions that interact with cairo directly work as expected. That said, I'm not really expecting this to get merged, but I wanted to get a public discussion going about how we could possibly support this upstream in an approved way.

Here's a couple videos from my Raspberry Pi:

Also adds new `flush()` function, and `format` getter.

This allows a specific Buffer instance to be the "backing store"
for a Canvas instance. This is especially useful when interfacing
directly with a Linux framebuffer, as it allows for faster
rendering and the `format` can be specified to match what the
framebuffer is expecting.

For example, the PiTFT screen expects 16-bits per pixel to
be written to `/dev/fb1`, so we can specify that the
CAIRO_FORMAT_RGB16_565 format be used for the Canvas instance.

Overall this API could be bikeshedded a bit perhaps, but
it's functional at least.

Here's a couple videos from my Raspberry Pi:

 - https://cloudup.com/iz--PO83YR2
 - https://cloudup.com/iJmL2M-mdw3
@LinusU
Copy link
Collaborator

LinusU commented Nov 23, 2015

This looks awesome! That being said, we should absolutely think about the api, but I don't see a problem with this functionality being added.

CAIRO_FORMAT_RGB30 was only added in Cairo 1.12 which is what's making the build fail. Could you add a guard and only export it when it's available?

@piranna
Copy link
Contributor

piranna commented Nov 27, 2015

I also did with the help of @ReneHollander some preliminar work on this topic at #571, and has a nice API to select the backend too. Maybe we could bring up some common ideas so this functionality could be included on upstream... :-)

My pull-request also add support to compile Cairo statically inside the canvas.node file, and works flawlessly on NodeOS.

@zbjornson
Copy link
Collaborator

I'm a fan of this. Is the only part of the API that gets funky getImageData, putImageData and createImageData, where the bytes-per-pixel changes from the HTML Canvas standard's 4BPP? While we could normalize the ImageData.data properties to always be 4BPP, that seems to defeat a lot of the purpose. Since this would be a non-standard API, it seems reasonable to expect the user to be aware of this difference. I'd also advocate adding a property to ImageData instances to indicate the bits per pixel.

@piranna
Copy link
Contributor

piranna commented May 4, 2017

Backends support was merged in master just this morning, so this could be done fairly easier. One of the first backends I want to port from my previous pull-request is FBDev, and it has support for several color depths. Regarding to the API, AFAIK @LinusU was pretending to remove the not-standard ones from the Canvas object, so probably it would work on the fly with 4BPP independently of how it works internally and/or add backend specific functionality on each one of the backends objects, I think this is the most clean API we could work with.

@pcordes
Copy link

pcordes commented May 4, 2017

@zbjornson: createImageData just allocates a new empty ImageData object, and is JS-only. It doesn't touch the cairo surface, so it only cares about the fixed (big-endian) RGBA pixel-format of ImageData (R is the first byte in order of ascending memory address, A is last, regardless of host endianness, because JS defines the format in terms of bytes, not uint32). And it's all-zero anyway: the implementation is return new ImageData(new Uint8ClampedArray(width * height * 4), width, height);

Oh, just noticed you were talking about using non-standard-format ImageData, too. Yes, it would be a lot simpler to only accept/return ImageData objects with non-standard pixel formats that match the surface, if the surface isn't the standard ARGB.

We could just return errors if the surface pixel format is non-standard, in any part of the code that doesn't (yet) handle that case. Alternate surface-formats would then be a not-fully-supported feature, I guess.


The C++ code behind putImageData and getImageData care about the pixel format of the cairo surface. Currently they assume cairo_image_surface_get_format(canvas->surface()) == CAIRO_FORMAT_ARGB32, which is native-endian ARGB with pre-multiplied alpha, so loading as uint32_t always puts alpha in the high 8 bits.

The full list of functions that care about surface pixel format, AFAIK:

  • Context2d::PutImageData: premultiply alpha and convert from ImageData pixel format to surface pixel format (big-endian RGBA to native-endian ARGB).
  • Context2d::GetImageData: undo alpha premultiplication and convert from surface format to ImageData format.
  • Image::decodeJPEGIntoSurface ( malloc(width*height*4), and a loop to convert RGB to ARGB with alpha=255.)
  • Context2d::blur loops over surface ARGB pixels. It also assumes that surface stride=width, which I think is a bug, and probably isn't true for widths that aren't a multiple of 16. (Cairo over-aligns rows for SIMD.)

If we have hand-vectorized SIMD for putImageData and getImageData (shuffle and apply/undo alpha premultiplication) like I'm working on (patch coming soon), it would be a lot of work to do that for multiple pixel formats. Maybe we could write naive scalar versions for other pixel formats (or just return an error), and only maintain SIMD versions for the default pixel format.


In the current (and still future default) case where the cairo surface is ARGB (4 bytes per pixel),
decodeJPEGIntoSurface could also be faster if it did the RGB->ARGB conversion in-place in the cairo surface memory, instead of allocating a separate RGB decode buffer and then copying (with format conversion) to the surface.

if (surfacebpp >= jpeg_decode_bpp) {
   decodejpeg(surface, jpeg);   // leaves the surface temporarily holding data in the wrong pixel format
   for (dst=end_of_surface, src=end_of_surface*3/4 ; ... ; dst-=4, src-=3) {
      surface[dst] = RGB_to_ARGB(surface[src]);
   }
}

The ARGB data will be written to memory that's still hot in cache from being read while it held RGB data. For a 1024x768 image, the initial distance between read and write is 768k, so it will easily stay hot in L3 cache. Only about the last 1/3rd of the conversion will stay hot in L2 cache between the read and write (as the dst and src pointers get closer), but it should still help a lot.

As a bonus, this leaves more of the surface hot in cache when finished, since there's no separate buffer we were reading from.

Obviously even better would be a JPEG decoder that can decode directly into the cairo surface's native format without requiring an extra conversion loop. But RGB -> ARGB conversion is cheap, especially with SIMD, so it's not a huge deal.

aeh added a commit to aeh/node-canvas that referenced this pull request Sep 25, 2017
slightly modified version of
Automattic#678 for canvas 1.6
aeh added a commit to aeh/node-canvas that referenced this pull request May 30, 2018
slightly modified version of
Automattic#678 for canvas 1.6
@zbjornson zbjornson force-pushed the master branch 2 times, most recently from 64ed3d8 to ff0f2ab Compare December 28, 2023 23:22
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