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

OSX SDL darkens colours proportional to increase in alpha. Also alters colours based on PNG colour profile #22

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

@SDLBugzilla SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: unspecified
Reported for operating system, platform: Mac OS X (All), All

Comments on the original bug report:

On 2009-10-29 08:06:06 +0000, Vittorio Giovara wrote:

starting from 1.2.14 with the new ImageIO backend, some images (we use pngs) have problems in setting the alpha channel correctly. The surface is loaded and converted correctly, as the colors match the ones in the other OSes, but when blending an alpha area, the alpha color is completely ignored.

here is the correct behaviour (1.2.13) -> http://dl.getdropbox.com/u/24468/screen-capture1.png

here is the (weird) behaviour (1.2.14 and 1.3) -> http://dl.getdropbox.com/u/24468/screen-capture2.png

here is a (normal) behaviour with red as alpha color -> http://dl.getdropbox.com/u/24468/screen-capture3.png

we have a strong suspect that one area might be doing ABGR, and the other area expecting the alpha to be the last byte and reading the R as an A.

On 2009-10-29 08:06:58 +0000, Vittorio Giovara wrote:

Created attachment 435
correct behaviour (1.2.13)

On 2009-10-29 08:09:16 +0000, Vittorio Giovara wrote:

Created attachment 436
wrong behaviour (1.2.14 + 1,3)

On 2009-10-29 08:10:41 +0000, Vittorio Giovara wrote:

Created attachment 437
correct behaviour with background red channel as alpha

On 2009-10-29 08:33:45 +0000, Vittorio Giovara wrote:

Created attachment 438
another (more evident) screenshot of the wrong behaviour

On 2009-10-29 09:13:05 +0000, Sam Lantinga wrote:

Are you able to reproduce this with showimage, or is this a problem in your application?

There were a few color issues in some games that turned out to be the game assuming the order of the RGB channels, but once they started checking the masks in the screen->format, everything was fine.

On 2009-10-29 10:30:50 +0000, Vittorio Giovara wrote:

(In reply to comment # 5)

Are you able to reproduce this with showimage, or is this a problem in your
application?

There were a few color issues in some games that turned out to be the game
assuming the order of the RGB channels, but once they started checking the
masks in the screen->format, everything was fine.

We're quite sure that the order of the channels are correct because the other images are displayed correctly -- the problem only arise when images with alpha are blended.

a similar issue was found here http://forums.libsdl.org/viewtopic.php?p=19729 where opacity is applied in two different ways on OSX and win.

btw our application is located at http://www.hedgewars.org/ and you can read the source here http://fireforge.net/scm/viewvc.php/trunk/hedgewars/?root=hedgewars (the IMG_Load is in uStore.pas)

On 2009-10-29 13:13:06 +0000, Vittorio Giovara wrote:

hm we noticed that by stripping color profiles from the png helped with the background image.
perhaps our theory was wrong, but does sdlimage handles color profiles correcly in the new backend?

On 2009-10-29 13:47:03 +0000, Vittorio Giovara wrote:

Ok, dumping the first column of a buggy SDL surface, we noticed an interesting pattern:

A B G R

SDL on OSX
206 | 177 | 131 | 125

GIMP
206 | 220 | 163 | 155

SDL on OSX
14 | 8 | 5 | 4

GIMP
14 | 157 | 92 | 84

Pattern seems to be that if you take:
177 * (255/206) ~ 219

and
8 * (255/14) ~ 146

Now, that 2nd one is not so accurate. But.
157 / (255 / 14) = 8.58 which if truncated is 8.

220 / (255 / 206) = 177.72 = 177

So we could be dealing with some curious modification proportional to alpha and a floor operation to boot.

Fortunately the fix for that is fairly trivial, until SDL is fixed, we can ifdef DARWIN to reverse the damage.
The truncation means some accuracy is lost, but hopefully not too much.

On 2009-10-29 21:03:12 +0000, Sam Lantinga wrote:

From nemo:

Hey. Turns out the problem was a bit weirder than someone simply messing up RGBA vs ABGR.
http://forums.libsdl.org/viewtopic.php?p=20011# 20011

On 2009-10-29 21:55:37 +0000, Sam Lantinga wrote:

Eric, can you look into this?

On 2009-11-09 19:45:09 +0000, Vittorio Giovara wrote:

is there any update on this?
in the meantime i have updated the workaround code

{$IFDEF DARWIN}
tmpP := tmpsurf^.pixels;
for i:= 0 to (tmpsurf^.pitch shr 2) * tmpsurf^.h - 1 do
begin
{$IFDEF ENDIAN_LITTLE}
tmpA:= tmpP^[i] shr 24 and $FF;
tmpR:= tmpP^[i] shr 16 and $FF;
tmpG:= tmpP^[i] shr 8 and $FF;
tmpB:= tmpP^[i] and $FF;
{$ELSE}
tmpA:= tmpP^[i] and $FF;
tmpR:= tmpP^[i] shr 8 and $FF;
tmpG:= tmpP^[i] shr 16 and $FF;
tmpB:= tmpP^[i] shr 24 and $FF;
{$ENDIF}
if tmpA <> 0 then
begin
tmpR:= round(tmpR * 255/tmpA);
tmpG:= round(tmpG * 255/tmpA);
tmpB:= round(tmpB * 255/tmpA);
end;

  			if tmpR > 255 then tmpR:= 255;
  			if tmpG > 255 then tmpG:= 255;
  			if tmpB > 255 then tmpB:= 255;

{$IFDEF ENDIAN_LITTLE}
tmpP^[i]:= (tmpA shl 24) or (tmpR shl 16) or (tmpG shl 8) or tmpB;
{$ELSE}
tmpP^[i]:= (tmpA) or (tmpR shl 8) or (tmpG shl 16) or (tmpB shl 24);
{$ENDIF}
end;
{$ENDIF}

On 2009-11-09 20:38:05 +0000, Sam Lantinga wrote:

Oh, it looks like Mac OS X is just pre-multiplying the alpha. Thanks for the great workaround code!

On 2009-11-09 20:40:13 +0000, Sam Lantinga wrote:

And of course looking at the code, I see we're explicitly asking for that with kCGImageAlphaPremultipliedFirst.

On 2009-11-09 22:02:11 +0000, Sam Lantinga wrote:

It turns out that ImageIO only supports pre-multiplied alpha output. Ugh.
http://lists.apple.com/archives/mac-opengl/2007/may/msg00056.html

On 2009-11-09 23:33:52 +0000, Sam Lantinga wrote:

Out of curiosity, what does your workaround do to this image?
http://www.w3.org/Graphics/PNG/alphatest.png

On 2009-11-10 00:45:42 +0000, Sam Lantinga wrote:

Nevermind, this is fixed now, thanks!
http://www.libsdl.org/tmp/SDL_image/SDL_image-1.2.9.tar.gz

On 2009-11-10 10:24:36 +0000, Vittorio Giovara wrote:

ok now it works fine, thanks!
there is just one thing that worries me is that by looking at the code
Uint8 *p = (Uint8 *)surface->pixels;
Uint8 A = p[3];
so we are using 8 bits for Alpha, 8 for Red, 8 for Green and 8 for Blue; then we make a division and save it on an eight bit variable

if the result of the operation is something like 99.99 then the result stored would be 99, truncating the .99, making the color a little darker -- not noticable -- but different from an alpha-free surface; every computation on color value would be likely to fail with this code.

What do you think of this possibility?

On 2009-11-10 13:11:28 +0000, Vittorio Giovara wrote:

PowerPC loading is broken

normal pngs work fine but pngs with alpha segfault
showimage's output is

showimage(52993) malloc: *** error for object 0x837200: incorrect checksum for freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug

(attached BlueWater.png)

On 2009-11-10 13:12:27 +0000, Vittorio Giovara wrote:

Created attachment 448
test image used in showimage to test ppc code

On 2009-11-10 23:09:44 +0000, Sam Lantinga wrote:

The crash is fixed, thanks!

As for the inaccuracy in the multiplication, we might be able to solve it by using 256 instead of 255, although that might break things the other direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant