Fix glTF coordinate conversion not converting tangents #20573
Merged
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.
Suggestion cannot be applied right now. Please check back later.
Objective
Fix glTF coordinate conversion not converting tangents.
Report: https://discord.com/channels/691052431525675048/692572690833473578/1405362252617355335
Thread: https://discord.com/channels/691052431525675048/1405451520836898848
Solution
Fixed by removing a redundant copy of the tangent attributes.
In #5370 attribute copying was moved to a more generic function (see
convert_attribute). But one code path was left behind, so the tangents are actually copied twice. This is technically a bug, but a harmless one since both copies are the same.Later on, #19633 added the option to convert attribute coordinates. But only the first tangent copy (in
convert_attribute) applies the conversion - the second copy will overwrite them with unconverted values. This PR removes the second copy.There's also some minor code quality tweaks - prefer
contains_attribute()toattribute().is_some(), and fixed some bad indentation in log macros. I can revert these if we want to play it safe.Testing
Tested a modified version of example
load_gltfwith and without featuregltf_convert_coordinates_default. Also tested after hacking the loader to use the code path where tangents are recalculated.