-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Improve handling of alpha when saving to jpeg. #15437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporarily changing the figure background and manually calculating the alpha-shading towards white is more low-level. It's harder to understand what's going on and it might be more susceptible of introducing bugs in the future.
Is the performance gain worth it?
@@ -563,23 +563,26 @@ def print_jpg(self, filename_or_obj, *args, dryrun=False, pil_kwargs=None, | |||
`PIL.Image.save` when saving the figure. These take precedence | |||
over *quality*, *optimize* and *progressive*. | |||
""" | |||
FigureCanvasAgg.draw(self) | |||
if dryrun: | |||
return | |||
# The image is "pasted" onto a white background image to safely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment needs to be adapted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, this is still pasting the figure over a white background.
Actually, another motivation for the change is to make buffer_rgba() return an array that can be directly passed to imsave() -- essentially doing the same thing as in #15435, but for jpeg. |
@@ -563,23 +563,25 @@ def print_jpg(self, filename_or_obj, *args, dryrun=False, pil_kwargs=None, | |||
`PIL.Image.save` when saving the figure. These take precedence | |||
over *quality*, *optimize* and *progressive*. | |||
""" | |||
FigureCanvasAgg.draw(self) | |||
# The image is "pasted" onto a white background to handle transparency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the term "paste" is a bit misleading
# The image is "pasted" onto a white background to handle transparency. | |
# Remove transparency by alpha-blending on an assumed white background |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, sounds good, edited accordingly
We can compose the figure facecolor against a white background before rendering the image, rather than after. This saves an image composition step.
Will this cause weird flickering on interactive views (maybe not regular figures that composite on white, but embedded/notebook/something else)? |
I'm not sure why it would? |
I was thinking of stuff like changing the DPI, but I see setting the canvas manager is handled in the call stack above this one. |
We can compose the figure facecolor against a white background before
rendering the image, rather than after. This saves an image composition
step.
PR Summary
PR Checklist