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

Fix CoreGraphics copy_raw_pixels swaping red and blue channels. #521

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

xStrom
Copy link
Member

@xStrom xStrom commented Jun 28, 2022

#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.

@xStrom xStrom added bug Something isn't working piet-coregraphics issue in the CoreGraphics backend labels Jun 28, 2022
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, your logic looks right to me. 👌

@xStrom xStrom merged commit b8201fe into linebender:master Jun 28, 2022
@xStrom xStrom deleted the coregraphics-copy-raw-pixels branch June 28, 2022 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working piet-coregraphics issue in the CoreGraphics backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants