UnRLEAlpha weirdness

Daniel2 · October 4, 2000, 4:39pm

Greetings,

Found in SDL_RLEaccel.c, line 1435

static void UnRLEAlpha(SDL_Surface *surface)
{
	...
	RLEDestFormat *df = surface->map->sw_data->aux_data;
	...
	int w = surface->w;
	int bpp = df->BytesPerPixel;

	...
}


what would cause 'bpp' to equal 105 (which happens to be the size

of my surfaces width ( surface->w ) ?

who owns this code and are they familiar with the problem?

my call stack is as follows all surfaces are in HW 16bpp mode.

SDL_ConvertSurface is being passed an 8 bit surface to convert it to 16
bit.

Call stack:

SDL_ConvertSurface(...)
SDL_LowerBlit(...)
SDL_MapSurface(...)
SDL_UnRLESurface(...)
UnRLEAlpha(...)

any ideas or usefull suggestions to figure this one out would be

much appreciated. in the mean time i will try writing a small and simple
test program to try and narrow the problem.

i am using VC6 under W2k on a K7 with a G400. The problem also

appears on my Pentium 120 under Win98.

thank you all.

-dv

slouken · October 4, 2000, 4:53pm

i am using VC6 under W2k on a K7 with a G400. The problem also
appears on my Pentium 120 under Win98.

Are you changing the default packing of structures?
I need to add padding to the SDL_Surface structure so it isn’t affected
by #pragma pack.

See ya,
-Sam Lantinga, Lead Programmer, Loki Entertainment Software

Mattias_Engdegard · October 4, 2000, 5:43pm

my call stack is as follows all surfaces are in HW 16bpp mode.
SDL_ConvertSurface is being passed an 8 bit surface to convert it to 16
bit.

SDL_ConvertSurface(…)
SDL_LowerBlit(…)
SDL_MapSurface(…)
SDL_UnRLESurface(…)
UnRLEAlpha(…)

This can’t be right then - UnRLEAlpha shouldn’t be called on an 8-bit
surface. Somehow SDL_RLEACCEL has been set in your surface->flags (by
mistake?), without SDL_SRCCOLORKEY being set. Please find out why and
when this happened

any ideas or usefull suggestions to figure this one out would be
much appreciated. in the mean time i will try writing a small and simple
test program to try and narrow the problem.

Great, would help a lot

Daniel2 · October 5, 2000, 2:39am

In article , you say…

i am using VC6 under W2k on a K7 with a G400. The problem also 
appears on my Pentium 120 under Win98.
Are you changing the default packing of structures?
I need to add padding to the SDL_Surface structure so it isn’t affected
by #pragma pack.

i am using 8 byte alignment (VC default).

-dv

Daniel2 · October 6, 2000, 3:05pm

This can’t be right then - UnRLEAlpha shouldn’t be called on an 8-bit
surface. Somehow SDL_RLEACCEL has been set in your surface->flags (by
mistake?), without SDL_SRCCOLORKEY being set. Please find out why and
when this happened

What is there a distinction between SDL_RLEACCELOK and

SDL_RLEACCEL ? What are their roles and what would happen if they went
out of synch ?

-dv

Mattias_Engdegard · October 6, 2000, 5:21pm

What is there a distinction between SDL_RLEACCELOK and
SDL_RLEACCEL ? What are their roles and what would happen if they went
out of synch ?

SDL_RLEACCELOK is an internal flag that user code shouldn’t care about;
it means that the user has requested RLE encoding but that it actually
hasn’t been done yet, since it’s done lazily.

The user should never change surface->flags manually in any way,
but use the appropriate API for doing so (SDL_SetColorKey, SDL_SetAlpha etc)

Daniel2 · October 7, 2000, 7:43am

This can’t be right then - UnRLEAlpha shouldn’t be called on an 8-bit
surface. Somehow SDL_RLEACCEL has been set in your surface->flags (by
mistake?), without SDL_SRCCOLORKEY being set. Please find out why and
when this happened

Greetings,

Found in SDL_Surface.c,

SDL_ConvertSurface(…)

on line 647 ( SDL_SetColorKey(…,…,…) )
and
on line 652 ( SDL_SetColorKey(…,…,…) )

…

if ( (surface_flags & SDL_SRCCOLORKEY) == SDL_SRCCOLORKEY ) {
if ( convert != NULL ) {
Uint8 keyR, keyG, keyB;

  SDL_GetRGB(...);
  SDL_SetColorKey(convert,

-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCELOK),
SDL_MapRGB(convert->format, keyR, keyG, keyB));
}
SDL_SetColorKey(surface,
-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCELOK),
colorkey);
}

…

Should this read:

  SDL_SetColorKey(convert,

-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCEL),
SDL_MapRGB(convert->format, keyR, keyG, keyB));

and

SDL_SetColorKey(surface,

-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCEL),
colorkey);

SDL_RLEACCELOK is supose to be private and therfore never passed
to SDL_SetColorKey by a client. This alteration fixes my problem, but i
have yet to fully understand why. Any ideas whats going on here?

thank you.

-dv

Mattias_Engdegard · October 7, 2000, 3:17pm

Should this read:
 SDL_SetColorKey(convert, 
-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCEL),
SDL_MapRGB(convert->format, keyR, keyG, keyB));

and

SDL_SetColorKey(surface,
-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCEL),
colorkey);

I don’t think so. It would help me more to know the exact input to
SDL_ConvertSurface(), including the format and flags of the image that
causes it to behave wrongly. A small program that exposes the possible
bug would be helpful as well. Just making random changes to the
code is not productive

Daniel2 · October 9, 2000, 3:16am

In article <200010071517.RAA05395 at octans.nada.kth.se>, you say…

Should this read:
 SDL_SetColorKey(convert, 
-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCEL),
SDL_MapRGB(convert->format, keyR, keyG, keyB));

and

SDL_SetColorKey(surface,
-> surface_flags&(SDL_SRCCOLORKEY|SDL_RLEACCEL),
colorkey);
I don’t think so. It would help me more to know the exact input to
SDL_ConvertSurface(), including the format and flags of the image that
causes it to behave wrongly. A small program that exposes the possible
bug would be helpful as well.

{
// init video
SDL_Init(SDL_INIT_VIDEO);
SDL_InitSubSystem(SDL_INIT_VIDEO);
SDL_Surface *screen = SDL_SetVideoMode(800, 600, 16, SDL_HWSURFACE);
assert(screen);

SDL_Surface *img = IMG_Load("walk.png"); // load 8 bit png.
assert(img);
SDL_Rect dstRect = {0,0,img->w,img->h};

SDL_SetColorKey(img, SDL_SRCCOLORKEY|SDL_RLEACCEL, 8);
SDL_Surface *sdlSurface = SDL_DisplayFormat(img);
SDL_FreeSurface(img);

// show original image
SDL_BlitSurface(sdlSurface, NULL, screen, &dstRect);
SDL_Flip(screen);

// dup image
SDL_Surface *dupSurface = SDL_DisplayFormat(sdlSurface);
assert(dupSurface);

// show dup image
dstRect.x += (sdlSurface->w + 10);	// show images side by side
SDL_BlitSurface(sdlSurface, NULL, screen, &dstRect);
SDL_Flip(screen);

// clean up
SDL_FreeSurface(dupSurface);
SDL_FreeSurface(sdlSurface);
SDL_Quit();

}

Just making random changes to the code is not productive

The changes I highlighted were not random but the result of 3 days of

grappling with the code and single stepping thousands of lines. My analysis
indicated a problem with the use of the RLE flags, I was hoping it would give
you a clue into what was happening.

the above fragment consistently reproduces my problem. i hope it helps

you nail this bastard bug which undermines your high performance RLE system.

good luck and please keep me informed as I have invested 40 hrs into this

bug and look forward to stuffing it and hanging it on my wall ASAP.

-dv

Daniel2 · October 9, 2000, 11:13am

Greetings,

// show dup image
dstRect.x += (±>w + 10); // show images side by side
SDL_BlitSurface(sdlSurface, NULL, screen, &dstRect);

should have read

SDL_BlitSurface(dupSurface, NULL, screen, &dstRect);

 this isn't really important because the code never gets this far and

both sdlSurface and dupSurface get corrupted during SDL_DisplayFormat(…) but
i thought the test should be as clean as possible.

-dv

Mattias_Engdegard · October 10, 2000, 7:15pm

[ test program ]

Thank you, it was very helpful. (I had to convert it to C first but nothing
to worry about.) I found the error and sent the patch to Sam. I’ve attached
it here to satiate your curiosity

It was SDL_SetColorKey that violated a format constraint: if RLEACCEL is set,
then either the image must have an alpha channel, or a valid colorkey
and SDL_SRCCOLORKEY must be set.

Index: src/video/SDL_surface.c===================================================================
RCS file: /cvs/SDL/src/video/SDL_surface.c,v
retrieving revision 1.8.2.21
diff -u -r1.8.2.21 SDL_surface.c
— src/video/SDL_surface.c 2000/09/28 20:10:39 1.8.2.21
+++ src/video/SDL_surface.c 2000/10/10 18:49:15
@@ -162,6 +162,12 @@
*/
int SDL_SetColorKey (SDL_Surface *surface, Uint32 flag, Uint32 key)
{

if(surface->flags & SDL_RLEACCEL

 && (!(flag & SDL_RLEACCEL) || key != surface->format->colorkey)) {

```
      /* un-encode existing RLE */
```
```
      SDL_UnRLESurface(surface, 1);
```
}
if ( flag ) {
SDL_VideoDevice *video = current_video;
SDL_VideoDevice *this = current_video;