From 537c46a42aecdacf00c2c2db449892c219104cfe Mon Sep 17 00:00:00 2001 From: Jan Krassnigg Date: Wed, 23 Mar 2022 15:45:09 +0100 Subject: [PATCH] Prevent nullptr access to normals-array in bitangent computation --- code/AssetLib/glTF2/glTF2Importer.cpp | 43 ++++++++++++++------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index e8b8562e6..293d3dea7 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -284,7 +284,7 @@ static aiMaterial *ImportMaterial(std::vector &embeddedTexIdxs, Asset &r, M aimat->AddProperty(&alphaMode, AI_MATKEY_GLTF_ALPHAMODE); aimat->AddProperty(&mat.alphaCutoff, 1, AI_MATKEY_GLTF_ALPHACUTOFF); - //pbrSpecularGlossiness + // pbrSpecularGlossiness if (mat.pbrSpecularGlossiness.isPresent) { PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value; @@ -606,7 +606,10 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { } } if (needTangents) { - if (target.tangent[0]->count != aim->mNumVertices) { + if (!aiAnimMesh.HasNormals()) { + // prevent nullptr access to aiAnimMesh.mNormals below when no normals are available + ASSIMP_LOG_WARN("Bitangents of target ", i, " in mesh \"", mesh.name, "\" can't be computed, because mesh has no normals."); + } else if (target.tangent[0]->count != aim->mNumVertices) { ASSIMP_LOG_WARN("Tangents of target ", i, " in mesh \"", mesh.name, "\" does not match the vertex count"); } else { Tangent *tangent = nullptr; @@ -698,12 +701,12 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { nFaces = count - 2; facePtr = faces = new aiFace[nFaces]; for (unsigned int i = 0; i < nFaces; ++i) { - //The ordering is to ensure that the triangles are all drawn with the same orientation + // The ordering is to ensure that the triangles are all drawn with the same orientation if ((i + 1) % 2 == 0) { - //For even n, vertices n + 1, n, and n + 2 define triangle n + // For even n, vertices n + 1, n, and n + 2 define triangle n SetFaceAndAdvance3(facePtr, aim->mNumVertices, data.GetUInt(i + 1), data.GetUInt(i), data.GetUInt(i + 2)); } else { - //For odd n, vertices n, n+1, and n+2 define triangle n + // For odd n, vertices n, n+1, and n+2 define triangle n SetFaceAndAdvance3(facePtr, aim->mNumVertices, data.GetUInt(i), data.GetUInt(i + 1), data.GetUInt(i + 2)); } } @@ -776,12 +779,12 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { nFaces = count - 2; facePtr = faces = new aiFace[nFaces]; for (unsigned int i = 0; i < nFaces; ++i) { - //The ordering is to ensure that the triangles are all drawn with the same orientation + // The ordering is to ensure that the triangles are all drawn with the same orientation if ((i + 1) % 2 == 0) { - //For even n, vertices n + 1, n, and n + 2 define triangle n + // For even n, vertices n + 1, n, and n + 2 define triangle n SetFaceAndAdvance3(facePtr, aim->mNumVertices, i + 1, i, i + 2); } else { - //For odd n, vertices n, n+1, and n+2 define triangle n + // For odd n, vertices n, n+1, and n+2 define triangle n SetFaceAndAdvance3(facePtr, aim->mNumVertices, i, i + 1, i + 2); } } @@ -904,14 +907,14 @@ void glTF2Importer::ImportLights(glTF2::Asset &r) { ail->mAttenuationLinear = 0.0; ail->mAttenuationQuadratic = 0.0; } else { - //in PBR attenuation is calculated using inverse square law which can be expressed - //using assimps equation: 1/(att0 + att1 * d + att2 * d*d) with the following parameters - //this is correct equation for the case when range (see - //https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual) - //is not present. When range is not present it is assumed that it is infinite and so numerator is 1. - //When range is present then numerator might be any value in range [0,1] and then assimps equation - //will not suffice. In this case range is added into metadata in ImportNode function - //and its up to implementation to read it when it wants to + // in PBR attenuation is calculated using inverse square law which can be expressed + // using assimps equation: 1/(att0 + att1 * d + att2 * d*d) with the following parameters + // this is correct equation for the case when range (see + // https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual) + // is not present. When range is not present it is assumed that it is infinite and so numerator is 1. + // When range is present then numerator might be any value in range [0,1] and then assimps equation + // will not suffice. In this case range is added into metadata in ImportNode function + // and its up to implementation to read it when it wants to ail->mAttenuationConstant = 0.0; ail->mAttenuationLinear = 0.0; ail->mAttenuationQuadratic = 1.0; @@ -1161,8 +1164,8 @@ aiNode *ImportNode(aiScene *pScene, glTF2::Asset &r, std::vector & if (node.light) { pScene->mLights[node.light.GetIndex()]->mName = ainode->mName; - //range is optional - see https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual - //it is added to meta data of parent node, because there is no other place to put it + // range is optional - see https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual + // it is added to meta data of parent node, because there is no other place to put it if (node.light->range.isPresent) { if (!ainode->mMetaData) { ainode->mMetaData = aiMetadata::Alloc(1); @@ -1556,9 +1559,9 @@ void glTF2Importer::ImportEmbeddedTextures(glTF2::Asset &r) { if (ext) { if (strcmp(ext, "jpeg") == 0) { ext = "jpg"; - } else if (strcmp(ext, "ktx2") == 0) { //basisu: ktx remains + } else if (strcmp(ext, "ktx2") == 0) { // basisu: ktx remains ext = "kx2"; - } else if (strcmp(ext, "basis") == 0) { //basisu + } else if (strcmp(ext, "basis") == 0) { // basisu ext = "bu"; }