Skip to content
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

Add notSkipAlphaChannel options. #136

Merged
merged 1 commit into from Oct 4, 2023
Merged

Conversation

noppefoxwolf
Copy link
Contributor

@noppefoxwolf noppefoxwolf commented Sep 25, 2023

It looks the WebKit always lookup alpha channel regardless of color type.

This APNG image has a color type that is true color(2).
But, Safari renders it with alpha channel.
7e3f65324c9de7f6

So, I added notSkipAlphaChannel to DecodingOptions.
This option always uses alpha channel when color type is true color.

I would like you to review my architecture, variable names and proposal.

Before After
Simulator Screenshot - iPhone 15 - 2023-10-02 at 23 01 34 Simulator Screenshot - iPhone 15 - 2023-10-02 at 23 02 37

@@ -29,6 +29,7 @@ class APNGDecoder {
let reader: Reader
let options: APNGImage.DecodingOptions
let cachePolicy: APNGImage.CachePolicy
let usesPrimitiveTrueColorAlphaChannel: Bool
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@noppefoxwolf

Do we actually need this here? Seems this property is only used in APNGImageRenderer when creating the bitmap info, so maybe just reading the options in the renderer class can be better (so we can eliminate the property)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review.
I moved this parameter to APNGImageRenderer and APNGImageView.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for it! I will merge it and then do some smaller adjustments to make the APIs more generic.

Revert "add usesPrimitiveTrueColorAlphaChannel options"

This reverts commit 5103792.

Update APNGImageRenderer.swift

add shouldRenderWithAlpha

add paramter
@onevcat onevcat merged commit 0e8bd63 into onevcat:master Oct 4, 2023
3 checks passed
@onevcat
Copy link
Owner

onevcat commented Oct 8, 2023

After some revise of the PNG spec, I guess I misunderstood the original meaning of tRNS chunk. Instead of merging this, I believe we should just use the .premultipliedLast alpha info when creating the bitmap even for true color (type 2). I will now revert these commits and then apply the correct fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants