gbadev.org forum archive

This is a read-only mirror of the content originally found on forum.gbadev.org (now offline), salvaged from Wayback machine copies. A new forum can be found here.

C/C++ > A problem with mode4 and individual pixel access.

#11627 - ramirez - Tue Oct 14, 2003 10:36 pm

Now, first of all I'd like to tell that I am fairly new in GBA development and currently coding in C using devkit advance. Me and my friend have been more into it for past few days and we have bumped into this error now and so we are looking for assistance.
The problem we are having is with mode4's individual pixel access. First of all we DO know that it uses palette and you can't directly transfer the data as 8-bit.
I wrote this simple function for putting individual pixel(s) to the buffer:
Code:
//all the typedefs are declared in typedefs.h

//structure for holding two 8-bit values; pretty self explotianary
typedef struct multiPixel
{
   u8 px1;
   u8 px2;
} mltPixel;

inline void PutPixel(u8 px, u8 x, u8 y)
{
  u8 offset = x%2; //should overwrite the 1st or 2nd pixel in the 16-bit data
  mltPixel * temp; //the structure variable for storing the pixels
  temp = (mltPixel*)&(VideoBuffer[y*120+x]); //the location to the 16-bit pixels
  //if the modulus was 1 (2nd pixel), we'll update the 2nd pixel and don't touch to the first
  if (offset)
  {
    temp->px2 = px;
  }
  //otherwise we do likewise
  else
  {
  temp->px1 = px;
  }
}

Now, we see no error in this script, and it compiles fine also. But the problem is that when we for example use this for alternating the individual pixels:
Code:
for (y=0; y<6; y++)
{
  for (x=0; x<6; x++)
  {
    PutPixel(1,x,y);
  }
}

The 1 is white color in the palette (255|(255<<5)|(255<<10)). And it displays the area, but not like you'd expect. While I want it to shot 6x6 rectangle, it shows a 12x6 area. And to be honest I am out of understanding.
So if anyone has any idea what we are doing wrong, we'd appreciate help.
Thanks in advance. :)

#11629 - Stroff - Tue Oct 14, 2003 10:50 pm

You can only write 16 bit values to videoram.
To plot a single pixel, you'll have to read a full 16 bit value, modify the upper or lower byte, than write back a full 16 bit value.

#11630 - sajiimori - Tue Oct 14, 2003 10:53 pm

There's also this:
Quote:

VideoBuffer[y*120+x]

Is VideoBuffer a u16* or a u8*? If it's a u16*, then adding x will move over 2*x pixels. If it's a u8*, multiplying by 120 only goes half a row. I would guess it's a u16* because you reported getting wider shapes than expected.

#11632 - jd - Tue Oct 14, 2003 11:04 pm

Whilst it isn't technically a bug, I should point out that the following is very, very slow..

ramirez wrote:

Code:

u8 offset = x%2;



..this is because modulus requires a division which the GBA has to emulate through software. In this case, the same result can be achieved using..

Code:

u8 offset = x&1;


Also, you should use int's for variables wherever possible (rather that u8s), otherwise the processor will have to mask off the upper bits each time it does a calculation.

#11634 - Burton Radons - Tue Oct 14, 2003 11:24 pm

GCC produces identical code in this case because the input is unsigned, so modulus and logical AND have identical results. If the input is signed the output range of modulus is different (-1, 0, or 1), so the operations aren't the same. But in that case with these parameters it's only two instructions slower.

#11657 - ramirez - Wed Oct 15, 2003 6:27 am

Well, I obviosly made mistake with the "[y*120+x]" since the VideoBuffer is 16-bit so actually with x = 2 I edited x = 4 in the buffer. However I created a 240 long unsigned char lookup table for the x and now I use:
"[y*120+lookupx[x]]" and it works fine, except that I have other weird problem.
Here is a part of the code:
Code:
for (x=0; x<120; x=x+2)
{
  PutPixel(1,x,y);
}

This should obviosly plot a pixel to every 2nd pixel in the screen while x is less than 120, but yet it draws pixel to EACH pixel in the screen while x is less than 120. Here is the current PutPixel() code:
Code:
inline void PutPixel(u8 px, u8 x, u8 y)
{
   u8 offset = x&2;
   mltPixel * temp;
   temp = (mltPixel*)&(VideoBuffer[y*120+lookupx[x]]);
   if (offset) {
      temp->px2 = px;
   }
   else {
      temp->px1 = px;
   }
}


Any help is appriciated again. :)

#11658 - Master S - Wed Oct 15, 2003 6:59 am

Just pointing a 2 byte sizeed structure into the video mem, and then changing one of the bytes in this structure dosent change that you are doing a 8bit write !!!

#11670 - sajiimori - Wed Oct 15, 2003 5:44 pm

A few things:

1) There's a new bug in your function. x&2 will check the second bit of x, not the lowest bit. You were probably thinking of x&1.

2) It seems like you missed Stroff's comment that you cannot write 8 bits at a time to VRAM. Assigning to an 8 bit field in a struct is writing 8 bits. You have to read out the existing 16 bit value, then modify it and store it again.

3) There is no guarantee that gcc will put the 2 fields of the struct in adjacent bytes. Even if it does in this version, it might change later, because the C standard allows compilers to pad structs for alignment.

4) I don't see the point of using a struct at all.

5) The lookup table presumably just maps x -> x/2, so you may as well shift x right one bit instead.

I'm not sure if this is right (mainly because I'm not sure if the left pixel is the high byte or the low byte):
Code:

inline void PutPixel(int color, int x, int y)
{
   u16* dest = VideoBuffer + y*120 + (x >> 1);

   if(x & 1)
      *dest = (*dest & 0xff00) | color;
   else
      *dest = (*dest & 0x00ff) | (color << 8);
}

#11676 - DekuTree64 - Wed Oct 15, 2003 6:28 pm

The left pixel is the low byte, right pixel is high, so switch those 2 statements and it will work. But in general, don't use a putpixel function unless you really only need to draw a few pixels. It's just too slow, and even worse on GBA than a PC because of the pixel alignment. Make your drawing functions write 2 or 4 pixels at a time directly with pointers whenever possible.
_________________
___________
The best optimization is to do nothing at all.
Therefore a fully optimized program doesn't exist.
-Deku

#11688 - jd - Wed Oct 15, 2003 11:35 pm

Alternatively, for maximum speed you could put a bit of ARM assembler in IWRAM like this: (this routine is untested, but it should work)

Code:

.TEXT
.SECTION    .iwram0,"ax",%progbits
.ALIGN
.ARM

.GLOBAL PutPixel
.EXTERN VideoBuffer

.pool

@ CODE_IN_IWRAM void PutPixel( int color, int x, int y )

PutPixel:
  ldr r12,=VideoBuffer
  add r12,r12,r1
  eor r12,r12,#1
  add r12,r12,r2,lsl #8
  ldrb r2,[r12,-r2,lsl #4]!
  ands r1,r1,#1
  addne r2,r2,r0,lsl #8
  addeq r2,r0,r2,lsl #8
  strh r2,[r12,-r1]
  bx lr

.pool


To add clipping, you could put the following instructions at the beginning of the PutPixel routine:

Code:

cmp r1,#239
bxhi lr
cmp r2,#159
bxhi lr


Of course, like DekuTree64 said, you should try to write out lots of pixels at once instead for the very best performance.

#21388 - lylh - Fri May 28, 2004 5:29 pm

wouldn't it be better to use:

Code:

inline void
PutPixel (int x, int y, int color)
{
  u16* dest = VideoBuffer + (y<<7)-(y<<3) + (x >> 1);
  *dest = (*dest & 0xffff) | color | (color << 8);
}


Also, remember if you are drawing a horizontal line, remove (x >> 1) (which divides by two), and downsize your loop instead ;)

#21417 - sajiimori - Fri May 28, 2004 10:33 pm

jd's is still faster.

#22343 - FPChris - Sat Jun 19, 2004 2:50 am

You could also just typecast the video memory as 8bit
Then read and write individual pixels

u8* cbuf = (u8*)(VideoBuffer);

cbuf[(y*240)+x] = color; //or what ever

#22345 - dagamer34 - Sat Jun 19, 2004 3:45 am

FPChris wrote:
You could also just typecast the video memory as 8bit
Then read and write individual pixels

u8* cbuf = (u8*)(VideoBuffer);

cbuf[(y*240)+x] = color; //or what ever


I don't think that works. The GBA Memory controller only writes to VRAM in 16/32 bits, not 8. Sorry, but it's not quite that simple (I wish I were!)
_________________
Little kids and Playstation 2's don't mix. :(

#22354 - FPChris - Sat Jun 19, 2004 5:27 am

Quote:
I don't think that works. The GBA Memory controller only writes to VRAM in 16/32 bits, not 8. Sorry, but it's not quite that simple (I wish I were!)


Hmm... I've done this and it runs on my GBA SP.

Chris

#22356 - FPChris - Sat Jun 19, 2004 5:44 am

Ok, I rechecked it. I am able to typecast an u8* but due
to may texture I was useing I did not realize it was still
writing u16 regaudless. The typecasting however didn't through
an error or fail on the hardware. Live and learn. Thanks.

Chris