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

SDL 2: Per Pixel Alpha error. #1289

Open
Pololot64 opened this issue Sep 7, 2019 · 32 comments
Labels
Milestone

Comments

@Pololot64
Copy link

@Pololot64 Pololot64 commented Sep 7, 2019

  • minimal working example to reproduce the issue
  • write unit test(s) (see test_src_alpha_issue_1289)
  • maybe some code cleanup somewhere?
  • add debugging printfs or so.
  • issue fixed

Images with alpha that are only white blit as grey. I am trying to lighten the screen but it is darkening it instead
Screenshot (8)
Screenshot (9)

@illume illume changed the title Pygame 2: Per Pixel Alpha error. SDL 2: Per Pixel Alpha error. Sep 7, 2019
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Sep 7, 2019

Hi.

Thanks for the issue report!

Does your issue have anything to do with this? #1254 (screen defaults to white instead of black bug)

Do you have a script that can reproduce the issue?

cheers,

@illume illume added this to the 2.0 milestone Sep 7, 2019
@Pololot64

This comment has been minimized.

Copy link
Author

@Pololot64 Pololot64 commented Sep 7, 2019

It does have to do with it. I found a workaround by blitting with "special_flags=(pygame.BLEND_RGBA_ADD)"

This will work for now. I see it is a known error :-)

@robertpfeiffer

This comment has been minimized.

Copy link
Contributor

@robertpfeiffer robertpfeiffer commented Sep 9, 2019

I don't understand the problem here. Can you please post a snippet of code that shows the problem? I'll fix #1254 if that is what causes your problem, but I'd like to have a test case first.

@Pololot64

This comment has been minimized.

Copy link
Author

@Pololot64 Pololot64 commented Sep 9, 2019

I am simply blitting an image with transparency onto a transparent surface created with:
lumen = pygame.Surface(size, SRCALPHA).convert_alpha()
Then I am blitting "lumen" onto the main canvas.

The transparent image is white but causes a dark overlay

@robertpfeiffer

This comment has been minimized.

Copy link
Contributor

@robertpfeiffer robertpfeiffer commented Sep 10, 2019

I thought I knew what you meant, but your comment made me more confused. It's definitely not #1254.

So you have an image with semi-transparency. You blit that image to a surface with per-pixel alpha. Then you blit the second surface to the screen. I have trouble reproducing the bug.

@Pololot64

This comment has been minimized.

Copy link
Author

@Pololot64 Pololot64 commented Sep 10, 2019

Lets say I blit an image that has only one color: [255, 255, 255, 100]. When blitted over something, this should lighten what is under it. Instead, If I blit it over a background with the color [255, 255, 255, 255], it turns grey. It should be invisible. Does that make more sense?

@robertpfeiffer

This comment has been minimized.

Copy link
Contributor

@robertpfeiffer robertpfeiffer commented Sep 12, 2019

This seems to work the same in 1.9.6 and 2.0-dev3

import pygame
pygame.init()

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

surf2=pygame.Surface((100,100)).convert_alpha()
surf2.fill((255,255,255,100))

surf3=pygame.Surface((100,100)).convert_alpha()
surf3.fill((0,0,0,100))

screen.blit(surf3, (50,50))
screen.blit(surf2, (0,0))

surf2.blit(surf3, (25,25))
screen.blit(surf2, (100,100))

pygame.display.flip()
input()
@Pololot64

This comment has been minimized.

Copy link
Author

@Pololot64 Pololot64 commented Sep 12, 2019

Ok. I am sorry I was not clear enough :-) I had to rush to do my responses. I will adapt your code:

#Use the attached photo white.png in the same directory as the script.
white

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.image.load("white.png")

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



surf2=pygame.Surface((100,100), pygame.SRCALPHA).convert_alpha()
surf2.blit(white_transparent_image, [0, 0])






screen.blit(surf2, (0,0))

#What is the value now showing on the screen where the image is blitted?

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

#Wait... (191, 191, 191, 255)
#Why does (255, 255, 255, 107) blitted on top of (255,255,255,255) become (191, 191, 191, 255)?
#Wouldn't white blitted on top of white stay white... why do the pixels become darker??

screen.blit(surf2, (100,100))

pygame.display.flip()
input()
@dlon

This comment has been minimized.

Copy link
Member

@dlon dlon commented Sep 13, 2019

Reproduces for me. Here's a test case without an image file:

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((255, 255, 255, 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])

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

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

pygame.display.flip()
input()
@dlon

This comment has been minimized.

Copy link
Member

@dlon dlon commented Sep 13, 2019

So the problem is that you're blending a semi-transparent white image with a black surface (surf2), which produces an opaque grey image. This is the expected behavior, I believe.

@dlon

This comment has been minimized.

Copy link
Member

@dlon dlon commented Sep 13, 2019

It does seem to produce strange results even if the target surface is transparent:

import pygame
pygame.display.init()

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


white_transparent_image = pygame.Surface((256, 256), pygame.SRCALPHA)
white_transparent_image.fill((255, 255, 255, 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])
print(surf2.get_at([10, 10]))

Setting the RGB value to white gives you the desired result (plus a small error):

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

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

surf2=pygame.Surface((256,256), pygame.SRCALPHA).convert_alpha()
surf2.fill((255,255,255,0))

# This returns (253, 253, 253, 99):
surf2.blit(white_transparent_image, [0, 0])
print(surf2.get_at([10, 10]))
@Pololot64

This comment has been minimized.

Copy link
Author

@Pololot64 Pololot64 commented Sep 13, 2019

What do you think is causing this...?

@dlon

This comment has been minimized.

Copy link
Member

@dlon dlon commented Sep 15, 2019

It's caused by the blend mode that is used. DstRGB = SrcAlpha * SrcRGB + (1 - SrcAlpha) * DstRGB doesn't produce the correct result. I think what we might want is this: DstRGB = SrcAlpha * SrcRGB + (1 - SrcAlpha) * DstAlpha * DstRGB

@Pololot64

This comment has been minimized.

Copy link
Author

@Pololot64 Pololot64 commented Sep 15, 2019

Since that is what should be fixed, should I close this issue or leave it open?

@Pololot64

This comment has been minimized.

Copy link
Author

@Pololot64 Pololot64 commented Sep 15, 2019

That explains how the issue was fixed when I changed the blendmode

@illume illume added the critical label Sep 23, 2019
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Oct 3, 2019

This is the minimal example I can find to show the difference.

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

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

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

This comment has been minimized.

Copy link
Member

@illume illume commented Oct 3, 2019

If you use this code, you will see that the surface is defaulting to black (in pygame2), rather than to white (in 1.9.6 with SDL1).

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

surf2=pygame.Surface((1, 1), pygame.SRCALPHA, 32)
print("surf2.get_at((0,0))", surf2.get_at((0,0)))
#surf2.fill((255, 255, 255, 255))
surf2.fill((0, 0, 0, 255))
surf2.blit(surf1, (0, 0))

print("surf1.get_at((0, 0))", surf1.get_at((0, 0)))
print("surf2.get_at((0,0))", surf2.get_at((0,0)))
@illume illume mentioned this issue Oct 4, 2019
1 of 1 task complete
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Oct 4, 2019

I pushed a failing test case here: #1372

(works for SDL1)

@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Oct 4, 2019

From https://wiki.libsdl.org/SDL_BlitSurface :

RGBA->RGBA:

    Source surface blend mode set to SDL_BLENDMODE_BLEND:
        alpha-blend (using the source alpha-channel and per-surface alpha) SDL_SRCCOLORKEY ignored. 
    Source surface blend mode set to SDL_BLENDMODE_NONE:
        copy all of RGBA to the destination. if SDL_SRCCOLORKEY set, only copy the pixels matching the RGB values of the source color key, ignoring alpha in the comparison. 

We see that if the source and dest have a blendmode then they blend. I guess previous behaviour is that they do not blend.

From https://wiki.libsdl.org/SDL_SetSurfaceBlendMode :

To copy a surface to another surface (or texture) without blending with the existing data, the blendmode of the SOURCE surface should be set to 'SDL_BLENDMODE_NONE'.

Perhaps a solution is to check if the src has a blendmode == blend, and if so remove the blend mode temporarily.

@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Oct 4, 2019

There's another test case that shows blending should be on when set_alpha(int) is used...

def test_set_alpha_value(self):
    """surf.set_alpha(x), where x != None, enables blending"""
    s = pygame.Surface((1,1), SRCALPHA, 32)
    s.fill((0, 255, 0, 128))
    s.set_alpha(255)

    s2 = pygame.Surface((1,1), SRCALPHA, 32)
    s2.fill((255, 0, 0, 255))
    s2.blit(s, (0, 0))
    self.assertGreater(s2.get_at((0, 0))[0], 0, "the red component should be above 0")
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 19, 2019

JoKing wrote:
I tracked down #742 to be because with SDL 2 in function set_alpha we use SDL_SetSurfaceAlphaMod and inside that function (in SDL source code) there is the line surface->map->info.flags &= ~SDL_COPY_MODULATE_ALPHA that only gets set if alpha == 255 (otherwise it is surface->map->info.flags |= SDL_COPY_MODULATE_ALPHA), you can see that in https://github.com/emscripten-ports/SDL2/blob/master/src/video/SDL_surface.c.
That has the effect on SDL_CalculateBlit (flag SDL_COPY_MODULATE_ALPHA is used in SDL_ChooseBlitFunc, all found in https://github.com/emscripten-ports/SDL2/blob/master/src/video/SDL_blit.c) so it sets blit to be NULL and that causes Blit combination not supported error (SDL_CalculateBlit in our code is used in surface.c on line 4088 where it returns 1 instead of 0). Sadly I don't know how to fix this but in my opinion this is the reason this code fails in SDL 2.
this could also be connected to the crit issue we are working on right now because it uses the same line (s.set_alpha(255)) in test_set_alpha_value

@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

#1372 (review)

Why should this be true? I would surf1.blit(surf2) to alpha-blend by default?

@lordmauve Good question. This test passes with pygame 1.9.6 and pygame 1.9.1, but not pygame 2.0.0.dev7 (Ubuntu 18.04). So it's a case of keeping backwards compatibility.

note for self: python test/surface_test.py SurfaceTypeTest.test_src_alpha_issue_1289


Sorry, the test_src_alpha_issue_1289 passes when run by itself in 2.0.0.dev7.

It's currently marked as skipped, because it crashes when run with other tests.

    @unittest.skip("causes failures in other tests if run, so skip")
    def test_src_alpha_issue_1289(self):
        """ blit should be white.
        """
        surf1 = pygame.Surface((1, 1), pygame.SRCALPHA, 32)
        surf1.fill((255, 255, 255, 100))

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

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

Here is how it fails when run with other tests...

======================================================================
FAIL: test_src_alpha_issue_1289 (pygame.tests.surface_test.SurfaceTypeTest)
blit should be white.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/pygame-2.0.0.dev7-py3.7-macosx-10.13-x86_64.egg/pygame/tests/surface_test.py", line 747, in test_src_alpha_issue_1289
    self.assertEqual(surf2.get_at((0, 0)), (255, 255, 255, 100))
AssertionError: (99, 99, 99, 99) != (255, 255, 255, 100)
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

Next step is to find a combination of tests that cause test_src_alpha_issue_1289 to fail. Then to make that as a single test case. This way it's easier to debug.

Weirdness. Now it's happening when run by itself. I guess it's probably intermittently failing.

vagrant@ubuntu-bionic:~/pygame$ python3 /vagrant_data/pygame/test/surface_test.py SurfaceTypeTest.test_src_alpha_issue_1289
pygame 2.0.0.dev7 (SDL 2.0.8, python 3.6.8)
Hello from the pygame community. https://www.pygame.org/contribute.html
s
----------------------------------------------------------------------
Ran 1 test in 0.000s

OK (skipped=1)
vagrant@ubuntu-bionic:~/pygame$ 
vagrant@ubuntu-bionic:~/pygame$ 
vagrant@ubuntu-bionic:~/pygame$ python3 /vagrant_data/pygame/test/surface_test.py SurfaceTypeTest.test_src_alpha_issue_1289
pygame 2.0.0.dev7 (SDL 2.0.8, python 3.6.8)
Hello from the pygame community. https://www.pygame.org/contribute.html
F
======================================================================
FAIL: test_src_alpha_issue_1289 (__main__.SurfaceTypeTest)
blit should be white.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/vagrant_data/pygame/test/surface_test.py", line 747, in test_src_alpha_issue_1289
    self.assertEqual(surf2.get_at((0, 0)), (255, 255, 255, 100))
AssertionError: (99, 99, 99, 99) != (255, 255, 255, 100)

----------------------------------------------------------------------
Ran 1 test in 0.004s

FAILED (failures=1)
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

There's a PR here with some experiments and tests...
#1517

python3 test/surface2_test.py

I made three tests for the SRCALPHA, and there's differences with all.
However, the difference where the destination surface has alpha 0 pixels is the largest.

SRC: (  0,   0,   0,   0)
DST: (255, 255, 255, 100)
SDL1:  -> (255, 255, 255, 100)
SDL2:  -> ( 99,  99,  99,  99)

SRC: (  0,   0,   0, 255)
DST: (255, 255, 255, 100)
SDL1:  -> (100, 100, 100, 255)
SDL2: -> ( 99,  99,  99, 253)

SRC: (  0,   0,   0,  10)
DST: (255, 255, 255, 100)
SDL1:  -> (100, 100, 100, 107)
SDL2: -> ( 99,  99,  99, 105)

Note: SDL_BLENDMODE_BLEND is set on both surfaces in the test_src_alpha_issue_1289 tests.

According to the SDL2 docs (https://wiki.libsdl.org/SDL_BlendMode), SDL_BLENDMODE_BLEND has this formula:

SDL_BLENDMODE_BLEND
alpha blending
dstRGB = (srcRGB * srcA) + (dstRGB * (1-srcA))
dstA = srcA + (dstA * (1-srcA))
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

What should it be?

It looks like SDL2 is actually using something close to this formula:

(srcRGB * srcA) + (dstRGB * (dstA))

>>> srcRGB = (1.0 / 255) * 0                                                                                                                                                                                      
>>> dstRGB = (1.0 / 255) * 255                                                                                                                                                                                    
>>> srcA = (1.0 / 255) * 0                                                                                                                                                                                        
>>> dstA = (1.0 / 255) * 100                                                                                                                                                                                      
>>>                                                                                                                                                                                                               
>>>                                                                                                                                                                                                               
>>> (srcA + (dstA * (1 - srcA))) * 255                                                                                                                                                                            
100.0
>>> (srcRGB * srcA) + (dstRGB * (1 - srcA))                                                                                                                                                                       
1.0
>>> (srcRGB * srcA) + (dstRGB * (1 - srcA)) * 255                                                                                                                                                                 
255.0
>>> (srcRGB * dstA) + (dstRGB * (1 - dstA)) * 255                                                                                                                                                                 
155.00000000000003
>>> (srcRGB * srcA) + (dstRGB * (1 - dstA)) * 255                                                                                                                                                                 
155.00000000000003
>>> (srcRGB * dstA) + (dstRGB * (1 - srcA)) * 255                                                                                                                                                                 
255.0
>>> (srcRGB * dstA) + (dstRGB * (srcA)) * 255                                                                                                                                                                     
0.0
>>> (srcRGB * srcA) + (dstRGB * (dstA)) * 255                                                                                                                                                                     
100.0
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

I tried to replicate the SDL2 blending mode in python... in this commit: f21aeb8

$ python3 test/surface2_test.py -k test_blending_formula
pygame 2.0.0.dev7 (SDL 2.0.10, python 3.7.4)
Hello from the pygame community. https://www.pygame.org/contribute.html

SRC: (0, 0, 0, 0)
DST: (255, 255, 255, 100)
(100.0, 100.0, 100.0, 100.0)
SDL1:  -> (255, 255, 255, 100)
SDL2:  -> ( 99,  99,  99,  99)

SRC: (0, 0, 0, 255)
DST: (255, 255, 255, 100)
(100.0, 100.0, 100.0, 255.0)
SDL1:  -> (100, 100, 100, 255)
SDL2: -> ( 99,  99,  99, 253)

SRC: (0, 0, 0, 10)
DST: (255, 255, 255, 100)
(100.0, 100.0, 100.0, 106.078431372549)
SDL1:  -> (100, 100, 100, 107)
SDL2: -> ( 99,  99,  99, 105)
@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

I made a close to zero src alpha (1 instead of 0) and SDL1 and SDL2 are close.

SRC: (0, 0, 0, 1)
DST: (255, 255, 255, 100)
(100.0, 100.0, 100.0, 100.60784313725489)
SDL1:  -> (100, 100, 100, 101)
SDL2:  -> ( 99,  99,  99,  99)

SDL1 has a special case where src alpha is 0. Feature, or bug... I don't know.

@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

Next step: make a pure C test case that shows the behavior for making post to the libsdl bug tracker and discussion forum. This will also make sure it's not something pygame is doing.

@lordmauve

This comment has been minimized.

Copy link
Contributor

@lordmauve lordmauve commented Nov 23, 2019

If the default behaviour for blit is to copy, then we need a separate blend mode to alpha blend correctly. The existing blend flags seem to be more unusual operations like additive or multiplicative blending.

Normal alpha blending, onto an RGBA surface, with a non-premultiplied input, is

dest.rgb = src.a * src.rgb + (1 - src.a) * dest.rgb
dest.a = src.a  + (1 - src.a) * dest.a

But this gives premultiplied output. If you want the buffer to end up with non-premultiplied alpha, for another blit operation, then you need to divide

if dest.a:
    dest.rgb /= dest.a

I think the trick is to always do pre-multiplied alpha.

Then the blend equations are always

dest.rgb = src.rgb + (1 - src.a) * dest.rgb
dest.a = src.a  + (1 - src.a) * dest.a

and you can always simple assume premultiplied alpha.

@lordmauve

This comment has been minimized.

Copy link
Contributor

@lordmauve lordmauve commented Nov 23, 2019

So working backwards, SDL_BLENDMODE_BLEND seems to follow the first of those equations, and therefore, src is non premultiplied, dest is premultiplied.

@lordmauve

This comment has been minimized.

Copy link
Contributor

@lordmauve lordmauve commented Nov 23, 2019

So you shouldn't be able to use the target of a blit as the direct input to another blit - otherwise you'll see an unexpected grey.

@illume

This comment has been minimized.

Copy link
Member

@illume illume commented Nov 23, 2019

It seems the default is to blend if they are SRCALPHA surfaces. SDL1 has a weird special case where src alpha is 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.