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++ > Having some trouble with object palette

#169780 - Bozebo - Mon Aug 03, 2009 3:10 pm

Hi. I am really struggling getting my sprite import system to work.
I am aware than the overall code is rather limiting, but I just need to make a basic breakout game for uni and I need to be able to load in the sprites and combine their palettes into the shared palette.
Firstly, you can download my code. (uses devkitPro, included).
Or just the code if don't want to waste 56MB on devkitPro.

I had sprites working when I was just testing the pacman sprite tutorial ;)
In an attempt to debug it I added some lines of code in main.cpp which fill the palette with a gradient from black to white, repeated to fill. Through that I am rather sure the issue is with the importSprite function in sprites.h, the second half of which checks to see if the colour for each pixel is already in the shared palette, and if so changes the OEMData value to that, if not it finds the first 0 - after transparent and black which are always pos 0 and 1 in the array - and puts it in there, and updates the position in OEMData.

I am fully aware that adding too many sprites would overload the palette and/or OEMData (especially if the sprites are not first known), so don't flame me for that ^_^ it is just a basic system so I can procure a sample game.

I have scoured my code and I cannot find out what the issue is, I have been working on this on and off for weeks. Can anybody help?

#169787 - Cearn - Mon Aug 03, 2009 7:08 pm

The main problem is that the code and graphics data is not in the proper format. For 256-color objects, the pixels are 8 bits wide, but the graphics arrays and VRAM (yes VRAM; OAMData doesn't point to OAM, but to VRAM) accesses are 16-bits in size. This is why you see all those gaps in the graphics.

The next function is a replacement that, although untested, should work. I've modified the interface a bit to weed out unused parts and handled the tile index via a return instead of passing it by reference.

Note that I'm assuming that the graphic arrays are formatted correctly: with 8 bits per pixel instead of 16. I'm also accounting for the fact that VRAM can not be written to in bytes by saving the saving the color index in a buffer on even pixels and writing two pixels at once on odd pixels.

Code:
#define OBJ_GFX      ((u16*)0x06010000)
#define OBJ_PALETTE   ((u16*)0x05000200)

/*!
   \param sprGfx   Sprite graphics data.
   \param sprPal   Sprite palette data.
   \param   size   Size of graphics data in pixels (?)
   \note   This only works for 8-bit graphics.
*/
int importSprite(const void *sprGfx, const u16 *sprPal,
   u32 size)
{
   int tileId= g_nextTile;
   
   u8 *srcGfx= (u8*)gfxData;         // Point to source gfx.
   u16 *dstGfx= &OBJ_GFX[tileId*64/2];   // OBJ-VRAM destination.
   u16 *dstPal= OBJ_PALETTE;         // Object palette pointer.
   
   u32 buf, color;
   u32 i, ipx, palSize=2;
   
   for(i=0; i<size; i++)         // Loop over all pixels.
   {
      ipx= srcGfx[i];
      if(ipx > 1)
      {
         color= sprPal[ipx];      // Get pixel color.
            
         // Search for color.
         for(ipx=1; ipx<palSize; ipx++)
            if(color == dstPal[ipx])
               break;
               
         // Not found: add new color.
         if(ipx==palSize)
         {
            dstPal[ipx]= color;
            palSize++;
            //# PONDER: what to do if palSize > 256 ?!?
         }
      }
      
      // NOTE: VRAM needs 2-pixel access, so even i store
      // to buffer and odd i actually write to VRAM.
      if(i%2)
         dstGfx[i/2]= buf | ipx<<8;
      else
         buf= ipx;
   }
   
   g_nextTile += size/64;

   return tileID;
}

I would also strongly recommend using either devkitPro's libgba or Tonc's code library as a basis for GBA code, as much of the headers and functions you find on the web are often incorrect and/or inefficient. For example, the rgb() macro is unsafe (no parentheses around r, g, b), the vSync() function actually waits till the Vdraw instead of the VBlank period and none of the IO registers are volatile. I'd also suggest making use of DKP's template projects instead of batch file constructions. They can take care of new source files automatically, properly handle up-to-date files and do not depend on your platform or folder structure.

For some general hints: do not use so many magic numbers; it's almost always better to create named constants instead. And if you do use magic numbers, do not use decimal values for bitmasks, which are better represented in hex. If you need an integer for local variables, use int (or s32/u32) and not one of the smaller types. The latter do not lead to lower memory use and are slower as well. And they overflow easier, which can lead to all sorts of bugs. To illustrate what I mean, consider setSprX() :
Code:
void setSprX(u8 id,u8 x){
  //empty rightmost 9 bits
  sprites[id].attribute1 &= 64512;
  //set the x value
  sprites[id].attribute1 += x; // |= would also work
}

The constant 64512 is meaningless and in fact also incorrect. 64512 =0xFC00, which is actually the mask for 10 bits, not 9. And to indicate that you want to clear something, it may be better to use ~0x1FF instead. Also, x is supposed to be 9 bits, but u8 is only 8-bit. You're throwing away a bit. This may seem insignificant, but that extra bit is what allows you to place the object partially outside the left side of the screen. A better version would be:
Code:
#define ATTR1_X_MASK    0x1FF

void setSprX(int id, int x)
{
    sprites[id].attribute1 &= ~ATTR1_X_MASK;       // Clear old X
    sprites[id].attribute1 |= (x & ATTR1_X_MASK);  // Insert new X
}


I'm sorry if I come off as a bit of a hardass about this, but learning to program for a new platform can be hard enough without all these easily remedied issues getting in the way. These mistakes are not your fault. The unfortunate fact is that most of the GBA tutorials have some pretty poor coding standards and it'd be unfortunate if you'd adopt them. It's best to learn how to do things properly right at the start, instead of having to unlearn bad habits at a later stage.

#169790 - wallacoloo - Mon Aug 03, 2009 7:25 pm

I found another thing that doesn't make sense to me:
Code:
u8 defineSprite(){
  spritesDefined ++;
  return spritesDefined -= 1;
}

This is incrementing spritesDefined by 1, then decrementing it by 1 and returning the value.
instead of "return spritesDefined -= 1;", use "return spritesDefined - 1;"

ATM, this isn't a big deal since you only have 1 sprite. But when you have more than one, I think they'd all have a sprite ID of 0.

#169791 - Bozebo - Mon Aug 03, 2009 7:49 pm

Ah thanks. You've solved the issue and ones that would have confused me later on. Thanks for all the advice too.
One thing though, I am not allowed to use a library for the task :( which makes sense. I probably wouldn't have used a library anyway because I am just like that, it's the best way to properly understand what is going on.

Code:

u8 defineSprite(){
  spritesDefined ++;
  return spritesDefined -= 1;
}

Aha, I can't believe I did that. Thanks for pointing it out >_<

#169792 - Cearn - Mon Aug 03, 2009 8:17 pm

Bozebo wrote:
Ah thanks. You've solved the issue and ones that would have confused me later on. Thanks for all the advice too.
One thing though, I am not allowed to use a library for the task :( which makes sense. I probably wouldn't have used a library anyway because I am just like that, it's the best way to properly understand what is going on.

Well, you don't have to use the whole library, just yoink whatever you can use. The register #defines and bitflags are usually grouped together in one or two headers. Since you're going to have to get new #defines or at least modify the ones you have now, you might as well just grab the complete and correct sets from existing libraries.

#169809 - Bozebo - Tue Aug 04, 2009 3:31 pm

edit:
ignore whats below, I made a stupid mistake:
Code:
u16 *dstGfx = &OAMData[nextTile+(size*64)];

instead of
Code:
u16 *dstGfx = &OAMData[(nextTile-512)*64];


thanks for all your help, it works nicely now

seeing as you are here.
Any advice on the algorithm to calculate bouncing of the ball off the blocks? If there was simply one block it would be easy, but with many near each other I suppose it has to keep track of previous positions and directions so it can loop through the possibilities so as to not get stuck inside a block - I may have the blocks locations define a "bitmap" of where the ball can and cannot be. Hey, at least it's only rectangles and a circle.

Or I could make a space invaders game.

-------------------------------------------------------------------------------------

I have it working (code-wise). But for some reasons the tiles only exist (as far as the hardware and emulator are concerned) as even numbers.
So it starts at 512, then the next tile is 514, then 516 and so on. What is that all about? All the tutorials and documentation don't say anything about that, surely it would go 512,513.....

Because of this there is nothing drawn on the screen, the tileId I have set in OAM is 512, as it should be. But the data itself has ended up in 564. So the leftmost 8x8 block of the 1d mapped sprite is 564, then the next block is numbered 566. It has completely confused me.

What's up with that?

edit:
here is what the function looks like now:
Code:

u16 nextTile = 512; //the next available tile for objects, mode 4

//imports sprite palette and data from rom
int importSprite(const void *gfxData,const u16 *sprPal,u32 size){
  u8 *srcGfx = (u8*)gfxData; //source in rom
  u16 *dstGfx = &OAMData[nextTile+(size*64)]; //destination in vram
 
  u16 colour; //16-bit bgr colour
  u16 buf; //buffer so 2 values can be written at once
  u32 sourcePix,ipx; //for loop variables
 
  //loop through every pixel
  for(sourcePix=0;sourcePix < size*64;sourcePix++){
    ipx = srcGfx[sourcePix]; //get the local palette position
    //skip transparent and black palette positions
    if(ipx > 1){
      colour = sprPal[ipx]; //get pixel colour
       
      //search for colour in the shared palette
      for(ipx = 1;ipx<256;ipx++)
        if(colour == OBJPaletteMem[ipx]) //colour is found
          break;
           
      //if not found
      if(ipx == 256){
        //find first 0 position, after transparent and black
        for(ipx = 1;ipx < 256;ipx ++)
          //if all colours are full, the logic will fail (256 cols)
          if(OBJPaletteMem[ipx] == 0)
            break;
        //set the colour
        OBJPaletteMem[ipx] = colour;
      }
    }
   
    //vram can only be written to in 16 bit; do this for every 2nd pix
    if(sourcePix%2){
      //add the pixels
      dstGfx[sourcePix/2] = buf | ipx<<8;
    } else {
      //store the pixel data in a buffer until there are 2
      buf = ipx;
    }
  }
 
  nextTile += size; //increment the next tile
  return nextTile - size; //return this sprite's starting tile
}

note:
"size" is the number of 8x8 tiles that the sprite uses... though it ends up messing things up because nextTile increments by size, though in some ways needs to be incremented by size*2 - which causes further bugs and issues. not to mention halving the number of possible 8x8 tiles.

Also, the tile data for objects seems to start at 0x06014000 yet you used 0x06010000, which is correct? Remember I am in mode 4. All the technical documents and tutorials and faqs seem to completely conflict with each other, yet each seem to work if they are followed to the letter individually.

#169810 - Cearn - Tue Aug 04, 2009 3:56 pm

Bozebo wrote:
I have it working (code-wise). But for some reasons the tiles only exist (as far as the hardware and emulator are concerned) as even numbers.
So it starts at 512, then the next tile is 514, then 516 and so on. What is that all about? All the tutorials and documentation don't say anything about that, surely it would go 512,513.....

Because of this there is nothing drawn on the screen, the tileId I have set in OAM is 512, as it should be. But the data itself has ended up in 564. So the leftmost 8x8 block of the 1d mapped sprite is 564, then the next block is numbered 566. It has completely confused me.

What's up with that?


the basic unit for objects is the 4bpp tile, which is 32 bytes in size. You're using 8bpp tiles, which are 64 bytes wide, which means that the next tile is 64/32=2 away, not just one. See tonc: obj-tiles.

Bozebo wrote:
Also, the tile data for objects seems to start at 0x06014000 yet you used 0x06010000, which is correct? Remember I am in mode 4. All the technical documents and tutorials and faqs seem to completely conflict with each other, yet each seem to work if they are followed to the letter individually.

Object VRAM starts at 0601:0000. However, in the bitmap video modes, tiles 0 to 511 are not accessible (well, they are, but that part of VRAM is used for the background, not objects). Only 512 and up are usable for objects. 0601:0000 + 512*32 = 0601:4000.

Also, there was reason why I renamed a few things. For example, OAMData does not lie in OAM, so the name is misleading. The object graphics are in VRAM. For global variables, it's generally a good idea to use an identifier that shows it as such. nextTile is too generic for a global variable; make sure that globals have a very specific name. If you want an example of why generic names for globals can be problematic, go to http://www.rinkworks.com/stupid/cs_programming.shtml and search for "alpha". Lastly, the reason I used a local pointer for OBJPaletteMem was because local names can be shorter (which can make core easier to scan), local variables are faster, and because the actual algorithm (the loop) is now completely independent, meaning that it could be used for other 8-bit situations as well.