-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: Restore hatch color tracking edge color #7976
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2210,14 +2210,14 @@ def alpha_cmd(self, alpha, forced, effective_alphas): | |
name = self.file.alphaState(effective_alphas) | ||
return [name, Op.setgstate] | ||
|
||
def hatch_cmd(self, hatch): | ||
def hatch_cmd(self, hatch, hatch_color): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain this extra argument instead of using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of the way the pdf backend deals with the fact that internally pdf's have a stack-based graphics context so when we change the gc in the draw methods, the It is possible that the pdf backend could be cleaned up, but that is a much bigger project. sorry this is rambley, but it took me a while to sort this out last night. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I understand… There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel the same way 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, this is made more complicated by how we implement hatching in pdf: it replaces the fill color with a named pattern. I think the backend_pdf changes are correct, and you added tests for the new functionality so we should be fine. |
||
if not hatch: | ||
if self._fillcolor is not None: | ||
return self.fillcolor_cmd(self._fillcolor) | ||
else: | ||
return [Name('DeviceRGB'), Op.setcolorspace_nonstroke] | ||
else: | ||
hatch_style = (self._hatch_color, self._fillcolor, hatch) | ||
hatch_style = (hatch_color, self._fillcolor, hatch) | ||
name = self.file.hatchPattern(hatch_style) | ||
return [Name('Pattern'), Op.setcolorspace_nonstroke, | ||
name, Op.setcolor_nonstroke] | ||
|
@@ -2281,7 +2281,8 @@ def clip_cmd(self, cliprect, clippath): | |
(('_linewidth',), linewidth_cmd), | ||
(('_dashes',), dash_cmd), | ||
(('_rgb',), rgb_cmd), | ||
(('_hatch',), hatch_cmd), # must come after fillcolor and rgb | ||
# must come after fillcolor and rgb | ||
(('_hatch', '_hatch_color'), hatch_cmd), | ||
) | ||
|
||
# TODO: _linestyle | ||
|
@@ -2312,7 +2313,7 @@ def delta(self, other): | |
break | ||
|
||
# Need to update hatching if we also updated fillcolor | ||
if params == ('_hatch',) and fill_performed: | ||
if params == ('_hatch', '_hatch_color') and fill_performed: | ||
different = True | ||
|
||
if different: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -136,6 +136,7 @@ def __init__(self, | |
self._linewidths = [0] | ||
self._is_filled = True # May be modified by set_facecolor(). | ||
|
||
self._hatch_color = mcolors.to_rgba(mpl.rcParams['hatch.color']) | ||
self.set_facecolor(facecolors) | ||
self.set_edgecolor(edgecolors) | ||
self.set_linewidth(linewidths) | ||
|
@@ -293,6 +294,12 @@ def draw(self, renderer): | |
|
||
if self._hatch: | ||
gc.set_hatch(self._hatch) | ||
try: | ||
gc.set_hatch_color(self._hatch_color) | ||
except AttributeError: | ||
# if we end up with a GC that does not have this method | ||
warnings.warn("Your backend does not support setting the " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you define There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is defined as a functional function it the base class, this is in case there are backends in the wild that do not derive from our base class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we suggest them to derive from our base class if they don't have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? On the draw method side we are ok duck-typing and the gc objects should belong to the backends so they should free to implement how ever they want. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just that it'd be an easy fix to get this working. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this particular case, it seems better practice than duck typing :) |
||
"hatch color.") | ||
|
||
if self.get_sketch_params() is not None: | ||
gc.set_sketch_params(*self.get_sketch_params()) | ||
|
@@ -690,12 +697,15 @@ def get_edgecolor(self): | |
get_edgecolors = get_edgecolor | ||
|
||
def _set_edgecolor(self, c): | ||
set_hatch_color = True | ||
if c is None: | ||
if (mpl.rcParams['patch.force_edgecolor'] or | ||
not self._is_filled or self._edge_default): | ||
c = mpl.rcParams['patch.edgecolor'] | ||
else: | ||
c = 'none' | ||
set_hatch_color = False | ||
|
||
self._is_stroked = True | ||
try: | ||
if c.lower() == 'none': | ||
|
@@ -710,6 +720,8 @@ def _set_edgecolor(self, c): | |
except AttributeError: | ||
pass | ||
self._edgecolors = mcolors.to_rgba_array(c, self._alpha) | ||
if set_hatch_color and len(self._edgecolors): | ||
self._hatch_color = tuple(self._edgecolors[0]) | ||
self.stale = True | ||
|
||
def set_edgecolor(self, c): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NelleV Added the 'please derive from us' verbiage here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you canceling by hand the old travis-ci tests or have we activated the new beta feature to cancel tests on PRs if a newer version exists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am canceling them by hand.