From 24b761acdadfb5613b0c5b79f161b621ffe87907 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 18 May 2022 18:03:10 -0700 Subject: [PATCH 1/3] Fifo recorder: Fix position's type being used for normals/colors/texture coords In most cases, games will use the same type for all vertex components (either Index8 or Index16 or Direct). However, RS2's deflection towers use Index16 for the texture coordinate and Index8 for everything else, meaning the texture coordinates were recorded incorrectly (the first byte was used, so only indices 0 and 1 were recorded instead of 0 through 0x0192). Worse still, some background elements in RS2 use direct positions but indexed normals or texture coordinates, and those would not be recorded at all. This is a regression from b5fd35f95145ecc8f88a179229ed69b390eb76be. --- Source/Core/Core/FifoPlayer/FifoRecorder.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp index 4464615495..f403090e2c 100644 --- a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp +++ b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp @@ -98,7 +98,7 @@ void FifoRecorder::FifoRecordAnalyzer::OnPrimitiveCommand(OpcodeDecoder::Primiti const u32 norm_size = VertexLoader_Normal::GetSize(vtx_desc.low.Normal, vtx_attr.g0.NormalFormat, vtx_attr.g0.NormalElements, vtx_attr.g0.NormalIndex3); - ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Position, offset, vertex_size, num_vertices, + ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset, vertex_size, num_vertices, vertex_data); offset += norm_size; @@ -106,7 +106,7 @@ 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.Position, offset, vertex_size, + ProcessVertexComponent(CPArray::Color0 + i, vtx_desc.low.Color[i], offset, vertex_size, num_vertices, vertex_data); offset += color_size; } @@ -114,7 +114,7 @@ void FifoRecorder::FifoRecordAnalyzer::OnPrimitiveCommand(OpcodeDecoder::Primiti { 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.low.Position, offset, vertex_size, + ProcessVertexComponent(CPArray::TexCoord0 + i, vtx_desc.high.TexCoord[i], offset, vertex_size, num_vertices, vertex_data); offset += tc_size; } From c9ff2a9b3d588a9c88a5c6ee64ecb9e2dd33c539 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 18 May 2022 19:40:18 -0700 Subject: [PATCH 2/3] Fifo recorder: Fix NormalIndex3 This should fix recording the wall in the staircase leading to the basement in Luigi's Mansion (though I haven't tested it, as I don't own a copy of Luigi's Mansion). This uses NormalIndex3, and the index for the normal vector (generally 0x02XX or 0x01XX) there is always lower than the tangent or binormal (generally 0x07XX). Other games seem to usually have a similar range of indices for the normal, tangent, and binormal, so this issue wouldn't affect them. --- Source/Core/Core/FifoPlayer/FifoRecorder.cpp | 43 ++++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp index f403090e2c..10d315fb45 100644 --- a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp +++ b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp @@ -53,7 +53,7 @@ public: private: void ProcessVertexComponent(CPArray array_index, VertexComponentFormat array_type, u32 component_offset, u32 vertex_size, u16 num_vertices, - const u8* vertex_data); + const u8* vertex_data, u32 byte_offset = 0); FifoRecorder* const m_owner; CPState m_cpmem; @@ -98,8 +98,35 @@ void FifoRecorder::FifoRecordAnalyzer::OnPrimitiveCommand(OpcodeDecoder::Primiti const u32 norm_size = VertexLoader_Normal::GetSize(vtx_desc.low.Normal, vtx_attr.g0.NormalFormat, vtx_attr.g0.NormalElements, vtx_attr.g0.NormalIndex3); - ProcessVertexComponent(CPArray::Normal, vtx_desc.low.Normal, offset, vertex_size, num_vertices, - vertex_data); + if (vtx_attr.g0.NormalIndex3 && IsIndexed(vtx_desc.low.Normal) && + vtx_attr.g0.NormalElements == NormalComponentCount::NTB) + { + // We're in 3-index mode, and we're using an indexed format and have the + // normal/tangent/binormal, so we actually need to deal with 3-index mode. + const u32 index_size = vtx_desc.low.Normal == VertexComponentFormat::Index16 ? 2 : 1; + ASSERT(norm_size == index_size * 3); + // 3-index mode uses one index each for the normal, tangent and binormal; + // the tangent and binormal are internally offset. + // The offset is based on the component size, not to the index itself; + // for instance, with 32-bit float normals, each normal vector is 3*sizeof(float) = 12 bytes, + // so the normal vector is offset by 0 bytes, the tangent by 12, and the binormal by 24. + // Using a byte offset instead of increasing the index means that using the same index for all + // elements is the same as not using the 3-index mode (increasing the index would give differing + // 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 + 2 * index_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); + } offset += norm_size; for (u32 i = 0; i < vtx_desc.low.Color.Size(); i++) @@ -123,11 +150,9 @@ 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) +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) { // Skip if not indexed array if (!IsIndexed(array_type)) @@ -167,7 +192,7 @@ void FifoRecorder::FifoRecordAnalyzer::ProcessVertexComponent(CPArray array_inde } } - const u32 array_start = m_cpmem.array_bases[array_index]; + 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); m_owner->UseMemory(array_start, array_size, MemoryUpdate::VERTEX_STREAM); From bac75de79c4511a6d3c46261b52829bd92c871e5 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 18 May 2022 21:10:46 -0700 Subject: [PATCH 3/3] 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); }