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

Fix iOS image loading process outputting alpha-premultiplied colours #6454

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frenzibyte
Copy link
Member

Marking as draft pending performance profiling and figuring out why there are still noticeable artifacts in intro even with this PR...;

By the nature of iOS, no image can be provided as raw pixel data without the colour components already being alpha-premultiplied, so we have to unapply that to have things working.

Since the processing logic was directly converting the image to raw pixel data in RGBA8 format, premultiplication becomes a very lossy process as the alpha component approaches zero. In other words, a pixel of (79, 79, 79, 4) when premultiplied becomes ~(1, 1, 1, 4), unapplying on the result yields (64, 64, 64, 4), which is off from the original by 15 degrees.

Therefore, I've rewritten the logic to render in floating-point format, giving enough precision to handle the case above. I'm not 100% sure about the performance implications of this but I would like to hope it's far less than loading the image through ImageSharp.


// unapply alpha pre-multiplication resulted by CGBitmapContext.
Copy link
Member

Choose a reason for hiding this comment

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

On a quick read it doesn't look like there's a CGBitmapContext anymore?

Also just a general question: did you use some other code as reference for this? It would be good to have some links to other similar implementation to reassure me that this isn't completely back to front (because I feel like it might be, and that the correct method is to adjust things so we can set the src/dest blend to support this at a metal level).

Copy link
Member Author

Choose a reason for hiding this comment

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

On a quick read it doesn't look like there's a CGBitmapContext anymore?

Yeah this comment is stale, should mention CIContext instead.

Also just a general question: did you use some other code as reference for this? It would be good to have some links to other similar implementation to reassure me that this isn't completely back to front (because I feel like it might be, and that the correct method is to adjust things so we can set the src/dest blend to support this at a metal level).

This is not referenced from anywhere else, it was written as such after various attempts of internet searching to achieve the desired behaviour, specifically after being aware of CIContext in a random stack overflow thread (I should also mention that CGBitmapContext supports floating-point components but the numbers are completely imprecise, seemingly converted from premultiplied byte values).

As to whether this is back to front or not, it is back to front in terms of iOS/Metal most likely. As I've read more about this before submitting this PR it does seem like the correct approach is to configure Metal to blend premultiplied colours more correctly, but I've gone with this PR first as I have a feeling it will not be a simple one at all. The masking/texture shaders will probably have to be thoroughly re-checked to handle premultiplied colours correctly, alongside figuring out what it takes to make Metal blend premultiplied colours.

I'll investigate that today and see where it leads me.

@peppy peppy self-requested a review December 13, 2024 02:04
peppy
peppy previously approved these changes Dec 13, 2024
@peppy peppy self-requested a review December 13, 2024 02:06
@frenzibyte frenzibyte self-assigned this Dec 15, 2024
@peppy peppy dismissed their stale review December 15, 2024 12:57

mistake

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

Successfully merging this pull request may close these issues.

iOS image loading using incorrect premultiplied mode for skinned resources
2 participants