Merge pull request #9765 from Techjar/bbox-pixel-quads-2-electric-boogaloo

VideoCommon: Move bounding box pixel quads rounding to shader

15 files changed, 52 insertions, 69 deletions

diff --git a/Source/Core/VideoBackends/D3D/D3DRender.cpp b/Source/Core/VideoBackends/D3D/D3DRender.cpp
index 904de1953c..1290a63630 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::BBoxReadImpl(int index)
+u16 Renderer::BBoxRead(int index)
 {
   return static_cast<u16>(BBox::Get(index));
 }
 
-void Renderer::BBoxWriteImpl(int index, u16 value)
+void Renderer::BBoxWrite(int index, u16 value)
 {
   BBox::Set(index, value);
 }
 
-void Renderer::BBoxFlushImpl()
+void Renderer::BBoxFlush()
 {
   BBox::Flush();
 }
diff --git a/Source/Core/VideoBackends/D3D/D3DRender.h b/Source/Core/VideoBackends/D3D/D3DRender.h
index 727e1243b9..2e008d87a7 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 BBoxReadImpl(int index) override;
-  void BBoxWriteImpl(int index, u16 value) override;
-  void BBoxFlushImpl() override;
+  u16 BBoxRead(int index) override;
+  void BBoxWrite(int index, u16 value) override;
+  void BBoxFlush() override;
 
   void Flush() override;
   void WaitForGPUIdle() override;
diff --git a/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp b/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp
index cfd259f9f2..ce9ac2504b 100644
--- a/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp
+++ b/Source/Core/VideoBackends/D3D12/D3D12Renderer.cpp
@@ -106,17 +106,17 @@ std::unique_ptr<AbstractPipeline> Renderer::CreatePipeline(const AbstractPipelin
   return DXPipeline::Create(config, cache_data, cache_data_length);
 }
 
-u16 Renderer::BBoxReadImpl(int index)
+u16 Renderer::BBoxRead(int index)
 {
   return static_cast<u16>(m_bounding_box->Get(index));
 }
 
-void Renderer::BBoxWriteImpl(int index, u16 value)
+void Renderer::BBoxWrite(int index, u16 value)
 {
   m_bounding_box->Set(index, value);
 }
 
-void Renderer::BBoxFlushImpl()
+void Renderer::BBoxFlush()
 {
   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 623a767676..4a26871a73 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 BBoxReadImpl(int index) override;
-  void BBoxWriteImpl(int index, u16 value) override;
-  void BBoxFlushImpl() override;
+  u16 BBoxRead(int index) override;
+  void BBoxWrite(int index, u16 value) override;
+  void BBoxFlush() override;
 
   void Flush() override;
   void WaitForGPUIdle() override;
diff --git a/Source/Core/VideoBackends/Null/NullRender.h b/Source/Core/VideoBackends/Null/NullRender.h
index 13bd3feae9..21bd9c9c2c 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 BBoxReadImpl(int index) override { return 0; }
-  void BBoxWriteImpl(int index, u16 value) override {}
+  u16 BBoxRead(int index) override { return 0; }
+  void BBoxWrite(int index, u16 value) override {}
 
   void ClearScreen(const MathUtil::Rectangle<int>& 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 d3435c51d5..5f6796d761 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<int>& rc)
   glScissor(rc.left, rc.top, rc.GetWidth(), rc.GetHeight());
 }
 
-u16 Renderer::BBoxReadImpl(int index)
+u16 Renderer::BBoxRead(int index)
 {
   // swap 2 and 3 for top/bottom
   if (index >= 2)
@@ -870,7 +870,7 @@ u16 Renderer::BBoxReadImpl(int index)
   return static_cast<u16>(value);
 }
 
-void Renderer::BBoxWriteImpl(int index, u16 value)
+void Renderer::BBoxWrite(int index, u16 value)
 {
   s32 swapped_value = value;
   if (index >= 2)
@@ -882,7 +882,7 @@ void Renderer::BBoxWriteImpl(int index, u16 value)
   BoundingBox::Set(index, swapped_value);
 }
 
-void Renderer::BBoxFlushImpl()
+void Renderer::BBoxFlush()
 {
   BoundingBox::Flush();
 }
diff --git a/Source/Core/VideoBackends/OGL/OGLRender.h b/Source/Core/VideoBackends/OGL/OGLRender.h
index c9b3b04896..399138ae07 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 BBoxReadImpl(int index) override;
-  void BBoxWriteImpl(int index, u16 value) override;
-  void BBoxFlushImpl() override;
+  u16 BBoxRead(int index) override;
+  void BBoxWrite(int index, u16 value) override;
+  void BBoxFlush() override;
 
   void BeginUtilityDrawing() override;
   void EndUtilityDrawing() override;
diff --git a/Source/Core/VideoBackends/Software/SWRenderer.cpp b/Source/Core/VideoBackends/Software/SWRenderer.cpp
index 51433d311f..c128e6bd1a 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::BBoxReadImpl(int index)
+u16 SWRenderer::BBoxRead(int index)
 {
   return BoundingBox::GetCoordinate(static_cast<BoundingBox::Coordinate>(index));
 }
 
-void SWRenderer::BBoxWriteImpl(int index, u16 value)
+void SWRenderer::BBoxWrite(int index, u16 value)
 {
   BoundingBox::SetCoordinate(static_cast<BoundingBox::Coordinate>(index), value);
 }
diff --git a/Source/Core/VideoBackends/Software/SWRenderer.h b/Source/Core/VideoBackends/Software/SWRenderer.h
index f2434d92c0..df23e123be 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 BBoxReadImpl(int index) override;
-  void BBoxWriteImpl(int index, u16 value) override;
+  u16 BBoxRead(int index) override;
+  void BBoxWrite(int index, u16 value) override;
 
   void RenderXFBToScreen(const MathUtil::Rectangle<int>& target_rc,
                          const AbstractTexture* source_texture,
diff --git a/Source/Core/VideoBackends/Software/Tev.cpp b/Source/Core/VideoBackends/Software/Tev.cpp
index be6f14d610..3c932b9566 100644
--- a/Source/Core/VideoBackends/Software/Tev.cpp
+++ b/Source/Core/VideoBackends/Software/Tev.cpp
@@ -838,8 +838,10 @@ void Tev::Draw()
     EfbInterface::IncPerfCounterQuadCount(PQ_ZCOMP_OUTPUT);
   }
 
-  BoundingBox::Update(static_cast<u16>(Position[0]), static_cast<u16>(Position[0]),
-                      static_cast<u16>(Position[1]), static_cast<u16>(Position[1]));
+  // 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.
+  BoundingBox::Update(static_cast<u16>(Position[0] & ~1), static_cast<u16>(Position[0] | 1),
+                      static_cast<u16>(Position[1] & ~1), static_cast<u16>(Position[1] | 1));
 
 #if ALLOW_TEV_DUMPS
   if (g_ActiveConfig.bDumpTevStages)
diff --git a/Source/Core/VideoBackends/Vulkan/VKRenderer.cpp b/Source/Core/VideoBackends/Vulkan/VKRenderer.cpp
index bbac6201c7..23d0c0d21a 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<const VKPipeline*>(pipeline));
 }
 
-u16 Renderer::BBoxReadImpl(int index)
+u16 Renderer::BBoxRead(int index)
 {
   return static_cast<u16>(m_bounding_box->Get(index));
 }
 
-void Renderer::BBoxWriteImpl(int index, u16 value)
+void Renderer::BBoxWrite(int index, u16 value)
 {
   m_bounding_box->Set(index, value);
 }
 
-void Renderer::BBoxFlushImpl()
+void Renderer::BBoxFlush()
 {
   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 b7e931d669..43ed3bd45b 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 BBoxReadImpl(int index) override;
-  void BBoxWriteImpl(int index, u16 value) override;
-  void BBoxFlushImpl() override;
+  u16 BBoxRead(int index) override;
+  void BBoxWrite(int index, u16 value) override;
+  void BBoxFlush() override;
 
   void Flush() override;
   void WaitForGPUIdle() override;
diff --git a/Source/Core/VideoCommon/PixelShaderGen.cpp b/Source/Core/VideoCommon/PixelShaderGen.cpp
index a2bc73257e..16fb7766e9 100644
--- a/Source/Core/VideoCommon/PixelShaderGen.cpp
+++ b/Source/Core/VideoCommon/PixelShaderGen.cpp
@@ -492,19 +492,30 @@ void UpdateBoundingBox(float2 rawpos) {{
   // such that width = right - left + 1. This has been verified on hardware.
   int2 pos = iround(rawpos * cefbscale + offset);
 
+  // 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.
+#ifdef API_OPENGL
+  // Need to flip the operands for Y on OpenGL because of lower-left origin.
+  int2 pos_tl = int2(pos.x & ~1, pos.y | 1);
+  int2 pos_br = int2(pos.x | 1, pos.y & ~1);
+#else
+  int2 pos_tl = pos & ~1;
+  int2 pos_br = pos | 1;
+#endif
+
 #ifdef SUPPORTS_SUBGROUP_REDUCTION
   if (CAN_USE_SUBGROUP_REDUCTION) {{
-    int2 min_pos = IS_HELPER_INVOCATION ? int2(2147483647, 2147483647) : pos;
-    int2 max_pos = IS_HELPER_INVOCATION ? int2(-2147483648, -2147483648) : pos;
+    int2 min_pos = IS_HELPER_INVOCATION ? int2(2147483647, 2147483647) : pos_tl;
+    int2 max_pos = IS_HELPER_INVOCATION ? int2(-2147483648, -2147483648) : pos_br;
     SUBGROUP_MIN(min_pos);
     SUBGROUP_MAX(max_pos);
     if (IS_FIRST_ACTIVE_INVOCATION)
       UpdateBoundingBoxBuffer(min_pos, max_pos);
   }} else {{
-    UpdateBoundingBoxBuffer(pos, pos);
+    UpdateBoundingBoxBuffer(pos_tl, pos_br);
   }}
 #else
-  UpdateBoundingBoxBuffer(pos, pos);
+  UpdateBoundingBoxBuffer(pos_tl, pos_br);
 #endif
 }}
 
diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp
index 0ff5b52902..93f8fa2279 100644
--- a/Source/Core/VideoCommon/RenderBase.cpp
+++ b/Source/Core/VideoCommon/RenderBase.cpp
@@ -185,32 +185,6 @@ void Renderer::ReinterpretPixelData(EFBReinterpretType convtype)
   g_framebuffer_manager->ReinterpretPixelData(convtype);
 }
 
-u16 Renderer::BBoxRead(int 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)
-{
-  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 1deedd4f76..48cef0bdad 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);
 
-  u16 BBoxRead(int index);
-  void BBoxWrite(int index, u16 value);
-  void BBoxFlush();
+  virtual u16 BBoxRead(int index) = 0;
+  virtual void BBoxWrite(int index, u16 value) = 0;
+  virtual void BBoxFlush() {}
 
   virtual void Flush() {}
   virtual void WaitForGPUIdle() {}
@@ -301,10 +301,6 @@ 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;