Merge pull request #9773 from Techjar/bbox-default-values

VideoCommon: Add fallback handling for bounding box when disabled or unsupported
dolphin-emu · Jun 1, 2021 · a3c89ac · a3c89ac
2 parents c404452 + 8cfe492
commit a3c89ac
Show file tree

Hide file tree

Showing 16 changed files with 78 additions and 43 deletions.
diff --git 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/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<u16>(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
@@ -61,9 +61,9 @@ class Renderer : public ::Renderer
   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
@@ -106,17 +106,17 @@ std::unique_ptr<AbstractPipeline> 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<u16>(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
@@ -47,9 +47,9 @@ class Renderer final : public ::Renderer
                                                    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
@@ -34,8 +34,8 @@ class Renderer final : public ::Renderer
 
   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<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
@@ -854,7 +854,7 @@ void Renderer::SetScissorRect(const MathUtil::Rectangle<int>& 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<u16>(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
@@ -126,9 +126,9 @@ class Renderer : public ::Renderer
   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
@@ -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<BoundingBox::Coordinate>(index));
 }
 
-void SWRenderer::BBoxWrite(int index, u16 value)
+void SWRenderer::BBoxWriteImpl(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
@@ -39,8 +39,8 @@ class SWRenderer final : public Renderer
 
   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<int>& target_rc,
                          const AbstractTexture* source_texture,

diff --git 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::BBoxRead(int index)
+u16 Renderer::BBoxReadImpl(int index)
 {
   return static_cast<u16>(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
@@ -54,9 +54,9 @@ class Renderer : public ::Renderer
 
   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/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
@@ -185,6 +185,30 @@ void Renderer::ReinterpretPixelData(EFBReinterpretType convtype)
   g_framebuffer_manager->ReinterpretPixelData(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);
+}
+
+void Renderer::BBoxFlush()
+{
+  BBoxFlushImpl();
+}
+
 u32 Renderer::AccessEFB(EFBAccessType type, u32 x, u32 y, u32 poke_data)
 {
   if (type == EFBAccessType::PeekColor)
@@ -1733,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
@@ -211,9 +211,9 @@ class Renderer
   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 @@ class Renderer
   // 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;
 
@@ -390,6 +394,18 @@ class Renderer
   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<u16, 4> 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
@@ -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);