Skip to content

Fix PillowWriter GIF frame skew: frame_size must match the rendered RGBA buffer - #32189

Closed
fudianchn wants to merge 1 commit into
matplotlib:mainfrom
fudianchn:fix-pillowwriter-frame-size-rounding
Closed

Fix PillowWriter GIF frame skew: frame_size must match the rendered RGBA buffer#32189
fudianchn wants to merge 1 commit into
matplotlib:mainfrom
fudianchn:fix-pillowwriter-frame-size-rounding

Conversation

@fudianchn

@fudianchn fudianchn commented Aug 8, 2026

Copy link
Copy Markdown

Problem

Since 3.11.1, GIF frames written by animation.PillowWriter are skewed to the left (e.g. figsize (8.2, 7) at 100 dpi). #32186

AbstractMovieWriter.frame_size computes the frame dimensions with plain int(w * dpi) (truncation), but the RGBA buffer PillowWriter.grab_frame grabs via savefig(format="rgba") is sized by FigureCanvasBase.get_width_height, which—since #32038—rounds up when the value is within 1e-8 px of the integer:

return tuple(int(size / (1 if physical else self.device_pixel_ratio) + 1e-8)

For 8.2 in * 100 dpi = 819.9999999999999, frame_size truncates to 819 while the buffer is 820. PIL.Image.frombuffer("RGBA", (819, 700), buf, …) then reads the 820-wide rows as 819-wide and the image is skewed.

fig, ax = plt.subplots(figsize=(8.2, 7), dpi=100)
ax.plot([0, 1], [0, 1])
ani = animation.FuncAnimation(fig, lambda f: ax.plot([0, 1], [0, f]), frames=1, blit=False)
ani.save("test.gif", writer=animation.PillowWriter(fps=1), dpi=100)

Fix

Apply the same + 1e-8 rounding get_width_height(physical=True) uses, so frame_size always matches the buffer that gets grabbed. This is the “missed point to pass the corrected size through” noted in the bisect comment on the issue.

         w, h = self.fig.get_size_inches()
-        return int(w * self.dpi), int(h * self.dpi)
+        return int(w * self.dpi + 1e-8), int(h * self.dpi + 1e-8)

frame_size lives on AbstractMovieWriter, so every writer (ffmpeg/imagemagick via -s %dx%d, etc.) now also reports the corrected size consistently with the frames it writes.

Verification

Verified fail-before / pass-after on matplotlib 3.11.1:

before: frame_size=(819, 700)  rgba buffer=820×700  → skewed
after:  frame_size=(820, 700)  matches buffer        → correct

The rounding only changes the result when w * dpi is within 1e-8 below an integer (the bug case); for typical sizes (6@100, 8@72, 10.5@100, 5@100, 4.8@100) the value is identical, so there is no behavior change for non-affected figures.

Added a regression test asserting frame_size equals the width of the RGBA buffer produced by savefig(format="rgba").

Closes #32186

@fudianchn
Fix PillowWriter frame_size to match the rendered RGBA buffer
3c7d7a6 
AbstractMovieWriter.frame_size computed frame dimensions with plain
int(w * dpi), which truncates, while the RGBA buffer PillowWriter grabs via
savefig(format="rgba") is sized by FigureCanvasBase.get_width_height, which
rounds up when within 1e-8 px of the integer (since matplotlib#32038). For figure sizes
where w * dpi lands just below an integer (e.g. 8.2 in at 100 dpi =
819.9999...), frame_size was 819 while the buffer was 820, so
PIL.Image.frombuffer misread the rows and skewed the GIF frames.

Use the same 1e-8 rounding as get_width_height so frame_size always matches
the rendered buffer.

Closes matplotlib#32186
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process.

You can also join us on discourse chat for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@rcomer

rcomer commented Aug 9, 2026

Copy link
Copy Markdown
Member

This PR shows indications of having been generated by AI. Please check it against our policy and, if it complies, update the PR summary to use our template.
https://matplotlib.org/devdocs/devel/contribute.html#use-of-generative-ai

@rcomer rcomer added the status: autoclose candidate PRs that are not yet ready for review and may be automatically closed in two weeks label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

⏰ This pull request might be automatically closed in two weeks from now.

Thank you for your contribution to Matplotlib and for the effort you have put into this PR. This pull request does not yet meet the quality and clarity standards needed for an effective review. Project maintainers have limited time for code reviews, and our goal is to prioritize well-prepared contributions to keep Matplotlib maintainable.

Matplotlib maintainers cannot provide one-to-one guidance on this PR. However, if you ask focused, well-researched questions, a community member may be willing to help. 💬

To increase the chance of a productive review:

As the author, you are responsible for driving this PR, which entails doing necessary background research as well as presenting its context and your thought process. If you are a new contributor, or do not know how to fulfill these requirements, we recommend that you familiarize yourself with Matplotlib's development conventions or engage with the community via our Discourse or one of our meetings before submitting code.

If you substantially improve this PR within two weeks, leave a comment and a team member may remove the status: autoclose candidate label and the PR stays open. Cosmetic changes or incomplete fixes will not be sufficient. Maintainers will assess improvements on their own schedule. Please do not ping (@) maintainers.

@fudianchn

fudianchn commented Aug 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Hi @rcomer, thanks for the pointer.

To be transparent: I used AI assistance (Claude) when implementing the fix, and for translating and polishing my comments here into English (I'm not a native English speaker).

Reading the policy, I realize my broader pattern of contributing across many projects at the same time falls under "increasing breadth of contributions," which the policy specifically asks against. To be honest about the why: I subscribe to two 20× plans — Claude Code and Codex — because one on its own isn't enough for me, but two together is more than I can always fully use, so rather than let the surplus go to waste I wanted to put it toward community contributions. I work as a full-stack developer, so these technologies do fall within my stack, and I personally reviewed and tested each contribution — but that's still no excuse for spreading myself so thin and adding to your reviewer load. I'm sorry for the noise, and I'll close this PR.

For #32186, the fix is a one-liner — AbstractMovieWriter.frame_size should round with the same + 1e-8 that FigureCanvasBase.get_width_height uses, so the frame dimensions always match the RGBA buffer that gets grabbed. The bisect comment on the issue already points there.

Thanks for the clear feedback.

@fudianchn fudianchn closed this Aug 9, 2026
@ayshih

ayshih commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

For #32186, the fix is a one-liner — AbstractMovieWriter.frame_size should round with the same + 1e-8 that FigureCanvasBase.get_width_height uses, so the frame dimensions always match the RGBA buffer that gets grabbed. The bisect comment on the issue already points there.

Even though your suggested fix works, it's not the "best" way to fix it because AbstractMovieWriter instead ought to be grabbing the canvas size rather than recalculating it, which is a design consideration that AI might not immediately grasp. See #32194 for my alternative fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

first-contribution status: autoclose candidate PRs that are not yet ready for review and may be automatically closed in two weeks topic: animation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: GIF frames become skewed when using PillowWriter in matplotlib 3.11.1

3 participants

@fudianchn @rcomer @ayshih