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_BlitSurface bug in empty destination #3338

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 1 comment
Closed

SDL_BlitSurface bug in empty destination #3338

SDLBugzilla opened this issue Feb 11, 2021 · 1 comment
Labels
waiting Waiting on user response

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

Reported in version: 2.0.9
Reported for operating system, platform: Windows 10, x86_64

Comments on the original bug report:

On 2019-07-21 11:05:24 +0000, mz37 wrote:

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

On 2019-07-30 17:49:36 +0000, Ryan C. Gordon wrote:

(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.

On 2019-09-03 10:55:44 +0000, Sylvain wrote:

Maybe you can attach a test-case because the BlitSurface depends on the pixel format of the source and destination surface.

On 2019-09-20 20:47:35 +0000, Ryan C. Gordon wrote:

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.

On 2019-09-20 20:48:40 +0000, Ryan C. Gordon wrote:

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.

On 2019-12-03 07:39:22 +0000, Ryan C. Gordon wrote:

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.

On 2019-12-03 19:38:54 +0000, mz37 wrote:

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

On 2019-12-04 17:38:55 +0000, Sylvain wrote:

mz37, this is a very usual pixel format, and very likely without issue.

So the best would be that you provide a testcase!

Thanks

On 2019-12-04 19:45:18 +0000, mz37 wrote:

please check this example:
https://pastebin.com/Pgh7f982
this will describe the problem

On 2019-12-04 21:18:54 +0000, Sylvain wrote:

Checking the values, it seems to match SDL_BLENDMODE_BLEND formula:

https://hg.libsdl.org/SDL/file/3b03741c0095/include/SDL_blendmode.h#l37

On 2020-03-24 20:09:26 +0000, Ryan C. Gordon wrote:

Did we decide this bug should be closed as invalid?

--ryan.

On 2020-03-25 07:42:21 +0000, Sylvain wrote:

not 100% sure, what about you mz37 did you double-check the blending calculus ?

On 2020-05-06 12:22:28 +0000, Dan Lawrence wrote:

I've been investigating this problem for the pygame library where we have a lengthy bug chain about it here:

pygame/pygame#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.

On 2020-05-14 16:01:47 +0000, Dan Lawrence wrote:

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.

@SDLBugzilla SDLBugzilla added bug waiting Waiting on user response labels Feb 11, 2021
@1bsyl
Copy link
Contributor

1bsyl commented Feb 11, 2022

I think this is ok with this issue. Here the test case.
s1 has blending
s2 must not have blending mode (other it would blend twice).
main_bug_3338_scale_alpha.c.txt

@1bsyl 1bsyl closed this as completed Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting Waiting on user response
Projects
None yet
Development

No branches or pull requests

2 participants