8000 FIX: only apply alpha once for images by tacaswell · Pull Request #6551 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

FIX: only apply alpha once for images #6551

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

FIX: only apply alpha once for images #6551

wants to merge 1 commit into from

Conversation

tacaswell
Copy link
Member

The alpha of the image pixels is folded in as part of the image
rendering process, it does not need to be done a second time in the gc.

Fixes #6540

attn @mdboom

@tacaswell
FIX: only apply alpha once for images
ae3e8ed 
The alpha of the image pixels is folded in as part of the image
rendering process, it does not need to be done a second time in the gc.

Fixes matplotlib#6540
@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Jun 8, 2016
@tacaswell
Copy link
Member Author

I 'tested' by looking at png (via qtagg), svg and pdf and they all look ok.

@WeatherGod
Copy link
Member

Wouldn't it be better to include the supplied SSCCE as a test so that we
can detect this in the future?

On Tue, Jun 7, 2016 at 11:17 PM, Thomas A Caswell notifications@github.com
wrote:

I 'tested' by looking at png (via qtagg), svg and pdf and they all look ok.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#6551 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AARy-IZRPIWRlXhdBRjB8OdlapKRFz3Gks5qJjRDgaJpZM4Iwjrf
.

@jenshnielsen
Copy link
Member

@WeatherGod I think it already fails on a bunch of image alpha tests (on travis) following this change so they will need to be verified and updated before this can be merged.

@tacaswell
Copy link
Member Author

Sorry, I did not do my due-diligence on this PR and threw it up right before I went to bed last night.

@QuLogic
Copy link
Member
QuLogic commented Jun 22, 2016

Ping? Tests are failing here.

@tacaswell
Copy link
Member Author

This has issues (order is expected, actual, diff)

I think the pdf change is correct, the rest probably are not

pdf

image_alpha-expected_pdf
image_alpha_pdf
image_alpha_svg-failed-diff

svg

image_alpha-expected_svg
image_alpha_svg
image_alpha_svg-failed-diff

png

image_alpha-expected
image_alpha
image_alpha-failed-diff

@WeatherGod
Copy link
Member

pdf & svg now has the left panel with no alpha (when previously, they did
have alpha applied). png now has alpha applied only once for both panels
(previously they had alpha double-applied). Is that the expected result? At
least in this latest result, all of the right panels are now matching.

On Tue, Jun 21, 2016 at 9:50 PM, Thomas A Caswell notifications@github.com
wrote:

This has issues (order is expected, actual, diff)

I think the pdf change is correct, the rest probably are not

pdf

[image: image_alpha-expected_pdf]
https://cloud.githubusercontent.com/assets/199813/16252085/619eb478-37f9-11e6-9f30-a89f3295dd14.png
[image: image_alpha_pdf]
https://cloud.githubusercontent.com/assets/199813/16252086/619fa55e-37f9-11e6-80b7-4f3a116deec8.png
[image: image_alpha_svg-failed-diff]

https://cloud.githubusercontent.com/assets/199813/16252084/619df150-37f9-11e6-91d4-11591e4699e3.png

svg

[image: image_alpha-expected_svg]
https://cloud.githubusercontent.com/assets/199813/16252147/c965c9e8-37f9-11e6-837d-8e244427d074.png
[image: image_alpha_svg]
https://cloud.githubusercontent.com/assets/199813/16252153/d6e95a3a-37f9-11e6-8cc9-cab700937200.png
[image: image_alpha_svg-failed-diff]

https://cloud.githubusercontent.com/assets/199813/16252148/c9674020-37f9-11e6-9f55-ac077d80443d.png

png

[image: image_alpha-expected]
https://cloud.githubusercontent.com/assets/199813/16252159/e143b6a6-37f9-11e6-9a91-f2cfd1b5f13c.png
[image: image_alpha]
https://cloud.githubusercontent.com/assets/199813/16252160/e14414ac-37f9-11e6-9a60-4b8866e3d9a3.png
[image: image_alpha-failed-diff]
https://cloud.githubusercontent.com/assets/199813/16252161/e14675c6-37f9-11e6-87c9-a2286bec25e1.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#6551 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AARy-PMbol2JFgB_5MdzE2wr-WgTHKIkks5qOJT9gaJpZM4Iwjrf
.

@tacaswell
Copy link
Member Author

Everything should have alpha applied once!

@mdboom mdboom mentioned this pull request Jun 28, 2016
Merged
@tacaswell tacaswell closed this Jun 29, 2016
@tacaswell tacaswell deleted the fix_image_alpha branch June 29, 2016 02:54
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
topic: color/alpha
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@tacaswell @WeatherGod @jenshnielsen @QuLogic
0