We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4729 - SDL_BlitSurface bug in empty destination
Summary: SDL_BlitSurface bug in empty destination
Status: WAITING
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 2.0.9
Hardware: x86_64 Windows 10
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-21 11:05 UTC by mz37
Modified: 2020-05-14 16:01 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description mz37 2019-07-21 11:05:24 UTC
when using the pixel color like 
    SDL_Color src = {0xff, 0xff, 0xff, 0x34};

 and the destination pixel's alpha equal 0 , then the result will be all color channel equal to zero and alpha equal to 0xff

I guess that the problem is in function `SDL_SoftBlit` .
I tried to read the code, but i didn't found where is the actual pixels calcultion
Comment 1 Ryan C. Gordon 2019-07-30 17:49:36 UTC
(Sorry if you get several emails like this, we're marking a bunch of bugs.)

We're hoping to ship SDL 2.0.11 on a much shorter timeframe than we have historically done releases, so I'm starting to tag bugs we hope to have closed in this release cycle.

Note that this tag means we just intend to scrutinize this bug for the 2.0.11 release: we may fix it, reject it, or even push it back to a later release for now, but this helps give us both a goal and a wishlist for the next release.

If this bug has been quiet for a few months and you have new information (such as, "this is definitely still broken" or "this got fixed at some point"), please feel free to retest and/or add more notes to the bug.

--ryan.
Comment 2 Sylvain 2019-09-03 10:55:44 UTC
Maybe you can attach a test-case because the BlitSurface depends on the pixel format of the source and destination surface.
Comment 3 Ryan C. Gordon 2019-09-20 20:47:35 UTC
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 4 Ryan C. Gordon 2019-09-20 20:48:40 UTC
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 5 Ryan C. Gordon 2019-12-03 07:39:22 UTC
Bumping this off target-2.0.12. mz37, if you can give us more details of what the source and destination surface formats are, we can try to track this down, but otherwise we'll eventually mark this as WORKSFORME and close it.

Thanks,
--ryan.
Comment 6 mz37 2019-12-03 19:38:54 UTC
first all my surface are created like that:

SDL_CreateRGBSurface(0, w, h, 32, rmask, gmask, bmask, amask);

and the colors format is:

#if SDL_BYTEORDER == SDL_BIG_ENDIAN
#define rmask  0xff000000
#define gmask  0x00ff0000
#define bmask  0x0000ff00
#define amask  0x000000ff

#define rshift 24
#define gshift 16
#define bshift 8
#define ashift 0

#else
#define rmask  0x000000ff
#define gmask  0x0000ff00
#define bmask  0x00ff0000
#define amask  0xff000000

#define rshift 0
#define gshift 8
#define bshift 16
#define ashift 24

#endif
Comment 7 Sylvain 2019-12-04 17:38:55 UTC
mz37, this is a very usual pixel format, and very likely without issue.

So the best would be that you provide a testcase!

Thanks
Comment 8 mz37 2019-12-04 19:45:18 UTC
please check this example:
https://pastebin.com/Pgh7f982
this will describe the problem
Comment 9 Sylvain 2019-12-04 21:18:54 UTC
Checking the values, it seems to match SDL_BLENDMODE_BLEND formula:

https://hg.libsdl.org/SDL/file/3b03741c0095/include/SDL_blendmode.h#l37
Comment 10 Ryan C. Gordon 2020-03-24 20:09:26 UTC
Did we decide this bug should be closed as invalid?

--ryan.
Comment 11 Sylvain 2020-03-25 07:42:21 UTC
not 100% sure, what about you mz37 did you double-check the blending calculus ?
Comment 12 Dan Lawrence 2020-05-06 12:22:28 UTC
I've been investigating this problem for the pygame library where we have a lengthy bug chain about it here:

https://github.com/pygame/pygame/issues/1289

There are really two issues here.

1. The immediate change in behaviour versus SDL1: The issue here is that SDL2 is doing the 'correct' thing for regular alpha blending, but the behaviour is different versus the 'incorrect' but useful behaviour is SDL1. In SDL1 with a fully transparent and black surface there was a special behaviour not to do alpha blending with the destination colours. I don't necessarily think this means SDL2 should put the 'bug'/'behaviour' back in, but it is a difference in behaviour between the two versions that makes cross-version compatability difficult.

2. The underlying issue (since the dawn of SDL) is the absence of support for pre-multiplied alpha blending in SDL_BlitSurface. I am almost certain that the original hack in SDL1 was added as a quick way to mimic pre-multiplied alpha blending in the most common case where regular alpha blending looks most wrong. The real long term fix for this issue would be to add another path to the branching described here:

https://wiki.libsdl.org/SDL_BlitSurface

That checks for a new blend mode called something like:

SDL_BLENDMODE_PREMULALPHA

on the source surface when doing RGBA->RGBA blits and if so, does a slightly cheaper blend using the formula for pre-multiplied alpha as described here:

https://microsoft.github.io/Win2D/html/PremultipliedAlpha.htm

Which basically boils down to:

dstRGB = srcRGB + (dstRGB * (1-srcA))
dstA = srcA + (dstA * (1-srcA))


We do have a version of this in pygame, but like all the pygame software blits it is much slower than the SDL version.

I'll straight up admit I don't know anything about how this function is implemented SDL side but it seems like it should largely be a case of just doing less work (one less multiply per pixel). 

I think there is reasonable demand for such a change - you can find people searching for pre-multiplied alpha blending in SDL in mentions on the internet going back about a decade. I also believe that many of the 'newer' (in relative terms) platforms like Android and OSX natively *only* support pre-multiplied alpha (in image loading libraries) and SDL has had to un-multiply the alpha to make things work sometimes.

Anyway, TLDR - pre-multiplied alpha blits will solve all of SDLs blitting problems and it might even be slightly faster too.
Comment 13 Dan Lawrence 2020-05-14 16:01:47 UTC
To follow up - I ended up implementing some intrinsic (MMX, SSE2/Arm Neon) pre-multiplied alpha blendmodes in pygame which make it run acceptably fast. 

If someone smarter than me wants to port it back to SDL that's absolutely fine by me since it was based off of SDL's normal alpha blendmode anyway.