PPU Random Stripes of Color

Discuss emulation of the Nintendo Entertainment System and Famicom.

Moderator: Moderators

Post Reply
ace314159
Posts: 27
Joined: Tue Apr 17, 2018 3:40 am

PPU Random Stripes of Color

Post by ace314159 »

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: Select all

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: Select all

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: Select all

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
User avatar
Quietust
Posts: 1920
Joined: Sun Sep 19, 2004 10:59 pm
Contact:

Re: PPU Random Stripes of Color

Post by Quietust »

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: Select all

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: Select all

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.
ace314159
Posts: 27
Joined: Tue Apr 17, 2018 3:40 am

Re: PPU Random Stripes of Color

Post by ace314159 »

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: Select all

uint8_t lowBGBit  = (this->lowBGShiftRegister  >> (15 - this->fineXScroll)) & 0x1;
uint8_t highBGBit = ((this->highBGShiftRegister >> (15 - this->fineXScroll)) & 0x1) << 1;
Reloading code:

Code: Select all

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
User avatar
Dwedit
Posts: 4924
Joined: Fri Nov 19, 2004 7:35 pm
Contact:

Re: PPU Random Stripes of Color

Post by Dwedit »

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!
User avatar
tokumaru
Posts: 12427
Joined: Sat Feb 12, 2005 9:43 pm
Location: Rio de Janeiro - Brazil

Re: PPU Random Stripes of Color

Post by tokumaru »

Doesn't SMB also rely on palette mirroring? Maybe that's worth checking out too.
ReaperSMS
Posts: 174
Joined: Sun Sep 19, 2004 11:07 pm

Re: PPU Random Stripes of Color

Post by ReaperSMS »

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.
ace314159
Posts: 27
Joined: Tue Apr 17, 2018 3:40 am

Re: PPU Random Stripes of Color

Post by ace314159 »

@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
lidnariq
Posts: 11432
Joined: Sun Apr 13, 2008 11:12 am

Re: PPU Random Stripes of Color

Post by lidnariq »

What Dwedit said—the attributes start too far to the left by 7 pixels.
ace314159
Posts: 27
Joined: Tue Apr 17, 2018 3:40 am

Re: PPU Random Stripes of Color

Post by ace314159 »

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: Select all

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.
ReaperSMS
Posts: 174
Joined: Sun Sep 19, 2004 11:07 pm

Re: PPU Random Stripes of Color

Post by ReaperSMS »

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.
ace314159
Posts: 27
Joined: Tue Apr 17, 2018 3:40 am

Re: PPU Random Stripes of Color

Post by ace314159 »

Code: Select all

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.
ReaperSMS
Posts: 174
Joined: Sun Sep 19, 2004 11:07 pm

Re: PPU Random Stripes of Color

Post by ReaperSMS »

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.
ace314159
Posts: 27
Joined: Tue Apr 17, 2018 3:40 am

Re: PPU Random Stripes of Color

Post by ace314159 »

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?
tepples
Posts: 22708
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: PPU Random Stripes of Color

Post by tepples »

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.
Post Reply