Fix #3647 [backport to 1.4.x] #3676
Conversation
| @@ -557,6 +557,9 @@ def draw(self, renderer): | |||
| self._set_gc_clip(gc) | |||
|
|
|||
| if self._bbox: | |||
| self._bbox.update(dict(clip_box=self.clipbox, | |||
There was a problem hiding this comment.
This should be done when the bounding box is created and when the various self.*clip* properties are done, not on every draw. This does fix the problem, but is rather inelegant.
There was a problem hiding this comment.
Actually I do not understand why this is inelegant, compared to tracking the clip values in the dictionary all the time. _bbox is not the Rectangle itself or something but just a dict.
There was a problem hiding this comment.
@tacaswell: I'd appreciate, if you comment on that.
There was a problem hiding this comment.
Sorry, this apparently fell off by radar.
If we are going to store equivalent state in two places we should not let them get out of sync with each other.
There was a problem hiding this comment.
What about making the combined dict a local variable, like
bbox_params = dict(self._bbox, clip_box=self.clipbox, ...
I'm somehow against syncing some state, that is internal and exactly determined by some other state.
|
What do you mean with "[backport to 1.4.x]"? It seems to me, that the difference between cairo and agg here is that clipping is reset with every set_clip_* in agg but in cairo set_clip_* is a noop when passed |
|
The PR is against master, not the maintenance branch. The backport to 1.4.x is a note to who ever merges this to cherry-pick the merge commit back to 1.4.x so that this bug will be fixed in the next bug-fix release (1.4.x), not just on the next 1.Y release. Can you provide line numbers? I am not familiar enough with this code (in either backend). cc @mdboom |
|
for cairo it is at backend_cairo.py#L354 |
|
From looking at the code, I would guess that the git blame says that change to svg was added in 58c0054 by JDH merging a patch from Michiel (which I am assuming is @mdehoon). The svg code was last touched in 2009, the backend_bases code was last touched in 2004(!). |
|
Of "svg", "pdf", "ps", "agg" and "cairo" only "cairo" does clip the bbox. |
|
Sorry, when I typed svg above I think I meant cairo. |
Add regression tests
|
This looks great! Thank you. For future reference, there is no notification when commits get pushed (or if there is, please tell me where that setting is ;) ), if you leave a note it will get the attention of who ever you have been talking to on the thread (I get emails for comments on issues I have commented in, and gh notifications for all mpl issues that get comments). For this bug, I think this is the correct solution (making sure clip information is propagated to all associated artists when it is set), however I think I will make a new issue re the inconsistent behavior of the |
BUG : Fix #3647, text boxes are not clipped
BUG : Fix #3647, text boxes are not clipped Conflicts: lib/matplotlib/tests/test_text.py cherry-pick picked up too many tests in test_text.py
|
backported as 76cd412 |
This fixes #3647, for me on the tkagg backend.
What still is strange is that the bug does not occur for e.g. cairo backend.