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

Some tweaks to the pre-multiply alpha blending #1834

Merged
merged 7 commits into from May 26, 2020

Conversation

MyreMylar
Copy link
Contributor

@MyreMylar MyreMylar commented May 22, 2020

Goals:

  • Trying to get all the test to pass on SDL1 on CI.
  • Improve accuracy of blend formula based on this SDL issue: https://bugzilla.libsdl.org/show_bug.cgi?id=4761
  • Handle 24 bit surfaces and 8 bit surfaces properly.
  • Make SSE2 acceleration work in SDL1 on windows.
  • Make Arm/Raspberry Pi compile alphablit.c with SDL1 changes.

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 22, 2020

Was also looking into this test case:

import pygame


pygame.init()

screen=pygame.display.set_mode((200,200))
screen.fill((255,255,255,255))

# First make surf2 have a transparent background with SRCALPHA
# This is a white image with transparency
white_transparent_image = pygame.Surface((100, 100), pygame.SRCALPHA)
white_transparent_image.fill((107, 107, 107, 107))  # colour is pre-multipled by the alpha

# Just check a value of a pixel in the image
print(white_transparent_image.get_at([10, 10]))
# This returns (107, 107, 107, 107) for me... ok so far

# First compose our transparent image onto a black transparent one
surf2 = pygame.Surface((100, 100), pygame.SRCALPHA).convert_alpha()
print(surf2.get_at([10, 10]))
# This returns (0, 0, 0, 0) for me
surf2.blit(white_transparent_image, [0, 0], special_flags=pygame.BLEND_PREMULTIPLIED)
print(surf2.get_at([10, 10]))
# After composing this returns (107, 107, 107, 107) for me

# finally blit on the screen
screen.blit(surf2, (0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)

# What is the value now showing on the screen where the image is blitted?
print(screen.get_at([10, 10]))
# This shows (255, 255, 255, 255) for me.
screen.blit(surf2, (100, 100), special_flags=pygame.BLEND_PREMULTIPLIED)

pygame.display.flip()
input()

..,modified for pre-multipled alpha from the original issue here:
#1289 (comment)

With this commit and the test case above I get the following output:

(107, 107, 107, 107)
(0, 0, 0, 0)
(107, 107, 107, 107)
(255, 255, 255, 255)

With SDL 1 & 2 which I believe preserves the spirit of the original problem (not getting a white screen & wanting compatability between SDL1 & SDL2)

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 24, 2020

I also tried the test case from this comment:
#1289 (comment)

import pygame
pygame.init()

screen=pygame.display.set_mode((200,200))
screen.fill((255,255,255,255))
pygame.display.flip()

white_transparent_image = pygame.Surface((256, 256), pygame.SRCALPHA)
white_transparent_image.fill((100, 100, 100, 100))

#This returns (255, 255, 255, 100):
print(white_transparent_image.get_at([10, 10]))

surf2=pygame.Surface((256,256), pygame.SRCALPHA)
surf2.blit(white_transparent_image, [0, 0], special_flags=pygame.BLEND_PREMULTIPLIED)

# this shows a grey screen:
screen.blit(surf2, (0,0), special_flags=pygame.BLEND_PREMULTIPLIED)
# this gives the correct result (white):
#screen.blit(white_transparent_image, (0,0))

print(screen.get_at([10, 10]))

pygame.display.flip()
input()

again premultiplying the fill colours by the alpha and using BLEND_PREMULTIPLIED when blitting and get this output:

(100, 100, 100, 100)
(255, 255, 255, 255)

Which I believe is correct, though this test case is basically the same as the previous one. I think converting code like this would be generally easier with a function on a pygame.Color you could call to pre-multiply the colour by the alpha for you accurately.

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 24, 2020

Trying the test case from this comment with the same pattern of changes:

#1289 (comment)

import pygame

pygame.display.init()

pygame.display.set_mode((120, 120))

white_transparent_image = pygame.Surface((256, 256), pygame.SRCALPHA)
white_transparent_image.fill((100, 100, 100, 100))

# This returns (255, 255, 255, 100):
print(white_transparent_image.get_at([10, 10]))

surf2 = pygame.Surface((256, 256), pygame.SRCALPHA).convert_alpha()
surf2.fill((0, 0, 0, 0))
# This returns (0, 0, 0, 0):
print(surf2.get_at([10, 10]))

# This returns (99, 99, 99, 99):
surf2.blit(white_transparent_image, [0, 0], special_flags=pygame.BLEND_PREMULTIPLIED)
print(surf2.get_at([10, 10]))

Output now:

(100, 100, 100, 100)
(0, 0, 0, 0)
(100, 100, 100, 100)

Which is what you would expect.

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 24, 2020

Again, same changes on @illume's minimal test case from here:

#1289 (comment)

import pygame

surf1 = pygame.Surface((1, 1), pygame.SRCALPHA)
surf1.fill((100, 100, 100, 100))

surf2 = pygame.Surface((1, 1), pygame.SRCALPHA)
surf2.blit(surf1, (0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)

print("surf1.get_at((0, 0))", surf1.get_at((0, 0)))
print("surf2.get_at((0, 0))", surf2.get_at((0, 0)))

Result:

surf1.get_at((0, 0)) (100, 100, 100, 100)
surf2.get_at((0, 0)) (100, 100, 100, 100)

Which is what you would expect.

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 24, 2020

Checking the tests from this comment:

#1289 (comment)

with the same change over to premultiplied alpha:

import unittest
import pygame


class BlitIssueTest(unittest.TestCase):

    def test_src_alpha_issue_1289_0(self):
        """
        """
        surf1 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf1.fill((100, 100, 100, 100))

        surf2 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf2.fill((0, 0, 0, 0))
        self.assertEqual(surf2.get_at((0, 0)), (0, 0, 0, 0))
        surf2.blit(surf1, (0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)

        self.assertEqual(surf1.get_at((0, 0)), (100, 100, 100, 100))
        self.assertEqual(surf2.get_at((0, 0)), (100, 100, 100, 100))

        # (100, 100, 100, 100)   (0, 0, 0, 0)
        # SDL1:  -> (100, 100, 100, 100)
        # SDL2:  -> (100, 100, 100, 100)

    def test_src_alpha_issue_1289_1(self):
        """
        """
        surf1 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf1.fill((100, 100, 100, 100))

        surf2 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf2.fill((0, 0, 0, 1))
        self.assertEqual(surf2.get_at((0, 0)), (0, 0, 0, 1))
        surf2.blit(surf1, (0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)

        self.assertEqual(surf1.get_at((0, 0)), (100, 100, 100, 100))
        self.assertEqual(surf2.get_at((0, 0)), (100, 100, 100, 101))

        # (100, 100, 100, 100)   (0, 0, 0, 1)
        # SDL1:  -> (100, 100, 100, 101)
        # SDL2:  -> (100, 100, 100, 101)

    def test_src_alpha_issue_1289_2(self):
        """
        """
        surf1 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf1.fill((100, 100, 100, 100))

        surf2 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf2.fill((0, 0, 0, 255))
        self.assertEqual(surf2.get_at((0, 0)), (0, 0, 0, 255))
        surf2.blit(surf1, (0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)

        self.assertEqual(surf1.get_at((0, 0)), (100, 100, 100, 100))
        self.assertEqual(surf2.get_at((0, 0)), (100, 100, 100, 255))

        # (100, 100, 100, 100) (0, 0, 0, 255)
        # SDL1: -> (100, 100, 100, 255)
        # SDL2: -> (100, 100, 100, 255)

    def test_src_alpha_issue_1289_3(self):
        """
        """
        surf1 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf1.fill((100, 100, 100, 100))

        surf2 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf2.fill((0, 0, 0, 10))
        self.assertEqual(surf2.get_at((0, 0)), (0, 0, 0, 10))
        surf2.blit(surf1, (0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)

        self.assertEqual(surf1.get_at((0, 0)), (100, 100, 100, 100))
        self.assertEqual(surf2.get_at((0, 0)), (100, 100, 100, 106))

        # (100, 100, 100, 100) (0, 0, 0, 10)
        # SDL1: -> (100, 100, 100, 106)
        # SDL2: -> (100, 100, 100, 106)


if __name__ == "__main__":
    unittest.main()

They all pass with the same results on SDL1 and SDL2 and I think the results seem reasonable.

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 24, 2020

So after scanning through this long issue (partly my fault), and trying all the test cases identified:

#1289

I believe it covers two issues:

  1. There was no obvious way to do pre-multiplied alpha blending before in pygame. This is what causes the OP's issue as they are trying to do alpha compositing (blitting two layers with alpha ontop of one another) with the default, 'straight', SDL_BLENDMODE_BLEND (formula here) and it works exactly like the formula, i.e. it looks bad because that is the basic problem with that type of alpha blending (see here for a decent overview). This was true in pygame 1 & pygame 2 at the time because even though we had a basic BLEND_PREMULTIPLIED mode, it had no documentation and ran much slower than regular alpha blending because it had no intrinsic acceleration.

  2. The second issue is the differences between the implementations of 'straight' alpha blending in SDL1 and SLD2. This can be broken down into two sub issues:

    • SDL1 uses a very slightly different blending formula, likely because it is slightly faster - this causes alpha blending results to differ by about 1 per colour channel between SDL1 and SDL 2. It's not visually noticeable and doesn't really cause that much bother.
    • SDL 1 had different behaviour when a pixel had exactly 0 alpha. In that specific case it behaved more like pre-multiplied alpha, not for any mathematical reason but probably because whoever wrote the formula noticed that the normal 'straight alpha' formula looks at it's worst/weirdest when you have a black coloured surface with 0 alpha and you try to blit onto it with another alpha'd surface.

Conclusion

On the SDL side, my feeling is that SDL probably will not reintroduce the special case 0 behaviour without a good reason because it is mathematically incorrect and inconsistent with their documentation. There is an issue about the general accuracy of the alpha formula here, which might change the output of SDL's normal blend mode slightly if the formula is changed. I don't think it matters that much whatever the outcome as the differences are pretty minor, though I did attempt to use the modified formula from that issue in this changelist as it seemed more accurate and wasn't noticeably slower.

I think that anyone having alpha compositing problems of this nature should probably be directed into trying out pre-multiplied blending in pygame 2 as that appears to be the generally accepted solution across the digital graphics world. It's possible in the future that SDL will add it's own pre-multiplied blend mode that is superior to pygame's and we could look at using that then.

On our own side, I think that as well as this blendmode we need two additional features to support it:
1. A .premultiply_alpha() method on pygame.Color that will take the current colour channel values and multiply them by the current alpha channel value. I think it should probably also set a flag to only let you do this process once.
2. A .convert_premul_alpha() method on pygame.Surface that modifies a surface in the same way.

Those would help adapt existing programs, though of course users could also load images with the alpha already pre-multiplied as some (though not all) image programs support this.

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 24, 2020

I think I should probably let this get merged in on it's own as it fixes some previously skipped tests, and then add the support functions in a different pull request later on.

@MyreMylar MyreMylar marked this pull request as ready for review May 24, 2020 13:59
src_c/pgcompat.h Outdated Show resolved Hide resolved
@illume
Copy link
Member

illume commented May 25, 2020

I think I should probably let this get merged in on it's own as it fixes some previously skipped tests, and then add the support functions in a different pull request later on.

Which skipped tests does this fix? (I don't see any tests with the skip removed)

@illume
Copy link
Member

illume commented May 25, 2020

Very good. I read through all your posts to try and understand it, but I think I'll come back to it again later.

It sounds like you've got the blitter to work like it did in SDL1 with SDL2?

@MyreMylar
Copy link
Contributor Author

I think I should probably let this get merged in on it's own as it fixes some previously skipped tests, and then add the support functions in a different pull request later on.

Which skipped tests does this fix? (I don't see any tests with the skip removed)

I removed an if from the code that was skipping a chunk of the pre-multiply blend tests on SDL1.

@illume
Copy link
Member

illume commented May 25, 2020

I removed an if from the code that was skipping a chunk of the pre-multiply blend tests on SDL1.

Ah yes. I see now.

@illume illume added Surface pygame.Surface and removed needs-review labels May 26, 2020
Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

🎉 thanks :)

@illume illume merged commit a03f8a7 into pygame:master May 26, 2020
@MyreMylar MyreMylar deleted the pre-mul-alpha-tweaks branch June 5, 2020 16:41
@notpygame notpygame mentioned this pull request Sep 24, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Surface pygame.Surface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants