Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug with PatchCollection in PDF output #1860

Merged
merged 2 commits into from Mar 28, 2013

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Mar 27, 2013

The following script:

import numpy as np
np.random.seed(12345)

from matplotlib.patches import Circle
from matplotlib.collections import PatchCollection
import matplotlib.pyplot as plt

c = Circle((8, 8), radius=4, facecolor='none', edgecolor='green')
print(c.get_fill())
p = PatchCollection([c], match_original=True)

fig = plt.figure()
ax = fig.add_subplot(1,1,1)
ax.imshow(np.random.random((16,16)), interpolation='nearest')
ax.patch.set_color('0.5')
ax.add_collection(p)
fig.savefig('test.pdf')

Produces the following figure:

test

so it looks like there is something weird going on - the circle is cutting a hole through the image! This is with the latest dev version, but I think the bug affects 1.2.1 too.

@pelson
Copy link
Member

pelson commented Mar 27, 2013

Confirmed. I've updated the title since this is explicitly a Collection issue. Adding the patch to the axes doesn't have this problem (It works as expected).

@pelson
Copy link
Member

pelson commented Mar 27, 2013

Even more explicitly, I think the problem is with the match_original keyword to PathPatch. Changing the collection creation to:

p = PatchCollection([c], facecolor='none')

Also solves the problem.

@astrofrog
Copy link
Contributor Author

Thanks for the workaround!

In case anyone else has this issue, I found a slightly improved workaround, if one wants to keep all the other attributes (edgecolor, etc.) which is just to keep match_original=True and then call:

p.set_facecolor('none')

@pelson
Copy link
Member

pelson commented Mar 27, 2013

Interestingly with the pdf backend the line's alpha is bound to the alpha of the face color, i.e.:

p = PatchCollection([c], facecolors=((0, 0.1, 0.1, 0.1), ), edgecolor='green')

produces a circle with a green line, but the line has an alpha of 0.1. This is not the behaviour with Agg.

@jkseppan & @mdboom - I'm out of my depth now, any advice?

@pelson
Copy link
Member

pelson commented Mar 27, 2013

FWIW, changing the following helped remove the circle, though I don't fully understand the impacts:

--- a/lib/matplotlib/backend_bases.py
+++ b/lib/matplotlib/backend_bases.py
@@ -395,15 +396,15 @@ class RendererBase:
                 if Nlinestyles:
                     gc0.set_dashes(*linestyles[i % Nlinestyles])
             if rgbFace is not None and len(rgbFace) == 4:
-                if rgbFace[3] == 0:
-                    rgbFace = None
-                else:
+#                if rgbFace[3] == 0:
+#                    rgbFace = None
+#                else:
                     gc0.set_alpha(rgbFace[3])
                     rgbFace = rgbFace[:3]
             gc0.set_antialiased(antialiaseds[i % Naa])
             if Nurls:
                 gc0.set_url(urls[i % Nurls])

@WeatherGod
Copy link
Member

An interesting data-point, this bug does not happen in v1.1.1.

@mdboom
Copy link
Member

mdboom commented Mar 27, 2013

This arises because:

  1. Objects that are reused (which is what happens in a collection) in PDF must have their stroke and fill hard coded -- so there's no way to create an XObject which is sometimes filled and sometimes not.

  2. There is code in the PDF backend to determine whether the reused object it creates should be filled or not, but it ignores the alpha-values (created when fill is set to 'none').

  3. PDF doesn't consider alpha part of the color, but uses alpha pushing and popping, so we can't just "fill" it with an invisible color.

I'm not sure how to fix this yet, but I thought that might be illuminating.

There is a broader bug here which is that collections can not have different alphas for different instances of the same reused object.

Since 1.1.1 was working, I'll try a git bisect to see if anything simple pops up, but I suspect this is due to an important refactoring of the collections handling in PDF that otherwise had more problems than we have now...

@mdboom
Copy link
Member

mdboom commented Mar 27, 2013

FWIWI, git bisect points at 4dd3de1.

@astrofrog
Copy link
Contributor Author

The fact that this can be worked around by doing p.set_facecolor('none') on the PatchCollection suggests to me that it's not really a backend problem - or am I mistaken?

@mdboom
Copy link
Member

mdboom commented Mar 27, 2013

That workaround only works if all of the collection items are to have no fill. If the collection mixes filled and unfilled, we'd still have this problem.

…d correctly in the PDF backend. Also fixes a related bug that alpha could not be set on individual members of a collection.
@mdboom
Copy link
Member

mdboom commented Mar 27, 2013

I believe the attached patch fixes the issue. @astrofrog: Can you confirm?

@jkseppan
Copy link
Member

The patch by @mdboom looks good to me on a first read. The logic for handling transparency correctly seems to be pretty complicated. It's probably partially because the Agg and PDF rendering models are not quite the same, but I can't help wondering if there isn't something that we could do in the frontend to simplify it.

@mdboom
Copy link
Member

mdboom commented Mar 27, 2013

In a first pass, I tried to create a separate XObject for each combination of (path_shape, fill_alpha, stroke_alpha), and then use the correct one when generating the use object commands. However, that just got extremely complicated really fast, so I ultimately arrived at this which is to use XObjects and use when they are a natural fit to the data, and fall back to just drawing each path independently when not. It's suboptimal, for sure, but it seems like a pretty unusual case to support in the first place, and I decided to do the simplest thing that's less likely to be buggy than to be "clever" about it.

@dmcdougall
Copy link
Member

Out of interest, what does the output look like with a PDF generated with the cairo backend?

@mdboom
Copy link
Member

mdboom commented Mar 27, 2013

@dmcdougall : Good question. The Cairo output is correct, but that's mainly because it doesn't do any optimizations (reusing of path objects in a collection).

@astrofrog
Copy link
Contributor Author

@mdboom - this patch works for me - thanks for the quick fix!

@pelson
Copy link
Member

pelson commented Mar 28, 2013

👍 from me. Looks like a good fix with a good test. All that is missing is some documentation for the change 😉

@mdboom
Copy link
Member

mdboom commented Mar 28, 2013

@pelson: as you wish 😉

@pelson
Copy link
Member

pelson commented Mar 28, 2013

😄 thanks @mdboom

pelson added a commit that referenced this pull request Mar 28, 2013
Bug with PatchCollection in PDF output
@pelson pelson merged commit 372fd71 into matplotlib:v1.2.x Mar 28, 2013
HubertHolin pushed a commit to HubertHolin/matplotlib that referenced this pull request Apr 8, 2013
…d correctly in the PDF backend. Also fixes a related bug that alpha could not be set on individual members of a collection.
@mdboom mdboom deleted the collection-fill branch August 7, 2014 13:49
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.

None yet

6 participants