From 736466a5d91c77cdcd3966e48b071c4f52658159 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 17 May 2022 11:27:49 -0700 Subject: [PATCH 1/4] XFMemory: Rename hostinfo to invtxspec --- Source/Core/VideoCommon/XFMemory.h | 34 +++++++++++++++--------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Source/Core/VideoCommon/XFMemory.h b/Source/Core/VideoCommon/XFMemory.h index 836f221e47..23c4088f05 100644 --- a/Source/Core/VideoCommon/XFMemory.h +++ b/Source/Core/VideoCommon/XFMemory.h @@ -416,23 +416,23 @@ struct Projection struct XFMemory { - float posMatrices[256]; // 0x0000 - 0x00ff - u32 unk0[768]; // 0x0100 - 0x03ff - float normalMatrices[96]; // 0x0400 - 0x045f - u32 unk1[160]; // 0x0460 - 0x04ff - float postMatrices[256]; // 0x0500 - 0x05ff - Light lights[8]; // 0x0600 - 0x067f - u32 unk2[2432]; // 0x0680 - 0x0fff - u32 error; // 0x1000 - u32 diag; // 0x1001 - u32 state0; // 0x1002 - u32 state1; // 0x1003 - u32 xfClock; // 0x1004 - ClipDisable clipDisable; // 0x1005 - u32 perf0; // 0x1006 - u32 perf1; // 0x1007 - INVTXSPEC hostinfo; // 0x1008 number of textures,colors,normals from vertex input - NumColorChannel numChan; // 0x1009 + float posMatrices[256]; // 0x0000 - 0x00ff + u32 unk0[768]; // 0x0100 - 0x03ff + float normalMatrices[96]; // 0x0400 - 0x045f + u32 unk1[160]; // 0x0460 - 0x04ff + float postMatrices[256]; // 0x0500 - 0x05ff + Light lights[8]; // 0x0600 - 0x067f + u32 unk2[2432]; // 0x0680 - 0x0fff + u32 error; // 0x1000 + u32 diag; // 0x1001 + u32 state0; // 0x1002 + u32 state1; // 0x1003 + u32 xfClock; // 0x1004 + ClipDisable clipDisable; // 0x1005 + u32 perf0; // 0x1006 + u32 perf1; // 0x1007 + INVTXSPEC invtxspec; // 0x1008 + NumColorChannel numChan; // 0x1009 u32 ambColor[NUM_XF_COLOR_CHANNELS]; // 0x100a, 0x100b u32 matColor[NUM_XF_COLOR_CHANNELS]; // 0x100c, 0x100d LitChannel color[NUM_XF_COLOR_CHANNELS]; // 0x100e, 0x100f From 46bcdc4372209f343ff28994770c0666a0d6c3ac Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 17 May 2022 11:54:24 -0700 Subject: [PATCH 2/4] Rename CP and XF normal component count enums and update their descriptions --- Source/Core/DolphinQt/FIFO/FIFOAnalyzer.cpp | 3 +- Source/Core/VideoCommon/CPMemory.h | 11 ++-- Source/Core/VideoCommon/VertexLoader.cpp | 2 +- Source/Core/VideoCommon/VertexLoaderARM64.cpp | 2 +- Source/Core/VideoCommon/VertexLoaderBase.cpp | 2 +- Source/Core/VideoCommon/VertexLoaderX64.cpp | 2 +- .../Core/VideoCommon/VertexLoader_Normal.cpp | 66 +++++++++---------- Source/Core/VideoCommon/XFMemory.h | 8 +-- .../VideoCommon/VertexLoaderTest.cpp | 2 +- 9 files changed, 49 insertions(+), 49 deletions(-) diff --git a/Source/Core/DolphinQt/FIFO/FIFOAnalyzer.cpp b/Source/Core/DolphinQt/FIFO/FIFOAnalyzer.cpp index 4deb2df6ff..586982e0e4 100644 --- a/Source/Core/DolphinQt/FIFO/FIFOAnalyzer.cpp +++ b/Source/Core/DolphinQt/FIFO/FIFOAnalyzer.cpp @@ -677,10 +677,9 @@ public: } process_component(vtx_desc.low.Position, vtx_attr.g0.PosFormat, vtx_attr.g0.PosElements == CoordComponentCount::XY ? 2 : 3); - // TODO: Is this calculation correct? const u32 normal_component_count = vtx_desc.low.Normal == VertexComponentFormat::Direct ? 3 : 1; - const u32 normal_elements = vtx_attr.g0.NormalElements == NormalComponentCount::NBT ? 3 : 1; + const u32 normal_elements = vtx_attr.g0.NormalElements == NormalComponentCount::NTB ? 3 : 1; process_component(vtx_desc.low.Normal, vtx_attr.g0.NormalFormat, normal_component_count * normal_elements, vtx_attr.g0.NormalIndex3 ? normal_elements : 1); diff --git a/Source/Core/VideoCommon/CPMemory.h b/Source/Core/VideoCommon/CPMemory.h index 167f667c42..7ce56c05cd 100644 --- a/Source/Core/VideoCommon/CPMemory.h +++ b/Source/Core/VideoCommon/CPMemory.h @@ -162,12 +162,12 @@ struct fmt::formatter : EnumFormatter -struct fmt::formatter : EnumFormatter +struct fmt::formatter : EnumFormatter { - constexpr formatter() : EnumFormatter({"1 (n)", "3 (n, b, t)"}) {} + constexpr formatter() : EnumFormatter({"1 (normal)", "3 (normal, tangent, binormal)"}) {} }; enum class ColorComponentCount @@ -348,8 +348,9 @@ struct fmt::formatter { static constexpr std::array byte_dequant = { "shift does not apply to u8/s8 components", "shift applies to u8/s8 components"}; - static constexpr std::array normalindex3 = {"single index per normal", - "triple-index per nine-normal"}; + static constexpr std::array normalindex3 = { + "single index shared by normal, tangent, and binormal", + "three indices, one each for normal, tangent, and binormal"}; return fmt::format_to(ctx.out(), "Position elements: {}\n" diff --git a/Source/Core/VideoCommon/VertexLoader.cpp b/Source/Core/VideoCommon/VertexLoader.cpp index 751778dd24..8176027b45 100644 --- a/Source/Core/VideoCommon/VertexLoader.cpp +++ b/Source/Core/VideoCommon/VertexLoader.cpp @@ -129,7 +129,7 @@ void VertexLoader::CompileVertexTranslator() } WriteCall(pFunc); - for (int i = 0; i < (m_VtxAttr.g0.NormalElements == NormalComponentCount::NBT ? 3 : 1); i++) + for (int i = 0; i < (m_VtxAttr.g0.NormalElements == NormalComponentCount::NTB ? 3 : 1); i++) { m_native_vtx_decl.normals[i].components = 3; m_native_vtx_decl.normals[i].enable = true; diff --git a/Source/Core/VideoCommon/VertexLoaderARM64.cpp b/Source/Core/VideoCommon/VertexLoaderARM64.cpp index eff8a29993..9388c21a7b 100644 --- a/Source/Core/VideoCommon/VertexLoaderARM64.cpp +++ b/Source/Core/VideoCommon/VertexLoaderARM64.cpp @@ -472,7 +472,7 @@ void VertexLoaderARM64::GenerateVertexLoader() { static const u8 map[8] = {7, 6, 15, 14}; const u8 scaling_exponent = map[u32(m_VtxAttr.g0.NormalFormat.Value())]; - const int limit = m_VtxAttr.g0.NormalElements == NormalComponentCount::NBT ? 3 : 1; + const int limit = m_VtxAttr.g0.NormalElements == NormalComponentCount::NTB ? 3 : 1; s32 offset = -1; for (int i = 0; i < limit; i++) diff --git a/Source/Core/VideoCommon/VertexLoaderBase.cpp b/Source/Core/VideoCommon/VertexLoaderBase.cpp index 2b6c3cdd21..678246783d 100644 --- a/Source/Core/VideoCommon/VertexLoaderBase.cpp +++ b/Source/Core/VideoCommon/VertexLoaderBase.cpp @@ -152,7 +152,7 @@ u32 VertexLoaderBase::GetVertexComponents(const TVtxDesc& vtx_desc, const VAT& v if (vtx_desc.low.Normal != VertexComponentFormat::NotPresent) { components |= VB_HAS_NORMAL; - if (vtx_attr.g0.NormalElements == NormalComponentCount::NBT) + if (vtx_attr.g0.NormalElements == NormalComponentCount::NTB) components |= VB_HAS_TANGENT | VB_HAS_BINORMAL; } for (u32 i = 0; i < vtx_desc.low.Color.Size(); i++) diff --git a/Source/Core/VideoCommon/VertexLoaderX64.cpp b/Source/Core/VideoCommon/VertexLoaderX64.cpp index aebba7680d..63752429aa 100644 --- a/Source/Core/VideoCommon/VertexLoaderX64.cpp +++ b/Source/Core/VideoCommon/VertexLoaderX64.cpp @@ -461,7 +461,7 @@ void VertexLoaderX64::GenerateVertexLoader() { static const u8 map[8] = {7, 6, 15, 14}; const u8 scaling_exponent = map[u32(m_VtxAttr.g0.NormalFormat.Value())]; - const int limit = m_VtxAttr.g0.NormalElements == NormalComponentCount::NBT ? 3 : 1; + const int limit = m_VtxAttr.g0.NormalElements == NormalComponentCount::NTB ? 3 : 1; for (int i = 0; i < limit; i++) { diff --git a/Source/Core/VideoCommon/VertexLoader_Normal.cpp b/Source/Core/VideoCommon/VertexLoader_Normal.cpp index db955639d7..6813e4128c 100644 --- a/Source/Core/VideoCommon/VertexLoader_Normal.cpp +++ b/Source/Core/VideoCommon/VertexLoader_Normal.cpp @@ -123,7 +123,7 @@ struct Set using Common::EnumMap; using Formats = EnumMap; -using Elements = EnumMap; +using Elements = EnumMap; using Indices = std::array; using Types = EnumMap; @@ -140,11 +140,11 @@ constexpr Types InitializeTable() table[VCF::Direct][false][NCC::N][FMT::UShort] = Normal_Direct(); table[VCF::Direct][false][NCC::N][FMT::Short] = Normal_Direct(); table[VCF::Direct][false][NCC::N][FMT::Float] = Normal_Direct(); - table[VCF::Direct][false][NCC::NBT][FMT::UByte] = Normal_Direct(); - table[VCF::Direct][false][NCC::NBT][FMT::Byte] = Normal_Direct(); - table[VCF::Direct][false][NCC::NBT][FMT::UShort] = Normal_Direct(); - table[VCF::Direct][false][NCC::NBT][FMT::Short] = Normal_Direct(); - table[VCF::Direct][false][NCC::NBT][FMT::Float] = Normal_Direct(); + table[VCF::Direct][false][NCC::NTB][FMT::UByte] = Normal_Direct(); + table[VCF::Direct][false][NCC::NTB][FMT::Byte] = Normal_Direct(); + table[VCF::Direct][false][NCC::NTB][FMT::UShort] = Normal_Direct(); + table[VCF::Direct][false][NCC::NTB][FMT::Short] = Normal_Direct(); + table[VCF::Direct][false][NCC::NTB][FMT::Float] = Normal_Direct(); // Same as above, since there are no indices table[VCF::Direct][true][NCC::N][FMT::UByte] = Normal_Direct(); @@ -152,57 +152,57 @@ constexpr Types InitializeTable() table[VCF::Direct][true][NCC::N][FMT::UShort] = Normal_Direct(); table[VCF::Direct][true][NCC::N][FMT::Short] = Normal_Direct(); table[VCF::Direct][true][NCC::N][FMT::Float] = Normal_Direct(); - table[VCF::Direct][true][NCC::NBT][FMT::UByte] = Normal_Direct(); - table[VCF::Direct][true][NCC::NBT][FMT::Byte] = Normal_Direct(); - table[VCF::Direct][true][NCC::NBT][FMT::UShort] = Normal_Direct(); - table[VCF::Direct][true][NCC::NBT][FMT::Short] = Normal_Direct(); - table[VCF::Direct][true][NCC::NBT][FMT::Float] = Normal_Direct(); + table[VCF::Direct][true][NCC::NTB][FMT::UByte] = Normal_Direct(); + table[VCF::Direct][true][NCC::NTB][FMT::Byte] = Normal_Direct(); + table[VCF::Direct][true][NCC::NTB][FMT::UShort] = Normal_Direct(); + table[VCF::Direct][true][NCC::NTB][FMT::Short] = Normal_Direct(); + table[VCF::Direct][true][NCC::NTB][FMT::Float] = Normal_Direct(); table[VCF::Index8][false][NCC::N][FMT::UByte] = Normal_Index(); table[VCF::Index8][false][NCC::N][FMT::Byte] = Normal_Index(); table[VCF::Index8][false][NCC::N][FMT::UShort] = Normal_Index(); table[VCF::Index8][false][NCC::N][FMT::Short] = Normal_Index(); table[VCF::Index8][false][NCC::N][FMT::Float] = Normal_Index(); - table[VCF::Index8][false][NCC::NBT][FMT::UByte] = Normal_Index(); - table[VCF::Index8][false][NCC::NBT][FMT::Byte] = Normal_Index(); - table[VCF::Index8][false][NCC::NBT][FMT::UShort] = Normal_Index(); - table[VCF::Index8][false][NCC::NBT][FMT::Short] = Normal_Index(); - table[VCF::Index8][false][NCC::NBT][FMT::Float] = Normal_Index(); + table[VCF::Index8][false][NCC::NTB][FMT::UByte] = Normal_Index(); + table[VCF::Index8][false][NCC::NTB][FMT::Byte] = Normal_Index(); + table[VCF::Index8][false][NCC::NTB][FMT::UShort] = Normal_Index(); + table[VCF::Index8][false][NCC::NTB][FMT::Short] = Normal_Index(); + table[VCF::Index8][false][NCC::NTB][FMT::Float] = Normal_Index(); - // Same for NormalComponentCount::N; differs for NBT + // Same for NormalComponentCount::N; differs for NTB table[VCF::Index8][true][NCC::N][FMT::UByte] = Normal_Index(); table[VCF::Index8][true][NCC::N][FMT::Byte] = Normal_Index(); table[VCF::Index8][true][NCC::N][FMT::UShort] = Normal_Index(); table[VCF::Index8][true][NCC::N][FMT::Short] = Normal_Index(); table[VCF::Index8][true][NCC::N][FMT::Float] = Normal_Index(); - table[VCF::Index8][true][NCC::NBT][FMT::UByte] = Normal_Index_Indices3(); - table[VCF::Index8][true][NCC::NBT][FMT::Byte] = Normal_Index_Indices3(); - table[VCF::Index8][true][NCC::NBT][FMT::UShort] = Normal_Index_Indices3(); - table[VCF::Index8][true][NCC::NBT][FMT::Short] = Normal_Index_Indices3(); - table[VCF::Index8][true][NCC::NBT][FMT::Float] = Normal_Index_Indices3(); + table[VCF::Index8][true][NCC::NTB][FMT::UByte] = Normal_Index_Indices3(); + table[VCF::Index8][true][NCC::NTB][FMT::Byte] = Normal_Index_Indices3(); + table[VCF::Index8][true][NCC::NTB][FMT::UShort] = Normal_Index_Indices3(); + table[VCF::Index8][true][NCC::NTB][FMT::Short] = Normal_Index_Indices3(); + table[VCF::Index8][true][NCC::NTB][FMT::Float] = Normal_Index_Indices3(); table[VCF::Index16][false][NCC::N][FMT::UByte] = Normal_Index(); table[VCF::Index16][false][NCC::N][FMT::Byte] = Normal_Index(); table[VCF::Index16][false][NCC::N][FMT::UShort] = Normal_Index(); table[VCF::Index16][false][NCC::N][FMT::Short] = Normal_Index(); table[VCF::Index16][false][NCC::N][FMT::Float] = Normal_Index(); - table[VCF::Index16][false][NCC::NBT][FMT::UByte] = Normal_Index(); - table[VCF::Index16][false][NCC::NBT][FMT::Byte] = Normal_Index(); - table[VCF::Index16][false][NCC::NBT][FMT::UShort] = Normal_Index(); - table[VCF::Index16][false][NCC::NBT][FMT::Short] = Normal_Index(); - table[VCF::Index16][false][NCC::NBT][FMT::Float] = Normal_Index(); + table[VCF::Index16][false][NCC::NTB][FMT::UByte] = Normal_Index(); + table[VCF::Index16][false][NCC::NTB][FMT::Byte] = Normal_Index(); + table[VCF::Index16][false][NCC::NTB][FMT::UShort] = Normal_Index(); + table[VCF::Index16][false][NCC::NTB][FMT::Short] = Normal_Index(); + table[VCF::Index16][false][NCC::NTB][FMT::Float] = Normal_Index(); - // Same for NormalComponentCount::N; differs for NBT + // Same for NormalComponentCount::N; differs for NTB table[VCF::Index16][true][NCC::N][FMT::UByte] = Normal_Index(); table[VCF::Index16][true][NCC::N][FMT::Byte] = Normal_Index(); table[VCF::Index16][true][NCC::N][FMT::UShort] = Normal_Index(); table[VCF::Index16][true][NCC::N][FMT::Short] = Normal_Index(); table[VCF::Index16][true][NCC::N][FMT::Float] = Normal_Index(); - table[VCF::Index16][true][NCC::NBT][FMT::UByte] = Normal_Index_Indices3(); - table[VCF::Index16][true][NCC::NBT][FMT::Byte] = Normal_Index_Indices3(); - table[VCF::Index16][true][NCC::NBT][FMT::UShort] = Normal_Index_Indices3(); - table[VCF::Index16][true][NCC::NBT][FMT::Short] = Normal_Index_Indices3(); - table[VCF::Index16][true][NCC::NBT][FMT::Float] = Normal_Index_Indices3(); + table[VCF::Index16][true][NCC::NTB][FMT::UByte] = Normal_Index_Indices3(); + table[VCF::Index16][true][NCC::NTB][FMT::Byte] = Normal_Index_Indices3(); + table[VCF::Index16][true][NCC::NTB][FMT::UShort] = Normal_Index_Indices3(); + table[VCF::Index16][true][NCC::NTB][FMT::Short] = Normal_Index_Indices3(); + table[VCF::Index16][true][NCC::NTB][FMT::Float] = Normal_Index_Indices3(); return table; } diff --git a/Source/Core/VideoCommon/XFMemory.h b/Source/Core/VideoCommon/XFMemory.h index 23c4088f05..aa8a28a608 100644 --- a/Source/Core/VideoCommon/XFMemory.h +++ b/Source/Core/VideoCommon/XFMemory.h @@ -44,13 +44,13 @@ struct fmt::formatter : EnumFormatter enum class NormalCount : u32 { None = 0, - Normals = 1, - NormalsBinormals = 2 + Normal = 1, + NormalTangentBinormal = 2 }; template <> -struct fmt::formatter : EnumFormatter +struct fmt::formatter : EnumFormatter { - constexpr formatter() : EnumFormatter({"None", "Normals only", "Normals and binormals"}) {} + constexpr formatter() : EnumFormatter({"None", "Normal only", "Normal, tangent, and binormal"}) {} }; // Texture generation type diff --git a/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp b/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp index e72fe28c29..b12dad1661 100644 --- a/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp +++ b/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp @@ -330,7 +330,7 @@ TEST_F(VertexLoaderTest, LargeFloatVertexSpeed) m_vtx_attr.g0.PosElements = CoordComponentCount::XYZ; m_vtx_attr.g0.PosFormat = ComponentFormat::Float; - m_vtx_attr.g0.NormalElements = NormalComponentCount::NBT; + m_vtx_attr.g0.NormalElements = NormalComponentCount::NTB; m_vtx_attr.g0.NormalFormat = ComponentFormat::Float; m_vtx_attr.g0.Color0Elements = ColorComponentCount::RGBA; m_vtx_attr.g0.Color0Comp = ColorFormat::RGBA8888; From 38a75f6a49706840edf7a6f5b94e224468ebbabd Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 17 May 2022 13:42:31 -0700 Subject: [PATCH 3/4] 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) From 8df55b492c52401f88393295479d6fbeba92ad9b Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 17 May 2022 13:58:54 -0700 Subject: [PATCH 4/4] Show a panic alert if the CP matrix indices don't match the XF matrix indices This almost certainly never happens, but if it does we want to know. --- Source/Core/Core/DolphinAnalytics.cpp | 3 ++- Source/Core/Core/DolphinAnalytics.h | 4 ++++ .../Core/VideoBackends/Software/SWVertexLoader.cpp | 14 -------------- Source/Core/VideoCommon/VertexLoaderManager.cpp | 12 ++++++++++++ 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/Source/Core/Core/DolphinAnalytics.cpp b/Source/Core/Core/DolphinAnalytics.cpp index ab5ea2c772..42eb7febe1 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", @@ -164,6 +164,7 @@ constexpr std::array GAME_QUIRKS_NAMES{ "mismatched-gpu-colors-between-cp-and-xf", "mismatched-gpu-normals-between-cp-and-xf", "mismatched-gpu-tex-coords-between-cp-and-xf", + "mismatched-gpu-matrix-indices-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 6e8acb8a8f..c708ad3a13 100644 --- a/Source/Core/Core/DolphinAnalytics.h +++ b/Source/Core/Core/DolphinAnalytics.h @@ -87,6 +87,10 @@ enum class GameQuirk MISMATCHED_GPU_COLORS_BETWEEN_CP_AND_XF, MISMATCHED_GPU_NORMALS_BETWEEN_CP_AND_XF, MISMATCHED_GPU_TEX_COORDS_BETWEEN_CP_AND_XF, + // Both CP and XF have normally-identical matrix index information. We currently always + // use the CP one in the hardware renderers and the XF one in the software renderer, + // but testing is needed to find out which of these is actually used for what. + MISMATCHED_GPU_MATRIX_INDICES_BETWEEN_CP_AND_XF, COUNT, }; diff --git a/Source/Core/VideoBackends/Software/SWVertexLoader.cpp b/Source/Core/VideoBackends/Software/SWVertexLoader.cpp index 4b9825132e..aaf91d9891 100644 --- a/Source/Core/VideoBackends/Software/SWVertexLoader.cpp +++ b/Source/Core/VideoBackends/Software/SWVertexLoader.cpp @@ -106,20 +106,6 @@ void SWVertexLoader::DrawCurrentBatch(u32 base_index, u32 num_indices, u32 base_ void SWVertexLoader::SetFormat() { - // matrix index from xf regs or cp memory? - if (xfmem.MatrixIndexA.PosNormalMtxIdx != g_main_cp_state.matrix_index_a.PosNormalMtxIdx || - xfmem.MatrixIndexA.Tex0MtxIdx != g_main_cp_state.matrix_index_a.Tex0MtxIdx || - xfmem.MatrixIndexA.Tex1MtxIdx != g_main_cp_state.matrix_index_a.Tex1MtxIdx || - xfmem.MatrixIndexA.Tex2MtxIdx != g_main_cp_state.matrix_index_a.Tex2MtxIdx || - xfmem.MatrixIndexA.Tex3MtxIdx != g_main_cp_state.matrix_index_a.Tex3MtxIdx || - xfmem.MatrixIndexB.Tex4MtxIdx != g_main_cp_state.matrix_index_b.Tex4MtxIdx || - xfmem.MatrixIndexB.Tex5MtxIdx != g_main_cp_state.matrix_index_b.Tex5MtxIdx || - xfmem.MatrixIndexB.Tex6MtxIdx != g_main_cp_state.matrix_index_b.Tex6MtxIdx || - xfmem.MatrixIndexB.Tex7MtxIdx != g_main_cp_state.matrix_index_b.Tex7MtxIdx) - { - ERROR_LOG_FMT(VIDEO, "Matrix indices don't match"); - } - m_vertex.posMtx = xfmem.MatrixIndexA.PosNormalMtxIdx; m_vertex.texMtx[0] = xfmem.MatrixIndexA.Tex0MtxIdx; m_vertex.texMtx[1] = xfmem.MatrixIndexA.Tex1MtxIdx; diff --git a/Source/Core/VideoCommon/VertexLoaderManager.cpp b/Source/Core/VideoCommon/VertexLoaderManager.cpp index eea5183a81..f3fc4d9e7e 100644 --- a/Source/Core/VideoCommon/VertexLoaderManager.cpp +++ b/Source/Core/VideoCommon/VertexLoaderManager.cpp @@ -321,6 +321,18 @@ static void CheckCPConfiguration(int vtx_attr_group) // Don't bail out, though; we can still render something successfully // (real hardware seems to hang in this case, though) } + + if (g_main_cp_state.matrix_index_a.Hex != xfmem.MatrixIndexA.Hex || + g_main_cp_state.matrix_index_b.Hex != xfmem.MatrixIndexB.Hex) + { + PanicAlertFmt("Mismatched matrix index configuration between CP and XF stages - " + "index A: {:08x}/{:08x}, index B {:08x}/{:08x}. " + "Please report on the issue tracker.", + g_main_cp_state.matrix_index_a.Hex, xfmem.MatrixIndexA.Hex, + g_main_cp_state.matrix_index_b.Hex, xfmem.MatrixIndexB.Hex); + DolphinAnalytics::Instance().ReportGameQuirk( + GameQuirk::MISMATCHED_GPU_MATRIX_INDICES_BETWEEN_CP_AND_XF); + } } int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, DataReader src,