NES: Added option to emulate sprite eval bug that causes unexpected p…

…ixels to be shown at X=255

This bug exists only on older PPU revisions (2C02B and earlier)

Core/NES/NesPpu.cpp

@@ -950,6 +950,47 @@ template<class T> void NesPpu<T>::ProcessScanlineImpl()
 	}
 }
 
+template<class T> void NesPpu<T>::ProcessSpriteEvaluationStart()
+{
+	_sprite0Added = false;
+	_spriteInRange = false;
+	_secondaryOamAddr = 0;
+
+	_overflowBugCounter = 0;
+
+	_oamCopyDone = false;
+
+	//Sprite evaluation does not necessarily start on the first byte of OAM
+	//it can start on any byte (based on the OAM address in 2003), and interprets
+	//that byte as the "sprite 0" Y coordinate.
+	_spriteAddrH = (_spriteRamAddr >> 2) & 0x3F;
+	_spriteAddrL = _spriteRamAddr & 0x03;
+
+	_firstVisibleSpriteAddr = _spriteAddrH * 4;
+	_lastVisibleSpriteAddr = _firstVisibleSpriteAddr;
+}
+
+template<class T> void NesPpu<T>::ProcessSpriteEvaluationEnd()
+{
+	_sprite0Visible = _sprite0Added;
+	_spriteCount = (_secondaryOamAddr >> 2);
+
+	if(_settings->GetNesConfig().EnablePpuSpriteEvalBug) {
+		//(Not entirely confirmed - but matches observed behavior)
+		//For early PPUs (2C02B and earlier), after sprite eval wraps back to the start of OAM,
+		//all subsequent sprites appear to be considered as "out of range", causing only their
+		//Y coordinate to be copied to secondary OAM, and then skipping to the next sprite.
+		//However, if the last Y position copied to secondary OAM by this process happens to be 
+		//"in range", it will be end up being shown as a sprite. The sprite's remaining 3 bytes
+		//will be $FF (because secondary OAM was cleared at the start of the scanline), causing
+		//it to display pixels from sprite tile $FF at X=255, with h+v mirroring and sprite palette 3.
+		bool inRange = (_scanline >= _oamCopybuffer && _scanline < _oamCopybuffer + (_control.LargeSprites ? 16 : 8));
+		if(inRange && _spriteCount < 8) {
+			_spriteCount++;
+		}
+	}
+}
+
 template<class T> void NesPpu<T>::ProcessSpriteEvaluation()
 {
 	if(IsRenderingEnabled() || (_region == ConsoleRegion::Pal && _scanline >= _palSpriteEvalScanline)) {
@@ -958,41 +999,27 @@ template<class T> void NesPpu<T>::ProcessSpriteEvaluation()
 			_oamCopybuffer = 0xFF;
 			_secondarySpriteRam[(_cycle - 1) >> 1] = 0xFF;
 		} else {
-			if(_cycle == 65) {
-				_sprite0Added = false;
-				_spriteInRange = false;
-				_secondaryOamAddr = 0;
-
-				_overflowBugCounter = 0;
-
-				_oamCopyDone = false;
-
-				//Sprite evaluation does not necessarily start on the first byte of OAM
-				//it can start on any byte (based on the OAM address in 2003), and interprets
-				//that byte as the "sprite 0" Y coordinate.
-				_spriteAddrH = (_spriteRamAddr >> 2) & 0x3F;
-				_spriteAddrL = _spriteRamAddr & 0x03;
-
-				_firstVisibleSpriteAddr = _spriteAddrH * 4;
-				_lastVisibleSpriteAddr = _firstVisibleSpriteAddr;
-			} else if(_cycle == 256) {
-				_sprite0Visible = _sprite0Added;
-				_spriteCount = (_secondaryOamAddr >> 2);
-			}
-
 			if(_cycle & 0x01) {
+				if(_cycle == 65) {
+					ProcessSpriteEvaluationStart();
+				}
+
 				//Read a byte from the primary OAM on odd cycles
 				_oamCopybuffer = ReadSpriteRam(_spriteRamAddr);
 			} else {
-				if(_oamCopyDone) {
+				if(_cycle == 256) {
+					ProcessSpriteEvaluationEnd();
+				}
+
+				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];
 					}
 				} else {
 					if(!_spriteInRange && _scanline >= _oamCopybuffer && _scanline < _oamCopybuffer + (_control.LargeSprites ? 16 : 8)) {
-						_spriteInRange = true;
+						_spriteInRange = !_oamCopyDone;
 					}
 
 					if(_secondaryOamAddr < 0x20) {
@@ -1013,6 +1040,10 @@ template<class T> void NesPpu<T>::ProcessSpriteEvaluation()
 							if(_spriteAddrL >= 4) {
 								_spriteAddrH = (_spriteAddrH + 1) & 0x3F;
 								_spriteAddrL = 0;
+
+								if(_spriteAddrH == 0) {
+									_oamCopyDone = true;
+								}
 							}
 
 							//Note: Using "(_secondaryOamAddr & 0x03) == 0" instead of "_spriteAddrL == 0" is required
@@ -1021,6 +1052,7 @@ template<class T> void NesPpu<T>::ProcessSpriteEvaluation()
 							if((_secondaryOamAddr & 0x03) == 0) {
 								//Done copying all 4 bytes
 								_spriteInRange = false;
+								_lastVisibleSpriteAddr = _spriteAddrH * 4;
 
 								if(_spriteAddrL != 0) {
 									//Normally, if the sprite eval started on a non-multiple-of-4 address, it would
@@ -1033,11 +1065,6 @@ template<class T> void NesPpu<T>::ProcessSpriteEvaluation()
 										_spriteAddrL = 0;
 									}
 								}
-
-								_lastVisibleSpriteAddr = _spriteAddrH * 4;
-								if(_spriteAddrH == 0) {
-									_oamCopyDone = true;
-								}
 							}
 						} else {
 							//Nothing to copy, skip to next sprite

Core/NES/NesPpu.h

@@ -58,6 +58,8 @@ class NesPpu : public BaseNesPpu
 	void ProcessScanlineFirstCycle();
 	__forceinline void ProcessScanlineImpl();
 	__forceinline void ProcessSpriteEvaluation();
+	__noinline void ProcessSpriteEvaluationStart();
+	__noinline void ProcessSpriteEvaluationEnd();
 
 	void BeginVBlank();
 	void TriggerNmi();

Core/Shared/EmuSettings.cpp

@@ -58,6 +58,7 @@ void EmuSettings::Serialize(Serializer& s)
 			SV(_nes.RestrictPpuAccessOnFirstFrame);
 			SV(_nes.EnableCpuTestMode);
 			SV(_nes.EnableDmcSampleDuplicationGlitch);
+			SV(_nes.EnablePpuSpriteEvalBug);
 			SV(_nes.PpuExtraScanlinesAfterNmi); SV(_nes.PpuExtraScanlinesBeforeNmi);
 			SV(_nes.Region);
 			SV(_nes.LightDetectionRadius);

Core/Shared/SettingTypes.h

@@ -635,6 +635,7 @@ struct NesConfig
 
 	bool EnableOamDecay = false;
 	bool EnablePpuOamRowCorruption = false;
+	bool EnablePpuSpriteEvalBug = false;
 	bool DisableOamAddrBug = false;
 	bool DisablePaletteRead = false;
 	bool DisablePpu2004Reads = false;

UI/Config/NesConfig.cs

@@ -63,6 +63,7 @@ public class NesConfig : BaseConfig<NesConfig>
 		//Emulation
 		[Reactive] public bool EnableOamDecay { get; set; } = false;
 		[Reactive] public bool EnablePpuOamRowCorruption { get; set; } = false;
+		[Reactive] public bool EnablePpuSpriteEvalBug { get; set; } = false;
 		[Reactive] public bool DisableOamAddrBug { get; set; } = false;
 		[Reactive] public bool DisablePaletteRead { get; set; } = false;
 		[Reactive] public bool DisablePpu2004Reads { get; set; } = false;
@@ -186,6 +187,7 @@ public void ApplyConfig()
 
 				EnableOamDecay = EnableOamDecay,
 				EnablePpuOamRowCorruption = EnablePpuOamRowCorruption,
+				EnablePpuSpriteEvalBug = EnablePpuSpriteEvalBug,
 				DisableOamAddrBug = DisableOamAddrBug,
 				DisablePaletteRead = DisablePaletteRead,
 				DisablePpu2004Reads = DisablePpu2004Reads,
@@ -317,9 +319,10 @@ public struct InteropNesConfig
 		[MarshalAs(UnmanagedType.I1)] public bool AllowInvalidInput;
 		[MarshalAs(UnmanagedType.I1)] public bool DisableGameGenieBusConflicts;
 		[MarshalAs(UnmanagedType.I1)] public bool DisableFlashSaves;
-		
+
 		[MarshalAs(UnmanagedType.I1)] public bool EnableOamDecay;
 		[MarshalAs(UnmanagedType.I1)] public bool EnablePpuOamRowCorruption;
+		[MarshalAs(UnmanagedType.I1)] public bool EnablePpuSpriteEvalBug;
 		[MarshalAs(UnmanagedType.I1)] public bool DisableOamAddrBug;
 		[MarshalAs(UnmanagedType.I1)] public bool DisablePaletteRead;
 		[MarshalAs(UnmanagedType.I1)] public bool DisablePpu2004Reads;

UI/Localization/resources.en.xml

@@ -333,7 +333,8 @@
 			<Control ID="lblMiscSettings">Miscellaneous Settings</Control>
 			<Control ID="chkDisableOamAddrBug">Disable PPU OAMADDR bug emulation</Control>
 			<Control ID="chkDisablePaletteRead">Disable PPU palette reads</Control>
-			<Control ID="chkDisablePpu2004Reads">Disable PPU $2004 reads (Famicom behavior)</Control>
+			<Control ID="chkDisablePpu2004Reads">Disable PPU $2004 reads</Control>
+			<Control ID="chkEnablePpuSpriteEvalBug">Enable erroneous sprite pixels at X=255 (2C02B and earlier)</Control>
 			<Control ID="chkDisablePpuReset">Do not reset PPU when resetting console (Famicom behavior)</Control>
 			<Control ID="chkDisableGameGenieBusConflicts">Disable Game Genie bus conflict emulation</Control>
 			<Control ID="chkDisableFlashSaves">Disable flash saves (UNROM512 / GTROM / Rainbow)</Control>

UI/Views/NesConfigView.axaml

@@ -327,6 +327,7 @@
 						<c:CheckBoxWarning IsChecked="{Binding Config.DisableOamAddrBug}" Text="{l:Translate chkDisableOamAddrBug}" />
 						<c:CheckBoxWarning IsChecked="{Binding Config.DisablePaletteRead}" Text="{l:Translate chkDisablePaletteRead}" />
 						<c:CheckBoxWarning IsChecked="{Binding Config.DisablePpu2004Reads}" Text="{l:Translate chkDisablePpu2004Reads}" />
+						<c:CheckBoxWarning IsChecked="{Binding Config.EnablePpuSpriteEvalBug}" Text="{l:Translate chkEnablePpuSpriteEvalBug}" />
 						<c:CheckBoxWarning IsChecked="{Binding Config.EnableCpuTestMode}" Text="{l:Translate chkEnableCpuTestMode}" />
 						<c:CheckBoxWarning IsChecked="{Binding Config.EnableDmcSampleDuplicationGlitch}" Text="{l:Translate chkEnableDmcSampleDuplicationGlitch}" />
 						<c:CheckBoxWarning IsChecked="{Binding Config.DisableGameGenieBusConflicts}" Text="{l:Translate chkDisableGameGenieBusConflicts}" />