From ba4e366ae60e51c390cd048e806493cc16b46681 Mon Sep 17 00:00:00 2001 From: Fiskbit <2811896+Fiskbit@users.noreply.github.com> Date: Sat, 9 May 2026 12:33:25 -0700 Subject: [PATCH 1/2] NES: Improve OAM corruption accuracy. Our understanding of OAM row corruption has significantly improved since Mesen's implementation of the feature that was based on oam_flicker_test.nes results. This change improves the accuracy of the two OAM corruption emulation options. Primary OAM (OAM1) and secondary OAM (OAM2) live in the same block of memory, made up of 9-byte rows where the first 8 bytes are consecutive OAM1 bytes and the last byte is an OAM2 byte. The PPU accesses OAM on every dot. On some dots, it uses the OAM1 address ($2003), and on other dots, it uses its internal OAM2 address. If the address changes during an OAM access (the 2nd half of a dot) and this causes the selected row to change, then it can trigger either analog corruption (the new row gets 'random' data) or a row copy (the old row's 9 bytes are copied to the new row). More information can be found at nesdev.org/wiki/OAM_internals. Address selection is fairly simple. When rendering is off, OAM1ADDR is always selected. When rendering is on, OAM1ADDR is selected on odd dots in 1-256, and otherwise OAM2ADDR is selected. Corruption occurs on the NES in 3 situations: 1. Writing to $2003 on NTSC PPUs, because writes are not synchronized at all and so the CPU open bus value and the new value can both cause the row to change mid-access. Depending on the timing, this can do nothing, cause analog corruption, or cause row copy. This was fixed in PAL PPUs, but we don't know about Dendy. 2. Toggling rendering on an OAM2ADDR dot. Depending on the direction and the timing, this can do nothing or cause analog corruption of or row copy to the new row. 3. On 2C02E and later PPUs, having a row mismatch if rendering is enabled at the start of pre-render. This always causes a row copy from the OAM1ADDR row to the OAM2ADDR row and is required for Huge Insect to spawn enemies in level 1. (2C02C PPUs have some corruption in this situation, too, but it is not yet understood.) This change adds support for OAM corruption in situations 1 and 2 using the "Enable PPU $2003/rendering-toggle OAM corruption" setting, which is off by default. This does not take alignment into account at all and only does row copy, not analog corruption. Recommended for developers, but probably undesirable for actual users. This setting was previously called "Enable PPU OAM row corruption emulation". Note that situation 1 ($2003 writes) is not perfectly matching blargg's oam_read test results in the most vulnerable alignment yet, suggesting there is more research to be done on this. Situation 3 is now supported under the "Disable PPU frame-start OAM corruption (2C02D and earlier)" setting, so this corruption happens by default. Mesen was already doing some corruption here to support Huge Insect, with the previous version of this setting being called "Disable PPU OAMADDR bug emulation". These changes make the corruption more accurate and apply in more situations, so there is a chance it will break some user software, but because it is required for a game, we've opted to have it on by default. This feature should match hardware very closely, as it doesn't have the timing and analog aspects seen in situations 1 and 2. To support these, sprite rendering was reworked a bit to more accurately manage the OAM1 and OAM2 addresses during rendering. This should make sprite rendering behavior more accurate when rendering is toggled mid-frame, although there are still inaccuracies that come up if you do this. The OAM2 address can also now be seen in the register viewer below $2003. This was tested against a suite of NES tests, AccuracyCoin $2004 Stress Test, and oam_flicker_test.nes/oam_flicker_test_reenable.nes. The $2003-based corruption causes a few tests to change behavior when enabled (which is expected), but oam_read and related tests are not failing exactly right yet. Tests otherwise pass. --- Core/NES/BaseNesPpu.cpp | 1 + Core/NES/BaseNesPpu.h | 1 - Core/NES/NesPpu.cpp | 171 ++++++++---------- Core/NES/NesPpu.h | 3 +- Core/NES/NesTypes.h | 1 + .../RegisterViewer/NesRegisterViewer.cs | 3 +- UI/Interop/ConsoleState/NesState.cs | 1 + UI/Localization/resources.en.xml | 4 +- 8 files changed, 83 insertions(+), 102 deletions(-) diff --git a/Core/NES/BaseNesPpu.cpp b/Core/NES/BaseNesPpu.cpp index ea5d46660..25911f7ff 100644 --- a/Core/NES/BaseNesPpu.cpp +++ b/Core/NES/BaseNesPpu.cpp @@ -16,6 +16,7 @@ void BaseNesPpu::GetState(NesPpuState& state) state.VideoRamAddr = _videoRamAddr; state.TmpVideoRamAddr = _tmpVideoRamAddr; state.SpriteRamAddr = _spriteRamAddr; + state.SecondaryOamAddr = _secondaryOamAddr; state.ScrollX = _xScroll; state.WriteToggle = _writeToggle; diff --git a/Core/NES/BaseNesPpu.h b/Core/NES/BaseNesPpu.h index 4b0ad92ec..21b53f87b 100644 --- a/Core/NES/BaseNesPpu.h +++ b/Core/NES/BaseNesPpu.h @@ -116,7 +116,6 @@ class BaseNesPpu : public INesMemoryHandler, public ISerializable int32_t _openBusDecayStamp[8] = {}; uint64_t _oamDecayCycles[0x20] = {}; - bool _corruptOamRow[32] = {}; bool IsRenderingEnabled(); void UpdateGrayscaleAndIntensifyBits(); diff --git a/Core/NES/NesPpu.cpp b/Core/NES/NesPpu.cpp index 79dd7e9d4..dc316df22 100644 --- a/Core/NES/NesPpu.cpp +++ b/Core/NES/NesPpu.cpp @@ -56,9 +56,6 @@ template NesPpu::NesPpu(NesConsole* console) memcpy(_paletteRam, paletteRamBootValues, sizeof(_paletteRam)); } - //This should (presumably) persist across resets - memset(_corruptOamRow, 0, sizeof(_corruptOamRow)); - //'v' is not cleared on reset, but it set to 0 on power on _videoRamAddr = 0; @@ -316,13 +313,10 @@ template uint8_t NesPpu::PeekRam(uint16_t addr) case PpuRegisters::SpriteData: if(!_console->GetNesConfig().DisablePpu2004Reads) { if(_scanline <= 239 && IsRenderingEnabled()) { - if(_cycle >= 257 && _cycle <= 320) { - uint8_t step = ((_cycle - 257) % 8) > 3 ? 3 : ((_cycle - 257) % 8); - uint8_t oamAddr = (_cycle - 257) / 8 * 4 + step; - returnValue = _secondarySpriteRam[oamAddr]; - } else { - returnValue = _oamCopybuffer; + if((_cycle >= 257 && _cycle <= 340) || _cycle == 0) { + returnValue = _secondarySpriteRam[_secondaryOamAddr & 0x1F]; } + returnValue = _oamCopybuffer; } else { returnValue = _spriteRam[_spriteRamAddr]; } @@ -368,19 +362,17 @@ template uint8_t NesPpu::ReadRam(uint16_t addr) case PpuRegisters::SpriteData: if(!_console->GetNesConfig().DisablePpu2004Reads) { if(_scanline <= 239 && IsRenderingEnabled()) { - //While the screen is begin drawn - if(_cycle >= 257 && _cycle <= 320) { + //While the screen is being drawn + if((_cycle >= 257 && _cycle <= 340) || _cycle == 0) { //If we're doing sprite rendering, set OAM copy buffer to its proper value. This is done here for performance. //It's faster to only do this here when it's needed, rather than splitting LoadSpriteTileInfo() into an 8-step process - uint8_t step = ((_cycle - 257) % 8) > 3 ? 3 : ((_cycle - 257) % 8); - _secondaryOamAddr = (_cycle - 257) / 8 * 4 + step; - _oamCopybuffer = _secondarySpriteRam[_secondaryOamAddr]; + _oamCopybuffer = _secondarySpriteRam[_secondaryOamAddr & 0x1F]; } //Return the value that PPU is currently using for sprite evaluation/rendering returnValue = _oamCopybuffer; } else { returnValue = ReadSpriteRam(_spriteRamAddr); - _emu->ProcessPpuWrite(_spriteRamAddr, returnValue, MemoryType::NesSpriteRam); + _emu->ProcessPpuRead(_spriteRamAddr, returnValue, MemoryType::NesSpriteRam); } openBusMask = 0x00; } @@ -447,6 +439,19 @@ template void NesPpu::WriteRam(uint16_t addr, uint8_t value) break; case PpuRegisters::SpriteAddr: + //If OAM1ADDR is selected, updating the address can corrupt 2 rows (the open bus row and the destination row). + //This is actually alignment-dependent, but we're currently modeling worst-case behavior. + if(_settings->GetNesConfig().EnablePpuOamRowCorruption && _region == ConsoleRegion::Ntsc && (!_prevRenderingEnabled || _scanline >= 240 || (_cycle < 257 && (_cycle & 1)))) { + uint8_t sourceRow = _spriteRamAddr >> 3; + uint8_t openBusRow = _console->GetMemoryManager()->GetOpenBus() >> 3; + uint8_t destRow = value >> 3; + CorruptOamRow(sourceRow, openBusRow); + CorruptOamRow(openBusRow, destRow); + } + //TODO There is some kind of delay on committing the $2003 value on PAL (this is how it fixes the corruption issue). + //This currently isn't emulated. + //"In the 2C02, writes to $2003 took effect immediately. In the 2C07, they're buffered through pclk0+pclk1+pclk0[+pclk1+pclk0]. + //I'm guessing that's what it took to prevent corruption. (it's actually pclk0+pclk1+pclk0 NOR pclk0+pclk1+pclk0+pclk1+pclk0)" _spriteRamAddr = value; break; @@ -891,25 +896,20 @@ template void NesPpu::ProcessScanlineImpl() if(_scanline >= 0) { ((T*)this)->DrawPixel(); - if(IsRenderingEnabled()) { + if(_prevRenderingEnabled) { ShiftTileRegisters(); } //"Secondary OAM clear and sprite evaluation do not occur on the pre-render line" ProcessSpriteEvaluation(); - } else if(_cycle < 9) { + } else if(_cycle == 1) { //Pre-render scanline logic - if(_cycle == 1) { - _statusFlags.VerticalBlank = false; - _console->GetCpu()->ClearNmiFlag(); - } - if(_spriteRamAddr >= 0x08 && IsRenderingEnabled() && !_settings->GetNesConfig().DisableOamAddrBug) { - //This should only be done if rendering is enabled (otherwise oam_stress test fails immediately) - //"If OAMADDR is not less than eight when rendering starts, the eight bytes starting at OAMADDR & 0xF8 are copied to the first eight bytes of OAM" - WriteSpriteRam(_cycle - 1, ReadSpriteRam((_spriteRamAddr & 0xF8) + _cycle - 1)); - } + _statusFlags.VerticalBlank = false; + _console->GetCpu()->ClearNmiFlag(); } } else if(_cycle >= 257 && _cycle <= 320) { - if(IsRenderingEnabled()) { + if(_prevRenderingEnabled) { + _sprite0Visible = _sprite0Added; + //"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; @@ -934,6 +934,7 @@ template void NesPpu::ProcessScanlineImpl() if(_cycle == 320) { LoadExtraSprites(); } + _secondaryOamAddr += !((_cycle - 1) & 4); } if(_cycle == 257) { _spriteIndex = 0; @@ -941,13 +942,17 @@ template void NesPpu::ProcessScanlineImpl() if(_prevRenderingEnabled) { //copy horizontal scrolling value from t _videoRamAddr = (_videoRamAddr & ~0x041F) | (_tmpVideoRamAddr & 0x041F); + + //Fix up OAM2ADDR, which got incremented an extra time earlier. By doing this here, we avoid an extra if every dot. + _secondaryOamAddr--; } } } else if(_cycle >= 321 && _cycle <= 336) { LoadTileInfo(); if(_cycle == 321) { - if(IsRenderingEnabled()) { + if(_prevRenderingEnabled) { + _secondaryOamAddr++; _oamCopybuffer = _secondarySpriteRam[0]; } } else if(_prevRenderingEnabled && (_cycle == 328 || _cycle == 336)) { @@ -990,8 +995,6 @@ template void NesPpu::ProcessSpriteEvaluationStart() template void NesPpu::ProcessSpriteEvaluationEnd() { - _sprite0Visible = _sprite0Added; - //Add 3 to address to count any partially-copied sprite. //If eval is misaligned and wraps back to the start of OAM, the copy can //be stopped mid-sprite (e.g only 1 to 3 bytes are copied to secondary OAM) @@ -1011,15 +1014,19 @@ template void NesPpu::ProcessSpriteEvaluationEnd() _spriteCount++; } } + + _secondaryOamAddr = 0; } template void NesPpu::ProcessSpriteEvaluation() { - if(IsRenderingEnabled() || (_region == ConsoleRegion::Pal && _scanline >= _palSpriteEvalScanline)) { + if(_prevRenderingEnabled || (_region == ConsoleRegion::Pal && _scanline >= _palSpriteEvalScanline)) { if(_cycle < 65) { //Clear secondary OAM at between cycle 1 and 64 _oamCopybuffer = 0xFF; - _secondarySpriteRam[(_cycle - 1) >> 1] = 0xFF; + _secondarySpriteRam[_secondaryOamAddr & 0x1F] = 0xFF; + + _secondaryOamAddr += !(_cycle & 1); } else { if(_cycle & 0x01) { if(_cycle == 65) { @@ -1029,9 +1036,9 @@ template void NesPpu::ProcessSpriteEvaluation() //Read a byte from the primary OAM on odd cycles _oamCopybuffer = ReadSpriteRam(_spriteRamAddr); } else { - if(_cycle == 256) { - ProcessSpriteEvaluationEnd(); - } + //TODO These really should be local variables, because they're just a convenient way of handling the two parts of OAM1ADDR. + _spriteAddrH = _spriteRamAddr >> 2; + _spriteAddrL = _spriteRamAddr & 3; if(_oamCopyDone && !_settings->GetNesConfig().EnablePpuSpriteEvalBug) { _spriteAddrH = (_spriteAddrH + 1) & 0x3F; @@ -1069,6 +1076,7 @@ template void NesPpu::ProcessSpriteEvaluation() //Note: Using "(_secondaryOamAddr & 0x03) == 0" instead of "_spriteAddrL == 0" is required //to replicate a hardware bug noticed in oam_flicker_test_reenable when disabling & re-enabling //rendering on a single scanline + //TODO This should actually be using a timer (which runs even with rendering off) rather than the OAM2 address. if((_secondaryOamAddr & 0x03) == 0) { //Done copying all 4 bytes _spriteInRange = false; @@ -1134,6 +1142,9 @@ template void NesPpu::ProcessSpriteEvaluation() } } _spriteRamAddr = (_spriteAddrL & 0x03) | (_spriteAddrH << 2); + if(_cycle == 256) { + ProcessSpriteEvaluationEnd(); + } } } } @@ -1279,49 +1290,6 @@ template void NesPpu::DebugUpdateFrameBuffer(bool toGrayscale) } } -template void NesPpu::SetOamCorruptionFlags() -{ - if(!_console->GetNesConfig().EnablePpuOamRowCorruption) { - return; - } - - //Note: Still pending more research, but this currently matches a portion of the issues that have been observed - //When rendering is disabled in some sections of the screen, either: - // A- During Secondary OAM clear (first ~64 cycles) - // B- During OAM tile fetching (cycle ~256 to cycle ~320) - //then OAM memory gets corrupted the next time the PPU starts rendering again (usually at the start of the next frame) - //This usually causes the first "row" of OAM (the first 8 bytes) to get copied over another, causing some sprites to be missing - //and causing an extra set of the first 2 sprites to appear on the screen (not possible to see them except via any overflow they may cause) - - if(_cycle >= 0 && _cycle < 64) { - //Every 2 dots causes the corruption to shift down 1 OAM row (8 bytes) - _corruptOamRow[_cycle >> 1] = true; - } else if(_cycle >= 256 && _cycle < 320) { - //This section is in 8-dot segments. - //The first 3 dot increment the corrupted row by 1, and then the last 5 dots corrupt the next row for 5 dots. - uint8_t base = (_cycle - 256) >> 3; - uint8_t offset = std::min(3, (_cycle - 256) & 0x07); - _corruptOamRow[base * 4 + offset] = true; - } -} - -template void NesPpu::ProcessOamCorruption() -{ - if(!_console->GetNesConfig().EnablePpuOamRowCorruption) { - return; - } - - //Copy first OAM row over another row, as needed by corruption flags (can be over itself, which causes no actual harm) - for(int i = 0; i < 32; i++) { - if(_corruptOamRow[i]) { - if(i > 0) { - memcpy(_spriteRam + i * 8, _spriteRam, 8); - } - _corruptOamRow[i] = false; - } - } -} - template void NesPpu::Exec() { if(_cycle < 340) { @@ -1369,10 +1337,6 @@ template void NesPpu::ProcessScanlineFirstCycle() //Force prerender scanline sprite fetches to load the dummy $FF tiles (fixes shaking in Ninja Gaiden 3 stage 1 after beating boss) _spriteCount = 0; - if(_renderingEnabled) { - ProcessOamCorruption(); - } - _emu->ProcessEvent(EventType::StartFrame); UpdateMinimumDrawCycles(); @@ -1392,10 +1356,17 @@ template void NesPpu::ProcessScanlineFirstCycle() _statusFlags.Sprite0Hit = false; _allowFullPpuAccess = true; + //2C02E+ bug where OAM reliably corrupts if the row changes at the start of pre-render. Required for Huge Insect. + if(IsRenderingEnabled() && !_settings->GetNesConfig().DisableOamAddrBug) { + CorruptOamRow(_spriteRamAddr >> 3, _secondaryOamAddr & 0x1F); + } + //Switch to alternate output buffer (VideoDecoder may still be decoding the last frame buffer) _currentOutputBuffer = (_currentOutputBuffer == _outputBuffers[0]) ? _outputBuffers[1] : _outputBuffers[0]; _emu->AddDebugEvent(DebugEventType::BgColorChange); } else if(_prevRenderingEnabled) { + _secondaryOamAddr = 0; + if(_scanline > 0 || (!(_frameCount & 0x01) || _region != ConsoleRegion::Ntsc || GetPpuModel() != PpuModel::Ppu2C02)) { //Set bus address to the tile address calculated from the unused NT fetches at the end of the previous scanline //This doesn't happen on scanline 0 if the last dot of the previous frame was skipped @@ -1403,6 +1374,9 @@ template void NesPpu::ProcessScanlineFirstCycle() } } } else if(_scanline == 240) { + if(_prevRenderingEnabled) { + _secondaryOamAddr = 0; + } //At the start of vblank, the bus address is set back to VideoRamAddr. //According to Visual NES, this occurs on scanline 240, cycle 1, but is done here on cycle 0 for performance reasons SetBusAddress(_videoRamAddr & 0x3FFF); @@ -1418,6 +1392,17 @@ template void NesPpu::ProcessScanlineFirstCycle() } } +template void NesPpu::CorruptOamRow(uint8_t sourceRow, uint8_t destRow) +{ + if(sourceRow != destRow) { + uint8_t sourceByte = (sourceRow << 3) & 0xFF; + uint8_t destByte = (destRow << 3) & 0xFF; + + memcpy(_spriteRam + destByte, _spriteRam + sourceByte, 8); + _secondarySpriteRam[destRow] = _secondarySpriteRam[sourceRow]; + } +} + template void NesPpu::UpdateState() { _needStateUpdate = false; @@ -1427,13 +1412,15 @@ template void NesPpu::UpdateState() _emu->AddDebugEvent(DebugEventType::BgColorChange); _prevRenderingEnabled = _renderingEnabled; if(_scanline < 240) { - if(_prevRenderingEnabled) { - //Rendering was just enabled, perform oam corruption if any is pending - ProcessOamCorruption(); - } else if(!_prevRenderingEnabled) { - //Rendering was just disabled by a write to $2001, check for oam row corruption glitch - SetOamCorruptionFlags(); + if(_settings->GetNesConfig().EnablePpuOamRowCorruption && (_cycle >= 257 || (_cycle & 1) == 0)) { + if(_renderingEnabled) { + CorruptOamRow(_spriteRamAddr >> 3, _secondaryOamAddr & 0x1F); //OAM1 row to OAM2 row. + } else { + CorruptOamRow(_secondaryOamAddr & 0x1F, _spriteRamAddr >> 3); //OAM2 row to OAM1 row. + } + } + if(!_prevRenderingEnabled) { //When rendering is disabled midscreen, set the vram bus back to the value of 'v' SetBusAddress(_videoRamAddr & 0x3FFF); @@ -1444,12 +1431,6 @@ template void NesPpu::UpdateState() // if rendering is disabled on an odd cycle, the increment will wait until the next odd cycle (at which point it will be incremented by 1) //In practice, there is no way to see the difference, so we just increment by 1 at the end of the next cycle after rendering was disabled _spriteRamAddr++; - - //Also corrupt H/L to replicate a bug found in oam_flicker_test_reenable when rendering is disabled around scanlines 128-136 - //Reenabling the causes the OAM evaluation to restart misaligned, and ends up generating a single sprite that's offset by 1 - //such that it's Y=tile index, index = attributes, attributes = x, and X = the next sprite's Y value - _spriteAddrH = (_spriteRamAddr >> 2) & 0x3F; - _spriteAddrL = _spriteRamAddr & 0x03; } } } @@ -1645,8 +1626,6 @@ template void NesPpu::Serialize(Serializer& s) _oamDecayCycles[i] = _console->GetCpu()->GetCycleCount(); } - memset(_corruptOamRow, 0, sizeof(_corruptOamRow)); - for(int i = 0; i < 257; i++) { _hasSprite[i] = true; } diff --git a/Core/NES/NesPpu.h b/Core/NES/NesPpu.h index 7298ba943..abe4e5a1a 100644 --- a/Core/NES/NesPpu.h +++ b/Core/NES/NesPpu.h @@ -70,8 +70,7 @@ class NesPpu : public BaseNesPpu __forceinline uint8_t ReadSpriteRam(uint8_t addr); __forceinline void WriteSpriteRam(uint8_t addr, uint8_t value); - void SetOamCorruptionFlags(); - void ProcessOamCorruption(); + __forceinline void CorruptOamRow(uint8_t sourceRow, uint8_t destRow); __forceinline uint8_t GetPixelColor(); diff --git a/Core/NES/NesTypes.h b/Core/NES/NesTypes.h index 7e30bd15c..71d4379ff 100644 --- a/Core/NES/NesTypes.h +++ b/Core/NES/NesTypes.h @@ -258,6 +258,7 @@ struct NesPpuState : public BaseState uint8_t ScrollX; bool WriteToggle; uint8_t SpriteRamAddr; + uint8_t SecondaryOamAddr; }; struct ApuLengthCounterState diff --git a/UI/Debugger/RegisterViewer/NesRegisterViewer.cs b/UI/Debugger/RegisterViewer/NesRegisterViewer.cs index e8e09e643..22666207c 100644 --- a/UI/Debugger/RegisterViewer/NesRegisterViewer.cs +++ b/UI/Debugger/RegisterViewer/NesRegisterViewer.cs @@ -58,7 +58,8 @@ private static RegisterViewerTab GetNesPpuTab(ref NesState state) new RegEntry("$2002.6", "Sprite 0 hit", ppu.StatusFlags.Sprite0Hit), new RegEntry("$2002.7", "Vertical blank", ppu.StatusFlags.VerticalBlank), - new RegEntry("$2003", "OAM address", ppu.SpriteRamAddr, Format.X8), + new RegEntry("$2003", "OAM1 address", ppu.SpriteRamAddr, Format.X8), + new RegEntry("", "OAM2 address", ppu.SecondaryOamAddr & 0x1F, Format.X8), new RegEntry("$2005-2006", "VRAM Address / Scrolling"), new RegEntry("", "VRAM Address", ppu.VideoRamAddr, Format.X16), diff --git a/UI/Interop/ConsoleState/NesState.cs b/UI/Interop/ConsoleState/NesState.cs index e2425ac4d..d80a2e500 100644 --- a/UI/Interop/ConsoleState/NesState.cs +++ b/UI/Interop/ConsoleState/NesState.cs @@ -52,6 +52,7 @@ public struct NesPpuState : BaseState public byte ScrollX; [MarshalAs(UnmanagedType.I1)] public bool WriteToggle; public byte SpriteRamAddr; + public byte SecondaryOamAddr; }; [Flags] diff --git a/UI/Localization/resources.en.xml b/UI/Localization/resources.en.xml index 27d18b85a..b1c46ba36 100644 --- a/UI/Localization/resources.en.xml +++ b/UI/Localization/resources.en.xml @@ -342,13 +342,13 @@ Randomize power-on state for mappers Randomize power-on/reset CPU/PPU alignment Enable PPU OAM decay - Enable PPU OAM row corruption emulation + Enable PPU $2003/rendering-toggle OAM corruption Enable PPU $2006 scroll glitch emulation Enable PPU $2000/$2005/$2006 first-write scroll glitch emulation Clear PPU $2000/1/5/6/7 throughout first frame after reset/power on Miscellaneous Settings - Disable PPU OAMADDR bug emulation + Disable PPU frame-start OAM corruption (2C02D and earlier) Disable PPU palette reads Disable PPU $2004 reads Enable erroneous sprite pixels at X=255 (2C02B and earlier) From a09635a8f9a7ddbb787a45973b5787b2e77baeb6 Mon Sep 17 00:00:00 2001 From: Fiskbit <2811896+Fiskbit@users.noreply.github.com> Date: Sun, 17 May 2026 22:16:50 -0700 Subject: [PATCH 2/2] Changes based on code review. --- Core/NES/BaseNesPpu.cpp | 1 + Core/NES/NesPpu.cpp | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Core/NES/BaseNesPpu.cpp b/Core/NES/BaseNesPpu.cpp index 25911f7ff..111691014 100644 --- a/Core/NES/BaseNesPpu.cpp +++ b/Core/NES/BaseNesPpu.cpp @@ -41,6 +41,7 @@ void BaseNesPpu::SetState(NesPpuState& state) _xScroll = state.ScrollX; _writeToggle = state.WriteToggle; _spriteRamAddr = state.SpriteRamAddr; + _secondaryOamAddr = state.SecondaryOamAddr; _cycle = state.Cycle; _scanline = state.Scanline; diff --git a/Core/NES/NesPpu.cpp b/Core/NES/NesPpu.cpp index dc316df22..11eaf4f26 100644 --- a/Core/NES/NesPpu.cpp +++ b/Core/NES/NesPpu.cpp @@ -934,6 +934,8 @@ template void NesPpu::ProcessScanlineImpl() if(_cycle == 320) { LoadExtraSprites(); } + //Increment OAM2ADDR during the first 4 dots of each set of 8. There is special behavior where the first set of 8 skips the + //first increment, and an extra increment is done after the last set of 8. _secondaryOamAddr += !((_cycle - 1) & 4); } if(_cycle == 257) { @@ -952,8 +954,8 @@ template void NesPpu::ProcessScanlineImpl() if(_cycle == 321) { if(_prevRenderingEnabled) { + //Do the final increment, which follows the last set of 8 sprite fetch dots. _secondaryOamAddr++; - _oamCopybuffer = _secondarySpriteRam[0]; } } else if(_prevRenderingEnabled && (_cycle == 328 || _cycle == 336)) { _lowBitShift <<= 8;