From a9c1dcf6565517b75ec33e935931249afdb3427c Mon Sep 17 00:00:00 2001 From: Stenzek Date: Wed, 1 Apr 2020 13:16:07 +1000 Subject: [PATCH 1/2] VertexManagerBase: Skip drawing objects with mismatched xf/bp stages Hardware tests have shown that if the number of texgens/channels do not match, you get garbage rendering. Presumably because the output registers from the XF stage are fed into the incorrect input registers for TEV/BP. Currently, this causes Dolphin to crash/generate invalid shaders with an assertion failure in the hardware backends. Instead, we log an error. Perhaps in the future we should just spit out all texgens/colors anyway from both stages, and let cross-stage optimization take care of DCE'ing it away. But doing so would require changing the UIDs and invalidating everyone's shader caches. --- Source/Core/VideoCommon/VertexManagerBase.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Source/Core/VideoCommon/VertexManagerBase.cpp b/Source/Core/VideoCommon/VertexManagerBase.cpp index 7fcd4935c0..56f9f68b26 100644 --- a/Source/Core/VideoCommon/VertexManagerBase.cpp +++ b/Source/Core/VideoCommon/VertexManagerBase.cpp @@ -344,6 +344,17 @@ void VertexManagerBase::Flush() m_is_flushed = true; + if (xfmem.numTexGen.numTexGens != bpmem.genMode.numtexgens || + xfmem.numChan.numColorChans != bpmem.genMode.numcolchans) + { + ERROR_LOG(VIDEO, + "Mismatched configuration between XF and BP stages - %u/%u texgens, %u/%u colors. " + "Skipping draw. Please report on the issue tracker.", + xfmem.numTexGen.numTexGens, bpmem.genMode.numtexgens.Value(), + xfmem.numChan.numColorChans, bpmem.genMode.numcolchans.Value()); + return; + } + #if defined(_DEBUG) || defined(DEBUGFAST) PRIM_LOG("frame%d:\n texgen=%u, numchan=%u, dualtex=%u, ztex=%u, cole=%u, alpe=%u, ze=%u", g_ActiveConfig.iSaveTargetId, xfmem.numTexGen.numTexGens, xfmem.numChan.numColorChans, From ff7180cac422cf20c174031d7d41c05961234828 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Wed, 1 Apr 2020 17:32:18 +1000 Subject: [PATCH 2/2] Analytics: Add quirk for mismatched xf/bp texgens/colors --- Source/Core/Core/Analytics.cpp | 24 +++++++++---------- Source/Core/Core/Analytics.h | 7 ++++++ Source/Core/VideoCommon/VertexManagerBase.cpp | 15 ++++++++++++ 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/Source/Core/Core/Analytics.cpp b/Source/Core/Core/Analytics.cpp index 6b28eeb153..af289b4ff5 100644 --- a/Source/Core/Core/Analytics.cpp +++ b/Source/Core/Core/Analytics.cpp @@ -132,18 +132,18 @@ void DolphinAnalytics::ReportGameStart() } // Keep in sync with enum class GameQuirk definition. -constexpr std::array GAME_QUIRKS_NAMES{ - "icache-matters", - "directly-reads-wiimote-input", - "uses-DVDLowStopLaser", - "uses-DVDLowOffset", - "uses-DVDLowReadDiskBca", - "uses-DVDLowRequestDiscStatus", - "uses-DVDLowRequestRetryNumber", - "uses-DVDLowSerMeasControl", - "uses-different-partition-command", - "uses-di-interrupt-command", -}; +constexpr std::array GAME_QUIRKS_NAMES{"icache-matters", + "directly-reads-wiimote-input", + "uses-DVDLowStopLaser", + "uses-DVDLowOffset", + "uses-DVDLowReadDiskBca", + "uses-DVDLowRequestDiscStatus", + "uses-DVDLowRequestRetryNumber", + "uses-DVDLowSerMeasControl", + "uses-different-partition-command", + "uses-di-interrupt-command", + "mismatched-gpu-texgens-between-xf-and-bp", + "mismatched-gpu-colors-between-xf-and-bp"}; 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/Analytics.h b/Source/Core/Core/Analytics.h index f78cafbac6..94f7eb5a78 100644 --- a/Source/Core/Core/Analytics.h +++ b/Source/Core/Core/Analytics.h @@ -47,6 +47,13 @@ enum class GameQuirk // (DVDLowClearCoverInterrupt is used, though) USES_DI_INTERRUPT_MASK_COMMAND, + // Some games configure a mismatched number of texture coordinates or colors between the transform + // and TEV/BP stages of the rendering pipeline. Currently, Dolphin just skips over these objects + // as the hardware renderers are not equipped to handle the case where the registers between + // stages are mismatched. + MISMATCHED_GPU_TEXGENS_BETWEEN_XF_AND_BP, + MISMATCHED_GPU_COLORS_BETWEEN_XF_AND_BP, + COUNT, }; diff --git a/Source/Core/VideoCommon/VertexManagerBase.cpp b/Source/Core/VideoCommon/VertexManagerBase.cpp index 56f9f68b26..4fb4cbeb41 100644 --- a/Source/Core/VideoCommon/VertexManagerBase.cpp +++ b/Source/Core/VideoCommon/VertexManagerBase.cpp @@ -14,6 +14,7 @@ #include "Common/Logging/Log.h" #include "Common/MathUtil.h" +#include "Core/Analytics.h" #include "Core/ConfigManager.h" #include "VideoCommon/BPMemory.h" @@ -352,6 +353,20 @@ void VertexManagerBase::Flush() "Skipping draw. Please report on the issue tracker.", xfmem.numTexGen.numTexGens, bpmem.genMode.numtexgens.Value(), xfmem.numChan.numColorChans, bpmem.genMode.numcolchans.Value()); + + // 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 (xfmem.numTexGen.numTexGens != bpmem.genMode.numtexgens) + { + DolphinAnalytics::Instance().ReportGameQuirk( + GameQuirk::MISMATCHED_GPU_TEXGENS_BETWEEN_XF_AND_BP); + } + if (xfmem.numChan.numColorChans != bpmem.genMode.numcolchans) + { + DolphinAnalytics::Instance().ReportGameQuirk( + GameQuirk::MISMATCHED_GPU_TEXGENS_BETWEEN_XF_AND_BP); + } + return; }