Skip to content

Various accuracy improvements #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Fiskbit merged 8 commits into from
Apr 9, 2026
Merged

Various accuracy improvements #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4adc78c
Various accuracy improvements
100thCoin Mar 17, 2026
ef0434e
Updated shift register behavior
100thCoin Mar 17, 2026
eb334d9
Added a State Machine for PPUDATA Writes
100thCoin Mar 18, 2026
baa31e5
Fixed the one dot delay between reads/writes of VRAM and v being incr…
100thCoin Mar 18, 2026
8cf98f2
Added the PPU DATA State Machine to the Serializer
100thCoin Mar 31, 2026
005119a
Added comments to the bit plane initialization
100thCoin Apr 9, 2026
d51d5ce
Made changes based on Fiskbit's comments
100thCoin Apr 9, 2026
6841fdd
Fixed the change I made to RTS
100thCoin Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Core/NES/BaseNesPpu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ class BaseNesPpu : public INesMemoryHandler, public ISerializable
bool _needVideoRamIncrement = false;
bool _allowFullPpuAccess = false;

uint8_t _ppuMemoryDataReadStateMachine = 0;
uint8_t _ppuMemoryDataWriteStateMachine = 0;
uint8_t _ppuMemoryDataWriteLatch = 0;
uint8_t _memoryReadBuffer = 0;
PPUStatusFlags _statusFlags = {};

Expand Down
5 changes: 3 additions & 2 deletions Core/NES/NesCpu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ class NesCpu : public ISerializable
DummyRead();

if(CheckPageCrossed(PC(), offset)) {
DummyRead();
MemoryRead(((PC() & 0xFF00) | ((PC() + offset) & 0xFF)), MemoryOperationType::DummyRead);
}

SetPC(PC() + offset);
Expand Down Expand Up @@ -523,8 +523,9 @@ class NesCpu : public ISerializable
}

void RTS() {
DummyRead();
MemoryRead(0x100 + SP(), MemoryOperationType::DummyRead);
uint16_t addr = PopWord();
SetPC(addr);
DummyRead();
SetPC(addr + 1);
}
Expand Down
92 changes: 59 additions & 33 deletions Core/NES/NesPpu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,8 @@ template<class T> uint8_t NesPpu<T>::ReadRam(uint16_t addr)
openBusMask = 0xFF;
} else {
returnValue = _memoryReadBuffer;
_memoryReadBuffer = ReadVram(_ppuBusAddress & 0x3FFF, MemoryOperationType::Read);
// The PPU Read Buffer is not updated until the CPU read ends, and 2 more ppu cycles have passed.
_ppuMemoryDataReadStateMachine = 5;

if((_ppuBusAddress & 0x3FFF) >= 0x3F00 && !_console->GetNesConfig().DisablePaletteRead) {
//Note: When grayscale is turned on, the read values also have the grayscale mask applied to them
Expand All @@ -395,7 +396,6 @@ template<class T> uint8_t NesPpu<T>::ReadRam(uint16_t addr)

_ignoreVramRead = 6;
_needStateUpdate = true;
_needVideoRamIncrement = true;
}
break;

Expand Down Expand Up @@ -444,7 +444,7 @@ template<class T> void NesPpu<T>::WriteRam(uint16_t addr, uint8_t value)
} else {
//"Writes to OAMDATA during rendering (on the pre-render line and the visible lines 0-239, provided either sprite or background rendering is enabled) do not modify values in OAM,
//but do perform a glitchy increment of OAMADDR, bumping only the high 6 bits"
_spriteRamAddr = (_spriteRamAddr + 4) & 0xFF;
_spriteRamAddr = (_spriteRamAddr + 4) & 0xFC;
_emu->BreakIfDebugging(CpuType::Nes, BreakSource::NesInvalidOamWrite);
}
break;
Expand Down Expand Up @@ -485,20 +485,10 @@ template<class T> void NesPpu<T>::WriteRam(uint16_t addr, uint8_t value)
break;

case PpuRegisters::VideoMemoryData:
if((_ppuBusAddress & 0x3FFF) >= 0x3F00) {
WritePaletteRam(_ppuBusAddress, value);
_emu->ProcessPpuWrite<CpuType::Nes>(_ppuBusAddress, value, MemoryType::NesPpuMemory);
} else {
if(_scanline >= 240 || !IsRenderingEnabled()) {
_mapper->WriteVram(_ppuBusAddress & 0x3FFF, value);
} else {
//During rendering, the value written is ignored, and instead the address' LSB is used (not confirmed, based on Visual NES)
_mapper->WriteVram(_ppuBusAddress & 0x3FFF, _ppuBusAddress & 0xFF);
_emu->BreakIfDebugging(CpuType::Nes, BreakSource::NesInvalidVramAccess);
}
}
// The write to VRAM does not occur until the CPU write ends, and 2 more ppu cycles have passed.
_ppuMemoryDataWriteStateMachine = 5;
_ppuMemoryDataWriteLatch = value;
_needStateUpdate = true;
_needVideoRamIncrement = true;
break;

case PpuRegisters::SpriteDMA:
Expand Down Expand Up @@ -674,8 +664,10 @@ template<class T> void NesPpu<T>::LoadTileInfo()
_previousTilePalette = _currentTilePalette;
_currentTilePalette = _tile.PaletteOffset;

_lowBitShift |= _tile.LowByte;
_highBitShift &= 0xFF00; // This shift register pulls in 1's, so we need to mask away the lower 8 bits before bringing in the data from the tile fetch.
_highBitShift |= _tile.HighByte;
_lowBitShift &= 0xFF00; // Similar to the high bit plane, it's possible there are 1's in the lower 8 bits if you toggle rendering at the right time.
_lowBitShift |= _tile.LowByte;

uint8_t tileIndex = ReadVram(GetNameTableAddr());
_tile.TileAddr = (tileIndex << 4) | (_videoRamAddr >> 12) | _control.BackgroundPatternAddr;
Expand Down Expand Up @@ -812,6 +804,7 @@ template<class T> void NesPpu<T>::ShiftTileRegisters()
{
_lowBitShift <<= 1;
_highBitShift <<= 1;
_highBitShift |= 1;
}

template<class T> uint8_t NesPpu<T>::GetPixelColor()
Expand Down Expand Up @@ -880,8 +873,9 @@ template<class T> void NesPpu<T>::ProcessScanlineImpl()

if(_scanline >= 0) {
((T*)this)->DrawPixel();
ShiftTileRegisters();

if(IsRenderingEnabled()) {
ShiftTileRegisters();
}
//"Secondary OAM clear and sprite evaluation do not occur on the pre-render line"
ProcessSpriteEvaluation();
} else if(_cycle < 9) {
Expand All @@ -897,14 +891,6 @@ template<class T> void NesPpu<T>::ProcessScanlineImpl()
}
}
} else if(_cycle >= 257 && _cycle <= 320) {
if(_cycle == 257) {
_spriteIndex = 0;
memset(_hasSprite, 0, sizeof(_hasSprite));
if(_prevRenderingEnabled) {
//copy horizontal scrolling value from t
_videoRamAddr = (_videoRamAddr & ~0x041F) | (_tmpVideoRamAddr & 0x041F);
}
}
if(IsRenderingEnabled()) {
//"OAMADDR is set to 0 during each of ticks 257-320 (the sprite tile loading interval) of the pre-render and visible scanlines." (When rendering)
_spriteRamAddr = 0;
Expand All @@ -913,8 +899,8 @@ template<class T> void NesPpu<T>::ProcessScanlineImpl()
//Garbage NT sprite fetch (257, 265, 273, etc.) - Required for proper MC-ACC IRQs (MMC3 clone)
case 0: ReadVram(GetNameTableAddr()); break;

//Garbage AT sprite fetch
case 2: ReadVram(GetAttributeAddr()); break;
//Garbage NT sprite fetch
case 2: ReadVram(GetNameTableAddr()); break;

//Cycle 260, 268, etc. This is an approximation (each tile is actually loaded in 8 steps (e.g from 257 to 264))
case 4: LoadSpriteTileInfo(); break;
Expand All @@ -931,6 +917,14 @@ template<class T> void NesPpu<T>::ProcessScanlineImpl()
LoadExtraSprites();
}
}
if(_cycle == 257) {
_spriteIndex = 0;
memset(_hasSprite, 0, sizeof(_hasSprite));
if(_prevRenderingEnabled) {
//copy horizontal scrolling value from t
_videoRamAddr = (_videoRamAddr & ~0x041F) | (_tmpVideoRamAddr & 0x041F);
}
}
} else if(_cycle >= 321 && _cycle <= 336) {
LoadTileInfo();

Expand Down Expand Up @@ -1023,10 +1017,9 @@ template<class T> void NesPpu<T>::ProcessSpriteEvaluation()

if(_oamCopyDone && !_settings->GetNesConfig().EnablePpuSpriteEvalBug) {
_spriteAddrH = (_spriteAddrH + 1) & 0x3F;
if(_secondaryOamAddr >= 0x20) {
//"As seen above, a side effect of the OAM write disable signal is to turn writes to the secondary OAM into reads from it."
_oamCopybuffer = _secondarySpriteRam[_secondaryOamAddr & 0x1F];
}
//"As seen above, a side effect of the OAM write disable signal is to turn writes to the secondary OAM into reads from it."
_oamCopybuffer = _secondarySpriteRam[_secondaryOamAddr & 0x1F];

} else {
if(!_spriteInRange && _scanline >= _oamCopybuffer && _scanline < _oamCopybuffer + (_control.LargeSprites ? 16 : 8)) {
_spriteInRange = !_oamCopyDone;
Expand Down Expand Up @@ -1509,6 +1502,35 @@ template<class T> void NesPpu<T>::UpdateState()
_needVideoRamIncrement = false;
UpdateVideoRamAddr();
}

if(_ppuMemoryDataReadStateMachine > 0) {
_ppuMemoryDataReadStateMachine--;
if(_ppuMemoryDataReadStateMachine == 0) {
_memoryReadBuffer = ReadVram(_ppuBusAddress & 0x3FFF, MemoryOperationType::Read);
_needVideoRamIncrement = true;
}
_needStateUpdate = true;
}

if(_ppuMemoryDataWriteStateMachine > 0) {
_ppuMemoryDataWriteStateMachine--;
if(_ppuMemoryDataWriteStateMachine == 0) {
if((_ppuBusAddress & 0x3FFF) >= 0x3F00) {
WritePaletteRam(_ppuBusAddress, _ppuMemoryDataWriteLatch);
_emu->ProcessPpuWrite<CpuType::Nes>(_ppuBusAddress, _ppuMemoryDataWriteLatch, MemoryType::NesPpuMemory);
} else {
if(_scanline >= 240 || !IsRenderingEnabled()) {
_mapper->WriteVram(_ppuBusAddress & 0x3FFF, _ppuMemoryDataWriteLatch);
} else {
//During rendering, the value written is ignored, and instead the address' LSB is used (not confirmed, based on Visual NES)
_mapper->WriteVram(_ppuBusAddress & 0x3FFF, _ppuBusAddress & 0xFF);
_emu->BreakIfDebugging(CpuType::Nes, BreakSource::NesInvalidVramAccess);
}
}
_needVideoRamIncrement = true;
}
_needStateUpdate = true;
}
}

template<class T> uint32_t NesPpu<T>::GetPixelBrightness(uint8_t x, uint8_t y)
Expand Down Expand Up @@ -1566,6 +1588,10 @@ template<class T> void NesPpu<T>::Serialize(Serializer& s)

SV(_allowFullPpuAccess);

SV(_ppuMemoryDataReadStateMachine);
SV(_ppuMemoryDataWriteStateMachine);
SV(_ppuMemoryDataWriteLatch);

for(int i = 0; i < _spriteCount; i++) {
SVI(_spriteTiles[i].SpriteX); SVI(_spriteTiles[i].LowByte); SVI(_spriteTiles[i].HighByte); SVI(_spriteTiles[i].PaletteOffset); SVI(_spriteTiles[i].HorizontalMirror); SVI(_spriteTiles[i].BackgroundPriority);
}
Expand Down