Automattic / Automattic/node-canvas

Context2d::PutImageData doesn't appear to respect endian-dependent byte ordering of the canvas

Open
#908 0 comments 0 reactions 0 assignees View on GitHub
Bug
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.