Properly handle transparency for animations #5415

cbreeden · 2015-11-06T05:04:40Z

This PR is a first attempt at fixing #5335. I have added a flatten kwarg for savefig in the agg backend that will allow animation.py remove all transparency before sending it to its respective animator. I don't think it would be a good idea to merge this just yet, but was looking for feedback before proceeding.

I guess I would still need to write a test. I'm not sure I handled the dimensions correctly for _write_png, animations look correct from write='imagemagick_file but that's definitely worth double checking.

jenshnielsen · 2015-11-06T09:30:17Z

The fix for jpeg images in #5324 may be useful

mdboom · 2015-11-06T12:50:23Z

lib/matplotlib/backends/backend_agg.py

+                img = self.flatten_rgba(img)
+                _png.write_png(img, filename_or_obj, self.figure.dpi)
+            else:
+                _png.write_png(renderer._renderer, filename_or_obj, self.figure.dpi)


Minor point, but I think I like the way you organized the code in print_raw better than here: i.e. set a local variable img to the render contents and then optionally flatten it, ultimately passing img into write_png on a single line.

@mdboom I really wanted to do that at first, but I will be honest when I say that I didn't understand what was going on here:

matplotlib/src/_backend_agg.cpp

Line 28 in 0037595

RendererAgg::RendererAgg(unsigned int width, unsigned int height, double dpi)

. I wasn't exactly sure if write_png needed something from that or not.

No -- it shouldn't matter. _write_png just converts whatever is passed in to a numpy array.

mdboom · 2015-11-06T12:52:02Z

This looks good. It does mean we now have two ways to flatten images. This way is more general, though, as it doesn't rely on PIL. Can you modify print_jpeg to call out to the new flatten_rgba function?

Also, flatten_rgba should probably live in image.py since I anticipate it will be useful to other non-Agg backends in the future.

WeatherGod · 2015-11-06T14:41:48Z

I wonder how much this overlaps with the image.combine_image feature? I
know it isn't the same, but it would be good to keep similar concepts as
compact as possible.

On Fri, Nov 6, 2015 at 7:52 AM, Michael Droettboom <notifications@github.com

wrote:

This looks good. It does mean we now have two ways to flatten images. This
way is more general, though, as it doesn't rely on PIL. Can you modify
print_jpeg to call out to the new flatten_rgba function?

Also, flatten_rgba should probably live in image.py since I anticipate it
will be useful to other non-Agg backends in the future.

—
Reply to this email directly or view it on GitHub
#5415 (comment)
.

WeatherGod · 2015-11-06T14:42:08Z

Sorry, that is image.composite_image.

On Fri, Nov 6, 2015 at 9:41 AM, Benjamin Root ben.v.root@gmail.com wrote:

I wonder how much this overlaps with the image.combine_image feature? I
know it isn't the same, but it would be good to keep similar concepts as
compact as possible.

On Fri, Nov 6, 2015 at 7:52 AM, Michael Droettboom <
notifications@github.com> wrote:

This looks good. It does mean we now have two ways to flatten images.
This way is more general, though, as it doesn't rely on PIL. Can you modify
print_jpeg to call out to the new flatten_rgba function?

Also, flatten_rgba should probably live in image.py since I anticipate
it will be useful to other non-Agg backends in the future.

—
Reply to this email directly or view it on GitHub
#5415 (comment)
.

mdboom · 2015-11-06T15:00:15Z

I wonder how much this overlaps with the image.combine_image feature? I know it isn't the same, but it would be good to keep similar concepts as compact as possible.

That's a good point. I think the image compositing stuff is bound to be slower than this because it's not on a solid color, but you're right there is some functionality overlap there.

cbreeden · 2015-11-06T15:31:34Z

Just for reference, I believe the core of the image.composite_image code lives here:

matplotlib/lib/matplotlib/figure.py

Line 1095 in 0037595

else:

. I'm not sure if reusing that code would be beneficial or not. I will admit to not having known about the image.composite_image rcParam (I guess because this is new to 1.5?), and the description is given by:

Whether a vector graphics backend should composite several images
into a single image or not when saving. Useful when needing to edit the 
files further in Inkscape or other programs.

Let me just reiterate the issue that this PR is trying to correct. In some backends, transparency in the figures are enabled by default (nbAgg does this with and can be modified with nbagg.transparent). Matplotlib will then send images with an 8-bit alpha channel to their respective animators (sometimes using png as an intermediate file format). These animators are then forced to flatten the alpha channels and seem to get confused by either just truncating the alpha channel all together, or by picking some kind of threshold and setting the alpha to Full or None in the case of formats that handle boolean transparency (like gif).

I'm not sure it is worth having a configurable setting for turning on and off the flattening of RGBA before handing it off to animators. If clear_temp was accessible (I think I had to modify it from source) in the FileWrites then it might be beneficial to open the flatten keyword to the public in case someone really only wants each individual file for further processing.

cbreeden · 2015-11-06T17:00:56Z

whoops. Tried to --amend the 1759556 commit, but obviously it didn't work out the way I planned.

tacaswell · 2015-11-07T04:24:50Z

@cbreeden We seem to go through the pep8 discussion with every new contributor 😉 . It really does help on the large code base to have a more-or-less uniform style.

Install an auto-linter in your editor and it becomes much less annoying.

tacaswell · 2015-11-07T04:32:41Z

Can you also re-base to remove that merge commit?

cbreeden · 2015-11-07T14:17:59Z

lib/matplotlib/backends/backend_agg.py

+        # Flatten RGBA if used with fileformat that doesn't handle trnasparency
+        if kwargs.get('flatten', False):
+            w, h = int(renderer.width), int(renderer.height)
+            img = np.array(memoryview(img)).reshape((w, h, 4))


Can someone verify that this is the correct reshape? For png I needed to use (h, w, 4) to avoid scanning line offsets. The images look correct.

I think it should be (h, w, 4).

cbreeden · 2015-11-07T15:18:13Z

Pending the Travis CI tests, I think I'm done here. Unless someone wants me to add documentation for the added flatten flag for savefig in the nbagg for png. It might also be worth looking into other backends. I had this comment from stackoverflow (http://stackoverflow.com/questions/33283274/pixelated-fonts-when-plot-is-saved-as-jpeg) when first running into this problem:

I seem to get the same results as you with nbagg, but with Agg my jpg looks much better. With the MacOSX backend both the jpg and the tiff have the pixellated fonts – tom

It might be related, but I don't have a Mac to test.

cbreeden · 2015-11-07T19:08:36Z

Not sure why the build failed. I'm quite certain nothing I did should impact anything that test did. I tried running the code on my local python (3.4) and test_savefig_to_stringio worked fine. I think Travis CI just hasn't had its coffee yet.

jenshnielsen · 2015-11-11T15:55:21Z

@cbreeden about Travis you are right these failures are annoying but intermittent and not related to your work

tacaswell · 2015-11-24T05:08:20Z

@cbreeden This sadly needs a re-base now 😞

…w to properly handle transparency in animations

…rgba

Fix (h, w) consistency

cbreeden · 2015-11-25T02:02:06Z

@tacaswell no worries. I am quite new to git. In fact, I only picked it up so I could try to make this patch. Now I got the chance to handle my first rebase conflict; and I think I did it correctly, even 😥

tacaswell · 2016-07-16T20:36:55Z

@cbreeden Sorry this fell off my radar.

Could you rebase this again?

jklymak · 2018-08-16T21:55:14Z

I'm not seeing the original issue in nbagg, and I know some work was done to fix the transparency issues recently. I'm going to close as abandoned but if there is important work in here, please feel free to re-open....

tacaswell added the status: needs review label Nov 6, 2015

mdboom reviewed Nov 6, 2015
View reviewed changes

cbreeden force-pushed the transparent-animations branch 2 times, most recently from c693df6 to 5bb8754 Compare November 7, 2015 14:14

cbreeden reviewed Nov 7, 2015
View reviewed changes

cbreeden force-pushed the transparent-animations branch 2 times, most recently from 5af94d2 to f0e2617 Compare November 7, 2015 15:06

cbreeden force-pushed the transparent-animations branch from f0e2617 to f9140ec Compare November 7, 2015 15:31

cbreeden added 7 commits November 24, 2015 19:42

Add 'flatten' kwarg to backend_agg savefig for print_png and print_ra…

30c2a09

…w to properly handle transparency in animations

Fix width/height locations, and forgot to add alpha layer to flatten_…

b8175a1

…rgba

Move flatten_rgba to image.py and implment flatten_rgba for jpeg files.

2fa1889

Removed typo

e0f1fd8

Clarify comment

d5fbd89

Appease the PEP8 gods. Somewhat of a sacraficial lamb if you ask me.

a57be1f

Add tests for flatten kwarg

0193d55

Fix (h, w) consistency

Fix typo in tests

b2c4560

cbreeden force-pushed the transparent-animations branch from 05721b1 to b2c4560 Compare November 25, 2015 01:55

tacaswell added this to the 2.0.1 (next bug fix release) milestone Jul 16, 2016

tacaswell self-assigned this Jul 16, 2016

story645 added the topic: animation label Jul 20, 2016

dstansby added the status: needs revision label Jan 29, 2017

QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0.2 (next bug fix release) May 3, 2017

tacaswell modified the milestones: 2.1 (next point release), 2.0.3 (next bug fix release) May 31, 2017

tacaswell modified the milestone: 2.1 (next point release) Aug 29, 2017

jklymak closed this Aug 16, 2018

jklymak removed status: needs review status: needs revision labels Aug 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Properly handle transparency for animations #5415

Properly handle transparency for animations #5415

cbreeden commented Nov 6, 2015

jenshnielsen commented Nov 6, 2015

mdboom Nov 6, 2015

cbreeden Nov 6, 2015

mdboom Nov 6, 2015

mdboom commented Nov 6, 2015

WeatherGod commented Nov 6, 2015

WeatherGod commented Nov 6, 2015

mdboom commented Nov 6, 2015

cbreeden commented Nov 6, 2015

cbreeden commented Nov 6, 2015

tacaswell commented Nov 7, 2015

tacaswell commented Nov 7, 2015

cbreeden Nov 7, 2015

mdboom Nov 7, 2015

cbreeden commented Nov 7, 2015

cbreeden commented Nov 7, 2015

jenshnielsen commented Nov 11, 2015

tacaswell commented Nov 24, 2015

cbreeden commented Nov 25, 2015

tacaswell commented Jul 16, 2016

jklymak commented Aug 16, 2018

Properly handle transparency for animations #5415

Properly handle transparency for animations #5415

Conversation

cbreeden commented Nov 6, 2015

jenshnielsen commented Nov 6, 2015

mdboom Nov 6, 2015

Choose a reason for hiding this comment

cbreeden Nov 6, 2015

Choose a reason for hiding this comment

mdboom Nov 6, 2015

Choose a reason for hiding this comment

mdboom commented Nov 6, 2015

WeatherGod commented Nov 6, 2015

WeatherGod commented Nov 6, 2015

mdboom commented Nov 6, 2015

cbreeden commented Nov 6, 2015

cbreeden commented Nov 6, 2015

tacaswell commented Nov 7, 2015

tacaswell commented Nov 7, 2015

cbreeden Nov 7, 2015

Choose a reason for hiding this comment

mdboom Nov 7, 2015

Choose a reason for hiding this comment

cbreeden commented Nov 7, 2015

cbreeden commented Nov 7, 2015

jenshnielsen commented Nov 11, 2015

tacaswell commented Nov 24, 2015

cbreeden commented Nov 25, 2015

tacaswell commented Jul 16, 2016

jklymak commented Aug 16, 2018