Rastized background color

[tacaswell]

Addresses issue #2473

[mdboom]

This seems like a reasonable solution, given that we can't do alpha blending in postscript anyway. I wonder if there's a way to pass down the actual axes fill color and use that, though. We should also update the clear method itself in the same way.

[tacaswell]

Added a class variable so that the value isn't as hard coded. I didn't add a setter function because that seems like an API change to me which should go onto master once this is merged into 1.3.x and 1.3.x is merged into master.

[tacaswell]

I am confused by what travis is doing here. It complies fine on my box, but travis complains that the new class member does not exist in the scope.

[mdboom]

That's definitely better. But what I was getting at is that the axes background can be changed by the user, and maybe we should use that value. I think the easiest way is to probably add an optional argument to "clear" to pass in a color, and have the code that draws the quadmesh call clear explicitly first.

[tacaswell]

Do you want that added to this PR or a separate PR against master?

[mdboom]

Ideally added to this PR -- I think it all fits under the category of bugfix. But if it looks too convoluted to solve that way, I think this PR is an obvious improvement over the current state of affairs.

[tacaswell]

Sorry for going quiet.

I am entering the home stretch of my PhD (defense is scheduled for December). I needed this fixed for some of my figures, but I don't want to commit to adding much else.

[pelson]

Why was this value changed from (0, 0, 0, 0) to (1, 1, 1, 0)? This was a change I introduced in d8fce7b which I'm surprised is not triggering some of the tests I added to fail. Would you mind providing some more detail in the description of the PR please?

[pelson]

I'd like to put the brakes on this PR as I have a few concerns, mainly with regards to the modification of expected behaviour (the test which was changed). I think we need some more detail as to the motivation for the change (please update the description of the PR) - only then is it feasible to agree on wether this is a bug fix, or a feature enhancement, and indeed whether this change is desirable. Right now I'm undecided and very much open minded on this PR.

PRs of interest to this: #1899, #1954, #1868. Pinging @Westacular as someone who got their hands really dirty on the Agg colour handling - your comments would be very welcome here.

Finally, just because I'm not 100% sold on this PR @tacaswell, it doesn't mean I don't appreciate how much effort has been put into this PR, so thank you!

Cheers,

[tacaswell]

@pelson this addresses issue #2473 which has code to reproduce the problem I am trying to address.

It looks like the agg backend renders the background as (0, 0, 0, 0) (transparent black). This work fine for anything which can do alpha-blending, however eps can not, so if you rasterize selected artistist which do not fully fill their bounding box you get a black background filling the rest of the bounding box. By changing the color to (1, 1, 1, 0) (transparent white) I don't think think that backends which can do alpha blending will care (as transparent is transparent), but degrades more gracefully in cases when the backend can't deal with alpha.

The change to the test is needed because it is testing the array returned. I don't think that there is any visible change in the pngs.

That said, I have only on a few occasions dug down into the Agg rendering so am doing a bit of cargo-cult programming here. I needed issue #2473 fixed for some figures in my thesis and took what I thought was the most direct route, but make no claim it is the best route.

[pelson]

Thanks for the update @tacaswell. I see your motivation for this change now and @mdboom's comments make a lot more sense to me 😄
Obviously this change fixes the situation for a large number of cases, but on the other hand, it is not the correct solution and pretending that eps' major limitation doesn't exist can also cause problems. For instance, the eps and png output from your modified code are significantly different:

from pylab import *

figure(facecolor='red')
ax = axes()
ax.patch.set_facecolor('none')

plt.plot(0.8, 2.5, 'ob', zorder=-10)
X, Y = meshgrid(linspace(0, 1, 2048), linspace(0, 1, 1024))
Y *= 2 + sin(linspace(0, 2*np.pi, 2048))
C = np.sin(X + Y)
pcolormesh(X, Y, C, rasterized=True)

savefig('test.eps', facecolor='red')
show()

Whilst I don't have a problem with this change, the correct fix is much harder (and in general, may not be possible without adding transparency to eps files). My current thoughts are two approaches:

• render everything below the rasterized object to Agg as well as the eps backend (not a go-er IMHO)
• use clip paths on the rasterized image to clip out transparent pixels (I think this is doable, but probably hard)
• I've not tried it, but apparently there is a plugin for ghostscript which does add alpha to eps files (couldn't for the life of me find the link) - perhaps that is one way to go also
• Don't use eps - I'm guessing its the publisher insisting on eps files, right? Would PDF do?

👍 for merging this, but acknowledging that it isn't the "correct" solution.

Thanks @tacaswell

[mdboom]

@pelson: I agree with your summary, i.e., this is not a complete solution, but I think it's "less bad" than the status quo.

FWIW, Cairo (itself, not our Cairo backend) deals with alpha transparency in PS exactly as you describe in your first bullet: it rasterizes everything up to and including transparent objects. The cairo-based tool "pdftocairo" will do this as well. I'd suggest that that is not a terrible solution to point users to -- either using our Cairo backend or outputting PDF and converting with pdftocairo. This, of course, assumes a Unixy environment where it's easy to install cairo.

Your second suggestion is interesting -- in the general case, you could end up with really large clip paths, I suppose.

[tacaswell]

@pelson That is true, but without these changes, the white region would just be replaced with a black region and it is still just as broken. There are cases (such as what drove me to do this) where a white region is less broken than a black region.

As a side note, is there an easy way for me to change an issue into a PR? That would have lead to less confusion in this case.

[pelson]

There are cases where a white region is less broken than a black region.

That is precisely what I meant with the statements: Whilst I don't have a problem with this change, the correct fix is much harder ... :+1: for merging this, but acknowledging that it isn't the "correct" solution.

[tacaswell]

Sorry, just making sure every one was on the same page.

[tacaswell]

@mdboom Can this be cherry picked to master? (should I just do it?)

[pelson]

This will get merged back to master by hand at some point - @mdboom and myself do this fairly regularly. Would it be helpful to you for me to do that now?

[tacaswell]

Yes. I need this along with a few other branches on master to generate the figures for my thesis. It would make my life easier if this were in master

[pelson]

Ok @tacaswell - hopefully that will make life easier for you. Commit to master: 06d0144

[tacaswell]

thanks

[letmaik]

Sorry for reviving, but I just stumbled upon the same issue and would need a black clearing color. It's really a shame that it cannot be set manually from Python world. I tried calling fig._cachedRenderer._renderer.clear(..) as a quick hack but this needs a agg::rgba which I have no idea how to produce from Python.

[tacaswell]

@neothemachine Sorry about breaking things for you! iirc there was discussion of being able to pass this value through, could you make a new issue to make sure it does not get lost.