From 38a75f6a49706840edf7a6f5b94e224468ebbabd Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 17 May 2022 13:42:31 -0700 Subject: [PATCH] Show a panic alert if the CP vertex config doesn't match the XF vertex config This probably isn't triggered by real games, but it's possible to accidentally do it with libogc (which results in freezes on real hardware). --- Source/Core/Core/DolphinAnalytics.cpp | 5 +- Source/Core/Core/DolphinAnalytics.h | 7 ++ .../Core/VideoCommon/VertexLoaderManager.cpp | 76 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/Source/Core/Core/DolphinAnalytics.cpp b/Source/Core/Core/DolphinAnalytics.cpp index 5b67fd2c04..ab5ea2c772 100644 --- a/Source/Core/Core/DolphinAnalytics.cpp +++ b/Source/Core/Core/DolphinAnalytics.cpp @@ -136,7 +136,7 @@ void DolphinAnalytics::ReportGameStart() } // Keep in sync with enum class GameQuirk definition. -constexpr std::array GAME_QUIRKS_NAMES{ +constexpr std::array GAME_QUIRKS_NAMES{ "icache-matters", "directly-reads-wiimote-input", "uses-DVDLowStopLaser", @@ -161,6 +161,9 @@ constexpr std::array GAME_QUIRKS_NAMES{ "sets-xf-clipdisable-bit-0", "sets-xf-clipdisable-bit-1", "sets-xf-clipdisable-bit-2", + "mismatched-gpu-colors-between-cp-and-xf", + "mismatched-gpu-normals-between-cp-and-xf", + "mismatched-gpu-tex-coords-between-cp-and-xf", }; static_assert(GAME_QUIRKS_NAMES.size() == static_cast(GameQuirk::COUNT), "Game quirks names and enum definition are out of sync."); diff --git a/Source/Core/Core/DolphinAnalytics.h b/Source/Core/Core/DolphinAnalytics.h index 2d61adae82..6e8acb8a8f 100644 --- a/Source/Core/Core/DolphinAnalytics.h +++ b/Source/Core/Core/DolphinAnalytics.h @@ -81,6 +81,13 @@ enum class GameQuirk SETS_XF_CLIPDISABLE_BIT_1, SETS_XF_CLIPDISABLE_BIT_2, + // Similar to the XF-BP mismatch, CP and XF might be configured with different vertex formats. + // Real hardware seems to hang in this case, so games probably don't do this, but it would + // be good to know if anything does it. + MISMATCHED_GPU_COLORS_BETWEEN_CP_AND_XF, + MISMATCHED_GPU_NORMALS_BETWEEN_CP_AND_XF, + MISMATCHED_GPU_TEX_COORDS_BETWEEN_CP_AND_XF, + COUNT, }; diff --git a/Source/Core/VideoCommon/VertexLoaderManager.cpp b/Source/Core/VideoCommon/VertexLoaderManager.cpp index 7710b7cc20..eea5183a81 100644 --- a/Source/Core/VideoCommon/VertexLoaderManager.cpp +++ b/Source/Core/VideoCommon/VertexLoaderManager.cpp @@ -16,6 +16,7 @@ #include "Common/EnumMap.h" #include "Common/Logging/Log.h" +#include "Core/DolphinAnalytics.h" #include "Core/HW/Memmap.h" #include "VideoCommon/BPMemory.h" @@ -28,6 +29,7 @@ #include "VideoCommon/VertexLoaderBase.h" #include "VideoCommon/VertexManagerBase.h" #include "VideoCommon/VertexShaderManager.h" +#include "VideoCommon/XFMemory.h" namespace VertexLoaderManager { @@ -249,6 +251,78 @@ static VertexLoaderBase* RefreshLoader(int vtx_attr_group, bool preprocess = fal return loader; } +static void CheckCPConfiguration(int vtx_attr_group) +{ + // Validate that the XF input configuration matches the CP configuration + u32 num_cp_colors = std::count_if( + g_main_cp_state.vtx_desc.low.Color.begin(), g_main_cp_state.vtx_desc.low.Color.end(), + [](auto format) { return format != VertexComponentFormat::NotPresent; }); + u32 num_cp_tex_coords = std::count_if( + g_main_cp_state.vtx_desc.high.TexCoord.begin(), g_main_cp_state.vtx_desc.high.TexCoord.end(), + [](auto format) { return format != VertexComponentFormat::NotPresent; }); + + u32 num_cp_normals; + if (g_main_cp_state.vtx_desc.low.Normal == VertexComponentFormat::NotPresent) + num_cp_normals = 0; + else if (g_main_cp_state.vtx_attr[vtx_attr_group].g0.NormalElements == NormalComponentCount::NTB) + num_cp_normals = 3; + else + num_cp_normals = 1; + + std::optional num_xf_normals; + switch (xfmem.invtxspec.numnormals) + { + case NormalCount::None: + num_xf_normals = 0; + break; + case NormalCount::Normal: + num_xf_normals = 1; + break; + case NormalCount::NormalTangentBinormal: + num_xf_normals = 3; + break; + default: + PanicAlertFmt("xfmem.invtxspec.numnormals is invalid: {}", xfmem.invtxspec.numnormals); + break; + } + + if (num_cp_colors != xfmem.invtxspec.numcolors || num_cp_normals != num_xf_normals || + num_cp_tex_coords != xfmem.invtxspec.numtextures) + { + PanicAlertFmt("Mismatched configuration between CP and XF stages - {}/{} colors, {}/{} " + "normals, {}/{} texture coordinates. Please report on the issue tracker.\n\n" + "VCD: {:08x} {:08x}\nVAT {}: {:08x} {:08x} {:08x}\nXF vertex spec: {:08x}", + num_cp_colors, xfmem.invtxspec.numcolors, num_cp_normals, + num_xf_normals.has_value() ? fmt::to_string(num_xf_normals.value()) : "invalid", + num_cp_tex_coords, xfmem.invtxspec.numtextures, g_main_cp_state.vtx_desc.low.Hex, + g_main_cp_state.vtx_desc.high.Hex, vtx_attr_group, + g_main_cp_state.vtx_attr[vtx_attr_group].g0.Hex, + g_main_cp_state.vtx_attr[vtx_attr_group].g1.Hex, + g_main_cp_state.vtx_attr[vtx_attr_group].g2.Hex, xfmem.invtxspec.hex); + + // Analytics reporting so we can discover which games have this problem, that way when we + // eventually simulate the behavior we have test cases for it. + if (num_cp_colors != xfmem.invtxspec.numcolors) + { + DolphinAnalytics::Instance().ReportGameQuirk( + GameQuirk::MISMATCHED_GPU_COLORS_BETWEEN_CP_AND_XF); + } + if (num_cp_normals != num_xf_normals) + { + DolphinAnalytics::Instance().ReportGameQuirk( + GameQuirk::MISMATCHED_GPU_NORMALS_BETWEEN_CP_AND_XF); + } + if (num_cp_tex_coords != xfmem.invtxspec.numtextures) + { + DolphinAnalytics::Instance().ReportGameQuirk( + GameQuirk::MISMATCHED_GPU_TEX_COORDS_BETWEEN_CP_AND_XF); + } + + // Don't bail out, though; we can still render something successfully + // (real hardware seems to hang in this case, though) + } +} + int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, DataReader src, bool is_preprocess) { @@ -265,6 +339,8 @@ int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int coun if (is_preprocess) return size; + CheckCPConfiguration(vtx_attr_group); + // If the native vertex format changed, force a flush. if (loader->m_native_vertex_format != s_current_vtx_fmt || loader->m_native_components != g_current_components)