From 0f17990137e9d32e442b7baf000098af5ffae111 Mon Sep 17 00:00:00 2001 From: Techjar Date: Fri, 21 May 2021 21:54:19 -0400 Subject: [PATCH 1/2] VideoCommon: Split BBox* functions into common and backend implementation variants This will allow for some aspects of bounding box to be handled in VideoCommon instead of individual backends. --- Source/Core/VideoBackends/D3D/D3DRender.cpp | 4 ++-- Source/Core/VideoBackends/D3D/D3DRender.h | 4 ++-- 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 | 4 ++-- Source/Core/VideoBackends/OGL/OGLRender.h | 4 ++-- 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, 48 insertions(+), 29 deletions(-) diff --git a/Source/Core/VideoBackends/D3D/D3DRender.cpp b/Source/Core/VideoBackends/D3D/D3DRender.cpp index 8830039af8..ccecec79f9 100644 --- a/Source/Core/VideoBackends/D3D/D3DRender.cpp +++ b/Source/Core/VideoBackends/D3D/D3DRender.cpp @@ -264,12 +264,12 @@ 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); } diff --git a/Source/Core/VideoBackends/D3D/D3DRender.h b/Source/Core/VideoBackends/D3D/D3DRender.h index 68f99f0fe1..45fad0e1b9 100644 --- a/Source/Core/VideoBackends/D3D/D3DRender.h +++ b/Source/Core/VideoBackends/D3D/D3DRender.h @@ -61,8 +61,8 @@ public: void SetFullscreen(bool enable_fullscreen) override; bool IsFullscreen() const 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 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 5be25d3436..49f848c913 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) diff --git a/Source/Core/VideoBackends/OGL/OGLRender.h b/Source/Core/VideoBackends/OGL/OGLRender.h index d36a32fa87..9c6623f254 100644 --- a/Source/Core/VideoBackends/OGL/OGLRender.h +++ b/Source/Core/VideoBackends/OGL/OGLRender.h @@ -126,8 +126,8 @@ public: void BindBackbuffer(const ClearColor& clear_color = {}) override; void PresentBackbuffer() 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 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 be6b000bec8f37fd4440dfcfdeb7630ccd7bfec8 Mon Sep 17 00:00:00 2001 From: Techjar Date: Sat, 22 May 2021 01:13:55 -0400 Subject: [PATCH 2/2] VideoCommon: Account for pixel quads in bounding box calculation The GC/Wii GPU rasterizes in 2x2 pixel groups, so bounding box values will be rounded to the extents of these groups, rather than the exact pixel. To account for this, we'll round the top/left down to even and the bottom/right up to odd. I have verified that the values resulting from this change exactly match a real Wii. --- Source/Core/VideoCommon/RenderBase.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp index f93d498fc6..0ff5b52902 100644 --- a/Source/Core/VideoCommon/RenderBase.cpp +++ b/Source/Core/VideoCommon/RenderBase.cpp @@ -187,7 +187,18 @@ void Renderer::ReinterpretPixelData(EFBReinterpretType convtype) u16 Renderer::BBoxRead(int index) { - return BBoxReadImpl(index); + u16 value = BBoxReadImpl(index); + + // The GC/Wii GPU rasterizes in 2x2 pixel groups, so bounding box values will be rounded to the + // extents of these groups, rather than the exact pixel. + // This would have been handled in the pixel shader, but all attempts to do so did not work on + // OpenGL/NVIDIA, due to presumably mystical driver behavior with atomics. + if (index == 0 || index == 2) + value &= ~1; + else + value |= 1; + + return value; } void Renderer::BBoxWrite(int index, u16 value)