Fifo recorder: Fix various indexed vertex component bugs #10676
Merged
+51
−17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pokechu22
commented
May 19, 2022
| // 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). |
There was a problem hiding this comment.
Pokechu22
force-pushed
the
fifo-recorder-indices
branch
from
9158cfe
to
b1ddc39
Compare
Pokechu22
force-pushed
the
fifo-recorder-indices
branch
2 times, most recently
from
6be3264
to
3fde178
Compare
Pokechu22
force-pushed
the
fifo-recorder-indices
branch
from
3fde178
to
309518d
Compare
Pokechu22
force-pushed
the
fifo-recorder-indices
branch
2 times, most recently
from
4755a25
to
b6dc6c8
Compare
|
JMC found that this PR breaks recording textures in Lego Indiana Jones (example); I've found that it also breaks Lego Star Wars: The Complete Saga, and bisected it to |
…ure 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 b5fd35f.
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.
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.
Pokechu22
force-pushed
the
fifo-recorder-indices
branch
from
b6dc6c8
to
bac75de
Compare
|
Fixed. It was a typo: const u32 pos_size = VertexLoader_Position::GetSize(vtx_desc.low.Position, vtx_attr.g0.PosFormat,
vtx_attr.g0.PosElements);
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, pos_direct_size, offset,
+ ProcessVertexComponent(CPArray::Position, vtx_desc.low.Position, offset, pos_direct_size,
vertex_size, num_vertices, vertex_data);
offset += pos_size; |
|
Fixes recording fifologs in Lego Indiana Jones wii |
JMC47
approved these changes
Oct 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
See the individual commit messages for details.
These fifologs are for the first commit (a regression from #9718) (images: broken, fixed). JMC provided some luigi's mansion fifologs; here's images: broken, fixed.