Automattic / Automattic/node-canvas
Context2d::PutImageData doesn't appear to respect endian-dependent byte ordering of the canvas
- Dominant language
- JavaScript
- Stars
- 10.7k
- Forks
- 1.2k
- Avg merge
- 4d 8h
- Merged PRs (30d)
- 1
Description
I was looking at getImageData and putImageData to see how efficient they are. (Turns out they're not at all efficient, compiling to code that copies one byte at a time. See issue #909 that I just opened with suggestions for speedups).
`NAN_METHOD(Context2d::GetImageData)` [loads pixels from the canvas as `uint32_t`](https://github.com/Automattic/node-canvas/blob/master/src/CanvasRenderingContext2d.cc#L712), and picks them apart with shifts, and stores them in a fixed byte-order. So the canvas surface byte-order is endian-dependent.
`NAN_METHOD(Context2d::PutImageData)` [loads RGBA pixels from the ImageData in a fixed order](https://github.com/Automattic/node-canvas/blob/master/src/CanvasRenderingContext2d.cc#L601) as expected, but then stores onto the canvas in a fixed byte-order:
```
} else if (a == 255) {
*dstRow++ = b;
*dstRow++ = g;
*dstRow++ = r;
*dstRow++ = a;
```
Based on the documentation for `canvas.toBuffer('raw');` saying "native endian ARGB" for a memcpy from the canvas, I think PutImageData is the one that's wrong.
Presumably it's only been tested on little-endian platforms (e.g. x86, most ARM systems). Unless I'm missing something, I think it will break on a big-endian system. I don't have one to test on, but even without physical hardware it should be possible to test in an emulator.
The copy-loops would benefit from using an endian-conversion function like `ntohl` anyway, which would fix the issue. (See #909)
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.