From 076392a0f65fa11b8c3475b762b8b9794057ae11 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Fri, 12 Nov 2021 11:48:26 -0800 Subject: [PATCH] VideoCommon: Rework scissor handling This increases accuracy, fixing the white rendering in Major Minor's Majestic March. However, the hardware backends can only have one viewport and scissor rectangle at a time, while sometimes multiple are needed to accurately emulate what is happening. If possible, this will need to be fixed later. --- Source/Core/VideoBackends/OGL/OGLRender.cpp | 2 +- Source/Core/VideoCommon/BPFunctions.cpp | 193 +++++++++++++++--- Source/Core/VideoCommon/BPFunctions.h | 123 ++++++++++- Source/Core/VideoCommon/BPStructs.cpp | 5 +- Source/Core/VideoCommon/RenderBase.cpp | 6 +- .../Core/VideoCommon/VertexShaderManager.cpp | 2 +- 6 files changed, 283 insertions(+), 48 deletions(-) diff --git a/Source/Core/VideoBackends/OGL/OGLRender.cpp b/Source/Core/VideoBackends/OGL/OGLRender.cpp index fae0d7a0ad..4c686941fe 100644 --- a/Source/Core/VideoBackends/OGL/OGLRender.cpp +++ b/Source/Core/VideoBackends/OGL/OGLRender.cpp @@ -958,7 +958,7 @@ void Renderer::ClearScreen(const MathUtil::Rectangle& rc, bool colorEnable, glDepthMask(m_current_depth_state.updateenable); // Scissor rect must be restored. - BPFunctions::SetScissor(); + BPFunctions::SetScissorAndViewport(); } void Renderer::RenderXFBToScreen(const MathUtil::Rectangle& target_rc, diff --git a/Source/Core/VideoCommon/BPFunctions.cpp b/Source/Core/VideoCommon/BPFunctions.cpp index 9bc26fb9d2..c065f1cc5f 100644 --- a/Source/Core/VideoCommon/BPFunctions.cpp +++ b/Source/Core/VideoCommon/BPFunctions.cpp @@ -7,6 +7,7 @@ #include #include +#include "Common/Assert.h" #include "Common/CommonTypes.h" #include "Common/Logging/Log.h" @@ -37,48 +38,172 @@ void SetGenerationMode() g_vertex_manager->SetRasterizationStateChanged(); } -void SetScissor() +int ScissorRect::GetArea() const { - /* NOTE: the minimum value here for the scissor rect is -342. - * GX SDK functions internally add an offset of 342 to scissor coords to - * ensure that the register was always unsigned. - * - * The code that was here before tried to "undo" this offset, but - * since we always take the difference, the +342 added to both - * sides cancels out. */ + return rect.GetWidth() * rect.GetHeight(); +} - /* NOTE: With a positive scissor offset, the scissor rect is shifted left and/or up; - * With a negative scissor offset, the scissor rect is shifted right and/or down. - * - * GX SDK functions internally add an offset of 342 to scissor offset. - * The scissor offset is always even, so to save space, the scissor offset register - * is scaled down by 2. So, if somebody calls GX_SetScissorBoxOffset(20, 20); - * the registers will be set to ((20 + 342) / 2 = 181, 181). - * - * The scissor offset register is 10bit signed [-512, 511]. - * e.g. In Super Mario Galaxy 1 and 2, during the "Boss roar effect", - * for a scissor offset of (0, -464), the scissor offset register will be set to - * (171, (-464 + 342) / 2 = -61). - */ - s32 xoff = bpmem.scissorOffset.x * 2; - s32 yoff = bpmem.scissorOffset.y * 2; +int ScissorResult::GetViewportArea(const ScissorRect& rect) const +{ + int x0 = std::clamp(rect.rect.left + rect.x_off, viewport_left, viewport_right); + int x1 = std::clamp(rect.rect.right + rect.x_off, viewport_left, viewport_right); - MathUtil::Rectangle native_rc(bpmem.scissorTL.x - xoff, bpmem.scissorTL.y - yoff, - bpmem.scissorBR.x - xoff + 1, bpmem.scissorBR.y - yoff + 1); - native_rc.ClampUL(0, 0, EFB_WIDTH, EFB_HEIGHT); + int y0 = std::clamp(rect.rect.top + rect.y_off, viewport_top, viewport_bottom); + int y1 = std::clamp(rect.rect.bottom + rect.y_off, viewport_top, viewport_bottom); - auto target_rc = g_renderer->ConvertEFBRectangle(native_rc); + return (x1 - x0) * (y1 - y0); +} + +// Compare so that a sorted collection of rectangles has the best one last, so that if they're drawn +// in order, the best one is the one that is drawn last (and thus over the rest). +// The exact iteration order on hardware hasn't been tested, but silly things can happen where a +// polygon can intersect with itself; this only applies outside of the viewport region (in areas +// that would normally be affected by clipping). No game is known to care about this. +bool ScissorResult::IsWorse(const ScissorRect& lhs, const ScissorRect& rhs) const +{ + // First, penalize any rect that is not in the viewport + int lhs_area = GetViewportArea(lhs); + int rhs_area = GetViewportArea(rhs); + + if (lhs_area != rhs_area) + return lhs_area < rhs_area; + + // Now compare on total areas, without regard for the viewport + return lhs.GetArea() < rhs.GetArea(); +} + +namespace +{ +// Dynamically sized small array of ScissorRanges (used as an heap-less alternative to std::vector +// to reduce allocation overhead) +struct RangeList +{ + static constexpr u32 MAX_RANGES = 9; + + u32 m_num_ranges = 0; + std::array m_ranges{}; + + void AddRange(int offset, int start, int end) + { + DEBUG_ASSERT(m_num_ranges < MAX_RANGES); + m_ranges[m_num_ranges] = ScissorRange(offset, start, end); + m_num_ranges++; + } + auto begin() const { return m_ranges.begin(); } + auto end() const { return m_ranges.begin() + m_num_ranges; } + + u32 size() { return m_num_ranges; } +}; + +static RangeList ComputeScissorRanges(int start, int end, int offset, int efb_dim) +{ + RangeList ranges; + + for (int extra_off = -4096; extra_off <= 4096; extra_off += 1024) + { + int new_off = offset + extra_off; + int new_start = std::clamp(start - new_off, 0, efb_dim); + int new_end = std::clamp(end - new_off + 1, 0, efb_dim); + if (new_start < new_end) + { + ranges.AddRange(new_off, new_start, new_end); + } + } + + return ranges; +} +} // namespace + +ScissorResult::ScissorResult(const BPMemory& bpmemory, const XFMemory& xfmemory) + : ScissorResult(bpmemory, + std::minmax(xfmemory.viewport.xOrig - xfmemory.viewport.wd, + xfmemory.viewport.xOrig + xfmemory.viewport.wd), + std::minmax(xfmemory.viewport.yOrig - xfmemory.viewport.ht, + xfmemory.viewport.yOrig + xfmemory.viewport.ht)) +{ +} +ScissorResult::ScissorResult(const BPMemory& bpmemory, std::pair viewport_x, + std::pair viewport_y) + : scissor_tl{.hex = bpmemory.scissorTL.hex}, scissor_br{.hex = bpmemory.scissorBR.hex}, + scissor_off{.hex = bpmemory.scissorOffset.hex}, viewport_left(viewport_x.first), + viewport_right(viewport_x.second), viewport_top(viewport_y.first), + viewport_bottom(viewport_y.second) +{ + // Range is [left, right] and [top, bottom] (closed intervals) + const int left = scissor_tl.x; + const int right = scissor_br.x; + const int top = scissor_tl.y; + const int bottom = scissor_br.y; + // When left > right or top > bottom, nothing renders (even with wrapping from the offsets) + if (left > right || top > bottom) + return; + + // Note that both the offsets and the coordinates have 342 added to them internally by GX + // functions (for the offsets, this is before they are divided by 2/right shifted). This code + // could undo both sets of offsets, but it doesn't need to since they cancel out when subtracting + // (and those offsets actually matter for the left > right and top > bottom checks). + const int x_off = scissor_off.x << 1; + const int y_off = scissor_off.y << 1; + + RangeList x_ranges = ComputeScissorRanges(left, right, x_off, EFB_WIDTH); + RangeList y_ranges = ComputeScissorRanges(top, bottom, y_off, EFB_HEIGHT); + + m_result.reserve(x_ranges.size() * y_ranges.size()); + + // Now we need to form actual rectangles from the x and y ranges, + // which is a simple Cartesian product of x_ranges_clamped and y_ranges_clamped. + // Each rectangle is also a Cartesian product of x_range and y_range, with + // the rectangles being half-open (of the form [x0, x1) X [y0, y1)). + for (const auto& x_range : x_ranges) + { + DEBUG_ASSERT(x_range.start < x_range.end); + DEBUG_ASSERT(x_range.end <= EFB_WIDTH); + for (const auto& y_range : y_ranges) + { + DEBUG_ASSERT(y_range.start < y_range.end); + DEBUG_ASSERT(y_range.end <= EFB_HEIGHT); + m_result.emplace_back(x_range, y_range); + } + } + + auto cmp = [&](const ScissorRect& lhs, const ScissorRect& rhs) { return IsWorse(lhs, rhs); }; + std::sort(m_result.begin(), m_result.end(), cmp); +} + +ScissorRect ScissorResult::Best() const +{ + // For now, simply choose the best rectangle (see ScissorResult::IsWorse). + // This does mean we calculate all rectangles and only choose one, which is not optimal, but this + // is called infrequently. Eventually, all backends will support multiple scissor rects. + if (!m_result.empty()) + { + return m_result.back(); + } + else + { + // But if we have no rectangles, use a bogus one that's out of bounds. + // Ideally, all backends will support multiple scissor rects, in which case this won't be + // needed. + return ScissorRect(ScissorRange{0, 1000, 1001}, ScissorRange{0, 1000, 1001}); + } +} + +ScissorResult ComputeScissorRects() +{ + return ScissorResult{bpmem, xfmem}; +} + +void SetScissorAndViewport() +{ + auto native_rc = ComputeScissorRects().Best(); + + auto target_rc = g_renderer->ConvertEFBRectangle(native_rc.rect); auto converted_rc = g_renderer->ConvertFramebufferRectangle(target_rc, g_renderer->GetCurrentFramebuffer()); g_renderer->SetScissorRect(converted_rc); -} -void SetViewport() -{ - const s32 xoff = bpmem.scissorOffset.x * 2; - const s32 yoff = bpmem.scissorOffset.y * 2; - float raw_x = xfmem.viewport.xOrig - xfmem.viewport.wd - xoff; - float raw_y = xfmem.viewport.yOrig + xfmem.viewport.ht - yoff; + float raw_x = (xfmem.viewport.xOrig - native_rc.x_off) - xfmem.viewport.wd; + float raw_y = (xfmem.viewport.yOrig - native_rc.y_off) + xfmem.viewport.ht; float raw_width = 2.0f * xfmem.viewport.wd; float raw_height = -2.0f * xfmem.viewport.ht; if (g_ActiveConfig.UseVertexRounding()) diff --git a/Source/Core/VideoCommon/BPFunctions.h b/Source/Core/VideoCommon/BPFunctions.h index 22bbea6487..eb0d67edf2 100644 --- a/Source/Core/VideoCommon/BPFunctions.h +++ b/Source/Core/VideoCommon/BPFunctions.h @@ -7,16 +7,131 @@ #pragma once -#include "Common/MathUtil.h" +#include +#include -struct BPCmd; +#include "Common/MathUtil.h" +#include "VideoCommon/BPMemory.h" +struct XFMemory; namespace BPFunctions { +struct ScissorRange +{ + constexpr ScissorRange() = default; + constexpr ScissorRange(int offset, int start, int end) : offset(offset), start(start), end(end) {} + int offset = 0; + int start = 0; + int end = 0; +}; + +struct ScissorRect +{ + constexpr ScissorRect(ScissorRange x_range, ScissorRange y_range) + : // Rectangle ctor takes x0, y0, x1, y1. + rect(x_range.start, y_range.start, x_range.end, y_range.end), x_off(x_range.offset), + y_off(y_range.offset) + { + } + + MathUtil::Rectangle rect; + int x_off; + int y_off; + + int GetArea() const; +}; + +// Although the GameCube/Wii have only one scissor configuration and only one viewport +// configuration, some values can result in multiple parts of the screen being updated. +// This can happen if the scissor offset combined with the bottom or right coordinate ends up +// exceeding 1024; then, both sides of the screen will be drawn to, while the middle is not. +// Major Minor's Majestic March causes this to happen during loading screens and other scrolling +// effects, though it draws on top of one of them. +// This can also happen if the scissor rectangle is particularly large, but this will usually +// involve drawing content outside of the viewport, which Dolphin does not currently handle. +// +// The hardware backends can currently only use one viewport and scissor rectangle, so we need to +// pick the "best" rectangle based on how much of the viewport would be rendered to the screen. +// If we choose the wrong one, then content might not actually show up when the game is expecting it +// to. This does happen on Major Minor's Majestic March for the final few frames of the horizontal +// scrolling animation, but it isn't that important. Note that the assumption that a "best" +// rectangle exists is based on games only wanting to draw one rectangle, and accidentally +// configuring the scissor offset and size of the scissor rectangle such that multiple show up; +// there are no known games where this is not the case. +struct ScissorResult +{ + ScissorResult(const BPMemory& bpmem, const XFMemory& xfmem); + ~ScissorResult() = default; + ScissorResult(const ScissorResult& other) + : scissor_tl{.hex = other.scissor_tl.hex}, scissor_br{.hex = other.scissor_br.hex}, + scissor_off{.hex = other.scissor_off.hex}, viewport_left{other.viewport_left}, + viewport_right{other.viewport_right}, viewport_top{other.viewport_top}, + viewport_bottom{other.viewport_bottom}, m_result{other.m_result} + { + } + ScissorResult& operator=(const ScissorResult& other) + { + if (this == &other) + return *this; + scissor_tl.hex = other.scissor_tl.hex; + scissor_br.hex = other.scissor_br.hex; + scissor_off.hex = other.scissor_off.hex; + viewport_left = other.viewport_left; + viewport_right = other.viewport_right; + viewport_top = other.viewport_top; + viewport_bottom = other.viewport_bottom; + m_result = other.m_result; + return *this; + } + ScissorResult(ScissorResult&& other) + : scissor_tl{.hex = other.scissor_tl.hex}, scissor_br{.hex = other.scissor_br.hex}, + scissor_off{.hex = other.scissor_off.hex}, viewport_left{other.viewport_left}, + viewport_right{other.viewport_right}, viewport_top{other.viewport_top}, + viewport_bottom{other.viewport_bottom}, m_result{std::move(other.m_result)} + { + } + ScissorResult& operator=(ScissorResult&& other) + { + if (this == &other) + return *this; + scissor_tl.hex = other.scissor_tl.hex; + scissor_br.hex = other.scissor_br.hex; + scissor_off.hex = other.scissor_off.hex; + viewport_left = other.viewport_left; + viewport_right = other.viewport_right; + viewport_top = other.viewport_top; + viewport_bottom = other.viewport_bottom; + m_result = std::move(other.m_result); + return *this; + } + + // Input values, for use in statistics + ScissorPos scissor_tl; + ScissorPos scissor_br; + ScissorOffset scissor_off; + float viewport_left; + float viewport_right; + float viewport_top; + float viewport_bottom; + + // Actual result + std::vector m_result; + + ScissorRect Best() const; + +private: + ScissorResult(const BPMemory& bpmem, std::pair viewport_x, + std::pair viewport_y); + + int GetViewportArea(const ScissorRect& rect) const; + bool IsWorse(const ScissorRect& lhs, const ScissorRect& rhs) const; +}; + +ScissorResult ComputeScissorRects(); + void FlushPipeline(); void SetGenerationMode(); -void SetScissor(); -void SetViewport(); +void SetScissorAndViewport(); void SetDepthMode(); void SetBlendMode(); void ClearScreen(const MathUtil::Rectangle& rc); diff --git a/Source/Core/VideoCommon/BPStructs.cpp b/Source/Core/VideoCommon/BPStructs.cpp index 7e5ecf1689..c6746d444a 100644 --- a/Source/Core/VideoCommon/BPStructs.cpp +++ b/Source/Core/VideoCommon/BPStructs.cpp @@ -131,8 +131,6 @@ static void BPWritten(const BPCmd& bp, int cycles_into_future) case BPMEM_SCISSORTL: // Scissor Rectable Top, Left case BPMEM_SCISSORBR: // Scissor Rectable Bottom, Right case BPMEM_SCISSOROFFSET: // Scissor Offset - SetScissor(); - SetViewport(); VertexShaderManager::SetViewportChanged(); GeometryShaderManager::SetViewportChanged(); return; @@ -1272,8 +1270,7 @@ void BPReload() // let's not risk actually replaying any writes. // note that PixelShaderManager is already covered since it has its own DoState. SetGenerationMode(); - SetScissor(); - SetViewport(); + SetScissorAndViewport(); SetDepthMode(); SetBlendMode(); OnPixelFormatChange(); diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp index 9d3cfedbd3..6c369621b3 100644 --- a/Source/Core/VideoCommon/RenderBase.cpp +++ b/Source/Core/VideoCommon/RenderBase.cpp @@ -160,8 +160,7 @@ void Renderer::EndUtilityDrawing() { // Reset framebuffer/scissor/viewport. Pipeline will be reset at next draw. g_framebuffer_manager->BindEFBFramebuffer(); - BPFunctions::SetScissor(); - BPFunctions::SetViewport(); + BPFunctions::SetScissorAndViewport(); } void Renderer::SetFramebuffer(AbstractFramebuffer* framebuffer) @@ -543,8 +542,7 @@ void Renderer::CheckForConfigChanges() // Viewport and scissor rect have to be reset since they will be scaled differently. if (changed_bits & CONFIG_CHANGE_BIT_TARGET_SIZE) { - BPFunctions::SetViewport(); - BPFunctions::SetScissor(); + BPFunctions::SetScissorAndViewport(); } // Stereo mode change requires recompiling our post processing pipeline and imgui pipelines for diff --git a/Source/Core/VideoCommon/VertexShaderManager.cpp b/Source/Core/VideoCommon/VertexShaderManager.cpp index 6ba7144345..fc8b9e69f8 100644 --- a/Source/Core/VideoCommon/VertexShaderManager.cpp +++ b/Source/Core/VideoCommon/VertexShaderManager.cpp @@ -298,7 +298,7 @@ void VertexShaderManager::SetConstants() } dirty = true; - BPFunctions::SetViewport(); + BPFunctions::SetScissorAndViewport(); } if (bProjectionChanged || g_freelook_camera.GetController()->IsDirty())