Skip to content

Adjust tobytes premul formula #2966

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

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Adjust tobytes premul formula #2966

merged 1 commit into from
Jul 25, 2024

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Jul 2, 2024

While I'm working on my tobytes improvement branch, I ported the code to use Surface.premul_alpha to handle RGBA_PREMULT and ARGB_PREMULT.

The problem is, Surface.premul_alpha uses a slightly different formula: ((pixel + 1) * alpha) >> 8 (which is equivalent to ((pixel + 1) * alpha) / 256) instead of the classic (pixel * alpha) / 255 formula, probably for performance reasons. I believe these two formulas are close enough to result in very similar looking outputs.

Hence this PR: I figured I should separate out the slight behaviour change into it's own PR and keep the refactor PR for later.

@ankith26 ankith26 requested a review from a team as a code owner July 2, 2024 14:12
@ankith26 ankith26 added the image pygame.image label Jul 3, 2024
Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

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

Makes sense, LGTM

Copy link
Member

@itzpr3d4t0r itzpr3d4t0r left a comment

Choose a reason for hiding this comment

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

The current setup is fine for now, though could be expanded in te future for better performance, with both simd and better scalar code.

Copy link
Member

@itzpr3d4t0r itzpr3d4t0r left a comment

Choose a reason for hiding this comment

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

LGTM!

@damusss damusss added this to the 2.5.1 milestone Jul 3, 2024
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM 👍

The formula is different for SIMD efficiency purposes basically. in SIMD land divides suck but shifts are great,

@MyreMylar MyreMylar merged commit c65d5fa into main Jul 25, 2024
39 checks passed
@damusss damusss deleted the ankith26-tobytes-premul branch July 25, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
image pygame.image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants