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

Only byte-swap 16-bit PNGs on little-endian (#7792) #7796

Merged
merged 1 commit into from
Jan 11, 2017

Conversation

AdamWill
Copy link
Contributor

@AdamWill AdamWill commented Jan 11, 2017

_png has some code that unconditionally byte-swaps 16-bit PNG
data (which is, per the spec, stored in big-endian order). This
isn't appropriate on a big-endian platform, though: this swap
being done unconditionally breaks the handling of 16-bit PNGs
on big-endian platforms (e.g. Fedora ppc64) (#7792). So, let's
use some macros numpy kindly defines for us to decide whether to
do this swap or not.

@QuLogic suggested using PyArray_EquivByteorders, but for some reason I kinda preferred using the underlying macros. If you would prefer doing it with PyArray_EquivByteorders(NPY_LITTLE, NPY_NATIVE) or so, I can do that, just ask. This gets the failure count for a Fedora ppc64 build down to 14 (and I checked it doesn't break x86_64).

_png has some code that unconditionally byte-swaps 16-bit PNG
data (which is, per the spec, stored in big-endian order). This
isn't appropriate on a big-endian platform, though: this swap
being done unconditionally breaks the handling of 16-bit PNGs
on big-endian platforms (e.g. Fedora ppc64) (matplotlib#7792). So, let's
use some macros numpy kindly defines for us to decide whether to
do this swap or not.
@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Jan 11, 2017
@AdamWill
Copy link
Contributor Author

Oh, in case you wondered, we don't have to check if NPY_BYTE_ORDER is defined, because numpy will error out if it can't determine it (the #error Unknown CPU: can not set endianness in npy_endian.h).

@codecov-io
Copy link

codecov-io commented Jan 11, 2017

Current coverage is 62.11% (diff: 100%)

Merging #7796 into master will decrease coverage by <.01%

@@             master      #7796   diff @@
==========================================
  Files           174        174          
  Lines         56028      56028          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          34805      34803     -2   
- Misses        21223      21225     +2   
  Partials          0          0          

Powered by Codecov. Last update 109251f...e96a2ca

@mdboom
Copy link
Member

mdboom commented Jan 11, 2017

👍 from me.

@mdboom mdboom merged commit e49947a into matplotlib:master Jan 11, 2017
@mdboom
Copy link
Member

mdboom commented Jan 11, 2017

Backported to v2.x as 26567b3

mdboom added a commit that referenced this pull request Jan 11, 2017
Only byte-swap 16-bit PNGs on little-endian (#7792)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants