From 838bd5fb554bdd06392ba969f64a311cfb6d4bb1 Mon Sep 17 00:00:00 2001 From: Persune Date: Mon, 23 Oct 2023 01:01:03 +0800 Subject: [PATCH 1/3] Correct dot crawl phase offset calculation - Thanks lidnariq! - The new formula is based on the color generator clock cycle count per frame. --- Core/NES/BaseNesPpu.h | 1 + Core/NES/BisqwitNtscFilter.cpp | 4 ++-- Core/NES/NesNtscFilter.cpp | 2 +- Core/NES/NesPpu.cpp | 19 ++++++++++++++----- Core/Shared/RenderedFrame.h | 6 +++--- Core/Shared/Video/BaseVideoFilter.cpp | 8 ++++---- Core/Shared/Video/BaseVideoFilter.h | 6 +++--- Core/Shared/Video/VideoDecoder.cpp | 2 +- 8 files changed, 29 insertions(+), 19 deletions(-) diff --git a/Core/NES/BaseNesPpu.h b/Core/NES/BaseNesPpu.h index dbce3a41b..e87fbac07 100644 --- 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 index 36a66f731..0e953e2f9 100644 --- a/Core/NES/BisqwitNtscFilter.cpp +++ b/Core/NES/BisqwitNtscFilter.cpp @@ -66,7 +66,7 @@ BisqwitNtscFilter::BisqwitNtscFilter(Emulator* emu) : BaseVideoFilter(emu) int scale = 8 / _resDivider; outputBuffer += frameInfo.Width * ((120 - GetOverscan().Top) * scale); - DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, (GetVideoPhase() * 4) + 327360); + DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, GetVideoPhaseOffset() + 120*341*_signalsPerPixel); _workDone = true; } @@ -90,7 +90,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 index f23c1e3e3..50c4332de 100644 --- a/Core/NES/NesNtscFilter.cpp +++ b/Core/NES/NesNtscFilter.cpp @@ -79,7 +79,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 index 507e47b90..39b4651f6 100644 --- a/Core/NES/NesPpu.cpp +++ b/Core/NES/NesPpu.cpp @@ -33,6 +33,7 @@ template NesPpu::NesPpu(NesConsole* console) _emu = console->GetEmulator(); _mapper = console->GetMapper(); _masterClock = 0; + _masterClockFrameStart = 0; _masterClockDivider = 4; _settings = _emu->GetSettings(); @@ -78,6 +79,7 @@ template NesPpu::NesPpu(NesConsole* console) template void NesPpu::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 void NesPpu::ProcessScanlineImpl() } if(_scanline >= 0) { + if(_scanline == 0 && _cycle == 1) { + // get the master clock at the first dot + _masterClockFrameStart = _masterClock; + } ((T*)this)->DrawPixel(); ShiftTileRegisters(); @@ -1109,15 +1115,18 @@ template void NesPpu::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()) { @@ -1453,7 +1462,7 @@ template void NesPpu::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 index d095046c6..4c8574705 100644 --- 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 InputData; RenderedFrame() @@ -27,14 +27,14 @@ struct RenderedFrame InputData({}) {} - RenderedFrame(void* buffer, uint32_t width, uint32_t height, double scale, uint32_t frameNumber, vector inputData, uint32_t videoPhase = 0) : + RenderedFrame(void* buffer, uint32_t width, uint32_t height, double scale, uint32_t frameNumber, vector 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 index d38ac27b5..0f0415da4 100644 --- a/Core/Shared/Video/BaseVideoFilter.cpp +++ b/Core/Shared/Video/BaseVideoFilter.cpp @@ -71,9 +71,9 @@ bool BaseVideoFilter::IsOddFrame() return _isOddFrame; } -uint32_t BaseVideoFilter::GetVideoPhase() +uint32_t BaseVideoFilter::GetVideoPhaseOffset() { - return _videoPhase; + return _videoPhaseOffset; } uint32_t BaseVideoFilter::GetBufferSize() @@ -92,12 +92,12 @@ FrameInfo BaseVideoFilter::GetFrameInfo(uint16_t* ppuOutputBuffer, bool enableOv return frameInfo; } -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 index 6784be840..e69f7f1b2 100644 --- 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 bool NtscFilterOptionsChanged(T& ntscSetup); @@ -47,7 +47,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 index 7a6be6b9f..bc3bbfb89 100644 --- 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(); From 352e4265446a4c2b063338c0f001c0bf660fc63a Mon Sep 17 00:00:00 2001 From: Persune Date: Mon, 23 Oct 2023 02:13:29 +0800 Subject: [PATCH 2/3] Use standard RGB-YIQ matrix coefficients https://forums.nesdev.org/viewtopic.php?p=172817#p172817 Bisqwit originally intended to match a certain palette using different display primaries ("FCC D65"). This resulted in colors that looked off. The modification here is to more closely match Bisqwit's own palette generator, which uses a more standard RGB-YIQ matrix. at hue 0, saturation 1.0, contrast 1.0, brightness 1.0, gamma 2.2 https://bisqwit.iki.fi/utils/nespalette.php IQ component coefficients are derived from the NTSC base matrix of luminance and color-difference. with color reduction factors and an additional 33 degree rotation of each respective component. https://www.nesdev.org/wiki/NTSC_video#Converting_YUV_to_signal_RGB --- Core/NES/BisqwitNtscFilter.cpp | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/Core/NES/BisqwitNtscFilter.cpp b/Core/NES/BisqwitNtscFilter.cpp index 0e953e2f9..1324eaa20 100644 --- a/Core/NES/BisqwitNtscFilter.cpp +++ b/Core/NES/BisqwitNtscFilter.cpp @@ -66,7 +66,7 @@ BisqwitNtscFilter::BisqwitNtscFilter(Emulator* emu) : BaseVideoFilter(emu) int scale = 8 / _resDivider; outputBuffer += frameInfo.Width * ((120 - GetOverscan().Top) * scale); - DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, GetVideoPhaseOffset() + 120*341*_signalsPerPixel); + DecodeFrame(120, 239 - GetOverscan().Bottom, _ppuOutputBuffer, outputBuffer, GetVideoPhaseOffset() + 120 * 341 * _signalsPerPixel); _workDone = true; } @@ -140,14 +140,28 @@ void BisqwitNtscFilter::OnBeforeApplyFilter() _y = contrast / _yWidth; - _ir = (int)(contrast * 1.994681e-6 * saturation / _iWidth); - _qr = (int)(contrast * 9.915742e-7 * saturation / _qWidth); + // https://forums.nesdev.org/viewtopic.php?p=172817#p172817 + // Bisqwit originally intended to match a certain palette using different + // display primaries ("FCC D65"). This resulted in colors that looked off. + // The modification here is to more closely match Bisqwit's own palette + // generator, which uses a more standard RGB-YIQ matrix. + // at hue 0, saturation 1.0, contrast 1.0, brightness 1.0, gamma 2.2 + // https://bisqwit.iki.fi/utils/nespalette.php - _ig = (int)(contrast * 9.151351e-8 * saturation / _iWidth); - _qg = (int)(contrast * -6.334805e-7 * saturation / _qWidth); + // ( * ) / (<_iWidth or _qWidth at 0> / 2000) + double saturationFactor = (167941.0 * 144044.0) / (12.0 * 2000.0); - _ib = (int)(contrast * -1.012984e-6 * saturation / _iWidth); - _qb = (int)(contrast * 1.667217e-6 * saturation / _qWidth); + // IQ coefficients are derived from the NTSC base matrix of luminance and color-difference + // with color reduction factors and an additional 33 degree rotation of each respective component. + // https://www.nesdev.org/wiki/NTSC_video#Converting_YUV_to_signal_RGB + _ir = (int)(contrast * ( 0.956084 / saturationFactor) * saturation / _iWidth); + _qr = (int)(contrast * ( 0.620888 / saturationFactor) * saturation / _qWidth); + + _ig = (int)(contrast * (-0.272281 / saturationFactor) * saturation / _iWidth); + _qg = (int)(contrast * (-0.646901 / saturationFactor) * saturation / _qWidth); + + _ib = (int)(contrast * (-1.105617 / saturationFactor) * saturation / _iWidth); + _qb = (int)(contrast * ( 1.702501 / saturationFactor) * saturation / _qWidth); } void BisqwitNtscFilter::RecursiveBlend(int iterationCount, uint64_t *output, uint64_t *currentLine, uint64_t *nextLine, int pixelsPerCycle, bool verticalBlend) From 4d65410f88ee67d791d0125c05d8f0b14c039f18 Mon Sep 17 00:00:00 2001 From: Persune Date: Mon, 11 Dec 2023 15:51:42 +0800 Subject: [PATCH 3/3] Fix RGB PPU default palette emphasis behavior --- Core/NES/NesDefaultVideoFilter.cpp | 50 +++++++++++++++++++----------- Core/NES/NesDefaultVideoFilter.h | 2 +- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/Core/NES/NesDefaultVideoFilter.cpp b/Core/NES/NesDefaultVideoFilter.cpp index 229997986..3924a66ba 100644 --- a/Core/NES/NesDefaultVideoFilter.cpp +++ b/Core/NES/NesDefaultVideoFilter.cpp @@ -42,31 +42,45 @@ NesDefaultVideoFilter::NesDefaultVideoFilter(Emulator* emu) : BaseVideoFilter(em InitLookupTable(); } -void NesDefaultVideoFilter::GenerateFullColorPalette(uint32_t paletteBuffer[512]) +void NesDefaultVideoFilter::GenerateFullColorPalette(uint32_t paletteBuffer[512], PpuModel model) { for(int i = 0; i < 64; i++) { for(int j = 1; j < 8; j++) { double redColor = (uint8_t)(paletteBuffer[i] >> 16); double greenColor = (uint8_t)(paletteBuffer[i] >> 8); double blueColor = (uint8_t)paletteBuffer[i]; - if((i & 0x0F) <= 0x0D) { - //Emphasis doesn't affect columns $xE and $xF - if(j & 0x01) { - //Intensify red - greenColor *= 0.84; - blueColor *= 0.84; - } - if(j & 0x02) { - //Intensify green - redColor *= 0.84; - blueColor *= 0.84; - } - if(j & 0x04) { - //Intensify blue - redColor *= 0.84; - greenColor *= 0.84; + if(model == PpuModel::Ppu2C02) { + if((i & 0x0F) <= 0x0D) { + //Emphasis doesn't affect columns $xE and $xF + if(j & 0x01) { + //Intensify red + greenColor *= 0.84; + blueColor *= 0.84; + } + if(j & 0x02) { + //Intensify green + redColor *= 0.84; + blueColor *= 0.84; + } + if(j & 0x04) { + //Intensify blue + redColor *= 0.84; + greenColor *= 0.84; + } } } + else { + // RGB PPUs do emphasis in a different way + //Intensify red + if(j & 0x01) + redColor = 0xFF; + //Intensify green + if(j & 0x02) + greenColor = 0xFF; + //Intensify blue + if(j & 0x04) + blueColor = 0xFF; + } uint8_t r = (uint8_t)(redColor > 255 ? 255 : redColor); uint8_t g = (uint8_t)(greenColor > 255 ? 255 : greenColor); @@ -98,7 +112,7 @@ void NesDefaultVideoFilter::GetFullPalette(uint32_t palette[512], NesConfig& nes } } else { memcpy(palette, _ppuPaletteArgb[(int)model], sizeof(_ppuPaletteArgb[(int)_ppuModel])); - GenerateFullColorPalette(palette); + GenerateFullColorPalette(palette, model); } } } diff --git a/Core/NES/NesDefaultVideoFilter.h b/Core/NES/NesDefaultVideoFilter.h index f5b083e81..36b6074e3 100644 --- a/Core/NES/NesDefaultVideoFilter.h +++ b/Core/NES/NesDefaultVideoFilter.h @@ -25,7 +25,7 @@ class NesDefaultVideoFilter : public BaseVideoFilter static void ApplyPalBorder(uint16_t* ppuOutputBuffer); - static void GenerateFullColorPalette(uint32_t paletteBuffer[512]); + static void GenerateFullColorPalette(uint32_t paletteBuffer[512], PpuModel model = PpuModel::Ppu2C02); static void GetFullPalette(uint32_t palette[512], NesConfig& nesCfg, PpuModel model); static uint32_t GetDefaultPixelBrightness(uint16_t colorIndex, PpuModel model);