Correct dot crawl phase offset calculation

Gumball2415 · Gumball2415 · commit fa7c32f51caa · 2023-10-23T01:01:03.000+08:00
- Thanks lidnariq!
- The new formula is based on the color generator clock cycle count per
  frame.
diff --git a/Core/NES/BaseNesPpu.h b/Core/NES/BaseNesPpu.h
@@ -16,6 +16,7 @@ class BaseNesPpu : public INesMemoryHandler, public ISerializable
 {
 protected:
 	uint64_t _masterClock = 0;
+	uint64_t _masterClockFrameStart = 0;
 	uint32_t _cycle = 0;
 	int16_t _scanline = 0;
 	bool _emulatorBgEnabled = false;
diff --git a/Core/NES/BisqwitNtscFilter.cpp b/Core/NES/BisqwitNtscFilter.cpp
@@ -65,7 +65,7 @@ BisqwitNtscFilter::BisqwitNtscFilter(Emulator* emu) : BaseVideoFilter(emu)
 			//Adjust outputbuffer to start at the middle of the picture
 			outputBuffer += frameInfo.Width * (frameInfo.Height / 2);
 
-			DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, (GetVideoPhase() * 4) + 327360);
+			DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, GetVideoPhaseOffset() + 120*341*_signalsPerPixel);
 
 			_workDone = true;
 		}
@@ -89,7 +89,7 @@ void BisqwitNtscFilter::ApplyFilter(uint16_t *ppuOutputBuffer)
 
 	_workDone = false;
 	_waitWork.Signal();
-	DecodeFrame(GetOverscan().Top, 120, ppuOutputBuffer, GetOutputBuffer(), (GetVideoPhase() * 4) + GetOverscan().Top*341*8);
+	DecodeFrame(GetOverscan().Top, 120, ppuOutputBuffer, GetOutputBuffer(), GetVideoPhaseOffset() + GetOverscan().Top * 341 * _signalsPerPixel);
 	while(!_workDone) {}
 }
 
diff --git a/Core/NES/NesNtscFilter.cpp b/Core/NES/NesNtscFilter.cpp
@@ -74,7 +74,7 @@ void NesNtscFilter::ApplyFilter(uint16_t *ppuOutputBuffer)
 		NesDefaultVideoFilter::ApplyPalBorder(ppuOutputBuffer);
 	}
 
-	nes_ntsc_blit(&_ntscData, ppuOutputBuffer, _baseFrameInfo.Width, GetVideoPhase(), _baseFrameInfo.Width, _baseFrameInfo.Height, _ntscBuffer, NES_NTSC_OUT_WIDTH(_baseFrameInfo.Width) * 4);
+	nes_ntsc_blit(&_ntscData, ppuOutputBuffer, _baseFrameInfo.Width, (GetVideoPhaseOffset() / 4), _baseFrameInfo.Width, _baseFrameInfo.Height, _ntscBuffer, NES_NTSC_OUT_WIDTH(_baseFrameInfo.Width) * 4);
 
 	for(uint32_t i = 0; i < frameInfo.Height; i+=2) {
 		memcpy(GetOutputBuffer()+i*frameInfo.Width, _ntscBuffer + yOffset + xOffset + (i/2)*baseWidth, frameInfo.Width * sizeof(uint32_t));
diff --git a/Core/NES/NesPpu.cpp b/Core/NES/NesPpu.cpp
@@ -33,6 +33,7 @@ template<class T> NesPpu<T>::NesPpu(NesConsole* console)
 	_emu = console->GetEmulator();
 	_mapper = console->GetMapper();
 	_masterClock = 0;
+	_masterClockFrameStart = 0;
 	_masterClockDivider = 4;
 	_settings = _emu->GetSettings();
 
@@ -78,6 +79,7 @@ template<class T> NesPpu<T>::NesPpu(NesConsole* console)
 template<class T> void NesPpu<T>::Reset(bool softReset)
 {
 	_masterClock = 0;
+	_masterClockFrameStart = 0;
 
 	//Reset OAM decay timestamps regardless of the reset PPU option
 	memset(_oamDecayCycles, 0, sizeof(_oamDecayCycles));
@@ -868,6 +870,10 @@ template<class T> void NesPpu<T>::ProcessScanlineImpl()
 		}
 
 		if(_scanline >= 0) {
+			if(_scanline == 0 && _cycle == 1) {
+				// get the master clock at the first dot
+				_masterClockFrameStart = _masterClock;
+			}
 			((T*)this)->DrawPixel();
 			ShiftTileRegisters();
 
@@ -1104,15 +1110,18 @@ template<class T> void NesPpu<T>::SendFrame()
 		_emu->GetNotificationManager()->SendNotification(ConsoleNotificationType::PpuFrameDone, _currentOutputBuffer);
 	}
 
-	//Get phase at the start of the current frame (341*241 cycles ago)
-	uint32_t videoPhase = ((_masterClock / _masterClockDivider) - 82181) % 3;
+	//Get chroma phase offset at the start of the current frame
+	//In units of color generator clocks
+	//https://www.nesdev.org/wiki/NTSC_video#Color_Artifacts
+	uint32_t videoPhaseOffset = (_masterClockFrameStart % 6) * 2;
+
 	NesConfig& cfg = _console->GetNesConfig();
 	if(_region != ConsoleRegion::Ntsc || cfg.PpuExtraScanlinesAfterNmi != 0 || cfg.PpuExtraScanlinesBeforeNmi != 0) {
 		//Force 2-phase pattern for PAL or when overclocking is used
-		videoPhase = _frameCount & 0x01;
+		videoPhaseOffset = (_frameCount & 0x01) * 4;
 	}
 
-	RenderedFrame frame(_currentOutputBuffer, NesConstants::ScreenWidth, NesConstants::ScreenHeight, 1.0, _frameCount, _console->GetControlManager()->GetPortStates(), videoPhase);
+	RenderedFrame frame(_currentOutputBuffer, NesConstants::ScreenWidth, NesConstants::ScreenHeight, 1.0, _frameCount, _console->GetControlManager()->GetPortStates(), videoPhaseOffset);
 	frame.Data = frameData; //HD packs
 
 	if(_console->GetVsMainConsole() || _console->GetVsSubConsole()) {
@@ -1462,7 +1471,7 @@ template<class T> void NesPpu<T>::Serialize(Serializer& s)
 	SV(_mask.IntensifyBlue); SV(_paletteRamMask); SV(_intensifyColorBits); SV(_statusFlags.SpriteOverflow); SV(_statusFlags.Sprite0Hit); SV(_statusFlags.VerticalBlank); SV(_scanline);
 	SV(_cycle); SV(_frameCount); SV(_memoryReadBuffer); SV(_region);
 
-	SV(_ppuBusAddress); SV(_masterClock);
+	SV(_ppuBusAddress); SV(_masterClock); SV(_masterClockFrameStart);
 
 	if(s.GetFormat() != SerializeFormat::Map) {
 		//Hide these entries from the Lua API
diff --git a/Core/Shared/RenderedFrame.h b/Core/Shared/RenderedFrame.h
@@ -11,7 +11,7 @@ struct RenderedFrame
 	uint32_t Height = 240;
 	double Scale = 1.0;
 	uint32_t FrameNumber = 0;
-	uint32_t VideoPhase = 0;
+	uint32_t VideoPhaseOffset = 0;
 	vector<ControllerData> InputData;
 
 	RenderedFrame()
@@ -27,14 +27,14 @@ struct RenderedFrame
 		InputData({})
 	{}
 
-	RenderedFrame(void* buffer, uint32_t width, uint32_t height, double scale, uint32_t frameNumber, vector<ControllerData> inputData, uint32_t videoPhase = 0) :
+	RenderedFrame(void* buffer, uint32_t width, uint32_t height, double scale, uint32_t frameNumber, vector<ControllerData> inputData, uint32_t videoPhaseOffset = 0) :
 		FrameBuffer(buffer),
 		Data(nullptr),
 		Width(width),
 		Height(height),
 		Scale(scale),
 		FrameNumber(frameNumber),
-		VideoPhase(videoPhase),
+		VideoPhaseOffset(videoPhaseOffset),
 		InputData(inputData)
 	{}
 };
diff --git a/Core/Shared/Video/BaseVideoFilter.cpp b/Core/Shared/Video/BaseVideoFilter.cpp
@@ -70,22 +70,22 @@ bool BaseVideoFilter::IsOddFrame()
 	return _isOddFrame;
 }
 
-uint32_t BaseVideoFilter::GetVideoPhase()
+uint32_t BaseVideoFilter::GetVideoPhaseOffset()
 {
-	return _videoPhase;
+	return _videoPhaseOffset;
 }
 
 uint32_t BaseVideoFilter::GetBufferSize()
 {
 	return _bufferSize * sizeof(uint32_t);
 }
 
-FrameInfo BaseVideoFilter::SendFrame(uint16_t *ppuOutputBuffer, uint32_t frameNumber, uint32_t videoPhase, void* frameData, bool enableOverscan)
+FrameInfo BaseVideoFilter::SendFrame(uint16_t *ppuOutputBuffer, uint32_t frameNumber, uint32_t videoPhaseOffset, void* frameData, bool enableOverscan)
 {
 	auto lock = _frameLock.AcquireSafe();
 	_overscan = enableOverscan ? _emu->GetSettings()->GetOverscan() : OverscanDimensions{};
 	_isOddFrame = frameNumber % 2;
-	_videoPhase = videoPhase;
+	_videoPhaseOffset = videoPhaseOffset;
 	_frameData = frameData;
 	_ppuOutputBuffer = ppuOutputBuffer;
 	OnBeforeApplyFilter();
diff --git a/Core/Shared/Video/BaseVideoFilter.h b/Core/Shared/Video/BaseVideoFilter.h
@@ -14,7 +14,7 @@ class BaseVideoFilter
 	SimpleLock _frameLock;
 	OverscanDimensions _overscan = {};
 	bool _isOddFrame = false;
-	uint32_t _videoPhase = 0;
+	uint32_t _videoPhaseOffset = 0;
 
 	void UpdateBufferSize();
 
@@ -33,7 +33,7 @@ class BaseVideoFilter
 	virtual void ApplyFilter(uint16_t *ppuOutputBuffer) = 0;
 	virtual void OnBeforeApplyFilter();
 	bool IsOddFrame();
-	uint32_t GetVideoPhase();
+	uint32_t GetVideoPhaseOffset();
 	uint32_t GetBufferSize();
 	
 	template<typename T> bool NtscFilterOptionsChanged(T& ntscSetup);
@@ -44,7 +44,7 @@ class BaseVideoFilter
 	virtual ~BaseVideoFilter();
 
 	uint32_t* GetOutputBuffer();
-	FrameInfo SendFrame(uint16_t *ppuOutputBuffer, uint32_t frameNumber, uint32_t videoPhase, void* frameData, bool enableOverscan = true);
+	FrameInfo SendFrame(uint16_t *ppuOutputBuffer, uint32_t frameNumber, uint32_t videoPhaseOffset, void* frameData, bool enableOverscan = true);
 	void TakeScreenshot(string romName, VideoFilterType filterType);
 	void TakeScreenshot(VideoFilterType filterType, string filename, std::stringstream *stream = nullptr);
 
diff --git a/Core/Shared/Video/VideoDecoder.cpp b/Core/Shared/Video/VideoDecoder.cpp
@@ -105,7 +105,7 @@ void VideoDecoder::DecodeFrame(bool forRewind)
 	}
 
 	_videoFilter->SetBaseFrameInfo(_baseFrameSize);
-	FrameInfo frameSize = _videoFilter->SendFrame((uint16_t*)_frame.FrameBuffer, _frame.FrameNumber, _frame.VideoPhase, _frame.Data);
+	FrameInfo frameSize = _videoFilter->SendFrame((uint16_t*)_frame.FrameBuffer, _frame.FrameNumber, _frame.VideoPhaseOffset, _frame.Data);
 
 	uint32_t* outputBuffer = _videoFilter->GetOutputBuffer();
 	

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ class BaseNesPpu : public INesMemoryHandler, public ISerializable`
`16`	`16`	`{`
`17`	`17`	`protected:`
`18`	`18`	`uint64_t _masterClock = 0;`
	`19`	`+ uint64_t _masterClockFrameStart = 0;`
`19`	`20`	`uint32_t _cycle = 0;`
`20`	`21`	`int16_t _scanline = 0;`
`21`	`22`	`bool _emulatorBgEnabled = false;`
Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ BisqwitNtscFilter::BisqwitNtscFilter(Emulator* emu) : BaseVideoFilter(emu)`
`65`	`65`	`//Adjust outputbuffer to start at the middle of the picture`
`66`	`66`	`outputBuffer += frameInfo.Width * (frameInfo.Height / 2);`
`67`	`67`
`68`		`- DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, (GetVideoPhase() * 4) + 327360);`
	`68`	`+ DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, GetVideoPhaseOffset() + 120341_signalsPerPixel);`
`69`	`69`
`70`	`70`	`_workDone = true;`
`71`	`71`	`}`
`@@ -89,7 +89,7 @@ void BisqwitNtscFilter::ApplyFilter(uint16_t *ppuOutputBuffer)`
`89`	`89`
`90`	`90`	`_workDone = false;`
`91`	`91`	`_waitWork.Signal();`
`92`		`- DecodeFrame(GetOverscan().Top, 120, ppuOutputBuffer, GetOutputBuffer(), (GetVideoPhase() * 4) + GetOverscan().Top3418);`
	`92`	`+ DecodeFrame(GetOverscan().Top, 120, ppuOutputBuffer, GetOutputBuffer(), GetVideoPhaseOffset() + GetOverscan().Top * 341 * _signalsPerPixel);`
`93`	`93`	`while(!_workDone) {}`
`94`	`94`	`}`
`95`	`95`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ void NesNtscFilter::ApplyFilter(uint16_t *ppuOutputBuffer)`
`74`	`74`	`NesDefaultVideoFilter::ApplyPalBorder(ppuOutputBuffer);`
`75`	`75`	`}`
`76`	`76`
`77`		`- nes_ntsc_blit(&_ntscData, ppuOutputBuffer, _baseFrameInfo.Width, GetVideoPhase(), _baseFrameInfo.Width, _baseFrameInfo.Height, _ntscBuffer, NES_NTSC_OUT_WIDTH(_baseFrameInfo.Width) * 4);`
	`77`	`+ nes_ntsc_blit(&_ntscData, ppuOutputBuffer, _baseFrameInfo.Width, (GetVideoPhaseOffset() / 4), _baseFrameInfo.Width, _baseFrameInfo.Height, _ntscBuffer, NES_NTSC_OUT_WIDTH(_baseFrameInfo.Width) * 4);`
`78`	`78`
`79`	`79`	`for(uint32_t i = 0; i < frameInfo.Height; i+=2) {`
`80`	`80`	`memcpy(GetOutputBuffer()+iframeInfo.Width, _ntscBuffer + yOffset + xOffset + (i/2)baseWidth, frameInfo.Width * sizeof(uint32_t));`
Original file line number	Diff line number	Diff line change
`@@ -70,22 +70,22 @@ bool BaseVideoFilter::IsOddFrame()`
`70`	`70`	`return _isOddFrame;`
`71`	`71`	`}`
`72`	`72`
`73`		`-uint32_t BaseVideoFilter::GetVideoPhase()`
	`73`	`+uint32_t BaseVideoFilter::GetVideoPhaseOffset()`
`74`	`74`	`{`
`75`		`- return _videoPhase;`
	`75`	`+ return _videoPhaseOffset;`
`76`	`76`	`}`
`77`	`77`
`78`	`78`	`uint32_t BaseVideoFilter::GetBufferSize()`
`79`	`79`	`{`
`80`	`80`	`return _bufferSize * sizeof(uint32_t);`
`81`	`81`	`}`
`82`	`82`
`83`		`-FrameInfo BaseVideoFilter::SendFrame(uint16_t ppuOutputBuffer, uint32_t frameNumber, uint32_t videoPhase, void frameData, bool enableOverscan)`
	`83`	`+FrameInfo BaseVideoFilter::SendFrame(uint16_t ppuOutputBuffer, uint32_t frameNumber, uint32_t videoPhaseOffset, void frameData, bool enableOverscan)`
`84`	`84`	`{`
`85`	`85`	`auto lock = _frameLock.AcquireSafe();`
`86`	`86`	`_overscan = enableOverscan ? _emu->GetSettings()->GetOverscan() : OverscanDimensions{};`
`87`	`87`	`_isOddFrame = frameNumber % 2;`
`88`		`- _videoPhase = videoPhase;`
	`88`	`+ _videoPhaseOffset = videoPhaseOffset;`
`89`	`89`	`_frameData = frameData;`
`90`	`90`	`_ppuOutputBuffer = ppuOutputBuffer;`
`91`	`91`	`OnBeforeApplyFilter();`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ void VideoDecoder::DecodeFrame(bool forRewind)`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`_videoFilter->SetBaseFrameInfo(_baseFrameSize);`
`108`		`- FrameInfo frameSize = _videoFilter->SendFrame((uint16_t*)_frame.FrameBuffer, _frame.FrameNumber, _frame.VideoPhase, _frame.Data);`
	`108`	`+ FrameInfo frameSize = _videoFilter->SendFrame((uint16_t*)_frame.FrameBuffer, _frame.FrameNumber, _frame.VideoPhaseOffset, _frame.Data);`
`109`	`109`
`110`	`110`	`uint32_t* outputBuffer = _videoFilter->GetOutputBuffer();`
`111`	`111`