From 2f1b639f0ad191a977bfaabdeff24ccef4914099 Mon Sep 17 00:00:00 2001 From: Techjar Date: Mon, 31 May 2021 17:40:08 -0400 Subject: [PATCH 1/2] VideoCommon: Restore BBox* forwarding functions --- Source/Core/VideoBackends/D3D/D3DRender.cpp | 6 +++--- Source/Core/VideoBackends/D3D/D3DRender.h | 6 +++--- Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp | 6 +++--- Source/Core/VideoBackends/D3D12/D3D12Renderer.h | 6 +++--- Source/Core/VideoBackends/Null/NullRender.h | 4 ++-- Source/Core/VideoBackends/OGL/OGLRender.cpp | 6 +++--- Source/Core/VideoBackends/OGL/OGLRender.h | 6 +++--- Source/Core/VideoBackends/Software/SWRenderer.cpp | 4 ++-- Source/Core/VideoBackends/Software/SWRenderer.h | 4 ++-- Source/Core/VideoBackends/Vulkan/VKRenderer.cpp | 6 +++--- Source/Core/VideoBackends/Vulkan/VKRenderer.h | 6 +++--- Source/Core/VideoCommon/RenderBase.cpp | 15 +++++++++++++++ Source/Core/VideoCommon/RenderBase.h | 10 +++++++--- 13 files changed, 52 insertions(+), 33 deletions(-) diff --git a/Source/Core/VideoBackends/D3D/D3DRender.cpp b/Source/Core/VideoBackends/D3D/D3DRender.cpp index 1290a63630..904de1953c 100644 --- a/Source/Core/VideoBackends/D3D/D3DRender.cpp +++ b/Source/Core/VideoBackends/D3D/D3DRender.cpp @@ -264,17 +264,17 @@ void Renderer::UnbindTexture(const AbstractTexture* texture) D3D::stateman->ApplyTextures(); } -u16 Renderer::BBoxRead(int index) +u16 Renderer::BBoxReadImpl(int index) { return static_cast(BBox::Get(index)); } -void Renderer::BBoxWrite(int index, u16 value) +void Renderer::BBoxWriteImpl(int index, u16 value) { BBox::Set(index, value); } -void Renderer::BBoxFlush() +void Renderer::BBoxFlushImpl() { BBox::Flush(); } diff --git a/Source/Core/VideoBackends/D3D/D3DRender.h b/Source/Core/VideoBackends/D3D/D3DRender.h index 2e008d87a7..727e1243b9 100644 --- a/Source/Core/VideoBackends/D3D/D3DRender.h +++ b/Source/Core/VideoBackends/D3D/D3DRender.h @@ -61,9 +61,9 @@ public: void SetFullscreen(bool enable_fullscreen) override; bool IsFullscreen() const override; - u16 BBoxRead(int index) override; - void BBoxWrite(int index, u16 value) override; - void BBoxFlush() override; + u16 BBoxReadImpl(int index) override; + void BBoxWriteImpl(int index, u16 value) override; + void BBoxFlushImpl() override; void Flush() override; void WaitForGPUIdle() override; diff --git a/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp b/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp index ce9ac2504b..cfd259f9f2 100644 --- a/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp +++ b/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp @@ -106,17 +106,17 @@ std::unique_ptr Renderer::CreatePipeline(const AbstractPipelin return DXPipeline::Create(config, cache_data, cache_data_length); } -u16 Renderer::BBoxRead(int index) +u16 Renderer::BBoxReadImpl(int index) { return static_cast(m_bounding_box->Get(index)); } -void Renderer::BBoxWrite(int index, u16 value) +void Renderer::BBoxWriteImpl(int index, u16 value) { m_bounding_box->Set(index, value); } -void Renderer::BBoxFlush() +void Renderer::BBoxFlushImpl() { m_bounding_box->Flush(); m_bounding_box->Invalidate(); diff --git a/Source/Core/VideoBackends/D3D12/D3D12Renderer.h b/Source/Core/VideoBackends/D3D12/D3D12Renderer.h index 4a26871a73..623a767676 100644 --- a/Source/Core/VideoBackends/D3D12/D3D12Renderer.h +++ b/Source/Core/VideoBackends/D3D12/D3D12Renderer.h @@ -47,9 +47,9 @@ public: const void* cache_data = nullptr, size_t cache_data_length = 0) override; - u16 BBoxRead(int index) override; - void BBoxWrite(int index, u16 value) override; - void BBoxFlush() override; + u16 BBoxReadImpl(int index) override; + void BBoxWriteImpl(int index, u16 value) override; + void BBoxFlushImpl() override; void Flush() override; void WaitForGPUIdle() override; diff --git a/Source/Core/VideoBackends/Null/NullRender.h b/Source/Core/VideoBackends/Null/NullRender.h index 21bd9c9c2c..13bd3feae9 100644 --- a/Source/Core/VideoBackends/Null/NullRender.h +++ b/Source/Core/VideoBackends/Null/NullRender.h @@ -34,8 +34,8 @@ public: u32 AccessEFB(EFBAccessType type, u32 x, u32 y, u32 poke_data) override { return 0; } void PokeEFB(EFBAccessType type, const EfbPokeData* points, size_t num_points) override {} - u16 BBoxRead(int index) override { return 0; } - void BBoxWrite(int index, u16 value) override {} + u16 BBoxReadImpl(int index) override { return 0; } + void BBoxWriteImpl(int index, u16 value) override {} void ClearScreen(const MathUtil::Rectangle& rc, bool colorEnable, bool alphaEnable, bool zEnable, u32 color, u32 z) override diff --git a/Source/Core/VideoBackends/OGL/OGLRender.cpp b/Source/Core/VideoBackends/OGL/OGLRender.cpp index 5f6796d761..d3435c51d5 100644 --- a/Source/Core/VideoBackends/OGL/OGLRender.cpp +++ b/Source/Core/VideoBackends/OGL/OGLRender.cpp @@ -854,7 +854,7 @@ void Renderer::SetScissorRect(const MathUtil::Rectangle& rc) glScissor(rc.left, rc.top, rc.GetWidth(), rc.GetHeight()); } -u16 Renderer::BBoxRead(int index) +u16 Renderer::BBoxReadImpl(int index) { // swap 2 and 3 for top/bottom if (index >= 2) @@ -870,7 +870,7 @@ u16 Renderer::BBoxRead(int index) return static_cast(value); } -void Renderer::BBoxWrite(int index, u16 value) +void Renderer::BBoxWriteImpl(int index, u16 value) { s32 swapped_value = value; if (index >= 2) @@ -882,7 +882,7 @@ void Renderer::BBoxWrite(int index, u16 value) BoundingBox::Set(index, swapped_value); } -void Renderer::BBoxFlush() +void Renderer::BBoxFlushImpl() { BoundingBox::Flush(); } diff --git a/Source/Core/VideoBackends/OGL/OGLRender.h b/Source/Core/VideoBackends/OGL/OGLRender.h index 399138ae07..c9b3b04896 100644 --- a/Source/Core/VideoBackends/OGL/OGLRender.h +++ b/Source/Core/VideoBackends/OGL/OGLRender.h @@ -126,9 +126,9 @@ public: void BindBackbuffer(const ClearColor& clear_color = {}) override; void PresentBackbuffer() override; - u16 BBoxRead(int index) override; - void BBoxWrite(int index, u16 value) override; - void BBoxFlush() override; + u16 BBoxReadImpl(int index) override; + void BBoxWriteImpl(int index, u16 value) override; + void BBoxFlushImpl() override; void BeginUtilityDrawing() override; void EndUtilityDrawing() override; diff --git a/Source/Core/VideoBackends/Software/SWRenderer.cpp b/Source/Core/VideoBackends/Software/SWRenderer.cpp index c128e6bd1a..51433d311f 100644 --- a/Source/Core/VideoBackends/Software/SWRenderer.cpp +++ b/Source/Core/VideoBackends/Software/SWRenderer.cpp @@ -126,12 +126,12 @@ u32 SWRenderer::AccessEFB(EFBAccessType type, u32 x, u32 y, u32 InputData) return value; } -u16 SWRenderer::BBoxRead(int index) +u16 SWRenderer::BBoxReadImpl(int index) { return BoundingBox::GetCoordinate(static_cast(index)); } -void SWRenderer::BBoxWrite(int index, u16 value) +void SWRenderer::BBoxWriteImpl(int index, u16 value) { BoundingBox::SetCoordinate(static_cast(index), value); } diff --git a/Source/Core/VideoBackends/Software/SWRenderer.h b/Source/Core/VideoBackends/Software/SWRenderer.h index df23e123be..f2434d92c0 100644 --- a/Source/Core/VideoBackends/Software/SWRenderer.h +++ b/Source/Core/VideoBackends/Software/SWRenderer.h @@ -39,8 +39,8 @@ public: u32 AccessEFB(EFBAccessType type, u32 x, u32 y, u32 poke_data) override; void PokeEFB(EFBAccessType type, const EfbPokeData* points, size_t num_points) override {} - u16 BBoxRead(int index) override; - void BBoxWrite(int index, u16 value) override; + u16 BBoxReadImpl(int index) override; + void BBoxWriteImpl(int index, u16 value) override; void RenderXFBToScreen(const MathUtil::Rectangle& target_rc, const AbstractTexture* source_texture, diff --git a/Source/Core/VideoBackends/Vulkan/VKRenderer.cpp b/Source/Core/VideoBackends/Vulkan/VKRenderer.cpp index 23d0c0d21a..bbac6201c7 100644 --- a/Source/Core/VideoBackends/Vulkan/VKRenderer.cpp +++ b/Source/Core/VideoBackends/Vulkan/VKRenderer.cpp @@ -131,17 +131,17 @@ void Renderer::SetPipeline(const AbstractPipeline* pipeline) StateTracker::GetInstance()->SetPipeline(static_cast(pipeline)); } -u16 Renderer::BBoxRead(int index) +u16 Renderer::BBoxReadImpl(int index) { return static_cast(m_bounding_box->Get(index)); } -void Renderer::BBoxWrite(int index, u16 value) +void Renderer::BBoxWriteImpl(int index, u16 value) { m_bounding_box->Set(index, value); } -void Renderer::BBoxFlush() +void Renderer::BBoxFlushImpl() { m_bounding_box->Flush(); m_bounding_box->Invalidate(); diff --git a/Source/Core/VideoBackends/Vulkan/VKRenderer.h b/Source/Core/VideoBackends/Vulkan/VKRenderer.h index 43ed3bd45b..b7e931d669 100644 --- a/Source/Core/VideoBackends/Vulkan/VKRenderer.h +++ b/Source/Core/VideoBackends/Vulkan/VKRenderer.h @@ -54,9 +54,9 @@ public: SwapChain* GetSwapChain() const { return m_swap_chain.get(); } BoundingBox* GetBoundingBox() const { return m_bounding_box.get(); } - u16 BBoxRead(int index) override; - void BBoxWrite(int index, u16 value) override; - void BBoxFlush() override; + u16 BBoxReadImpl(int index) override; + void BBoxWriteImpl(int index, u16 value) override; + void BBoxFlushImpl() override; void Flush() override; void WaitForGPUIdle() override; diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp index 93f8fa2279..f93d498fc6 100644 --- a/Source/Core/VideoCommon/RenderBase.cpp +++ b/Source/Core/VideoCommon/RenderBase.cpp @@ -185,6 +185,21 @@ void Renderer::ReinterpretPixelData(EFBReinterpretType convtype) g_framebuffer_manager->ReinterpretPixelData(convtype); } +u16 Renderer::BBoxRead(int index) +{ + return BBoxReadImpl(index); +} + +void Renderer::BBoxWrite(int index, u16 value) +{ + BBoxWriteImpl(index, value); +} + +void Renderer::BBoxFlush() +{ + BBoxFlushImpl(); +} + u32 Renderer::AccessEFB(EFBAccessType type, u32 x, u32 y, u32 poke_data) { if (type == EFBAccessType::PeekColor) diff --git a/Source/Core/VideoCommon/RenderBase.h b/Source/Core/VideoCommon/RenderBase.h index 48cef0bdad..1deedd4f76 100644 --- a/Source/Core/VideoCommon/RenderBase.h +++ b/Source/Core/VideoCommon/RenderBase.h @@ -211,9 +211,9 @@ public: virtual u32 AccessEFB(EFBAccessType type, u32 x, u32 y, u32 poke_data); virtual void PokeEFB(EFBAccessType type, const EfbPokeData* points, size_t num_points); - virtual u16 BBoxRead(int index) = 0; - virtual void BBoxWrite(int index, u16 value) = 0; - virtual void BBoxFlush() {} + u16 BBoxRead(int index); + void BBoxWrite(int index, u16 value); + void BBoxFlush(); virtual void Flush() {} virtual void WaitForGPUIdle() {} @@ -301,6 +301,10 @@ protected: // Should be called with the ImGui lock held. void DrawImGui(); + virtual u16 BBoxReadImpl(int index) = 0; + virtual void BBoxWriteImpl(int index, u16 value) = 0; + virtual void BBoxFlushImpl() {} + AbstractFramebuffer* m_current_framebuffer = nullptr; const AbstractPipeline* m_current_pipeline = nullptr; From 8cfe49295f105506a270ff350a9ea717e8af2172 Mon Sep 17 00:00:00 2001 From: Techjar Date: Mon, 31 May 2021 16:31:27 -0400 Subject: [PATCH 2/2] VideoCommon: Add fallback handling for bounding box when disabled or unsupported The SDK seems to write "default" bounding box values before every draw (1023 0 1023 0 are the only values encountered so far, which happen to be the extents allowed by the BP registers) to reset the registers for comparison in the pixel engine, and presumably to detect whether GX has updated the registers with real values. Handling these writes and returning them on read when bounding box emulation is disabled or unsupported, even without computing real values from rendering, seems to prevent games from corrupting memory or crashing. This obviously does not fix any effects that rely on bounding box emulation, but having the game not clobber its own code/data or just outright crash is a definite improvement. --- Source/Core/Core/State.cpp | 2 +- Source/Core/VideoCommon/BPStructs.cpp | 7 ++----- Source/Core/VideoCommon/RenderBase.cpp | 10 ++++++++++ Source/Core/VideoCommon/RenderBase.h | 12 ++++++++++++ Source/Core/VideoCommon/VideoBackendBase.cpp | 5 +---- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index 8e832d8e28..2ab2146c06 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -74,7 +74,7 @@ static Common::Event g_compressAndDumpStateSyncEvent; static std::thread g_save_thread; // Don't forget to increase this after doing changes on the savestate system -constexpr u32 STATE_VERSION = 130; // Last changed in PR 9545 +constexpr u32 STATE_VERSION = 131; // Last changed in PR 9773 // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list, diff --git a/Source/Core/VideoCommon/BPStructs.cpp b/Source/Core/VideoCommon/BPStructs.cpp index 8754152fe9..810cb4e129 100644 --- a/Source/Core/VideoCommon/BPStructs.cpp +++ b/Source/Core/VideoCommon/BPStructs.cpp @@ -445,11 +445,8 @@ static void BPWritten(const BPCmd& bp) const u8 offset = bp.address & 2; BoundingBox::Enable(); - if (g_ActiveConfig.backend_info.bSupportsBBox && g_ActiveConfig.bBBoxEnable) - { - g_renderer->BBoxWrite(offset, bp.newvalue & 0x3ff); - g_renderer->BBoxWrite(offset + 1, bp.newvalue >> 10); - } + g_renderer->BBoxWrite(offset, bp.newvalue & 0x3ff); + g_renderer->BBoxWrite(offset + 1, bp.newvalue >> 10); } return; case BPMEM_TEXINVALIDATE: diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp index f93d498fc6..5422d26e94 100644 --- a/Source/Core/VideoCommon/RenderBase.cpp +++ b/Source/Core/VideoCommon/RenderBase.cpp @@ -187,11 +187,20 @@ void Renderer::ReinterpretPixelData(EFBReinterpretType convtype) u16 Renderer::BBoxRead(int index) { + if (!g_ActiveConfig.bBBoxEnable || !g_ActiveConfig.backend_info.bSupportsBBox) + return m_bounding_box_fallback[index]; + return BBoxReadImpl(index); } void Renderer::BBoxWrite(int index, u16 value) { + if (!g_ActiveConfig.bBBoxEnable || !g_ActiveConfig.backend_info.bSupportsBBox) + { + m_bounding_box_fallback[index] = value; + return; + } + BBoxWriteImpl(index, value); } @@ -1748,6 +1757,7 @@ void Renderer::DoState(PointerWrap& p) p.Do(m_last_xfb_width); p.Do(m_last_xfb_stride); p.Do(m_last_xfb_height); + p.DoArray(m_bounding_box_fallback); if (p.GetMode() == PointerWrap::MODE_READ) { diff --git a/Source/Core/VideoCommon/RenderBase.h b/Source/Core/VideoCommon/RenderBase.h index 1deedd4f76..e07fbbc5fd 100644 --- a/Source/Core/VideoCommon/RenderBase.h +++ b/Source/Core/VideoCommon/RenderBase.h @@ -394,6 +394,18 @@ private: u32 m_last_xfb_stride = 0; u32 m_last_xfb_height = 0; + // Nintendo's SDK seems to write "default" bounding box values before every draw (1023 0 1023 0 + // are the only values encountered so far, which happen to be the extents allowed by the BP + // registers) to reset the registers for comparison in the pixel engine, and presumably to detect + // whether GX has updated the registers with real values. + // + // We can store these values when Bounding Box emulation is disabled and return them on read, + // which the game will interpret as "no pixels have been drawn" + // + // This produces much better results than just returning garbage, which can cause games like + // Ultimate Spider-Man to crash + std::array m_bounding_box_fallback = {}; + // NOTE: The methods below are called on the framedumping thread. void FrameDumpThreadFunc(); bool StartFrameDumpToFFMPEG(const FrameDump::FrameData&); diff --git a/Source/Core/VideoCommon/VideoBackendBase.cpp b/Source/Core/VideoCommon/VideoBackendBase.cpp index 2f7e92664a..f38601dcd7 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.cpp +++ b/Source/Core/VideoCommon/VideoBackendBase.cpp @@ -168,10 +168,8 @@ u16 VideoBackendBase::Video_GetBoundingBox(int index) "for this game."); } warn_once = false; - return 0; } - - if (!g_ActiveConfig.backend_info.bSupportsBBox) + else if (!g_ActiveConfig.backend_info.bSupportsBBox) { static bool warn_once = true; if (warn_once) @@ -182,7 +180,6 @@ u16 VideoBackendBase::Video_GetBoundingBox(int index) "freezes while running this game."); } warn_once = false; - return 0; } Fifo::SyncGPU(Fifo::SyncGPUReason::BBox);