Skip to content

Bug with PatchCollection in PDF output#1860

Merged
pelson merged 2 commits into
matplotlib:v1.2.xfrom
mdboom:collection-fill
Mar 28, 2013
Merged

Bug with PatchCollection in PDF output#1860
pelson merged 2 commits into
matplotlib:v1.2.xfrom
mdboom:collection-fill

Conversation

@mdboom

@mdboom mdboom commented Mar 27, 2013

Copy link
Copy Markdown
Member

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

pelson commented Mar 27, 2013

Copy link
Copy Markdown
Member

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

pelson commented Mar 27, 2013

Copy link
Copy Markdown
Member

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
Copy Markdown
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

pelson commented Mar 27, 2013

Copy link
Copy Markdown
Member

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

pelson commented Mar 27, 2013

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

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

@mdboom

mdboom commented Mar 27, 2013

Copy link
Copy Markdown
Member

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

mdboom commented Mar 27, 2013

Copy link
Copy Markdown
Member

FWIWI, git bisect points at 4dd3de1.

@astrofrog

Copy link
Copy Markdown
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

mdboom commented Mar 27, 2013

Copy link
Copy Markdown
Member

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

mdboom commented Mar 27, 2013

Copy link
Copy Markdown
Member

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

@jkseppan

Copy link
Copy Markdown
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

mdboom commented Mar 27, 2013

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

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

@mdboom

mdboom commented Mar 27, 2013

Copy link
Copy Markdown
Member

@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
Copy Markdown
Contributor Author

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

@pelson

pelson commented Mar 28, 2013

Copy link
Copy Markdown
Member

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

@mdboom

mdboom commented Mar 28, 2013

Copy link
Copy Markdown
Member

@pelson: as you wish 😉

@pelson

pelson commented Mar 28, 2013

Copy link
Copy Markdown
Member

😄 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.

6 participants