uber-archive / image-diff

Create image differential between two images
MIT License
2.46k stars 94 forks source link

Fix case where alpha value is an exponent #29

Closed jacobp100 closed 9 years ago

jacobp100 commented 9 years ago

From error,

! Exception ocurred when performing image diff for tiedChords-ipad-pro-portrait@1-p1
[Error: Expected `image-diff's stderr` to contain 'all' but received "/tmp/tmp-76818mswa0g.png PNG 1024x1366 1024x1366+0+0 8-bit DirectClass 14.2KB 0.010u 0:00.019
/tmp/tmp-76818m6035c.png PNG 1024x1366 1024x1366+0+0 8-bit DirectClass 14.2KB 0.010u 0:00.019
Image: /tmp/tmp-76818mswa0g.png
  Channel distortion: RMSE
    gray: 0.532272 (8.12195e-06)
    alpha: 0 (0)
    all: 0.460961 (7.03381e-06)
/tmp/tmp-76818mswa0g.png=>artifacts/diff/features/tie/tiedChords-ipad-pro-portrait@1-p1.png PNG 1024x1366 1024x1366+0+0 8-bit PseudoClass 3c 0.150u 0:00.149
"]
twolfson commented 9 years ago

Can we add a test case for this comment? Maybe we should break out the extraction method so we can test it more independently (i.e. without needing an image)

jacobp100 commented 9 years ago

Do you want a test with two images that are so similar to cause this scenario?

twolfson commented 9 years ago

Nah, I imagine that would require very large images to be stored in the repo. We should relocate the .match into its own function so it can be tested independently

jacobp100 commented 9 years ago

It’s certainly possible to add these tests. Although Imagemagick is pretty bad for maintaining compatibility, so it might not be the best use of time.

twolfson commented 9 years ago

Yea, but this will also put us in a more flexible state for the future. I am going to open a secondary PR to add that functionality. This PR looks good for now.

twolfson commented 9 years ago

I have opened up #30 which brings this PR over the finish line and ready for landing.

mlmorg commented 9 years ago

Sorry for the delay. Thanks!

twolfson commented 9 years ago

Woot, thanks @mlmorg =)