X Tutup
Skip to content

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

Merged
mdboom merged 1 commit intomatplotlib:masterfrom
AdamWill:png-swap-endian
Jan 11, 2017
Merged

Only byte-swap 16-bit PNGs on little-endian (#7792)#7796
mdboom merged 1 commit intomatplotlib:masterfrom
AdamWill:png-swap-endian

Conversation

@AdamWill
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
Member

mdboom commented Jan 11, 2017

👍 from me.

@mdboom mdboom merged commit e49947a into matplotlib:master Jan 11, 2017
@mdboom
Copy link
Copy Markdown
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.

4 participants

X Tutup