Possible fix for hatching problems inside legends (PDF backend) #4474
Conversation
|
attn @jkseppan |
| @@ -2301,7 +2302,13 @@ def delta(self, other): | |||
| if different: | |||
| break | |||
|
|
|||
| # Need to update hatching if we also updated fillcolor | |||
| if params == ('_hatch',) and fill_performed: | |||
There was a problem hiding this comment.
To check the logic, it only matter if the last command was filled so the fact that the first time through is always False is ok?
There was a problem hiding this comment.
I believe so. The writeHatches function seems to independently change the fill color (line 1189), so requires updating the hatching if the fill color is modified.
tacaswell
added
the
Release critical
|
The change looks fine to me, but the whole PDF hatching (originally implemented by me) is somewhat brittle and should probably be redone by turning the fill area into a temporary clip mask and drawing the hatch pattern over the whole area. This pull request adds a test for the fixed behaviour, so it should survive any future changes. Could the test be extended to other backends as well? I'm not entirely sure if the interaction between hatching and edge/fill colors is well defined, so a test across all relevant backends would help make sure it means the same thing everywhere. |
|
Good point on the other backends, that this fix didn't break anything means this code path is not tested anywhere else. |
|
I am going to go ahead and merge this as-is. @jpallister If you are feeling ambitious extending your test to the other backends in a PR would be great. |
BUG: fix for hatching problems inside PDF legends
This commit fixes the issue #4469, but I don't know whether this is the correct way to fix the problem.