From 9bd533ebe4cee1dfef9725f32fe0b9ca42f5680d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 10:58:03 -0500 Subject: [PATCH 1/2] VideoCommon/BoundingBox: Make interface for querying bounding box data Rather than expose the bounding box members directly, we can instead provide an interface for code to use. This makes it nicer to transition from global data, as the interface function names are already in place. --- .../VideoBackends/Software/SWRenderer.cpp | 4 +- Source/Core/VideoBackends/Software/Tev.cpp | 11 +--- Source/Core/VideoCommon/BPStructs.cpp | 6 +- Source/Core/VideoCommon/BoundingBox.cpp | 63 +++++++++++++++++-- Source/Core/VideoCommon/BoundingBox.h | 38 +++++++---- Source/Core/VideoCommon/PixelEngine.cpp | 2 +- Source/Core/VideoCommon/PixelShaderGen.cpp | 2 +- Source/Core/VideoCommon/VertexManagerBase.cpp | 2 +- 8 files changed, 92 insertions(+), 36 deletions(-) diff --git a/Source/Core/VideoBackends/Software/SWRenderer.cpp b/Source/Core/VideoBackends/Software/SWRenderer.cpp index 4d9b821f5a..c128e6bd1a 100644 --- a/Source/Core/VideoBackends/Software/SWRenderer.cpp +++ b/Source/Core/VideoBackends/Software/SWRenderer.cpp @@ -128,12 +128,12 @@ u32 SWRenderer::AccessEFB(EFBAccessType type, u32 x, u32 y, u32 InputData) u16 SWRenderer::BBoxRead(int index) { - return BoundingBox::coords[index]; + return BoundingBox::GetCoordinate(static_cast(index)); } void SWRenderer::BBoxWrite(int index, u16 value) { - BoundingBox::coords[index] = value; + BoundingBox::SetCoordinate(static_cast(index), value); } void SWRenderer::ClearScreen(const MathUtil::Rectangle& rc, bool colorEnable, bool alphaEnable, diff --git a/Source/Core/VideoBackends/Software/Tev.cpp b/Source/Core/VideoBackends/Software/Tev.cpp index 580ab58685..8d03cacac2 100644 --- a/Source/Core/VideoBackends/Software/Tev.cpp +++ b/Source/Core/VideoBackends/Software/Tev.cpp @@ -841,15 +841,8 @@ void Tev::Draw() EfbInterface::IncPerfCounterQuadCount(PQ_ZCOMP_OUTPUT); } - // branchless bounding box update - BoundingBox::coords[BoundingBox::LEFT] = - std::min((u16)Position[0], BoundingBox::coords[BoundingBox::LEFT]); - BoundingBox::coords[BoundingBox::RIGHT] = - std::max((u16)Position[0], BoundingBox::coords[BoundingBox::RIGHT]); - BoundingBox::coords[BoundingBox::TOP] = - std::min((u16)Position[1], BoundingBox::coords[BoundingBox::TOP]); - BoundingBox::coords[BoundingBox::BOTTOM] = - std::max((u16)Position[1], BoundingBox::coords[BoundingBox::BOTTOM]); + BoundingBox::Update(static_cast(Position[0]), static_cast(Position[0]), + static_cast(Position[1]), static_cast(Position[1])); #if ALLOW_TEV_DUMPS if (g_ActiveConfig.bDumpTevStages) diff --git a/Source/Core/VideoCommon/BPStructs.cpp b/Source/Core/VideoCommon/BPStructs.cpp index f9be647114..6cff479396 100644 --- a/Source/Core/VideoCommon/BPStructs.cpp +++ b/Source/Core/VideoCommon/BPStructs.cpp @@ -279,7 +279,7 @@ static void BPWritten(const BPCmd& bp) // here. Not sure if there's a better spot to put this. // the number of lines copied is determined by the y scale * source efb height - BoundingBox::active = false; + BoundingBox::Disable(); PixelShaderManager::SetBoundingBoxActive(false); float yScale; @@ -448,8 +448,8 @@ static void BPWritten(const BPCmd& bp) case BPMEM_CLEARBBOX1: case BPMEM_CLEARBBOX2: { - u8 offset = bp.address & 2; - BoundingBox::active = true; + const u8 offset = bp.address & 2; + BoundingBox::Enable(); PixelShaderManager::SetBoundingBoxActive(true); if (g_ActiveConfig.backend_info.bSupportsBBox && g_ActiveConfig.bBBoxEnable) diff --git a/Source/Core/VideoCommon/BoundingBox.cpp b/Source/Core/VideoCommon/BoundingBox.cpp index 4c9b6c4c3b..7c2127d353 100644 --- a/Source/Core/VideoCommon/BoundingBox.cpp +++ b/Source/Core/VideoCommon/BoundingBox.cpp @@ -3,20 +3,71 @@ // Refer to the license.txt file included. #include "VideoCommon/BoundingBox.h" + +#include +#include + #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" namespace BoundingBox { -// External vars -bool active = false; -u16 coords[4] = {0x80, 0xA0, 0x80, 0xA0}; +namespace +{ +// Whether or not bounding box is enabled. +bool s_is_active = false; + +// Current bounding box coordinates. +std::array s_coordinates{ + 0x80, + 0xA0, + 0x80, + 0xA0, +}; +} // Anonymous namespace + +void Enable() +{ + s_is_active = true; +} + +void Disable() +{ + s_is_active = false; +} + +bool IsEnabled() +{ + return s_is_active; +} + +u16 GetCoordinate(Coordinate coordinate) +{ + return s_coordinates[static_cast(coordinate)]; +} + +void SetCoordinate(Coordinate coordinate, u16 value) +{ + s_coordinates[static_cast(coordinate)] = value; +} + +void Update(u16 left, u16 right, u16 top, u16 bottom) +{ + const u16 new_left = std::min(left, GetCoordinate(Coordinate::Left)); + const u16 new_right = std::max(right, GetCoordinate(Coordinate::Right)); + const u16 new_top = std::min(top, GetCoordinate(Coordinate::Top)); + const u16 new_bottom = std::max(bottom, GetCoordinate(Coordinate::Bottom)); + + SetCoordinate(Coordinate::Left, new_left); + SetCoordinate(Coordinate::Right, new_right); + SetCoordinate(Coordinate::Top, new_top); + SetCoordinate(Coordinate::Bottom, new_bottom); +} -// Save state void DoState(PointerWrap& p) { - p.Do(active); - p.Do(coords); + p.Do(s_is_active); + p.DoArray(s_coordinates); } } // namespace BoundingBox diff --git a/Source/Core/VideoCommon/BoundingBox.h b/Source/Core/VideoCommon/BoundingBox.h index e10b97cc06..f4000c488f 100644 --- a/Source/Core/VideoCommon/BoundingBox.h +++ b/Source/Core/VideoCommon/BoundingBox.h @@ -11,21 +11,33 @@ class PointerWrap; // Bounding Box manager namespace BoundingBox { -// Determines if bounding box is active -extern bool active; - -// Bounding box current coordinates -extern u16 coords[4]; - -enum +// Indicates a coordinate of the bounding box. +enum class Coordinate { - LEFT = 0, - RIGHT = 1, - TOP = 2, - BOTTOM = 3 + Left, // The X coordinate of the left side of the bounding box. + Right, // The X coordinate of the right side of the bounding box. + Top, // The Y coordinate of the top of the bounding box. + Bottom, // The Y coordinate of the bottom of the bounding box. }; +// Enables bounding box. +void Enable(); + +// Disables bounding box. +void Disable(); + +// Determines if bounding box is enabled. +bool IsEnabled(); + +// Gets a particular coordinate for the bounding box. +u16 GetCoordinate(Coordinate coordinate); + +// Sets a particular coordinate for the bounding box. +void SetCoordinate(Coordinate coordinate, u16 value); + +// Updates all bounding box coordinates. +void Update(u16 left, u16 right, u16 top, u16 bottom); + // Save state void DoState(PointerWrap& p); - -}; // end of namespace BoundingBox +} // namespace BoundingBox diff --git a/Source/Core/VideoCommon/PixelEngine.cpp b/Source/Core/VideoCommon/PixelEngine.cpp index e5136e6d50..fc44841622 100644 --- a/Source/Core/VideoCommon/PixelEngine.cpp +++ b/Source/Core/VideoCommon/PixelEngine.cpp @@ -233,7 +233,7 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base) for (int i = 0; i < 4; ++i) { mmio->Register(base | (PE_BBOX_LEFT + 2 * i), MMIO::ComplexRead([i](u32) { - BoundingBox::active = false; + BoundingBox::Disable(); PixelShaderManager::SetBoundingBoxActive(false); return g_video_backend->Video_GetBoundingBox(i); }), diff --git a/Source/Core/VideoCommon/PixelShaderGen.cpp b/Source/Core/VideoCommon/PixelShaderGen.cpp index 89d0bd933d..ee750988f8 100644 --- a/Source/Core/VideoCommon/PixelShaderGen.cpp +++ b/Source/Core/VideoCommon/PixelShaderGen.cpp @@ -179,7 +179,7 @@ PixelShaderUid GetPixelShaderUid() uid_data->genMode_numindstages = bpmem.genMode.numindstages; uid_data->genMode_numtevstages = bpmem.genMode.numtevstages; uid_data->genMode_numtexgens = bpmem.genMode.numtexgens; - uid_data->bounding_box = g_ActiveConfig.bBBoxEnable && BoundingBox::active; + uid_data->bounding_box = g_ActiveConfig.bBBoxEnable && BoundingBox::IsEnabled(); uid_data->rgba6_format = bpmem.zcontrol.pixel_format == PEControl::RGBA6_Z24 && !g_ActiveConfig.bForceTrueColor; uid_data->dither = bpmem.blendmode.dither && uid_data->rgba6_format; diff --git a/Source/Core/VideoCommon/VertexManagerBase.cpp b/Source/Core/VideoCommon/VertexManagerBase.cpp index f1150dc340..ddbdd7e0f3 100644 --- a/Source/Core/VideoCommon/VertexManagerBase.cpp +++ b/Source/Core/VideoCommon/VertexManagerBase.cpp @@ -247,7 +247,7 @@ void VertexManagerBase::CommitBuffer(u32 num_vertices, u32 vertex_stride, u32 nu void VertexManagerBase::DrawCurrentBatch(u32 base_index, u32 num_indices, u32 base_vertex) { // If bounding box is enabled, we need to flush any changes first, then invalidate what we have. - if (::BoundingBox::active && g_ActiveConfig.bBBoxEnable && + if (BoundingBox::IsEnabled() && g_ActiveConfig.bBBoxEnable && g_ActiveConfig.backend_info.bSupportsBBox) { g_renderer->BBoxFlush(); From 2c9ec6cb8abba5a4dfd7a3792b0d866ed4898118 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 11:55:21 -0500 Subject: [PATCH 2/2] VideoCommon/BoundingBox: Move PixelShaderManager::SetBoundingBoxActive() calls into Enable()/Disable() Now that we have an actual interface to manage things, we can stop duplicating the calls to to the pixel shader manager and remove the need to remember to actually do so when disabling or enabling the bounding box. --- Source/Core/VideoCommon/BPStructs.cpp | 3 --- Source/Core/VideoCommon/BoundingBox.cpp | 3 +++ Source/Core/VideoCommon/PixelEngine.cpp | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Source/Core/VideoCommon/BPStructs.cpp b/Source/Core/VideoCommon/BPStructs.cpp index 6cff479396..e8b0ee1f32 100644 --- a/Source/Core/VideoCommon/BPStructs.cpp +++ b/Source/Core/VideoCommon/BPStructs.cpp @@ -278,9 +278,7 @@ static void BPWritten(const BPCmd& bp) // We should be able to get away with deactivating the current bbox tracking // here. Not sure if there's a better spot to put this. // the number of lines copied is determined by the y scale * source efb height - BoundingBox::Disable(); - PixelShaderManager::SetBoundingBoxActive(false); float yScale; if (PE_copy.scale_invert) @@ -450,7 +448,6 @@ static void BPWritten(const BPCmd& bp) { const u8 offset = bp.address & 2; BoundingBox::Enable(); - PixelShaderManager::SetBoundingBoxActive(true); if (g_ActiveConfig.backend_info.bSupportsBBox && g_ActiveConfig.bBBoxEnable) { diff --git a/Source/Core/VideoCommon/BoundingBox.cpp b/Source/Core/VideoCommon/BoundingBox.cpp index 7c2127d353..805431c7d6 100644 --- a/Source/Core/VideoCommon/BoundingBox.cpp +++ b/Source/Core/VideoCommon/BoundingBox.cpp @@ -9,6 +9,7 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" +#include "VideoCommon/PixelShaderManager.h" namespace BoundingBox { @@ -29,11 +30,13 @@ std::array s_coordinates{ void Enable() { s_is_active = true; + PixelShaderManager::SetBoundingBoxActive(s_is_active); } void Disable() { s_is_active = false; + PixelShaderManager::SetBoundingBoxActive(s_is_active); } bool IsEnabled() diff --git a/Source/Core/VideoCommon/PixelEngine.cpp b/Source/Core/VideoCommon/PixelEngine.cpp index fc44841622..d2a27c9d41 100644 --- a/Source/Core/VideoCommon/PixelEngine.cpp +++ b/Source/Core/VideoCommon/PixelEngine.cpp @@ -19,7 +19,6 @@ #include "VideoCommon/BoundingBox.h" #include "VideoCommon/Fifo.h" #include "VideoCommon/PerfQueryBase.h" -#include "VideoCommon/PixelShaderManager.h" #include "VideoCommon/VideoBackendBase.h" namespace PixelEngine @@ -234,7 +233,6 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base) { mmio->Register(base | (PE_BBOX_LEFT + 2 * i), MMIO::ComplexRead([i](u32) { BoundingBox::Disable(); - PixelShaderManager::SetBoundingBoxActive(false); return g_video_backend->Video_GetBoundingBox(i); }), MMIO::InvalidWrite());