Skip to content

Factor out common code between Patch.draw and FancyArrowPatch.draw. #11741

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 21, 2018
Merged

Factor out common code between Patch.draw and FancyArrowPatch.draw. #11741

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 51 additions & 71 deletions lib/matplotlib/patches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import contextlib
import functools
import math
from numbers import Number
Expand Down Expand Up @@ -481,11 +482,16 @@ def get_hatch(self):
'Return the current hatching pattern'
return self._hatch

@artist.allow_rasterization
def draw(self, renderer):
'Draw the :class:`Patch` to the given *renderer*.'
if not self.get_visible():
return
@contextlib.contextmanager
def _bind_draw_path_function(self, renderer):
"""
``draw()`` helper factored out for sharing with `FancyArrowPatch`.

Yields a callable ``dp`` such that calling ``dp(*args, **kwargs)`` is
equivalent to calling ``renderer1.draw_path(gc, *args, **kwargs)``
where ``renderer1`` and ``gc`` have been suitably set from ``renderer``
and the artist's properties.
"""

renderer.open_group('patch', self.get_gid())
gc = renderer.new_gc()
Expand All @@ -496,7 +502,7 @@ def draw(self, renderer):
if self._edgecolor[3] == 0:
lw = 0
gc.set_linewidth(lw)
gc.set_dashes(0, self._dashes)
gc.set_dashes(self._dashoffset, self._dashes)
gc.set_capstyle(self._capstyle)
gc.set_joinstyle(self._joinstyle)

Expand Down Expand Up @@ -525,21 +531,39 @@ def draw(self, renderer):
if self.get_sketch_params() is not None:
gc.set_sketch_params(*self.get_sketch_params())

path = self.get_path()
transform = self.get_transform()
tpath = transform.transform_path_non_affine(path)
affine = transform.get_affine()

if self.get_path_effects():
from matplotlib.patheffects import PathEffectRenderer
renderer = PathEffectRenderer(self.get_path_effects(), renderer)

renderer.draw_path(gc, tpath, affine, rgbFace)
# In `with _bind_draw_path_function(renderer) as draw_path: ...`
# (in the implementations of `draw()` below), calls to `draw_path(...)`
# will occur as if they took place here with `gc` inserted as
# additional first argument.
yield functools.partial(renderer.draw_path, gc)

gc.restore()
renderer.close_group('patch')
self.stale = False

@artist.allow_rasterization
def draw(self, renderer):
'Draw the :class:`Patch` to the given *renderer*.'
if not self.get_visible():
return

# Patch has traditionally ignored the dashoffset.
with cbook._setattr_cm(self, _dashoffset=0), \
self._bind_draw_path_function(renderer) as draw_path:
path = self.get_path()
transform = self.get_transform()
tpath = transform.transform_path_non_affine(path)
affine = transform.get_affine()
draw_path(tpath, affine,
# Work around a bug in the PDF and SVG renderers, which
# do not draw the hatches if the facecolor is fully
# transparent, but do if it is None.
self._facecolor if self._facecolor[3] else None)

def get_path(self):
"""
Return the path of this patch
Expand Down Expand Up @@ -4263,69 +4287,25 @@ def draw(self, renderer):
if not self.get_visible():
return

renderer.open_group('patch', self.get_gid())
gc = renderer.new_gc()

gc.set_foreground(self._edgecolor, isRGBA=True)

lw = self._linewidth
if self._edgecolor[3] == 0:
lw = 0
gc.set_linewidth(lw)
gc.set_dashes(self._dashoffset, self._dashes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The context uses gc.set_dashes(0, self._dashes) instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I had missed that. I restored the old behavior using setattr_cm (in Patch.draw), leaving a comment similar to the one for the capstyle in FancyArrowPatch.


gc.set_antialiased(self._antialiased)
self._set_gc_clip(gc)
gc.set_capstyle('round')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The context uses gc.set_capstyle(self._capstyle) instead. This is 'butt' by default (inherited from Patch). It's reasonable to draw the value from self._capstyle. Should the default for this value be 'round' to have backward compatibility or do you want to change it intentionally to be the same default as for Patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forced it to "round" above (in the setattr_cm). Yes, ignoring the capstyle was probably a long-standing bug (the kind of things that such refactors uncover...) but changing that would result in changes in the baseline images (... that's how I realized the discrepancy). I'll leave this as it is until someone actually complains or we have some othe reason to change the baseline images.

gc.set_snap(self.get_snap())
# FancyArrowPatch has traditionally forced the capstyle and joinstyle.
with cbook._setattr_cm(self, _capstyle='round', _joinstyle='round'), \
self._bind_draw_path_function(renderer) as draw_path:

rgbFace = self._facecolor
if rgbFace[3] == 0:
rgbFace = None # (some?) renderers expect this as no-fill signal
# FIXME : dpi_cor is for the dpi-dependecy of the linewidth. There
# could be room for improvement.
self.set_dpi_cor(renderer.points_to_pixels(1.))
path, fillable = self.get_path_in_displaycoord()

gc.set_alpha(self._alpha)
if not cbook.iterable(fillable):
path = [path]
fillable = [fillable]

if self._hatch:
gc.set_hatch(self._hatch)
if self._hatch_color is not None:
try:
gc.set_hatch_color(self._hatch_color)
except AttributeError:
# if we end up with a GC that does not have this method
cbook.warn_deprecated(
"3.1", "Your backend does not support setting the "
"hatch color; such backends will become unsupported "
"in Matplotlib 3.3.")
affine = transforms.IdentityTransform()

if self.get_sketch_params() is not None:
gc.set_sketch_params(*self.get_sketch_params())

# FIXME : dpi_cor is for the dpi-dependecy of the
# linewidth. There could be room for improvement.
#
# dpi_cor = renderer.points_to_pixels(1.)
self.set_dpi_cor(renderer.points_to_pixels(1.))
path, fillable = self.get_path_in_displaycoord()

if not cbook.iterable(fillable):
path = [path]
fillable = [fillable]

affine = transforms.IdentityTransform()

if self.get_path_effects():
from matplotlib.patheffects import PathEffectRenderer
renderer = PathEffectRenderer(self.get_path_effects(), renderer)

for p, f in zip(path, fillable):
if f:
renderer.draw_path(gc, p, affine, rgbFace)
else:
renderer.draw_path(gc, p, affine, None)

gc.restore()
renderer.close_group('patch')
self.stale = False
for p, f in zip(path, fillable):
draw_path(
p, affine,
self._facecolor if f and self._facecolor[3] else None)


class ConnectionPatch(FancyArrowPatch):
Expand Down