Conversation
This is a stub to make incremental implementation easier; the function interface may change a little in the future. The output will be 4 floats (tangent direction and winding) for each *corner* (index_count), and will be identical regardless of the input indexing.
Using position and texture coordinate data, we compute per-face tangent vector and orientation following the paper (eq 42/48). The rest of the algorithm will need to group and accumulate the resulting tangents per corner; for now we simply output "flat" tangents where the same value is stored in each of three corners. The output tangent vectors are normalized (except for degenerate triangles where the vector is zero).
Since the input vertices are not guaranteed to have any particular structure, including possibility of unindexed input or vertex splits due to non-tangent-related attributes, we need to deduplicate them. Since MikkTSpace relies on exact equality for this deduplication, we can use a hash map to achieve the same goal faster. For now we use a binary hash (Murmur) for the individual components and don't handle negative zero; this will be adjusted later.
Given the input triangles, we build per-vertex (post-remap) lists of triangle indices that contain this vertex; the triangle index is encoded with 2 low bits specifying corner id to make it easier to process later. Note that since the remap is sparse, we will have holes in the resulting adjacency table, which may be inefficient - fixing that would require tweaking the remap construction a little bit.
When two triangles share an edge and have matching orientations, the shared corners should have the same tangent group. We implement it by visiting all triangle pairs for each corner and checking if triangles are adjacent by extracting the remapped corner indices through the index/remap data. If two triangles match, we use union-find to merge the group ids; the resulting group id set is sparse for now, it may or may not be worth compacting it in the future.
For each corner, we now accumulate face tangents into group tangents based on the group association computed earlier. Each face tangent is reprojected onto tangent plane (which is consistent with the paper). Additionally, the accumulation uses angle weighting based on *projected* edge vectors. This is *not* described in the paper, and the use of projected angle is a little questionable but this is what the reference implementation does and unfortunately not doing that leads to different results. Finally, each group's tangent is renormalized and propagated to all corners where it's used. With this we finally complete the entire loop and generate tangents that mostly match the reference; what remains is performance tuning and rounding up edge cases.
acosf is quite slow and we need to call it for each corner, so it accounts for a noticeable percentage of the overall execution. Instead we can replace it with an approximation dating back to 1955: acos(x) can be approximated as sqrt(1-x) * polynomial for x>0, and for x<0 we can flip the sign and correct the result afterwards. For our needs, a 2-degree polynomial is sufficiently precise; the coefficients here were optimized via gradient descent to minimize error.
Also add a couple more since we have some mismatches vs reference that would be good to address.
Previously it was possible for a single group to contain triangles of opposing orientations if they were connected through a degenerate bridge. This would require a specific sequence of unification, where a degenerate triangle unifies with positive and negative. To avoid this, we now store a sign per-group and only allow merging if the signs of two groups are consistent; group unification updates the sign on the root so that this invariant is not violated. Also, for degen-only groups, we now use orientation -1 to match MikkTSpace reference better.
MikkT is an incorrect suffix because "T" refers to "tangent" and isn't really a formal algorithm name. The correct name is "MikkTSpace", but that seems odd as a function name suffix since "Tangents" is already spelled out. For now it seems cleaner to simply drop the suffix.
- Fix assertion for vertex UVs to permit stride=8 bytes - Use 1/0/0/-1 tangent for degenerate groups for consistency with MikkTSpace - Cast group index to size_t before addressing result[] to avoid overflow
Instead of a generic MurmurHash on the binary data we now use a variant of spatial hashing that we use elsewhere. To ensure correctness we need to normalize negative zero in any input component. For simplicity, we mix in normal components into positional components and handle UV separately via a simplified reduction (UV are rarely negative so masking sign off is a little cheaper, and they are rarely equal so folding them into a single extra component is sufficient).
For now we simply run the generation code without trying to analyze the splits or rebuild the full vertex buffer.
Also remove redundant MESHOPTIMIZER_EXPERIMENTAL from function definition.
- No need to check if det=0 again; rw is already zero - Add an assertion during adjacency builds - Edit and clarify some comments
This is similar to the adjacency structure we use in simplifier: instead of tracking counts[] explicitly, we can allocate an extra offset and use offsets[] as a range during adjacency lookups. This slightly reduces memory used by the adjacency data and slightly simplifies the build process. The previous implementation was copied from vcacheoptimizer where we still use counts[] because the structure is dynamic - the adjacency lists are shrunk as the algorithm progresses. Here, once we build the structure it never changes.
|
Exactly 420 lines in the implementation! Should have shipped this on April 20th :) |
Instead of just leaving the per-corner tangents as is, we now implement splitting for the existing index buffer; each vertex may be split if the associated tangents diverge in different corners. The splits are tracked through a linked-list-in-array; usually there's zero or 1 split per vertex so this is efficient. Another way to implement this involved deindexing & reindexing but that is much simpler and can perhaps go into the documentation instead.
We need this to support non-unsigned int index inputs, and it will be useful once we add options to generateTangents so that they can default to 0...
Owner
Author
|
Going to merge this as is for now but there will be a second PR next week with some significant improvements beyond MikkTSpace. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This change implements
meshopt_generateTangents, which generates per-corner tangent frames (tangent vector and orientation) following MikkTSpace algorithm.Motivation
mikktspace.cis an industry standard, but is often a bottleneck during import as it's not written for efficiency. It also has an interface that is too easy to get wrong (more than half of the engine integrations I have reviewed do not use the library correctly), a few implementation defects and some inconsistent promises about the generated data. It's also 1900 lines of code. Because engine processing pipelines typically usemeshoptimizerfor one reason or another, it would be great if they can switch to using meshoptimizer tangent generation code, and get compatible results at lower cost.Usage
The generated frames are per corner: three frames are generated for each triangle. The function supports both indexed and un-indexed inputs (similar to
generateVertexRemapet al); the generated results do not depend on the indexing, but the indexed inputs are processed faster.If you start from an unindexed input, the results of this function can be treated as another unindexed stream, and a reindexing pass will produce final vertex/index buffer.
If you start from an indexed input, it's recommended to duplicate vertices on the fly based on tangent information (see
tangentsexample indemo/main.cpp). While you can omit the duplication step and simply copy tangents into existing vertex data, note that MikkTSpace algorithm splits tangents on UV mirror seams (edges where UV mapping changes winding) - such seams may not have duplicated vertices in the original indexed topology.Compared to the reference implementation, this implementation is ~5-7x faster on unindexed inputs, ~10-11x faster on indexed inputs, and in most cases produces visually identical outputs.
Internals
The implementation follows the MikkTSpace paper faithfully, and only follows the reference implementation (
mikktspace.c) loosely. It is not based on the reference implementation. It is not guaranteed to produce the same results; however, short of floating point numerics and a couple rare edge cases, it should produce the same results on manifold inputs without degenerate triangles in UV space (triangles that map to a line/point).MikkTSpace reference implementation has inconsistencies in handling of non-manifold data (it arbitrarily groups triangles into pairs, which violates order independence) and in handling of degenerate triangles (here any algorithm will have to make arbitrary choices; the choices this implementation makes may be different). In either case it should be rare that the difference materially affects the rendered result, and in these cases you'd likely see MikkTSpace generated data flip between different mesh representations anyway.
Compared to the paper, this implementation incorporates the corner weighting by projected angle; this choice of weighting is unexplained and arbitrary, but is necessary to have parity with reference implementation on well-formed meshes. This is the only significant gap in the paper on normal meshes.
Compared to the reference implementation, this implementation uses a different strategy of unifying UV-degenerate triangles with neighbors, and a different strategy of identifying adjacent triangles for non-manifold inputs. The latter uses an actual order-independent approach; the former is inherently input dependent.
The reference implementation has a few defects that this implementation fixes; neither is a problem here:
Future work
The current implementation has a few known defects, also present in the reference implementation:
These can cause visual artifacts depending on the data representation, but they are difficult to fix in a rigorous manner. In first two cases the problems are specific to UV-degenerate triangles; the third case is dictated by reference implementation if the goal is to match it as closely as possible.
While MikkTSpace can generate both tangent & bitangent vectors separately, almost every usage in the industry uses the recommended cross product based reconstruction; hence this PR only supports that for now. It also expects the caller to figure out how to encode the tangent vectors. Additional functions could be provided to cover either use case in the future as part of the same
.cppfile.This contribution is sponsored by Valve.