{{ message }}
FIX: round copy_from_bbox regions out to whole pixels - #32264
Open
larsoner wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

PR summary
I noticed some visual drift while working on
evoked.animate_topomapcode in MNE-python, whittled down to MWE (forced full-repaint exposes the full Agg canvas to show the bug more obviously):gives this for main, with just the copy-from-bbox fix, then with that plus the Qt fix:
Screencap movies
main
Screen.Recording.2026-08-30.at.07.53.48.mov
Just copy-to-bbox fix
Screen.Recording.2026-08-30.at.07.54.36.mov
copy-to-box plus Qt (this PR's state)
Screen.Recording.2026-08-30.at.08.17.19.mov
I then refactored the code into a shared helper, and searched for other places for likely truncation errors, and fixed those too (gtk3agg, wxagg, cairo). Hopefully everything is consistently applied across backends now 🤞 Happy to reduce scope if you want, this is a (perhaps overly optimistic) "let's try to get everything consistent at once" push!
Related to #8837 (sadly, doesn't close because that issue tracks a second bug about GUIs being "unable to render the image with a non-integer origin").
Similar class of fix that #14461 made in Tk, and refactors that code to use a shared helper (DRY) to perhaps/hopefully address @anntzer comment there:
AI Disclosure
Claude Opus 5 was used to examine the issue and draft the changes. I reviewed and iterated (a lot) with it on the code (DRY helper, scoping backends, interactively testing, verifying fixes, etc.), and believe I ended up with the changes I likely would have made if I sat down for many hours working on this alone.
PR quality check
Plotting related features are demonstrated in an example(too subtle I think, but MWE above)Documentation complies with general and docstring guidelines(not a doc change)