From 80f5283b2fd3fac7dc612e2163b38786ebe1125e Mon Sep 17 00:00:00 2001
From: Mike Samsonov <misams@microsoft.com>
Date: Mon, 18 Nov 2019 16:57:01 +0000
Subject: [PATCH 1/3] Error string of Importer should contain a message in case
of an exception
---
code/Common/Importer.cpp | 4 +-
include/assimp/Exceptional.h | 10 +
test/models/glTF2/MissingBin/BoxTextured.gltf | 181 ++++++++++++++++++
test/unit/utglTF2ImportExport.cpp | 9 +
4 files changed, 202 insertions(+), 2 deletions(-)
create mode 100644 test/models/glTF2/MissingBin/BoxTextured.gltf
diff --git a/code/Common/Importer.cpp b/code/Common/Importer.cpp
index 91b50859a..880b8e83d 100644
--- a/code/Common/Importer.cpp
+++ b/code/Common/Importer.cpp
@@ -493,7 +493,7 @@ const aiScene* Importer::ReadFileFromMemory( const void* pBuffer,
ReadFile(fbuff,pFlags);
SetIOHandler(io);
- ASSIMP_END_EXCEPTION_REGION(const aiScene*);
+ ASSIMP_END_EXCEPTION_REGION_WITH_ERROR_STRING(const aiScene*, pimpl->mErrorString);
return pimpl->mScene;
}
@@ -710,7 +710,7 @@ const aiScene* Importer::ReadFile( const char* _pFile, unsigned int pFlags)
#endif // ! ASSIMP_CATCH_GLOBAL_EXCEPTIONS
// either successful or failure - the pointer expresses it anyways
- ASSIMP_END_EXCEPTION_REGION(const aiScene*);
+ ASSIMP_END_EXCEPTION_REGION_WITH_ERROR_STRING(const aiScene*, pimpl->mErrorString);
return pimpl->mScene;
}
diff --git a/include/assimp/Exceptional.h b/include/assimp/Exceptional.h
index 6bb6ce1e3..f0d23d0f3 100644
--- a/include/assimp/Exceptional.h
+++ b/include/assimp/Exceptional.h
@@ -119,6 +119,16 @@ struct ExceptionSwallower<void> {
{\
try {
+#define ASSIMP_END_EXCEPTION_REGION_WITH_ERROR_STRING(type, ASSIMP_END_EXCEPTION_REGION_errorString)\
+ } catch(const DeadlyImportError& e) {\
+ ASSIMP_END_EXCEPTION_REGION_errorString = e.what();\
+ return ExceptionSwallower<type>()();\
+ } catch(...) {\
+ ASSIMP_END_EXCEPTION_REGION_errorString = "Unknown exception";\
+ return ExceptionSwallower<type>()();\
+ }\
+}
+
#define ASSIMP_END_EXCEPTION_REGION(type)\
} catch(...) {\
return ExceptionSwallower<type>()();\
diff --git a/test/models/glTF2/MissingBin/BoxTextured.gltf b/test/models/glTF2/MissingBin/BoxTextured.gltf
new file mode 100644
index 000000000..88d65391e
--- /dev/null
+++ b/test/models/glTF2/MissingBin/BoxTextured.gltf
@@ -0,0 +1,181 @@
+{
+ "asset": {
+ "generator": "COLLADA2GLTF",
+ "version": "2.0"
+ },
+ "scene": 0,
+ "scenes": [
+ {
+ "nodes": [
+ 0
+ ]
+ }
+ ],
+ "nodes": [
+ {
+ "children": [
+ 1
+ ],
+ "matrix": [
+ 1.0,
+ 0.0,
+ 0.0,
+ 0.0,
+ 0.0,
+ 0.0,
+ -1.0,
+ 0.0,
+ 0.0,
+ 1.0,
+ 0.0,
+ 0.0,
+ 0.0,
+ 0.0,
+ 0.0,
+ 1.0
+ ]
+ },
+ {
+ "mesh": 0
+ }
+ ],
+ "meshes": [
+ {
+ "primitives": [
+ {
+ "attributes": {
+ "NORMAL": 1,
+ "POSITION": 2,
+ "TEXCOORD_0": 3
+ },
+ "indices": 0,
+ "mode": 4,
+ "material": 0
+ }
+ ],
+ "name": "Mesh"
+ }
+ ],
+ "accessors": [
+ {
+ "bufferView": 0,
+ "byteOffset": 0,
+ "componentType": 5123,
+ "count": 36,
+ "max": [
+ 23
+ ],
+ "min": [
+ 0
+ ],
+ "type": "SCALAR"
+ },
+ {
+ "bufferView": 1,
+ "byteOffset": 0,
+ "componentType": 5126,
+ "count": 24,
+ "max": [
+ 1.0,
+ 1.0,
+ 1.0
+ ],
+ "min": [
+ -1.0,
+ -1.0,
+ -1.0
+ ],
+ "type": "VEC3"
+ },
+ {
+ "bufferView": 1,
+ "byteOffset": 288,
+ "componentType": 5126,
+ "count": 24,
+ "max": [
+ 0.5,
+ 0.5,
+ 0.5
+ ],
+ "min": [
+ -0.5,
+ -0.5,
+ -0.5
+ ],
+ "type": "VEC3"
+ },
+ {
+ "bufferView": 2,
+ "byteOffset": 0,
+ "componentType": 5126,
+ "count": 24,
+ "max": [
+ 6.0,
+ 1.0
+ ],
+ "min": [
+ 0.0,
+ 0.0
+ ],
+ "type": "VEC2"
+ }
+ ],
+ "materials": [
+ {
+ "pbrMetallicRoughness": {
+ "baseColorTexture": {
+ "index": 0
+ },
+ "metallicFactor": 0.0
+ },
+ "name": "Texture"
+ }
+ ],
+ "textures": [
+ {
+ "sampler": 0,
+ "source": 0
+ }
+ ],
+ "images": [
+ {
+ "uri": "CesiumLogoFlat.png"
+ }
+ ],
+ "samplers": [
+ {
+ "magFilter": 9729,
+ "minFilter": 9986,
+ "wrapS": 33648,
+ "wrapT": 33071
+ }
+ ],
+ "bufferViews": [
+ {
+ "buffer": 0,
+ "byteOffset": 768,
+ "byteLength": 72,
+ "target": 34963
+ },
+ {
+ "buffer": 0,
+ "byteOffset": 0,
+ "byteLength": 576,
+ "byteStride": 12,
+ "target": 34962
+ },
+ {
+ "buffer": 0,
+ "byteOffset": 576,
+ "byteLength": 192,
+ "byteStride": 8,
+ "target": 34962
+ }
+ ],
+ "buffers": [
+ {
+ "byteLength": 840,
+ "uri": "BoxTextured0.bin"
+ }
+ ]
+}
diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp
index 6925098b9..3c1f6fea6 100644
--- a/test/unit/utglTF2ImportExport.cpp
+++ b/test/unit/utglTF2ImportExport.cpp
@@ -419,4 +419,13 @@ TEST_F( utglTF2ImportExport, crash_in_anim_mesh_destructor ) {
ASSERT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/glTF-Sample-Models/AnimatedMorphCube-glTF/AnimatedMorphCube_out.glTF"));
}
+TEST_F(utglTF2ImportExport, error_string_preserved) {
+ Assimp::Importer importer;
+ const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/MissingBin/BoxTextured.gltf",
+ aiProcess_ValidateDataStructure);
+ ASSERT_EQ(nullptr, scene);
+ std::string error = importer.GetErrorString();
+ ASSERT_NE(error.find("BoxTextured0.bin"), std::string::npos) << "Error string should contain an error about missing .bin file";
+}
+
#endif // ASSIMP_BUILD_NO_EXPORT
From b93c360b877b6cd73f10cc0b1bf5f670dd92a4b7 Mon Sep 17 00:00:00 2001
From: Mike Samsonov <misams@microsoft.com>
Date: Tue, 19 Nov 2019 16:10:34 +0000
Subject: [PATCH 2/3] memory leak fix
---
code/glTF2/glTF2Asset.inl | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/code/glTF2/glTF2Asset.inl b/code/glTF2/glTF2Asset.inl
index 6b47b1607..3c98a8150 100644
--- a/code/glTF2/glTF2Asset.inl
+++ b/code/glTF2/glTF2Asset.inl
@@ -270,13 +270,14 @@ Ref<T> LazyDict<T>::Retrieve(unsigned int i)
throw DeadlyImportError("GLTF: Object at index \"" + to_string(i) + "\" is not a JSON object");
}
- T* inst = new T();
+ // In case Read method throws an exception this will not leak
+ auto inst = std::make_unique<T>();
inst->id = std::string(mDictId) + "_" + to_string(i);
inst->oIndex = i;
ReadMember(obj, "name", inst->name);
inst->Read(obj, mAsset);
- return Add(inst);
+ return Add(inst.release());
}
template<class T>
From 6f7cb6af06a92e9d8404d4cf41f31e5bf67799af Mon Sep 17 00:00:00 2001
From: Mike Samsonov <misams@microsoft.com>
Date: Tue, 19 Nov 2019 16:58:48 +0000
Subject: [PATCH 3/3] revert memory leak fix
---
code/glTF2/glTF2Asset.inl | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/code/glTF2/glTF2Asset.inl b/code/glTF2/glTF2Asset.inl
index 3c98a8150..6b47b1607 100644
--- a/code/glTF2/glTF2Asset.inl
+++ b/code/glTF2/glTF2Asset.inl
@@ -270,14 +270,13 @@ Ref<T> LazyDict<T>::Retrieve(unsigned int i)
throw DeadlyImportError("GLTF: Object at index \"" + to_string(i) + "\" is not a JSON object");
}
- // In case Read method throws an exception this will not leak
- auto inst = std::make_unique<T>();
+ T* inst = new T();
inst->id = std::string(mDictId) + "_" + to_string(i);
inst->oIndex = i;
ReadMember(obj, "name", inst->name);
inst->Read(obj, mAsset);
- return Add(inst.release());
+ return Add(inst);
}
template<class T>