From 7353d25c139e97f19ce0de433f883a0bd7807d76 Mon Sep 17 00:00:00 2001 From: Haik Lorenz Date: Mon, 9 Oct 2017 15:47:17 +0200 Subject: [PATCH 1/9] Prevent failing stringstream to crash the export process Text exporters are using string streams to hold the file content first and then write them to the file in a single pass. If for whatever reason the stream has the fail bit set, tellp() will return pos_type(-1), which in turn makes the subsequent write crash - at least on Windows systems. One reason for the stream being in fail state is when its size exceeds 2^31 bytes, even on 64-bit systems (i.e., when very large scenes get exported). The fix is checking the fail() before even opening the file. --- code/ColladaExporter.cpp | 4 ++++ code/ObjExporter.cpp | 4 ++++ code/PlyExporter.cpp | 4 ++++ code/STLExporter.cpp | 8 ++++++++ code/XFileExporter.cpp | 4 ++++ 5 files changed, 24 insertions(+) diff --git a/code/ColladaExporter.cpp b/code/ColladaExporter.cpp index 1bac8b7f6..8f2e4047b 100644 --- a/code/ColladaExporter.cpp +++ b/code/ColladaExporter.cpp @@ -72,6 +72,10 @@ void ExportSceneCollada(const char* pFile, IOSystem* pIOSystem, const aiScene* p // invoke the exporter ColladaExporter iDoTheExportThing( pScene, pIOSystem, path, file); + + if (iDoTheExportThing.mOutput.fail()) { + throw DeadlyExportError("output data creation failed. Most likely the file became too large: " + std::string(pFile)); + } // we're still here - export successfully completed. Write result to the given IOSYstem std::unique_ptr outfile (pIOSystem->Open(pFile,"wt")); diff --git a/code/ObjExporter.cpp b/code/ObjExporter.cpp index d10dfcd45..ea3e19cd9 100644 --- a/code/ObjExporter.cpp +++ b/code/ObjExporter.cpp @@ -61,6 +61,10 @@ void ExportSceneObj(const char* pFile,IOSystem* pIOSystem, const aiScene* pScene // invoke the exporter ObjExporter exporter(pFile, pScene); + if (exporter.mOutput.fail() || exporter.mOutputMat.fail()) { + throw DeadlyExportError("output data creation failed. Most likely the file became too large: " + std::string(pFile)); + } + // we're still here - export successfully completed. Write both the main OBJ file and the material script { std::unique_ptr outfile (pIOSystem->Open(pFile,"wt")); diff --git a/code/PlyExporter.cpp b/code/PlyExporter.cpp index 1d14c9219..bd3c94830 100644 --- a/code/PlyExporter.cpp +++ b/code/PlyExporter.cpp @@ -69,6 +69,10 @@ void ExportScenePly(const char* pFile,IOSystem* pIOSystem, const aiScene* pScene // invoke the exporter PlyExporter exporter(pFile, pScene); + if (exporter.mOutput.fail()) { + throw DeadlyExportError("output data creation failed. Most likely the file became too large: " + std::string(pFile)); + } + // we're still here - export successfully completed. Write the file. std::unique_ptr outfile (pIOSystem->Open(pFile,"wt")); if(outfile == NULL) { diff --git a/code/STLExporter.cpp b/code/STLExporter.cpp index 3905cbcaf..c9d554655 100644 --- a/code/STLExporter.cpp +++ b/code/STLExporter.cpp @@ -61,6 +61,10 @@ void ExportSceneSTL(const char* pFile,IOSystem* pIOSystem, const aiScene* pScene // invoke the exporter STLExporter exporter(pFile, pScene); + if (exporter.mOutput.fail()) { + throw DeadlyExportError("output data creation failed. Most likely the file became too large: " + std::string(pFile)); + } + // we're still here - export successfully completed. Write the file. std::unique_ptr outfile (pIOSystem->Open(pFile,"wt")); if(outfile == NULL) { @@ -74,6 +78,10 @@ void ExportSceneSTLBinary(const char* pFile,IOSystem* pIOSystem, const aiScene* // invoke the exporter STLExporter exporter(pFile, pScene, true); + if (exporter.mOutput.fail()) { + throw DeadlyExportError("output data creation failed. Most likely the file became too large: " + std::string(pFile)); + } + // we're still here - export successfully completed. Write the file. std::unique_ptr outfile (pIOSystem->Open(pFile,"wb")); if(outfile == NULL) { diff --git a/code/XFileExporter.cpp b/code/XFileExporter.cpp index 30a0a21f8..1e77c8b81 100644 --- a/code/XFileExporter.cpp +++ b/code/XFileExporter.cpp @@ -78,6 +78,10 @@ void ExportSceneXFile(const char* pFile,IOSystem* pIOSystem, const aiScene* pSce // invoke the exporter XFileExporter iDoTheExportThing( pScene, pIOSystem, path, file, &props); + if (iDoTheExportThing.mOutput.fail()) { + throw DeadlyExportError("output data creation failed. Most likely the file became too large: " + std::string(pFile)); + } + // we're still here - export successfully completed. Write result to the given IOSYstem std::unique_ptr outfile (pIOSystem->Open(pFile,"wt")); if(outfile == NULL) { From b2eb599176dfa5e98285e992422515455ccf137a Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 16 Oct 2017 18:51:25 +0200 Subject: [PATCH 2/9] Update ColladaExporter.cpp Retrigger travis. --- code/ColladaExporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/ColladaExporter.cpp b/code/ColladaExporter.cpp index 691ef9500..53ee07388 100644 --- a/code/ColladaExporter.cpp +++ b/code/ColladaExporter.cpp @@ -108,7 +108,7 @@ ColladaExporter::ColladaExporter( const aiScene* pScene, IOSystem* pIOSystem, co // set up strings endstr = "\n"; - // start writing + // start writing the file WriteFile(); } From c71790c78da53d3b5c5e9ef6bd25b0c2aff59b98 Mon Sep 17 00:00:00 2001 From: Daniel Hritzkiv Date: Thu, 19 Oct 2017 12:01:40 -0400 Subject: [PATCH 3/9] Diffuse color and diffuse texture import and export improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These changes do a better of importing and exporting baseColor colors and textures, as well as diffuse colors and textures (in the case of pbrSpecularGlossiness) - baseColorFactor will be stored on both `$clr.diffuse` and `$mat.gltf.pbrMetallicRoughness.baseColorFactor`, and will be extracted from `$mat.gltf.pbrMetallicRoughness.baseColorFactor` first, and falling back to `$clr.diffuse`. The behaviour for baseColorTexture is similar - pbrSG’s diffuseFactor will now only be store on `$clr.diffuse` (overwriting any previous assignments to `$clr.diffuse`, e.g. from metallicRoughness’ baseColorFactor, as diffuseFactor is more analogous to diffuse color than baseColor), and will only extract from `$clr.diffuse` --- code/glTF2Asset.h | 4 ++-- code/glTF2Exporter.cpp | 35 +++++++++++++++++++++++++++-------- code/glTF2Exporter.h | 4 ++-- code/glTF2Importer.cpp | 8 ++++++-- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/code/glTF2Asset.h b/code/glTF2Asset.h index 63282dc6e..84211aeb6 100644 --- a/code/glTF2Asset.h +++ b/code/glTF2Asset.h @@ -164,16 +164,16 @@ namespace glTF2 //! Magic number for GLB files #define AI_GLB_MAGIC_NUMBER "glTF" + #define AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_BASE_COLOR_FACTOR "$mat.gltf.pbrMetallicRoughness.baseColorFactor", 0, 0 #define AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLIC_FACTOR "$mat.gltf.pbrMetallicRoughness.metallicFactor", 0, 0 #define AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_ROUGHNESS_FACTOR "$mat.gltf.pbrMetallicRoughness.roughnessFactor", 0, 0 + #define AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_BASE_COLOR_TEXTURE aiTextureType_DIFFUSE, 1 #define AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLICROUGHNESS_TEXTURE aiTextureType_UNKNOWN, 0 #define AI_MATKEY_GLTF_ALPHAMODE "$mat.gltf.alphaMode", 0, 0 #define AI_MATKEY_GLTF_ALPHACUTOFF "$mat.gltf.alphaCutoff", 0, 0 #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS "$mat.gltf.pbrSpecularGlossiness", 0, 0 - #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_DIFFUSE_FACTOR "$clr.diffuse", 0, 1 #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULAR_FACTOR "$clr.specular", 0, 1 #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR "$mat.gltf.pbrMetallicRoughness.glossinessFactor", 0, 0 - #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_DIFFUSE_TEXTURE aiTextureType_DIFFUSE, 1 #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULARGLOSSINESS_TEXTURE aiTextureType_UNKNOWN, 1 #define _AI_MATKEY_GLTF_TEXTURE_TEXCOORD_BASE "$tex.file.texCoord" diff --git a/code/glTF2Exporter.cpp b/code/glTF2Exporter.cpp index 8f46f7dc7..26ff77913 100644 --- a/code/glTF2Exporter.cpp +++ b/code/glTF2Exporter.cpp @@ -372,20 +372,28 @@ void glTF2Exporter::GetMatTex(const aiMaterial* mat, OcclusionTextureInfo& prop, } } -void glTF2Exporter::GetMatColor(const aiMaterial* mat, vec4& prop, const char* propName, int type, int idx) +aiReturn glTF2Exporter::GetMatColor(const aiMaterial* mat, vec4& prop, const char* propName, int type, int idx) { aiColor4D col; - if (mat->Get(propName, type, idx, col) == AI_SUCCESS) { + aiReturn result = mat->Get(propName, type, idx, col); + + if (result == AI_SUCCESS) { prop[0] = col.r; prop[1] = col.g; prop[2] = col.b; prop[3] = col.a; } + + return result; } -void glTF2Exporter::GetMatColor(const aiMaterial* mat, vec3& prop, const char* propName, int type, int idx) +aiReturn glTF2Exporter::GetMatColor(const aiMaterial* mat, vec3& prop, const char* propName, int type, int idx) { aiColor3D col; - if (mat->Get(propName, type, idx, col) == AI_SUCCESS) { + aiReturn result = mat->Get(propName, type, idx, col); + + if (result == AI_SUCCESS) { prop[0] = col.r; prop[1] = col.g; prop[2] = col.b; } + + return result; } void glTF2Exporter::ExportMaterials() @@ -406,9 +414,20 @@ void glTF2Exporter::ExportMaterials() m->name = name; - GetMatTex(mat, m->pbrMetallicRoughness.baseColorTexture, aiTextureType_DIFFUSE); + GetMatTex(mat, m->pbrMetallicRoughness.baseColorTexture, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_BASE_COLOR_TEXTURE); + + if (!m->pbrMetallicRoughness.baseColorTexture.texture) { + //if there wasn't a baseColorTexture defined in the source, fallback to any diffuse texture + GetMatTex(mat, m->pbrMetallicRoughness.baseColorTexture, aiTextureType_DIFFUSE); + } + GetMatTex(mat, m->pbrMetallicRoughness.metallicRoughnessTexture, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLICROUGHNESS_TEXTURE); - GetMatColor(mat, m->pbrMetallicRoughness.baseColorFactor, AI_MATKEY_COLOR_DIFFUSE); + + if (GetMatColor(mat, m->pbrMetallicRoughness.baseColorFactor, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_BASE_COLOR_FACTOR) != AI_SUCCESS) { + // if baseColorFactor wasn't defined, then the source is likely not a metallic roughness material. + //a fallback to any diffuse color should be used instead + GetMatColor(mat, m->pbrMetallicRoughness.baseColorFactor, AI_MATKEY_COLOR_DIFFUSE); + } if (mat->Get(AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLIC_FACTOR, m->pbrMetallicRoughness.metallicFactor) != AI_SUCCESS) { //if metallicFactor wasn't defined, then the source is likely not a PBR file, and the metallicFactor should be 0 @@ -451,11 +470,11 @@ void glTF2Exporter::ExportMaterials() PbrSpecularGlossiness pbrSG; - GetMatColor(mat, pbrSG.diffuseFactor, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_DIFFUSE_FACTOR); GetMatColor(mat, pbrSG.specularFactor, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULAR_FACTOR); mat->Get(AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR, pbrSG.glossinessFactor); - GetMatTex(mat, pbrSG.diffuseTexture, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_DIFFUSE_TEXTURE); GetMatTex(mat, pbrSG.specularGlossinessTexture, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULARGLOSSINESS_TEXTURE); + GetMatColor(mat, pbrSG.diffuseFactor, AI_MATKEY_COLOR_DIFFUSE); + GetMatTex(mat, pbrSG.diffuseTexture, aiTextureType_DIFFUSE); m->pbrSpecularGlossiness = Nullable(pbrSG); } diff --git a/code/glTF2Exporter.h b/code/glTF2Exporter.h index 3aed35ae6..e9f7c113a 100644 --- a/code/glTF2Exporter.h +++ b/code/glTF2Exporter.h @@ -115,8 +115,8 @@ namespace Assimp void GetMatTex(const aiMaterial* mat, glTF2::TextureInfo& prop, aiTextureType tt, unsigned int slot); void GetMatTex(const aiMaterial* mat, glTF2::NormalTextureInfo& prop, aiTextureType tt, unsigned int slot); void GetMatTex(const aiMaterial* mat, glTF2::OcclusionTextureInfo& prop, aiTextureType tt, unsigned int slot); - void GetMatColor(const aiMaterial* mat, glTF2::vec4& prop, const char* propName, int type, int idx); - void GetMatColor(const aiMaterial* mat, glTF2::vec3& prop, const char* propName, int type, int idx); + aiReturn GetMatColor(const aiMaterial* mat, glTF2::vec4& prop, const char* propName, int type, int idx); + aiReturn GetMatColor(const aiMaterial* mat, glTF2::vec3& prop, const char* propName, int type, int idx); void ExportMetadata(); void ExportMaterials(); void ExportMeshes(); diff --git a/code/glTF2Importer.cpp b/code/glTF2Importer.cpp index de9f39050..2faf1b926 100644 --- a/code/glTF2Importer.cpp +++ b/code/glTF2Importer.cpp @@ -228,7 +228,11 @@ void glTF2Importer::ImportMaterials(glTF2::Asset& r) } SetMaterialColorProperty(r, mat.pbrMetallicRoughness.baseColorFactor, aimat, AI_MATKEY_COLOR_DIFFUSE); + SetMaterialColorProperty(r, mat.pbrMetallicRoughness.baseColorFactor, aimat, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_BASE_COLOR_FACTOR); + SetMaterialTextureProperty(embeddedTexIdxs, r, mat.pbrMetallicRoughness.baseColorTexture, aimat, aiTextureType_DIFFUSE); + SetMaterialTextureProperty(embeddedTexIdxs, r, mat.pbrMetallicRoughness.baseColorTexture, aimat, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_BASE_COLOR_TEXTURE); + SetMaterialTextureProperty(embeddedTexIdxs, r, mat.pbrMetallicRoughness.metallicRoughnessTexture, aimat, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLICROUGHNESS_TEXTURE); aimat->AddProperty(&mat.pbrMetallicRoughness.metallicFactor, 1, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLIC_FACTOR); aimat->AddProperty(&mat.pbrMetallicRoughness.roughnessFactor, 1, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_ROUGHNESS_FACTOR); @@ -249,11 +253,11 @@ void glTF2Importer::ImportMaterials(glTF2::Asset& r) PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value; aimat->AddProperty(&mat.pbrSpecularGlossiness.isPresent, 1, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS); - SetMaterialColorProperty(r, pbrSG.diffuseFactor, aimat, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_DIFFUSE_FACTOR); SetMaterialColorProperty(r, pbrSG.specularFactor, aimat, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULAR_FACTOR); + SetMaterialColorProperty(r, pbrSG.diffuseFactor, aimat, AI_MATKEY_COLOR_DIFFUSE); aimat->AddProperty(&pbrSG.glossinessFactor, 1, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR); - SetMaterialTextureProperty(embeddedTexIdxs, r, pbrSG.diffuseTexture, aimat, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_DIFFUSE_TEXTURE); SetMaterialTextureProperty(embeddedTexIdxs, r, pbrSG.specularGlossinessTexture, aimat, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULARGLOSSINESS_TEXTURE); + SetMaterialTextureProperty(embeddedTexIdxs, r, pbrSG.diffuseTexture, aimat, aiTextureType_DIFFUSE); } } } From a898c1f2d1d579389d159ff1924766fcaf0a9dba Mon Sep 17 00:00:00 2001 From: Daniel Hritzkiv Date: Thu, 19 Oct 2017 12:24:25 -0400 Subject: [PATCH 4/9] SpecularFactor import and export improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The changes here (which only apply to reading from or writing to pbrSpecularGlossiness) will: - store and read specular color on `AI_MATKEY_COLOR_SPECULAR ` rather than `AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULAR_FACTOR` - store and read specular texture from `aiTextureType_SPECULAR` rather than `AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULARGLOSSINESS_TEXTURE`. Even though pbrSG’s specularGlossiness texture uses the alpha channel for glossiness, it will still work well enough with just the RGB channels of the image --- code/glTF2Asset.h | 2 -- code/glTF2Exporter.cpp | 4 ++-- code/glTF2Importer.cpp | 6 ++++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/code/glTF2Asset.h b/code/glTF2Asset.h index 84211aeb6..a98fe5ab2 100644 --- a/code/glTF2Asset.h +++ b/code/glTF2Asset.h @@ -172,9 +172,7 @@ namespace glTF2 #define AI_MATKEY_GLTF_ALPHAMODE "$mat.gltf.alphaMode", 0, 0 #define AI_MATKEY_GLTF_ALPHACUTOFF "$mat.gltf.alphaCutoff", 0, 0 #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS "$mat.gltf.pbrSpecularGlossiness", 0, 0 - #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULAR_FACTOR "$clr.specular", 0, 1 #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR "$mat.gltf.pbrMetallicRoughness.glossinessFactor", 0, 0 - #define AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULARGLOSSINESS_TEXTURE aiTextureType_UNKNOWN, 1 #define _AI_MATKEY_GLTF_TEXTURE_TEXCOORD_BASE "$tex.file.texCoord" #define _AI_MATKEY_GLTF_MAPPINGNAME_BASE "$tex.mappingname" diff --git a/code/glTF2Exporter.cpp b/code/glTF2Exporter.cpp index 26ff77913..1eb52a229 100644 --- a/code/glTF2Exporter.cpp +++ b/code/glTF2Exporter.cpp @@ -470,11 +470,11 @@ void glTF2Exporter::ExportMaterials() PbrSpecularGlossiness pbrSG; - GetMatColor(mat, pbrSG.specularFactor, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULAR_FACTOR); mat->Get(AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR, pbrSG.glossinessFactor); - GetMatTex(mat, pbrSG.specularGlossinessTexture, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULARGLOSSINESS_TEXTURE); GetMatColor(mat, pbrSG.diffuseFactor, AI_MATKEY_COLOR_DIFFUSE); + GetMatColor(mat, pbrSG.specularFactor, AI_MATKEY_COLOR_SPECULAR); GetMatTex(mat, pbrSG.diffuseTexture, aiTextureType_DIFFUSE); + GetMatTex(mat, pbrSG.specularGlossinessTexture, aiTextureType_SPECULAR); m->pbrSpecularGlossiness = Nullable(pbrSG); } diff --git a/code/glTF2Importer.cpp b/code/glTF2Importer.cpp index 2faf1b926..be7947b96 100644 --- a/code/glTF2Importer.cpp +++ b/code/glTF2Importer.cpp @@ -253,11 +253,13 @@ void glTF2Importer::ImportMaterials(glTF2::Asset& r) PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value; aimat->AddProperty(&mat.pbrSpecularGlossiness.isPresent, 1, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS); - SetMaterialColorProperty(r, pbrSG.specularFactor, aimat, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULAR_FACTOR); SetMaterialColorProperty(r, pbrSG.diffuseFactor, aimat, AI_MATKEY_COLOR_DIFFUSE); + SetMaterialColorProperty(r, pbrSG.specularFactor, aimat, AI_MATKEY_COLOR_SPECULAR); aimat->AddProperty(&pbrSG.glossinessFactor, 1, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR); - SetMaterialTextureProperty(embeddedTexIdxs, r, pbrSG.specularGlossinessTexture, aimat, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_SPECULARGLOSSINESS_TEXTURE); + SetMaterialTextureProperty(embeddedTexIdxs, r, pbrSG.diffuseTexture, aimat, aiTextureType_DIFFUSE); + + SetMaterialTextureProperty(embeddedTexIdxs, r, pbrSG.specularGlossinessTexture, aimat, aiTextureType_SPECULAR); } } } From 89358458f080ff74b515436980ef4432c4ea039f Mon Sep 17 00:00:00 2001 From: Daniel Hritzkiv Date: Thu, 19 Oct 2017 12:30:32 -0400 Subject: [PATCH 5/9] Approximate specularity / glossiness in metallicRoughness materials Before, models (of traditional lighting models) with specularity/glossiness would be completely flat when exported to metallicRoughness. These changes approximate glossiness (as an inverse of roughness, with specular intensity as a multiplier) both reading from gltf2 and writing to gltf2. --- code/glTF2Exporter.cpp | 34 ++++++++++++++++++++++++++++++++-- code/glTF2Importer.cpp | 7 +++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/code/glTF2Exporter.cpp b/code/glTF2Exporter.cpp index 1eb52a229..ad0e2647a 100644 --- a/code/glTF2Exporter.cpp +++ b/code/glTF2Exporter.cpp @@ -434,7 +434,29 @@ void glTF2Exporter::ExportMaterials() m->pbrMetallicRoughness.metallicFactor = 0; } - mat->Get(AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_ROUGHNESS_FACTOR, m->pbrMetallicRoughness.roughnessFactor); + // get roughness if source is gltf2 file + if (mat->Get(AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_ROUGHNESS_FACTOR, m->pbrMetallicRoughness.roughnessFactor) != AI_SUCCESS) { + // otherwise, try to derive and convert from specular + shininess values + aiColor4D specularColor; + ai_real shininess; + + if ( + mat->Get(AI_MATKEY_COLOR_SPECULAR, specularColor) == AI_SUCCESS && + mat->Get(AI_MATKEY_SHININESS, shininess) == AI_SUCCESS + ) { + // convert specular color to luminance + float specularIntensity = specularColor[0] * 0.2125 + specularColor[1] * 0.7154 + specularColor[2] * 0.0721; + float roughnessFactor = 1 - std::sqrt(shininess / 1000); + + roughnessFactor = std::powf(roughnessFactor, 2); + roughnessFactor = std::min(std::max(roughnessFactor, 0.0f), 1.0f); + + // low specular intensity values should produce a rough material even if shininess is high. + roughnessFactor = 1 - (roughnessFactor * specularIntensity); + + m->pbrMetallicRoughness.roughnessFactor = roughnessFactor; + } + } GetMatTex(mat, m->normalTexture, aiTextureType_NORMALS); GetMatTex(mat, m->occlusionTexture, aiTextureType_LIGHTMAP); @@ -470,9 +492,17 @@ void glTF2Exporter::ExportMaterials() PbrSpecularGlossiness pbrSG; - mat->Get(AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR, pbrSG.glossinessFactor); GetMatColor(mat, pbrSG.diffuseFactor, AI_MATKEY_COLOR_DIFFUSE); GetMatColor(mat, pbrSG.specularFactor, AI_MATKEY_COLOR_SPECULAR); + + if (mat->Get(AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR, pbrSG.glossinessFactor) != AI_SUCCESS) { + float shininess; + + if (mat->Get(AI_MATKEY_SHININESS, shininess)) { + pbrSG.glossinessFactor = shininess / 1000; + } + } + GetMatTex(mat, pbrSG.diffuseTexture, aiTextureType_DIFFUSE); GetMatTex(mat, pbrSG.specularGlossinessTexture, aiTextureType_SPECULAR); diff --git a/code/glTF2Importer.cpp b/code/glTF2Importer.cpp index be7947b96..292e0eeb2 100644 --- a/code/glTF2Importer.cpp +++ b/code/glTF2Importer.cpp @@ -234,9 +234,13 @@ void glTF2Importer::ImportMaterials(glTF2::Asset& r) SetMaterialTextureProperty(embeddedTexIdxs, r, mat.pbrMetallicRoughness.baseColorTexture, aimat, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_BASE_COLOR_TEXTURE); SetMaterialTextureProperty(embeddedTexIdxs, r, mat.pbrMetallicRoughness.metallicRoughnessTexture, aimat, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLICROUGHNESS_TEXTURE); + aimat->AddProperty(&mat.pbrMetallicRoughness.metallicFactor, 1, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_METALLIC_FACTOR); aimat->AddProperty(&mat.pbrMetallicRoughness.roughnessFactor, 1, AI_MATKEY_GLTF_PBRMETALLICROUGHNESS_ROUGHNESS_FACTOR); + float roughnessAsShininess = (1 - mat.pbrMetallicRoughness.roughnessFactor) * 1000; + aimat->AddProperty(&roughnessAsShininess, 1, AI_MATKEY_SHININESS); + SetMaterialTextureProperty(embeddedTexIdxs, r, mat.normalTexture, aimat, aiTextureType_NORMALS); SetMaterialTextureProperty(embeddedTexIdxs, r, mat.occlusionTexture, aimat, aiTextureType_LIGHTMAP); SetMaterialTextureProperty(embeddedTexIdxs, r, mat.emissiveTexture, aimat, aiTextureType_EMISSIVE); @@ -255,6 +259,9 @@ void glTF2Importer::ImportMaterials(glTF2::Asset& r) aimat->AddProperty(&mat.pbrSpecularGlossiness.isPresent, 1, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS); SetMaterialColorProperty(r, pbrSG.diffuseFactor, aimat, AI_MATKEY_COLOR_DIFFUSE); SetMaterialColorProperty(r, pbrSG.specularFactor, aimat, AI_MATKEY_COLOR_SPECULAR); + + float glossinessAsShininess = pbrSG.glossinessFactor * 1000.0f; + aimat->AddProperty(&glossinessAsShininess, 1, AI_MATKEY_SHININESS); aimat->AddProperty(&pbrSG.glossinessFactor, 1, AI_MATKEY_GLTF_PBRSPECULARGLOSSINESS_GLOSSINESS_FACTOR); SetMaterialTextureProperty(embeddedTexIdxs, r, pbrSG.diffuseTexture, aimat, aiTextureType_DIFFUSE); From 40147d253d0c6f392085519159631db73b61bd57 Mon Sep 17 00:00:00 2001 From: Daniel Hritzkiv Date: Thu, 19 Oct 2017 12:32:55 -0400 Subject: [PATCH 6/9] =?UTF-8?q?Prefer=20=E2=80=9CBLEND=E2=80=9D=20over=20?= =?UTF-8?q?=E2=80=9CMASK=E2=80=9D=20as=20an=20alphaMode=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit “BLEND” is a much nicer alphaMode value (if the hardware supports it – not a steep requirement) than “MASK” as mask is either fully opaque or fully transparent, depending on the alphaCutoff. This matches many other converters’ alphaMode default. --- code/glTF2Exporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/glTF2Exporter.cpp b/code/glTF2Exporter.cpp index ad0e2647a..46f201b52 100644 --- a/code/glTF2Exporter.cpp +++ b/code/glTF2Exporter.cpp @@ -475,7 +475,7 @@ void glTF2Exporter::ExportMaterials() if (mat->Get(AI_MATKEY_OPACITY, opacity) == AI_SUCCESS) { if (opacity < 1) { - m->alphaMode = "MASK"; + m->alphaMode = "BLEND"; m->pbrMetallicRoughness.baseColorFactor[3] *= opacity; } } From 6e888386029a4265a27e5480271c6ca381b26def Mon Sep 17 00:00:00 2001 From: Daniel Hritzkiv Date: Thu, 19 Oct 2017 13:40:03 -0400 Subject: [PATCH 7/9] powf -> pow Fix build errors on linux --- code/glTF2Exporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/glTF2Exporter.cpp b/code/glTF2Exporter.cpp index 46f201b52..9e6415522 100644 --- a/code/glTF2Exporter.cpp +++ b/code/glTF2Exporter.cpp @@ -448,7 +448,7 @@ void glTF2Exporter::ExportMaterials() float specularIntensity = specularColor[0] * 0.2125 + specularColor[1] * 0.7154 + specularColor[2] * 0.0721; float roughnessFactor = 1 - std::sqrt(shininess / 1000); - roughnessFactor = std::powf(roughnessFactor, 2); + roughnessFactor = std::pow(roughnessFactor, 2); roughnessFactor = std::min(std::max(roughnessFactor, 0.0f), 1.0f); // low specular intensity values should produce a rough material even if shininess is high. From cc8374dd80821a423c41aa1ff85c98d12ca8e015 Mon Sep 17 00:00:00 2001 From: Alexandre Avenel Date: Sat, 21 Oct 2017 20:36:43 +0200 Subject: [PATCH 8/9] Return exception when obj file contains invalid face indice --- code/ObjFileParser.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/ObjFileParser.cpp b/code/ObjFileParser.cpp index acf275b94..4b203a8c2 100644 --- a/code/ObjFileParser.cpp +++ b/code/ObjFileParser.cpp @@ -475,7 +475,11 @@ void ObjFileParser::getFace( aiPrimitiveType type ) { } else { reportErrorTokenInFace(); } + } else { + //On error, std::atoi will return 0 which is not a valid value + throw DeadlyImportError("OBJ: Invalid face indice"); } + } m_DataIt += iStep; } From 8b73ec7541e89e870140efa49e829fee5a32b9a0 Mon Sep 17 00:00:00 2001 From: Daniel Hritzkiv Date: Thu, 26 Oct 2017 11:33:33 -0400 Subject: [PATCH 9/9] Fix shininess to roughness conversion; Add comments Was accidentally flipping to value (1 - x) twice, thus negating the effect. --- code/glTF2Exporter.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/code/glTF2Exporter.cpp b/code/glTF2Exporter.cpp index 9e6415522..d8cff897c 100644 --- a/code/glTF2Exporter.cpp +++ b/code/glTF2Exporter.cpp @@ -446,15 +446,15 @@ void glTF2Exporter::ExportMaterials() ) { // convert specular color to luminance float specularIntensity = specularColor[0] * 0.2125 + specularColor[1] * 0.7154 + specularColor[2] * 0.0721; - float roughnessFactor = 1 - std::sqrt(shininess / 1000); - - roughnessFactor = std::pow(roughnessFactor, 2); - roughnessFactor = std::min(std::max(roughnessFactor, 0.0f), 1.0f); + //normalize shininess (assuming max is 1000) with an inverse exponentional curve + float normalizedShininess = std::sqrt(shininess / 1000); + //clamp the shininess value between 0 and 1 + normalizedShininess = std::min(std::max(normalizedShininess, 0.0f), 1.0f); // low specular intensity values should produce a rough material even if shininess is high. - roughnessFactor = 1 - (roughnessFactor * specularIntensity); + normalizedShininess = normalizedShininess * specularIntensity; - m->pbrMetallicRoughness.roughnessFactor = roughnessFactor; + m->pbrMetallicRoughness.roughnessFactor = 1 - normalizedShininess; } }