It is currently Fri Jun 22, 2018 5:56 pm

All times are UTC - 7 hours





Post new topic Reply to topic  [ 14 posts ] 
Author Message
PostPosted: Tue Apr 17, 2018 4:03 am 
Offline

Joined: Tue Apr 17, 2018 3:40 am
Posts: 8
Hi, I've been trying to develop an NES emulator and my PPU has been displaying stripes of incorrect color. The red color is color 0 of palette 0, so I've associated the problem with either shifting, loading, or reading the 8 bit attribute registers, but I'm not sure what's wrong.

From what I've read on the forums and wiki, in order to load the registers I have to pass the attribute byte through a 4 to 1 multiplexer with bit 1 of the coarse X and coarse Y as select bits and fill the registers with that:
Code:
uint8_t yBit = (this->currentVramAddr & 0x40) >> 5; // Bit 1 of coarse y in pos 1
uint8_t xBit = (this->currentVramAddr & 0x2) >> 1; // Bit 1 of carse x
// yx is used to select the corresponding 2 bits from the attribute byte
uint8_t paletteNum = (this->atByte >> ((yBit | xBit) * 2)) & 0x3;
this->highAttrShiftRegister = (paletteNum >> 1) * 0xFF;
this->lowAttrShiftRegister = (paletteNum & 0x1) * 0xFF;


I shift all 4 registers between cycles 2 and 257, and 322 and 337:
Code:
if(this->cycleNum >= 2 && this->cycleNum <= 257 || this->cycleNum >= 322 && this->cycleNum <= 337) {
   this->lowBGShiftRegister <<= 1;
   this->highBGShiftRegister <<= 1;         
   this->lowAttrShiftRegister <<= 1;
   this->highAttrShiftRegister <<= 1;         
}


I get the palette number by passing the shift register into a 8 to 1 multiplexer with the fine scroll as the select bits:
Code:
uint8_t lowAttrBit = (this->lowAttrShiftRegister >> (7 - this->fineXScroll)) & 0x1;
uint8_t highAttrBit = (this->highAttrShiftRegister >> (7 - this->fineXScroll - 1)) & 0x2;


Is my understanding correct? Thanks for any help!


Attachments:
Screenshot.png
Screenshot.png [ 13.95 KiB | Viewed 1098 times ]
Top
 Profile  
 
PostPosted: Tue Apr 17, 2018 4:24 pm 
Offline
User avatar

Joined: Sun Sep 19, 2004 10:59 pm
Posts: 1417
ace314159 wrote:
I get the palette number by passing the shift register into a 8 to 1 multiplexer with the fine scroll as the select bits:
Code:
uint8_t highAttrBit = (this->highAttrShiftRegister >> (7 - this->fineXScroll - 1)) & 0x2;


When fineXScroll is 7, that will try to shift right by a negative number, which I'm pretty sure is Undefined Behavior™.

What happens if you change it to this?
Code:
uint8_t highAttrBit = ((this->highAttrShiftRegister >> (7 - this->fineXScroll)) & 0x1) << 1;

_________________
Quietust, QMT Productions
P.S. If you don't get this note, let me know and I'll write you another.


Top
 Profile  
 
PostPosted: Tue Apr 17, 2018 9:26 pm 
Offline

Joined: Tue Apr 17, 2018 3:40 am
Posts: 8
I feel so stupid now! Thanks so much! The stripes are no longer there, but there are the random blocks which are either white or red instead of the correct color. I looked at Super Mario Bros, and saw that the incorrect color was there for certain pixels in the same column.

I'm pretty sure that my VRAM is fine, because I checked the dumps with Nintendulator at the end of the frame and saw that it matched (until 0x3000, the wiki said it doesn't matter because the ppu doesn't render from here).

Here's my code for determining the bg tile data. Maybe I messed something up here too:
Code:
uint8_t lowBGBit  = (this->lowBGShiftRegister  >> (15 - this->fineXScroll)) & 0x1;
uint8_t highBGBit = ((this->highBGShiftRegister >> (15 - this->fineXScroll)) & 0x1) << 1;

Reloading code:
Code:
if(this->cycleNum >= 9 && this->cycleNum <= 257 || this->cycleNum >= 329 && this->cycleNum <= 337) {
   this->highBGShiftRegister |= this->highTileByte;
   this->lowBGShiftRegister |= this->lowTileByte;

   uint8_t yBit = (this->currentVramAddr & 0x40) >> 5; // Bit 1 of coarse y in pos 1
   uint8_t xBit = (this->currentVramAddr & 0x2) >> 1; // Bit 1 of carse x
   // yx is used to select the corresponding 2 bits from the attribute byte
   uint8_t paletteNum = (this->atByte >> ((yBit | xBit) * 2)) & 0x3;
   this->highAttrShiftRegister = (paletteNum >> 1) * 0xFF;
   this->lowAttrShiftRegister = (paletteNum & 0x1) * 0xFF;
}


Attachments:
Capture.PNG
Capture.PNG [ 52.46 KiB | Viewed 1000 times ]
Capture.PNG
Capture.PNG [ 44.62 KiB | Viewed 1000 times ]
Top
 Profile  
 
PostPosted: Tue Apr 17, 2018 9:32 pm 
Offline
User avatar

Joined: Fri Nov 19, 2004 7:35 pm
Posts: 4051
It looks like the attributes aren't lined up with the tiles.
Super Mario Bros looks like it failed to read the title screen from CHR-ROM. PPU reads are effectively delayed by one read, so the game has to read a junk byte before reading the rest of the data.

_________________
Here come the fortune cookies! Here come the fortune cookies! They're wearing paper hats!


Top
 Profile  
 
PostPosted: Tue Apr 17, 2018 10:22 pm 
Offline
User avatar

Joined: Sat Feb 12, 2005 9:43 pm
Posts: 10523
Location: Rio de Janeiro - Brazil
Doesn't SMB also rely on palette mirroring? Maybe that's worth checking out too.


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 10:27 am 
Offline

Joined: Sun Sep 19, 2004 11:07 pm
Posts: 162
It does, but digging up memories of getting it working in an emulator suggests that when the palette mirroring is wrong, the sky turns black.

Is there an if wrapping that "Reloading code" that restricts it to just after the actual reads? The one as written looks like it would be reloading the shift register every pixel.


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 10:44 am 
Offline

Joined: Tue Apr 17, 2018 3:40 am
Posts: 8
@Dwedit, if you mean the accesses at $2007 described here, I've implemented it and tested it with blargg's test roms and the result is still the same for Mario Bros. But for Super Mario Bros, the play menu now appears, but the colors are still not correct.

@tokumaru I think I mirrored the palette correctly (at least according to blargg's test rom).

@ReaperSMS The if statement in the reloading code is checked every 8 cycles between dots 0-256 and 321-340 (inclusive) on all the visible scanlines + pre-render scanline. I run it on the cycles 1, 9, 17, 25, 33, etc. (basically when cycleNum % 8 == 1).


Attachments:
Capture.PNG
Capture.PNG [ 90.43 KiB | Viewed 916 times ]
Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 10:52 am 
Online

Joined: Sun Apr 13, 2008 11:12 am
Posts: 7227
Location: Seattle
What Dwedit said—the attributes start too far to the left by 7 pixels.


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 12:21 pm 
Offline

Joined: Tue Apr 17, 2018 3:40 am
Posts: 8
I looked through my code manyyy times, but I can't seem to figure out where the attribute bytes are going wrong. Am I doing something wrong during the scanlines? Here's my code:
Code:
if(this->scanlineNum < 240 && this->isRendering()) { // Pre-render Scanline and Visible Scanlines
   if(this->cycleNum <= 256 || this->cycleNum >= 321) {
      this->fetchData();
      if(this->scanlineNum != -1 && this->cycleNum <= 256) this->renderDot();
      if(this->cycleNum >= 2 && this->cycleNum <= 257 || this->cycleNum >= 322 && this->cycleNum <= 337) {
         this->lowBGShiftRegister <<= 1;
         this->highBGShiftRegister <<= 1;
         this->lowAttrShiftRegister <<= 1;
         this->highAttrShiftRegister <<= 1;
      }

      if(this->cycleNum % 8 == 0) this->incrementScrollX(); // Inc.hori(v)
      if(this->cycleNum == 256) this->incrementScrollY(); // Inc. vert(v)
   }
   if(this->cycleNum == 257) // hori(v) = hori(t)
      this->currentVramAddr = (this->currentVramAddr & ~0x41F) | (this->tempVramAddr & 0x41F);


   if(this->scanlineNum == 239 && this->cycleNum == 256) {
      Graphics::renderScreen();
   }
}


fetchData contains the reloading of data and preparation of new data. renderDot() contains the code for finding the pixel colors.


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 12:47 pm 
Offline

Joined: Sun Sep 19, 2004 11:07 pm
Posts: 162
what does fetchData() look like? Does it have the proper checks to fetch the right bytes at the right times?

I suspect you are trampling the attribute data immediately, rather than letting it properly shift itself out.


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 1:01 pm 
Offline

Joined: Tue Apr 17, 2018 3:40 am
Posts: 8
Code:
uint8_t fineY = (this->currentVramAddr >> 12) & 0x7;
uint16_t bgPage = (this->CTRL << 8) & 0x1000;
uint16_t v = this->currentVramAddr;
switch(this->cycleNum % 8) {
case 1:
   if(this->cycleNum >= 9 && this->cycleNum <= 257 || this->cycleNum >= 329 && this->cycleNum <= 337) {
      this->highBGShiftRegister |= this->highTileByte;
      this->lowBGShiftRegister |= this->lowTileByte;

      this->highAttrShiftRegister = (this->paletteNum >> 1) * 0xFF;
      this->lowAttrShiftRegister = (this->paletteNum & 0x1) * 0xFF;
   }
   break;
case 2: // NT Byte
   this->ntByte = mem.get8(0x2000 | (v & 0x0FFF));
   break;
case 4: // AT Byte
   {
   uint8_t atByte = mem.get8(0x23C0 | (v & 0x0C00) | ((v >> 4) & 0x38) | ((v >> 2) & 0x07));
   uint8_t yBit = (v & 0x40) >> 5; // Bit 1 of coarse y in pos 1
   uint8_t xBit = (v & 0x2) >> 1; // Bit 1 of carse x
   // yx is used to select the corresponding 2 bits from the attribute byte
   this->paletteNum = (atByte >> ((yBit | xBit) * 2)) & 0x3;
   }
   break;
case 6: // Low BG Tile Byte
   this->lowTileByte = mem.get8(bgPage | (this->ntByte << 4) | fineY);
   break;
case 0: // High BG Tile Byte
   this->highTileByte = mem.get8(bgPage | (this->ntByte << 4) | fineY | 8);
   break;
}   


I update the shift register every 8 dots, so I think I'm letting it shift.


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 2:36 pm 
Offline

Joined: Sun Sep 19, 2004 11:07 pm
Posts: 162
I think I see what's happening. You should make your attribute shift registers 16 bits wide, to match the pattern registers, and update them in a similar fashion, by ORing a 00 or FF into the lower byte.

Your current code is stomping your entire attribute shift register the moment the new tile gets merged in, but that tile won't hit the screen for 1-8 more pixels depending on the fine X scroll, hence your attributes appearing to be 7 pixels off.


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 9:13 pm 
Offline

Joined: Tue Apr 17, 2018 3:40 am
Posts: 8
That fixes everything! Thanks so much for the help!
Though, I'm wondering why the wiki mentions that the attribute shift register is only 8 bits wide. Am I doing something that forces me to use a 16 bit attribute shift register?


Top
 Profile  
 
PostPosted: Wed Apr 18, 2018 9:59 pm 
Offline

Joined: Sun Sep 19, 2004 11:12 pm
Posts: 20165
Location: NE Indiana, USA (NTSC)
It's because all pixels in a sliver (8x1-pixel area) have the same attribute, so the same bit can be shifted into the attribute register 8 times in a row.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 14 posts ] 

All times are UTC - 7 hours


Who is online

Users browsing this forum: Google [Bot] and 4 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group