FIX: CL avoid fully collapsed axes #11627
Conversation
|
This is minor, and a safety catch, so marking RC for 3.0.... |
|
Would it make sense, and be reasonably easy, to disable the bbox and constraint calculations when zoom-to-rect or zoom/pan are on? |
|
Yeah if there is a method to detect that’s what the redraw is for, I think that’d be the safest thing most of the time. Is it easy to know that the redraw was triggered for that reason? EDIT: done - easier than I though... |
| @@ -199,15 +207,27 @@ def do_constrained_layout(fig, renderer, h_pad, w_pad, | |||
|
|
|||
| fig._layoutbox.constrained_layout_called += 1 | |||
| fig._layoutbox.update_variables() | |||
| # Now set the position of the axes... | |||
| # check if layout worked. If not, don't change positions: | |||
There was a problem hiding this comment.
You could move this whole check to a local function layout_is_valid(fig) and then just:
if layout_is_valid(fig):
# Now set the position of the axes...
...
or even with a second function apply_layout(fig)
if layout_is_valid(fig):
apply_layout(fig):
else:
warnings.warn(...)
I think this would help keeping an overview what's going on.
There was a problem hiding this comment.
I guess this is a taste thing. 6 lines is kind of under my "move to its own function" threshold. When debugging I find it pretty annoying to have to chase down chains of three-line rabbit holes that do something trivial.
There was a problem hiding this comment.
You are height that this is a matter of taste
My rule of thumb is: A couple of lines that have a distinguished purpose usually make up a good function. Even more so, if I need a comment to explain what they are doing. The function name can often replace the comment.
There was a problem hiding this comment.
... done - agree that is a bit nicer...
7034418 to
362c34c
Compare
362c34c to
6d8c9c1
Compare
PR Summary
Applying zoom to an axes with constrained_layout caused problems because the bounding box for a line on the axes was not correctly calculated when constrained_layout needed it to be. That could lead to a collapsed axes, and singular matrices for other transformations.
This PR doesn't fix the underlying problem with the zoom not setting the transform quick enough, but since layout probably doesn't need to change much under zoom, that's likely OK. However, it does stop constrained layout from being applied if any of the axes have zero width or height.
Note this was not a problem before #10682, because now
ax.get_tightbboxchecks all the artists in the axes.The only drawback I can see is if someone deliberately had a zero-width/height axes on their figure. But I don't think constrained_layout would work well in that case anyways.
UPDATE: Per @efiring suggestion, I've also turned constrained_layout off if 'PAN' or 'ZOOM' are active. I think this is much safer.
PR Checklist