Weird surface behavior

So I have yet another question… surprise! So what I’m attempting to do out
of boredom is some simple 2d bump mapping effects but I seem to be having some
problems with copying surfaces (and I’m probably going about this all wrong
though it worked for 8 bit surfaces). Basically I have a 24bit surface,
‘surface’, created from SetVideoMode and a 24 bit surface, ‘color_map’,
created from IMG_Load (and is a png). I’ll be creating the bump map on a pixel
by pixel basis so I need to be able to copy single pixels to ‘surface’. I had
some problems so I decided to try and do just a pixel by pixel copy from
’color_map’ to ‘surface’, Flip the surface, and repeat. Both surfaces are
320x200 and here’s the behavior I’ve seen.

After 1 full pass of both surfaces, color_map is successfully copied to
surface and displayed correctly.
After the second pass, it seems that memory is being accessed that shouldn’t
be and garbage gets displayed about a third down the image and then the image
is fine. This is how the image stays for all the rest of the passes.
OR
The program segfaults at row 150. (I have two versions that are almost
identical and one continues drawing the other segfaults. I’m including the
segfaulting one).

Below I’ve included my code. The printfs are to determine where things go
screwy. Basically it prints out the pixel data for ‘surface’ and 'color_map’
before and after the copy. On the first pass ‘surface’ should be zero before
the copy and the same as ‘color_map’ after the copy. On all other passes,
‘surface’ should equal ‘color_map’ before and after the copy. Things seem to
go screwy around row 150 where ‘surface’ before the copy starts having data
which shouldn’t happen the first pass. And at row 200, which would be row 0 of
the second pass, both surfaces seem to have no data. The printf with "Frame:"
in it, just prints which row it’s currently on. Sorry for the drawn out
details, but I figured it’d help. Thanks!

#include <iostream.h>
#include <stdlib.h>
#include <math.h>
#include “SDL.h”
#include “SDL_image.h”

SDL_Surface *surface;

int main( )
{
int videoFlags;

if ( SDL_Init( SDL_INIT_VIDEO ) < 0 )
    return 1;

videoFlags  = SDL_DOUBLEBUF;
videoFlags |= SDL_HWPALETTE;
videoFlags |= SDL_HWSURFACE;
videoFlags |= SDL_HWACCEL;

surface = SDL_SetVideoMode( 320, 200, 24, videoFlags );

if ( !surface )
    return 1;

SDL_Surface *color_map = IMG_Load( "ged3.png" );

Uint32 frameCount = 0;

if ( SDL_MUSTLOCK( surface ) )
    SDL_LockSurface( surface );
Uint32 *surface_p = ( Uint32* )surface->pixels;
if ( SDL_MUSTLOCK( color_map ) )
    SDL_UnlockSurface( color_map );

if ( SDL_MUSTLOCK( color_map ) )
    SDL_LockSurface( color_map );
Uint32 *color_p = ( Uint32* )color_map->pixels;
if ( SDL_MUSTLOCK( surface ) )
    SDL_UnlockSurface( surface );


bool done = false;
while ( !done )
    {
        for ( int j = 0; j < 200; j++ )
            {
                printf( "Frame: %u\n", frameCount );
        
                if ( SDL_MUSTLOCK( surface ) )
                    SDL_LockSurface( surface );

                for ( int i = 0; i < 320; i++ )
                    {
                        unsigned int pos = j * 320 + i;
                        printf( "Before s: %u c: %u", surface_p[pos],

color_p[pos] );
surface_p[pos] = color_p[pos];
printf( " After s: %u c: %u i: %d j: %d\n",
surface_p[pos], color_p[pos], i, j );
}

                if ( SDL_MUSTLOCK( surface ) )
                    SDL_UnlockSurface( surface );

                SDL_Flip( surface );

                frameCount++;

                SDL_Event event;
                while ( SDL_PollEvent( &event ) )
                    {
                        switch ( event.type )
                            {
                            case SDL_QUIT:
                                done = true;
                                break;
                            case SDL_KEYDOWN:
                                if ( event.key.keysym.sym == SDLK_ESCAPE )
                                    {
                                        done = true;
                                        printf( "DONE!\n" );
                                    }
                                break;
                            }
                    }
            }

    }

SDL_FreeSurface( color_map );

SDL_Quit( );
return 0;

}–
Ti Leggett
leggett at eecs.tulane.edu

Ti Leggett wrote:

if ( SDL_MUSTLOCK( surface ) )
    SDL_LockSurface( surface );
Uint32 *surface_p = ( Uint32* )surface->pixels;
if ( SDL_MUSTLOCK( color_map ) )
    SDL_UnlockSurface( color_map );

Shouldn’t the Unlock be on ‘surface’?

if ( SDL_MUSTLOCK( color_map ) )
    SDL_LockSurface( color_map );
Uint32 *color_p = ( Uint32* )color_map->pixels;
if ( SDL_MUSTLOCK( surface ) )
    SDL_UnlockSurface( surface );

Shouldn’t the Unlock be on ‘color_map’?

Are you sure that surface->pixels and color_map->pixels are always
constant?
Also in the main loop don’t you need to lock a surface even to get read
access to it (I am not sure)?
Your main loop also seems to assume the pixel size is always a UInt32.
You should have a look at the GetPixel/SetPixel functions in the doco.
http://sdldoc.csn.ul.ie/guidevideo.php

Adam.

Ti Leggett wrote:

if ( SDL_MUSTLOCK( surface ) )
    SDL_LockSurface( surface );
Uint32 *surface_p = ( Uint32* )surface->pixels;
if ( SDL_MUSTLOCK( color_map ) )
    SDL_UnlockSurface( color_map );

Shouldn’t the Unlock be on ‘surface’?

So it should. Got mixed up, but it still has no effect.

color_map->pixels should remain constant since all I do is load the image once
and copy from it. surface->pixels should change only once (the first pass
through). After that it should remain the same since color_map shouldn’t
change.

Also in the main loop don’t you need to lock a surface even to get read
access to it (I am not sure)?

From what I’ve seen and what I’ve done before, no.

Your main loop also seems to assume the pixel size is always a UInt32.
You should have a look at the GetPixel/SetPixel functions in the doco.
http://sdldoc.csn.ul.ie/guidevideo.php

I think (there I go again), that pixel info is always Uint32 it’s just how
it’s stored in that 32 bits that’s different (i.e., 24 bit surfaces ignore the
first or last 8 bits depending on edianess). At least I assume so since both
GetRGB and GetRGBA both return Uint32. Then again, I could be wrong and that
could be my problem :)On 2001.08.22 00:56 Adam Gates wrote:


Ti Leggett
leggett at eecs.tulane.edu

Ti Leggett wrote:

Ti Leggett wrote:

if ( SDL_MUSTLOCK( surface ) )
    SDL_LockSurface( surface );
Uint32 *surface_p = ( Uint32* )surface->pixels;
if ( SDL_MUSTLOCK( color_map ) )
    SDL_UnlockSurface( color_map );

Shouldn’t the Unlock be on ‘surface’?

So it should. Got mixed up, but it still has no effect.

color_map->pixels should remain constant since all I do is load the image once
and copy from it. surface->pixels should change only once (the first pass
through). After that it should remain the same since color_map shouldn’t
change.

Also in the main loop don’t you need to lock a surface even to get read
access to it (I am not sure)?

From what I’ve seen and what I’ve done before, no.

Your main loop also seems to assume the pixel size is always a UInt32.
You should have a look at the GetPixel/SetPixel functions in the doco.
http://sdldoc.csn.ul.ie/guidevideo.php

I think (there I go again), that pixel info is always Uint32 it’s just how
it’s stored in that 32 bits that’s different (i.e., 24 bit surfaces ignore the
first or last 8 bits depending on edianess). At least I assume so since both
GetRGB and GetRGBA both return Uint32. Then again, I could be wrong and that
could be my problem :slight_smile:

You may be right on the other points, I’m not sure I’m just giving
ideas, but your last point about the pixel size is definately wrong.
GetRGB and GetRGBA both return Uint32 because all pixels will fit into
an Uint32 but in the pixel array the are packed according to the Bytes
Per Pixel (bpp) value.> On 2001.08.22 00:56 Adam Gates wrote:

Ti Leggett wrote:

color_map->pixels should remain constant since all I do is load the image once
and copy from it. surface->pixels should change only once (the first pass
through). After that it should remain the same since color_map shouldn’t
change.

Also in the main loop don’t you need to lock a surface even to get read
access to it (I am not sure)?

No.

  1. surface->pixels may return a different value each time the surface is
    locked, so don’t keep a pointer to pixels after unlocking
  2. you must lock surfaces for any pixel access, reading or writing

Alrighty. Then that brings up another question. How big of performance hit is
locking and unlocking theses surfaces repeatedly?On 2001.08.22 04:07 Mattias Engdegard wrote:

Ti Leggett wrote:

color_map->pixels should remain constant since all I do is load the image
once
and copy from it. surface->pixels should change only once (the first pass
through). After that it should remain the same since color_map shouldn’t
change.

Also in the main loop don’t you need to lock a surface even to get read
access to it (I am not sure)?

No.

  1. surface->pixels may return a different value each time the surface is
    locked, so don’t keep a pointer to pixels after unlocking
  2. you must lock surfaces for any pixel access, reading or writing


Ti Leggett
leggett at eecs.tulane.edu

I’ll buy that, but is it wrong then to just copy that data over as a Uint32
then? If so, how should I go about doing a pixel by pixel copy? I tried, at
one point doing a GetRGBA from color_map and then a SetRGBA to surface but
that bombed harder (I’m sure due to my using them wrong).On 2001.08.22 01:44 Adam Gates wrote:

You may be right on the other points, I’m not sure I’m just giving
ideas, but your last point about the pixel size is definately wrong.
GetRGB and GetRGBA both return Uint32 because all pixels will fit into
an Uint32 but in the pixel array the are packed according to the Bytes
Per Pixel (bpp) value.


Ti Leggett
leggett at eecs.tulane.edu

So I’ve changed it a bit based on prior suggestions. I changed it to use the
get/putpixel methods that are in the sdl docs so now the main loop looks like:

        for ( int j = 0; j < 200; j++ )
            {
                printf( "Frame: %u\n", frameCount );
        
                if ( SDL_MUSTLOCK( surface ) )
                    SDL_LockSurface( surface );
                Uint32 *surface_p = ( Uint32* )surface->pixels;

                if ( SDL_MUSTLOCK( color_map ) )
                    SDL_LockSurface( color_map );
                Uint32 *color_p = ( Uint32* )color_map->pixels;

                for ( int i = 0; i < 320; i++ )
                    {
                        unsigned int pos = j * 320 + i;
                        printf( "Before s: %u c: %u", surface_p[pos],

color_p[pos] );
Uint32 pixel = getpixel( color_map, i, j );
putpixel( surface, i, j, pixel );
printf( " After s: %u c: %u i: %d j: %d\n",
surface_p[pos], color_p[pos], i, j );
}

                if ( SDL_MUSTLOCK( color_map ) )
                    SDL_UnlockSurface( color_map );

                if ( SDL_MUSTLOCK( surface ) )
                    SDL_UnlockSurface( surface );

                SDL_Flip( surface );

                frameCount++;

And it still segfaults at that magic row 150.–
Ti Leggett
leggett at eecs.tulane.edu

Ti Leggett wrote:

Alrighty. Then that brings up another question. How big of performance hit is
locking and unlocking theses surfaces repeatedly?

Locking costs vary from zero for non-RLEed software surfaces to
a lot for some hardware surfaces or RLEed software surfaces. In general
locking isn’t a big deal if you do it say, once per frame per surface

Direct pixel access is one of SDL’s features but mostly it’s simpler to
copy chunks of pixels using BlitSurface, and often faster

[…]
Uint32 color_p = ( Uint32 )color_map->pixels;

                for ( int i = 0; i < 320; i++ )
                    {
                        unsigned int pos = j * 320 + i;
                        printf( "Before s: %u c: %u", surface_p[pos],

color_p[pos] );

Are you sure there is data at color_p[pos] ? If the size of a pixel stored
surface->pixels is smaller than 32 bits, this instruction is not valid.

                        Uint32 pixel = getpixel( color_map, i, j );
                        putpixel( surface, i, j, pixel );
                        printf( " After s: %u c: %u i: %d j: %d\n",

surface_p[pos], color_p[pos], i, j );

Same remark as above.

[…]

And it still segfaults at that magic row 150.

If you remove your printf, it may work.On Wed, 22 Aug 2001, Ti Leggett wrote:


Johann Deneux
http://www.esil.univ-mrs.fr/~jdeneux/projects/

Locking costs vary from zero for non-RLEed software surfaces to
a lot for some hardware surfaces or RLEed software surfaces. In general
locking isn’t a big deal if you do it say, once per frame per surface

Cool. Once I figure out how to do this I’ll only need to lock once, copy
all the data, and then Flip.

Direct pixel access is one of SDL’s features but mostly it’s simpler to
copy chunks of pixels using BlitSurface, and often faster

I’m new to this and I’m not quite sure how I would achieve bump mapping
without modifying each pixel… Thanks again!

TiOn Wed, 22 Aug 2001, Mattias Engdegard wrote:

Are you sure there is data at color_p[pos] ? If the size of a pixel stored
surface->pixels is smaller than 32 bits, this instruction is not valid.

I assume there should be data throughout the whole surface of color_map
since the complete image is a filled graphic. I’ve tried uchar, uint8,
uint16, and uint32. The first three don’t segfault, but they don’t show
the complete image either. I thought (I’m still doing it) that since these
are 24bit images they are stored in 32 bits, but it’s highly probable I’m
wrong. I’m testing out the printf removal right now, but I get like 1
frame / 17 secs due to having to run the program from my home machine
(don’t ask)…On Wed, 22 Aug 2001, Johann Deneux wrote:

And it still segfaults at that magic row 150.

If you remove your printf, it may work.


Johann Deneux
http://www.esil.univ-mrs.fr/~jdeneux/projects/


SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl

So I finally got it working! I’m not sure if it’s bump mapping correctly
but it is displaying correctly. I went ahead and use get/putpixel and
Get/MapRGBA to do the per pixel transfers. The printfs were segfaulting
and I’m now properly using locking surfaces. Big thanks to all who helped!–
Ti Leggett
leggett at eecs.tulane.edu

Are you working on a demo or intro ???

Will the final results be online ???

CU