Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

imsave should preserve alpha channel #1956

Merged
merged 1 commit into from May 3, 2013

Conversation

Projects
None yet
3 participants
Contributor

Westacular commented Apr 27, 2013

While looking at test_image, I noticed the comment that imsave was flattening alpha channel data. I figured this was related to #1868 (it is, partially), and decided to fix it.

I've updated the test to reflect that an imsave/imread round-trip should now preserve full RGBA data to within 1/255.

Passing the test, however, requires that the patches from #1868 also be applied.

@pelson pelson and 1 other commented on an outdated diff Apr 29, 2013

lib/matplotlib/image.py
@@ -1273,7 +1273,8 @@ def imsave(fname, arr, vmin=None, vmax=None, cmap=None, format=None,
fig = Figure(figsize=figsize, dpi=dpi, frameon=False)
canvas = FigureCanvas(fig)
im = fig.figimage(arr, cmap=cmap, vmin=vmin, vmax=vmax, origin=origin)
- fig.savefig(fname, dpi=dpi, format=format)
+ fig.patch.set_alpha(0)
@pelson

pelson Apr 29, 2013

Member

Does the transparent kwarg on savefig do the same thing? (http://matplotlib.org/faq/howto_faq.html#save-transparent-figures)

@Westacular

Westacular Apr 29, 2013

Contributor

You're right, it does. (Silly me, I'd forgotten about that option.)

I've updated the branch to use that instead.

@pelson pelson and 1 other commented on an outdated diff Apr 29, 2013

lib/matplotlib/tests/test_image.py
from numpy import random
random.seed(1)
data = random.rand(256, 128, 4)
+ # Where alpha value gets rounded down to 0, the rgb values all get set to 0
+ # during imsave, which messes up the comparison. Workaround is to make
+ # sure the min alpha value for any pixel is 1./255.
+ alphas = data[:, :, 3]
+ alphas[alphas < 1./255] = 1./255
@pelson

pelson Apr 29, 2013

Member

This is nasty. Is the data type of the random array to blame? I'd like to get to the bottom of this problem rather than workaround it. Are you keen?

@Westacular

Westacular Apr 29, 2013

Contributor

Well, I'm not sure there's a real problem to be fixed here. I believe what's happening is due to how the image is being blended with the background, and if a pixel has a value of (r,g,b,0), the result of the blending will just be the background's value. That is perfectly reasonable behaviour; if alpha=0 the r,g,b values are meaningless.

Prior to the blending, the pixel data is converted uint8 (using trunc(x*255) it looks like), so alpha<1./255 becomes 0. This also seems quite reasonable.

The alternative workarounds (instead of setting a minimum alpha) that come to mind are:

  • set the r,g,b values in data to 0 where alpha<1./255
  • implement a custom comparison where differences in the r,g,b values are ignored if alpha<1./255 on both sides of the comparison

Both those alternatives would (I believe) require more code.

@pelson

pelson May 1, 2013

Member

Hmmm, ok, I'm less convinced that it is so bad. If this is known (and desirable functionality) I suggest that you set the data values to zero after imsave so that we are testing the clearing of RGB values. So something like:

bad_alphas = alphas < 1./255
...
plt.imsave(buff, data)
...
data[bad_alphas] = 0

@pelson pelson and 1 other commented on an outdated diff May 1, 2013

lib/matplotlib/tests/test_image.py
buff = io.BytesIO()
plt.imsave(buff, data)
buff.seek(0)
arr_buf = plt.imread(buff)
- assert arr_buf.shape == data.shape
-
- # Unfortunately, the AGG process "flattens" the RGBA data
- # into an equivalent RGB data with no transparency. So we
- # Can't directly compare the arrays like we could in some
- # other imsave tests.
+ assert_array_almost_equal(data, arr_buf, decimal=2)
@pelson

pelson May 1, 2013

Member

If we converted the data to np.uint8 then back to float32 (or whatever it is here) would we still need the nearest check - i.e. could we use exact equality?

@Westacular

Westacular May 1, 2013

Contributor

Good idea, and it works like a charm. Doing the float->uint8->float32 conversion also simplifies your other suggestion down to data[data[:,:,3]==0] = 0, if data has already done the round trip.

@pelson

pelson May 1, 2013

Member

Amazing. That test looks much cleaner now - thanks for persevering with me 😄

@Westacular

Westacular May 1, 2013

Contributor

No problem, the test was much improved by your suggestions -- it's now much more explicit and exact about what the expected behaviours are.

Member

pelson commented May 1, 2013

Ah. Unfortunately the travis-ci instance has this test failing: https://s3.amazonaws.com/archive.travis-ci.org/jobs/6792238/log.txt
Surprisingly the numbers are quite different.

Contributor

Westacular commented May 1, 2013

That's because #1868 hasn't been merged. (This hits the same problem that that fixes.) On my test branch with both patches applied, the test passes.

Member

pelson commented May 1, 2013

That's because #1868 hasn't been merged.

Doh! Time to go home - its been a long day 😄

Contributor

Westacular commented May 2, 2013

Oh, oops. I think I half-botched a rebase of this branch, but the overall diff is still correct.

Member

pelson commented May 3, 2013

Oh, oops. I think I half-botched a rebase of this branch, but the overall diff is still correct.

I think we only need a single commit for this change, so would you mind going ahead and squashing your changes? if its more trouble than its worth, don't worry about it though.

Contributor

Westacular commented May 3, 2013

I think we only need a single commit for this change, so would you mind going ahead and squashing your changes?

Done (and managed to do it right this time.)

Owner

mdboom commented May 3, 2013

👍 -- @pelson: I leave it to you to push the green button since you've been following all this closer than I.

pelson added a commit that referenced this pull request May 3, 2013

Merge pull request #1956 from Westacular/imsave_alpha
Imsave now preserves the alpha channel.

@pelson pelson merged commit b6b9062 into matplotlib:master May 3, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment