PR Summary

This PR seeks to make the failure modes more consistent and manageable when blitting on theTkAgg backend. Draft for now because I don't think it is release critical and some feedback is needed on design choices.

Background

The thread safety changes of#18565 interact a bit awkwardly with the memory safety changes of#14478, namely when_tkagg.blit raises, it pops directly out of the tkintermainloop call and poisons the current instance of tkapp so that pumping the event loop always raises theValueError forever. This isn't a big deal for bad values ofcomp_rule orphotoimage since those are matplotlib's responsibilities, however as a user-facing API this prevents a client application from gracefully handling bad bboxes; they are basically fatal.

Why, if this is so bad, did nobody notice? In#14461 the bbox limits are clipped to a mostly-safe range, so onlyvery bad bboxes that fall entirely outside the canvas trigger the issue. (And only my app is crazy enough to generate such bboxes 🤦)

Proposed fix(es)

Extend safe-clipping

The initial commit in this PR makes the most minimal possible change to existing behavior, extending the safe-clipping to boxes outside the canvas; they are simply dropped, and with a fast path optimization to boot. This way ifValueErrors are generated, they are 100% our fault. Theif expression comes from thecheck in_tkagg.cpp but stripping the rules that are redundant to the clipping logic. The disadvantage is that the only way a user knows that their bbox is being clipped is by weird visual artifacts.

Raise aggressively

An alternative fix is to promote the_tkagg.cpp check into the first lines of_backend_tk.blit:

defblit(photoimage,aggimage,offsets,bbox=None):    ...ifbboxisnotNone:# Does anyone know why we are calling __array__ directly?        (x1,y1), (x2,y2)=bbox.__array__().astype(int).tolist()if (0>y1ory1>y2ory2>heightor0>x1orx1>x2orx2>width):raiseValueError("Attempting to draw out of bounds")bboxptr= (x1,x2,y1,y2)comp_rule=TK_PHOTO_COMPOSITE_OVERLAY    ...

Thisraise is now catchable and the check inside_tkagg.blit is redundant (though it is super cheap so it should remain in both places). Adding tests for this would finally cover this branch of the if statement. Also ALL bad bbox blits are errors now.

I see two disadvantages to this alternative. The integer conversion has to be different (truncated) so that full-canvas blits are not liable to pick up an extra pixel and error out, however this floor rounding behavior is consistent with other backends. Some user code that worked by silently clipping will now raise.

Open questions

Should we consider this alternative option as exposing latent bugs that should have been caught before, or as makingblit "less robust"? Should we reconsider the current rounding/clipping rules? How strict are other backends to weird bboxes and can we make things consistent across all of them?

PR Checklist

• Has pytest style unit tests (andpytest passes).
• IsFlake 8 compliant (runflake8 on changed files to check).
• New features are documented, with examples if plot related.
• Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
• Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
• New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
• API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).