From bac75de79c4511a6d3c46261b52829bd92c871e5 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 18 May 2022 21:10:46 -0700 Subject: [PATCH] Fifo recorder: Fix incorrect calculation of the size of an array The old calculation was stride * (max_index + 1), which fails if stride is less than the size of a component (for instance, if float XYZ positions are used, and the stride was set to 4 (i.e. sizeof(float)) instead of 12 (i.e. 3 * sizeof(float)), it would be missing the last 8 bytes of the final element in the array. Or, if stride was set to 0, then no bytes would be recorded at all (though that's not a useful configuration so it's unlikely to actually exist). I'm not aware of any games affected by this issue. --- Source/Core/Core/FifoPlayer/FifoRecorder.cpp | 45 ++++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp index 10d315fb45..d22163f136 100644 --- a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp +++ b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp @@ -52,8 +52,8 @@ public: private: void ProcessVertexComponent(CPArray array_index, VertexComponentFormat array_type, - u32 component_offset, u32 vertex_size, u16 num_vertices, - const u8* vertex_data, u32 byte_offset = 0); + u32 component_offset, u32 component_size, u32 vertex_size, + u16 num_vertices, const u8* vertex_data, u32 byte_offset = 0); FifoRecorder* const m_owner; CPState m_cpmem; @@ -91,13 +91,18 @@ void FifoRecorder::FifoRecordAnalyzer::OnPrimitiveCommand(OpcodeDecoder::Primiti } const u32 pos_size = VertexLoader_Position::GetSize(vtx_desc.low.Position, vtx_attr.g0.PosFormat, vtx_attr.g0.PosElements); - ProcessVertexComponent(CPArray::Position, vtx_desc.low.Position, offset, vertex_size, - num_vertices, vertex_data); + const u32 pos_direct_size = VertexLoader_Position::GetSize( + VertexComponentFormat::Direct, vtx_attr.g0.PosFormat, vtx_attr.g0.PosElements); + ProcessVertexComponent(CPArray::Position, vtx_desc.low.Position, offset, pos_direct_size, + vertex_size, num_vertices, vertex_data); offset += pos_size; const u32 norm_size = VertexLoader_Normal::GetSize(vtx_desc.low.Normal, vtx_attr.g0.NormalFormat, vtx_attr.g0.NormalElements, vtx_attr.g0.NormalIndex3); + const u32 norm_direct_size = + VertexLoader_Normal::GetSize(VertexComponentFormat::Direct, vtx_attr.g0.NormalFormat, + vtx_attr.g0.NormalElements, vtx_attr.g0.NormalIndex3); if (vtx_attr.g0.NormalIndex3 && IsIndexed(vtx_desc.low.Normal) && vtx_attr.g0.NormalElements == NormalComponentCount::NTB) { @@ -115,17 +120,17 @@ void FifoRecorder::FifoRecordAnalyzer::OnPrimitiveCommand(OpcodeDecoder::Primiti // results if the normal array's stride was something other than 12, for instance if vertices // were contiguous in main memory instead of individual components being used). const u32 element_size = GetElementSize(vtx_attr.g0.NormalFormat) * 3; - ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset, vertex_size, num_vertices, - vertex_data); - ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset + index_size, vertex_size, - num_vertices, vertex_data, element_size); + ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset, element_size, vertex_size, + num_vertices, vertex_data); + ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset + index_size, element_size, + vertex_size, num_vertices, vertex_data, element_size); ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset + 2 * index_size, - vertex_size, num_vertices, vertex_data, 2 * element_size); + element_size, vertex_size, num_vertices, vertex_data, 2 * element_size); } else { - ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset, vertex_size, num_vertices, - vertex_data); + ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset, norm_direct_size, + vertex_size, num_vertices, vertex_data); } offset += norm_size; @@ -133,16 +138,20 @@ void FifoRecorder::FifoRecordAnalyzer::OnPrimitiveCommand(OpcodeDecoder::Primiti { const u32 color_size = VertexLoader_Color::GetSize(vtx_desc.low.Color[i], vtx_attr.GetColorFormat(i)); - ProcessVertexComponent(CPArray::Color0 + i, vtx_desc.low.Color[i], offset, vertex_size, - num_vertices, vertex_data); + const u32 color_direct_size = + VertexLoader_Color::GetSize(VertexComponentFormat::Direct, vtx_attr.GetColorFormat(i)); + ProcessVertexComponent(CPArray::Color0 + i, vtx_desc.low.Color[i], offset, color_direct_size, + vertex_size, num_vertices, vertex_data); offset += color_size; } for (u32 i = 0; i < vtx_desc.high.TexCoord.Size(); i++) { const u32 tc_size = VertexLoader_TextCoord::GetSize( vtx_desc.high.TexCoord[i], vtx_attr.GetTexFormat(i), vtx_attr.GetTexElements(i)); - ProcessVertexComponent(CPArray::TexCoord0 + i, vtx_desc.high.TexCoord[i], offset, vertex_size, - num_vertices, vertex_data); + const u32 tc_direct_size = VertexLoader_TextCoord::GetSize( + VertexComponentFormat::Direct, vtx_attr.GetTexFormat(i), vtx_attr.GetTexElements(i)); + ProcessVertexComponent(CPArray::TexCoord0 + i, vtx_desc.high.TexCoord[i], offset, + tc_direct_size, vertex_size, num_vertices, vertex_data); offset += tc_size; } @@ -151,8 +160,8 @@ void FifoRecorder::FifoRecordAnalyzer::OnPrimitiveCommand(OpcodeDecoder::Primiti // If a component is indexed, the array it indexes into for data must be saved. void FifoRecorder::FifoRecordAnalyzer::ProcessVertexComponent( - CPArray array_index, VertexComponentFormat array_type, u32 component_offset, u32 vertex_size, - u16 num_vertices, const u8* vertex_data, u32 byte_offset) + CPArray array_index, VertexComponentFormat array_type, u32 component_offset, u32 component_size, + u32 vertex_size, u16 num_vertices, const u8* vertex_data, u32 byte_offset) { // Skip if not indexed array if (!IsIndexed(array_type)) @@ -193,7 +202,7 @@ void FifoRecorder::FifoRecordAnalyzer::ProcessVertexComponent( } const u32 array_start = m_cpmem.array_bases[array_index] + byte_offset; - const u32 array_size = m_cpmem.array_strides[array_index] * (max_index + 1); + const u32 array_size = m_cpmem.array_strides[array_index] * max_index + component_size; m_owner->UseMemory(array_start, array_size, MemoryUpdate::VERTEX_STREAM); }