linebender / piet

An abstraction for 2D graphics.
Apache License 2.0
1.24k stars 93 forks source link

[cairo] Fix for big-endian systems #448

Closed psychon closed 3 years ago

psychon commented 3 years ago

Cairo defines pixels as "a 32-bit quantity [...] stored in native-endian". However, the existing code was treating a pixel as four 8-bit quantities. Put differently, it was hardcoding little endian.

This commit changes the code to first calculate the pixel value as an u32 and then use u32::to_ne_bytes() to get back to "the world of bytes". For readability, this is done in a new helper function.

I did not actually check if this fixes anything on big endian, but at least it does not change anything for little endian according to the test-pictures examples.

Fixes: https://github.com/linebender/piet/issues/224 Signed-off-by: Uli Schlachter psychon@znc.in

cmyr commented 3 years ago

CI is failing because of the updated rustc, i'll push a fix for that and you can rebase. :)

JAicewizard commented 3 years ago

CI seems to be blocked by changes in this PR now. Maybe replace x with index or smth like that.

psychon commented 3 years ago

Here are updated criterion results (with "latest version of that criterion PR" vs "latest version of that criterion PR merged with this PR"):

make_image_2160p_Grayscale                                                                            
                        time:   [16.193 ms 16.198 ms 16.204 ms]
                        change: [-51.808% -51.698% -51.616%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  7 (7.00%) high mild
  1 (1.00%) high severe

make_image_2160p_Rgb    time:   [18.589 ms 18.602 ms 18.623 ms]                                 
                        change: [-48.528% -48.483% -48.423%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
  3 (3.00%) high mild
  12 (12.00%) high severe

make_image_2160p_RgbaSeparate                                                                            
                        time:   [40.119 ms 40.125 ms 40.131 ms]
                        change: [-43.341% -43.277% -43.230%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) high mild
  5 (5.00%) high severe

make_image_2160p_RgbaPremul                                                                            
                        time:   [25.172 ms 25.181 ms 25.192 ms]
                        change: [-56.145% -56.124% -56.102%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
  3 (3.00%) high mild
  9 (9.00%) high severe
cmyr commented 3 years ago

Thank you for your thoroughness!