Enhance checks and user experience around tangent arrays in meshes. #84252
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
akien-mga
reviewed
Oct 31, 2023
akien-mga
reviewed
Oct 31, 2023
akien-mga
reviewed
Oct 31, 2023
clayjohn
force-pushed
the
ensure_tangents
branch
from
8cc46dc to
422e252
Compare
akien-mga
approved these changes
Nov 1, 2023
Ensure `ensure_tangents` option actually creates tangent array. Even if it is just a dummy array. Allow mesh to generate its own tangents when using compression. This allows users to compress meshes without tangents. Warn users if they are trying to read from tangents without providing tangents.
clayjohn
force-pushed
the
ensure_tangents
branch
from
422e252 to
d1043a5
Compare
AThousandShips
approved these changes
Nov 1, 2023
|
Thanks! |
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.
Suggestion cannot be applied right now. Please check back later.
Fixes: #83236
Fixes: #83850
In both linked MRPs the user was attempting to read tangents in a shader from a mesh with no tangents. The behaviour in 4.1 was for Godot to supply a dummy tangent (1, 0, 0). That is no longer possible without a performance penalty (but it is possible see here).
Since it was never supported behaviour to read from non-existent tangents (see the strong warning in the docs for normal_texture) and the result of relying would typically look horrible. We decided not to bring back the previous behaviour and instead try to make the usage of tangents more intuitive and user-friendly.
We do that with three things:
ensure_tangentsoption actually creates tangent array. Even if it is just a dummy array.Previous, we gave up on generating a tangent array if there were no UVs in the mesh.
This ensures that most imported meshes have tangent arrays. It is how the "ensure tangents" option was intended to work and it aligns with the name. Users who don't want tangents can uncheck the option with no harm to their mesh. This is much preferable to the old behaviour as the tangent used will now actually be tangential to the normal.
Even if no tangent array is used, we need tangents to compress the mesh and we have good performance savings when using mesh compression. So this is a fallback to ensure that meshes can be compressed, even if just using normals
This is the needed for cases like #83236 where the user is generating their own mesh and is relying on normal mapping, but has forgotten to create tangents. This warning warns the user while pairing the material to a mesh (which only happens once).
Taken together, these 3 things should ensure that most users can carry on without any changes. Some users will run into the warning and have to generate tangents. But the end result should be much better for everyone.