linebender / piet

An abstraction for 2D graphics.
Apache License 2.0
1.23k stars 95 forks source link

Fix CoreGraphics copy_raw_pixels swaping red and blue channels. #521

Closed xStrom closed 2 years ago

xStrom commented 2 years ago

196 added stride support which is great, but the pixel byte order swapping introduced there seems wrong.

1) In the fast path, when the whole stride is used, the code just copies bytes. However when the stride is longer, blue and red are swapped. This seems highly suspicious by itself already. Why would the pixel format depend on the stride length? In my testing, it doesn't. 2) As I discovered during my work in #520, macOS gives us a fully utilized stride with our current test code. So the pixel swaping code never runs with our test images. The bug is hidden. 3) Changing SCALE in test-picture.rs to something else like 1.0 causes the output test image to have blue and red swapped. Confirming the bug.

Thus I have modified the code to keep the pixel component order as is, even when the stride is partially used.

I have manually tested this change to work correctly by temporarily removing the fast path and always using the new code.