diff --git a/Core/NES/BaseNesPpu.cpp b/Core/NES/BaseNesPpu.cpp index ea5d46660..111691014 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; @@ -40,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/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..11eaf4f26 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,9 @@ 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) { _spriteIndex = 0; @@ -941,14 +944,18 @@ 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()) { - _oamCopybuffer = _secondarySpriteRam[0]; + if(_prevRenderingEnabled) { + //Do the final increment, which follows the last set of 8 sprite fetch dots. + _secondaryOamAddr++; } } else if(_prevRenderingEnabled && (_cycle == 328 || _cycle == 336)) { _lowBitShift <<= 8; @@ -990,8 +997,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 +1016,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 +1038,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 +1078,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 +1144,9 @@ template void NesPpu::ProcessSpriteEvaluation() } } _spriteRamAddr = (_spriteAddrL & 0x03) | (_spriteAddrH << 2); + if(_cycle == 256) { + ProcessSpriteEvaluationEnd(); + } } } } @@ -1279,49 +1292,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 +1339,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 +1358,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 +1376,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 +1394,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 +1414,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 +1433,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 +1628,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)