From ba9e76910ef56f8ad1a9e8b236035863f7b7f1aa Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 28 Nov 2018 11:28:17 -0800 Subject: [PATCH] Fix up tangents in MeshAssimp to help with #528. We started to write our own implementation of mikktspace, but it required an unindexed mesh for input. This was awkward because assimp is already applying a pipeline of operations that includes tangent generation and indexing. It felt reduandant to add our own pipeline on top of that (unindexing => tangentgen => reindexing). Since assimp's CalcTangentsProcess method is already producing tangents that follow the orientation implied by texture coordinates, this simply needs to be tweaked to match glTF. (flip the bitangent) Also fixed a bug for the case where models have neither UV's nor tangents, where we were calling norm instead of normalize. :) --- samples/app/MeshAssimp.cpp | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/samples/app/MeshAssimp.cpp b/samples/app/MeshAssimp.cpp index 003d9ed56a0..ea1c26befd7 100644 --- a/samples/app/MeshAssimp.cpp +++ b/samples/app/MeshAssimp.cpp @@ -560,7 +560,7 @@ bool MeshAssimp::setFromFile(const Path& file, std::vector& outIndices float3 const* tangents = reinterpret_cast(mesh->mTangents); float3 const* bitangents = reinterpret_cast(mesh->mBitangents); float3 const* normals = reinterpret_cast(mesh->mNormals); - float3 const* texCoords = reinterpret_cast(mesh->mTextureCoords[0]); + float3 const* texCoords = reinterpret_cast(mesh->mTextureCoords[0]); const size_t numVertices = mesh->mNumVertices; @@ -573,29 +573,35 @@ bool MeshAssimp::setFromFile(const Path& file, std::vector& outIndices for (size_t j = 0; j < numVertices; j++) { float3 normal = normals[j]; - float3 texCoord = texCoords ? texCoords[j] : float3{0.0}; float3 tangent; float3 bitangent; - //If the tangent and bitangent don't exist, make arbitrary ones - // TODO: The glTF specification recommends using the MikkTSpace algorithm - // for computing tangent vectors in the absence of explicit tangents. + // Assimp always returns 3D tex coords but we only support 2D tex coords. + float2 texCoord = texCoords ? texCoords[j].xy : float2{0.0}; + + // If the tangent and bitangent don't exist, make arbitrary ones. This only + // occurs when the mesh is missing texture coordinates, because assimp + // computes tangents for us. (search up for aiProcess_CalcTangentSpace) if (!tangents) { - bitangent = norm(cross(normal, float3{1.0, 0.0, 0.0})); - tangent = norm(cross(normal, bitangent)); + bitangent = normalize(cross(normal, float3{1.0, 0.0, 0.0})); + tangent = normalize(cross(normal, bitangent)); } else { + // In assimp, the CalcTangentsProcess algorithm generates tangents in + // the +U direction and bitangents in the +V direction, but the glTF + // conformance suite (see NormalTangentTest) reveals that bitangents + // should be flipped. tangent = tangents[j]; - bitangent = bitangents[j]; + bitangent = -bitangents[j]; } quatf q = mat3f::packTangentFrame({tangent, bitangent, normal}); outTangents.push_back(packSnorm16(q.xyzw)); - outTexCoords.emplace_back(texCoord.xy); + outTexCoords.emplace_back(texCoord); outPositions.emplace_back(positions[j], 1.0_h); } - - // all faces should be triangles since we configure assimp to triangulate faces + // Populate the index buffer. All faces are triangles at this point because we + // asked assimp to perform triangulation. size_t indicesCount = numFaces * faces[0].mNumIndices; size_t indexBufferOffset = outIndices.size(); totalIndices += indicesCount; @@ -613,7 +619,6 @@ bool MeshAssimp::setFromFile(const Path& file, std::vector& outIndices aiString name; std::string materialName; - if (material->Get(AI_MATKEY_NAME, name) != AI_SUCCESS) { if (isGLTF) { while (outMaterials.find("_mat_" + std::to_string(matCount))