Offsets + Drawing and Getting Pixels

Hello all,

I’ve been working with SDL for a while now but have been working with some of
my code that uses offsets.

Offsets like Offset = y * ResolutionWidth + x; which I tend to like a little
better than just putting in x and y coordinates. I’ve tried every which way
which is why I’ve come here asking for some help.

Uint32 GetTexPixel(SDL_Surface *surface, int offset)
{
//slock(surface);
int bpp = surface->format->BytesPerPixel/8;

Uint8 *p = (Uint8 *)offset;

//sunlock(surface);

switch(bpp)
{
   case 1: return *p;

   case 2: return *(Uint16 *)p;

   case 3: if(SDL_BYTEORDER == SDL_BIG_ENDIAN)
			   return p[0] << 16 | p[1] << 8 | p[2];
	       else
			   return p[0] | p[1] << 8 | p[2] << 16;
   case 4: return *(Uint32 *)p;

   default: return 0;
}

}

There is a GetPixel with an offset I was able to make, but it doesn’t seem to
work with anything but 8 Bit Colour.

So if anyone has any suggestions why it doesn’t want to work I would be
appreciative. Thank you.

Hello Jinroh,

Wednesday, December 6, 2006, 3:20:31 AM, you wrote:

Hello all,

I’ve been working with SDL for a while now but have been working with some of
my code that uses offsets.

Offsets like Offset = y * ResolutionWidth + x; which I tend to like a little
better than just putting in x and y coordinates. I’ve tried every which way
which is why I’ve come here asking for some help.

Offset would be Offset = (y * surface->pitch) + x;

Pitch is the width of the surface, in bytes, including any extra there
might be to align the surface in video ram.

Also, the way you calculate “bpp” is wrong - BytesPerPixel is already
the right value, and doesn’t need dividing by 8.–
Best regards,
Peter mailto:@Peter_Mulholland

Hello Jinroh

int bpp = surface->format->BytesPerPixel/8;

Shouldn’t that be
int bpp = surface->format->BitsPerPixel / 8;
or just
int bpp = surface->format->BytesPerPixel;

Greetings–
"…there are two types of friends in this world.
Normal friends and the ones you can code with."

Jinroh wrote:

Hello all,

I’ve been working with SDL for a while now but have been working with some of
my code that uses offsets.

Offsets like Offset = y * ResolutionWidth + x; which I tend to like a little
better than just putting in x and y coordinates. I’ve tried every which way
which is why I’ve come here asking for some help.

<snip>

There is a GetPixel with an offset I was able to make, but it doesn’t seem to
work with anything but 8 Bit Colour.

So if anyone has any suggestions why it doesn’t want to work I would be
appreciative. Thank you.


SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl
i think this has been answered already. if BytesPerPixel is 1, 2, 3,
or 4 then 1/8, 2/8, 3/8, 4/8 is always 0.

also, why are you converting from an int to a pointer, then back to an int? that is bad coding and is likely to cause problems. i suggest
you dont mix those and use int and pointers correctly.

matt

Uint32 GetTexPixel(SDL_Surface *surface, int offset)
{
//slock(surface);
int bpp = surface->format->BytesPerPixel/8;

This is wrong, it you want bit per pixel, you must multiply by 8.
(anyway after reading the switch code, I saw you want byte per pixel,
so, no /8)

Uint8 *p = (Uint8 *)offset;

This is wrong, wrong and wrong!

Learn to use pointer.

int *p = &offset;
or if you wanna copy data

int *p;
*p = offset;

Anyway, Uint8 is not int and int is not int *. (not same lenght, not
even same signess)

//sunlock(surface);

switch(bpp)
{
case 1: return *p;

You return a 8 bit int, and your function is declared to return
32bit, in this context, you will get the right value, but this is
dangerous.

 case 2: return *(Uint16 *)p;

Do you at least know what casting a pointer does?

 case 3: if(SDL_BYTEORDER == SDL_BIG_ENDIAN)
  		   return p[0] << 16 | p[1] << 8 | p[2];
         else
  		   return p[0] | p[1] << 8 | p[2] << 16;

Dang, p is a 8it int (at least how you declared it), it will handle
8bit and not more.

 case 4: return *(Uint32 *)p;

 default: return 0;

}
}

You should really code in a cleaner way. C allows many things, even
very wrong.

For example, if you do

int **test;

addSomething()

use test[0] or test[1] will mostly works, but only because your are
"lucky". This is wrong. A and B must be static for it to work.

void addSomething() {
test = malloc(2 * sizeof(int*));
int a = 3;
int b = 5;
test[0] = &a;
test[1] = &b;

}

So, it’s not because it works that it is right.

What you do is dangerous, because:

return p[0] << 16 | p[1] << 8 | p[2]

Should not even works, but maybe it does.

p[n] will move the pointer of 8 bit because *p is Uint8

But as p[n] is 8 bit, you cannot shift it by 8 or even by 16.

I did not read what the code is supposed to do, because in this case,
you must look at your code error before thinking of the logic.

Hope this will help you.

Regards–
Kuon
CEO - Goyman.com SA
http://www.goyman.com/

“Computers should not stop working when the users’ brain does.”

-------------- next part --------------
A non-text attachment was scrubbed…
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2434 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20061207/a26e27f3/attachment.bin

nicholas, well said.

in addition you should use the rgb mask to find out where each byte or
color is, checking the endian of the machine is not enough. i found out
the hard way and had to debug that and now my app works on different archs.

what is the purpose of this function? perhaps one of could of us could
help code something better

matt

Nicolas Goy - kuon - ??? wrote:>>

Uint32 GetTexPixel(SDL_Surface *surface, int offset)
{
//slock(surface);
int bpp = surface->format->BytesPerPixel/8;

This is wrong, it you want bit per pixel, you must multiply by 8.
(anyway after reading the switch code, I saw you want byte per pixel,
so, no /8)

Uint8 *p = (Uint8 *)offset;

This is wrong, wrong and wrong!

Learn to use pointer.

int *p = &offset;
or if you wanna copy data

int *p;
*p = offset;

Anyway, Uint8 is not int and int is not int *. (not same lenght, not
even same signess)

//sunlock(surface);

switch(bpp)
{
case 1: return *p;

You return a 8 bit int, and your function is declared to return 32bit,
in this context, you will get the right value, but this is dangerous.

case 2: return *(Uint16 *)p;
Do you at least know what casting a pointer does?

case 3: if(SDL_BYTEORDER == SDL_BIG_ENDIAN)
return p[0] << 16 | p[1] << 8 | p[2];
else
return p[0] | p[1] << 8 | p[2] << 16;

Dang, p is a 8it int (at least how you declared it), it will handle
8bit and not more.

case 4: return *(Uint32 *)p;

default: return 0;
}
}

You should really code in a cleaner way. C allows many things, even
very wrong.

For example, if you do

int **test;

addSomething()

use test[0] or test[1] will mostly works, but only because your are
"lucky". This is wrong. A and B must be static for it to work.

void addSomething() {
test = malloc(2 * sizeof(int*));
int a = 3;
int b = 5;
test[0] = &a;
test[1] = &b;

}

So, it’s not because it works that it is right.

What you do is dangerous, because:

return p[0] << 16 | p[1] << 8 | p[2]

Should not even works, but maybe it does.

p[n] will move the pointer of 8 bit because *p is Uint8

But as p[n] is 8 bit, you cannot shift it by 8 or even by 16.

I did not read what the code is supposed to do, because in this case,
you must look at your code error before thinking of the logic.

Hope this will help you.

Regards

–Kuon
CEO - Goyman.com SA
http://www.goyman.com/

“Computers should not stop working when the users’ brain does.”



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

Hello all, thanks for the quick replys and helpful comments. Yeah, the original
code was kinda messy I got it from some SDL Tutorial that was discussing
offsets. I just c/p it and played around with it for a few minutes until it
worked, which it did but with only 8 BPP. I should’ve cleaned up the sloppy
pointering and things like that, but meh.

Anyway, a few days ago, I plugged my get and draw pixel functions in and
dissected the offets to get the x’s and y’s I needed for the pixels and altered
my rendering code and voila! Color correct texture mapping on my polygons.

Anyway, thanks again for the quick replys and I’ll take the info to heart
should I ever get in this situation again. Thanks a ton.