From 47ca18d132cd22e09ecee11b99b59d9b697f34b5 Mon Sep 17 00:00:00 2001
From: Mara Vogt <mvogt@uni-koblenz.de>
Date: Wed, 16 Jun 2021 14:17:34 +0200
Subject: [PATCH] [#63] code cleanUp deleted load_mesh function added
 documentation

---
 .../include/vkcv/asset/asset_loader.hpp       | 101 ++++----
 .../src/vkcv/asset/asset_loader.cpp           | 216 ++----------------
 .../Sponza/Sponta_20_keine_blumen.bin         |   3 -
 .../Sponza/Sponta_20_keine_blumen.gltf        |   3 -
 4 files changed, 64 insertions(+), 259 deletions(-)
 delete mode 100644 projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.bin
 delete mode 100644 projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.gltf

diff --git a/modules/asset_loader/include/vkcv/asset/asset_loader.hpp b/modules/asset_loader/include/vkcv/asset/asset_loader.hpp
index 64ece270..4107d57e 100644
--- a/modules/asset_loader/include/vkcv/asset/asset_loader.hpp
+++ b/modules/asset_loader/include/vkcv/asset/asset_loader.hpp
@@ -1,6 +1,6 @@
 #pragma once
 /**
- * @authors Trevor Hollmann
+ * @authors Trevor Hollmann, Mara Vogt, Susanne Dötsch
  * @file include/vkcv/asset/asset_loader.h
  * @brief Interface of the asset loader module for the vkcv framework.
  */
@@ -10,7 +10,7 @@
 #include <array>
 #include <cstdint>
 
-/* These macros define limits of the following structs. Implementations can
+/** These macros define limits of the following structs. Implementations can
  * test against these limits when performing sanity checks. The main constraint
  * expressed is that of the data type: Material indices are identified by a
  * uint8_t in the VertexGroup struct, so there can't be more than UINT8_MAX
@@ -20,7 +20,7 @@
 #define MAX_MATERIALS_PER_MESH UINT8_MAX
 #define MAX_VERTICES_PER_VERTEX_GROUP UINT32_MAX
 
-/* LOADING MESHES
+/** LOADING MESHES
  * The description of meshes is a hierarchy of structures with the Mesh at the
  * top.
  *
@@ -45,14 +45,14 @@
 
 namespace vkcv::asset {
 
-/* This enum matches modes in fx-gltf, the library returns a standard mode
+/** This enum matches modes in fx-gltf, the library returns a standard mode
  * (TRIANGLES) if no mode is given in the file. */
 enum class PrimitiveMode : uint8_t {
 	POINTS=0, LINES, LINELOOP, LINESTRIP, TRIANGLES, TRIANGLESTRIP,
 	TRIANGLEFAN
 };
 
-/* The indices in the index buffer can be of different bit width. */
+/** The indices in the index buffer can be of different bit width. */
 enum class IndexType : uint8_t { UNDEFINED=0, UINT8=1, UINT16=2, UINT32=3 };
 
 typedef struct {
@@ -62,6 +62,7 @@ typedef struct {
 	// define a different set of Min/Mag-filters than Vulkan.
 } Sampler;
 
+/** struct for defining the loaded texture */
 typedef struct {
 	int sampler;		// index into the sampler array of the Scene
 	uint8_t channels;	// number of channels
@@ -69,43 +70,11 @@ typedef struct {
 	std::vector<uint8_t> data;	// binary data of the decoded texture
 } Texture;
 
-
-/* The asset loader module only supports the PBR-MetallicRoughness model for
+/** The asset loader module only supports the PBR-MetallicRoughness model for
  * materials.*/
-
-/* With these enums, 0 is reserved to signal uninitialized or invalid data. */
-enum class PrimitiveType : uint32_t {
-    UNDEFINED = 0,
-    POSITION = 1,
-    NORMAL = 2,
-    TEXCOORD_0 = 3,
-	TEXCOORD_1 = 4
-};
-
-/* These integer values are used the same way in OpenGL, Vulkan and glTF. This
- * enum is not needed for translation, it's only for the programmers
- * convenience (easier to read in if/switch statements etc). While this enum
- * exists in (almost) the same definition in the fx-gltf library, we want to
- * avoid exposing that dependency, thus it is re-defined here. */
-enum class ComponentType : uint16_t {
-	NONE = 0, INT8 = 5120, UINT8 = 5121, INT16 = 5122, UINT16 = 5123,
-	UINT32 = 5125, FLOAT32 = 5126
-};
-
-
-/* This struct describes one vertex attribute of a vertex buffer. */
-typedef struct {
-    PrimitiveType type;			// POSITION, NORMAL, ...
-    uint32_t offset;			// offset in bytes
-    uint32_t length;			// length of ... in bytes
-    uint32_t stride;			// stride in bytes
-	ComponentType componentType;		// eg. 5126 for float
-    uint8_t  componentCount;	// eg. 3 for vec3
-} VertexAttribute;
-
 typedef struct {
 	uint16_t textureMask;	// bit mask with active texture targets
-	// Indices into the Mesh.textures array
+	// Indices into the Array.textures array
 	int baseColor, metalRough, normal, occlusion, emissive;
 	// Scaling factors for each texture target
 	struct { float r, g, b, a; } baseColorFactor;
@@ -115,14 +84,14 @@ typedef struct {
 	struct { float r, g, b; } emissiveFactor;
 } Material;
 
-/* Flags for the bit-mask in the Material struct. To check if a material has a
+/** Flags for the bit-mask in the Material struct. To check if a material has a
  * certain texture target, you can use the hasTexture() function below, passing
  * the material struct and the enum. */
 enum class PBRTextureTarget {
 	baseColor=1, metalRough=2, normal=4, occlusion=8, emissive=16
 };
 
-/* This macro translates the index of an enum in the defined order to an
+/** This macro translates the index of an enum in the defined order to an
  * integer with a single bit set in the corresponding place. It is used for
  * working with the bitmask of texture targets ("textureMask") in the Material
  * struct:
@@ -133,13 +102,42 @@ enum class PBRTextureTarget {
  * contact with bit-level operations. */
 #define bitflag(ENUM) (0x1u << ((unsigned)(ENUM)))
 
-/* To signal that a certain texture target is active in a Material struct, its
+/** To signal that a certain texture target is active in a Material struct, its
  * bit is set in the textureMask. You can use this function to check that:
  * Material mat = ...;
  * if (materialHasTexture(&mat, baseColor)) {...} */
 bool materialHasTexture(const Material *const m, const PBRTextureTarget t);
 
-/* This struct represents one (possibly the only) part of a mesh. There is
+/** With these enums, 0 is reserved to signal uninitialized or invalid data. */
+enum class PrimitiveType : uint32_t {
+    UNDEFINED = 0,
+    POSITION = 1,
+    NORMAL = 2,
+    TEXCOORD_0 = 3,
+    TEXCOORD_1 = 4
+};
+
+/** These integer values are used the same way in OpenGL, Vulkan and glTF. This
+ * enum is not needed for translation, it's only for the programmers
+ * convenience (easier to read in if/switch statements etc). While this enum
+ * exists in (almost) the same definition in the fx-gltf library, we want to
+ * avoid exposing that dependency, thus it is re-defined here. */
+enum class ComponentType : uint16_t {
+    NONE = 0, INT8 = 5120, UINT8 = 5121, INT16 = 5122, UINT16 = 5123,
+    UINT32 = 5125, FLOAT32 = 5126
+};
+
+/** This struct describes one vertex attribute of a vertex buffer. */
+typedef struct {
+    PrimitiveType type;			// POSITION, NORMAL, ...
+    uint32_t offset;			// offset in bytes
+    uint32_t length;			// length of ... in bytes
+    uint32_t stride;			// stride in bytes
+    ComponentType componentType;		// eg. 5126 for float
+    uint8_t  componentCount;	// eg. 3 for vec3
+} VertexAttribute;
+
+/** This struct represents one (possibly the only) part of a mesh. There is
  * always one vertexBuffer and zero or one indexBuffer (indexed rendering is
  * common but not always used). If there is no index buffer, this is indicated
  * by indexBuffer.data being empty. Each vertex buffer can have one or more
@@ -160,7 +158,7 @@ typedef struct {
 	int materialIndex;		// index to one of the materials
 } VertexGroup;
 
-/* This struct represents a single mesh as it was loaded from a glTF file. It
+/** This struct represents a single mesh as it was loaded from a glTF file. It
  * consists of at least one VertexGroup, which then references other resources
  * such as Materials. */
 typedef struct {
@@ -169,8 +167,7 @@ typedef struct {
 	std::vector<int> vertexGroups;
 } Mesh;
 
-
-/* The scene struct is simply a collection of objects in the scene as well as
+/** The scene struct is simply a collection of objects in the scene as well as
  * the resources used by those objects.
  * For now the only type of object are the meshes and they are represented in a
  * flat array.
@@ -184,18 +181,12 @@ typedef struct {
 } Scene;
 
 /**
- * In its first iteration the asset loader module will only allow loading
- * single meshes, one per glTF file.
- * It will later be extended to allow loading entire scenes from glTF files.
+ * Load every mesh from the glTF file, as well as materials and textures.
  *
- * @param path must be the path to a glTF file containing a single mesh.
- * @param mesh is a reference to a Mesh struct that will be filled with the
+ * @param path must be the path to a glTF or glb file.
+ * @param scene is a reference to a Scene struct that will be filled with the
  * 	content of the glTF file being loaded.
  * */
-int loadMesh(const std::string &path, Mesh &mesh);
-
-// TODO Either replace loadMesh with this new function loadScene, or implement
-// loadScene as a second function besides loadMesh...
 int loadScene(const std::string &path, Scene &scene);
 
 
diff --git a/modules/asset_loader/src/vkcv/asset/asset_loader.cpp b/modules/asset_loader/src/vkcv/asset/asset_loader.cpp
index 815ad61a..10e47025 100644
--- a/modules/asset_loader/src/vkcv/asset/asset_loader.cpp
+++ b/modules/asset_loader/src/vkcv/asset/asset_loader.cpp
@@ -52,7 +52,7 @@ void print_what (const std::exception& e, const std::string &path) {
 	}
 }
 
-/* Translate the component type used in the index accessor of fx-gltf to our
+/** Translate the component type used in the index accessor of fx-gltf to our
  * enum for index type. The reason we have defined an incompatible enum that
  * needs translation is that only a subset of component types is valid for
  * indices and we want to catch these incompatibilities here. */
@@ -66,12 +66,20 @@ enum IndexType getIndexType(const enum fx::gltf::Accessor::ComponentType &t)
 	case fx::gltf::Accessor::ComponentType::UnsignedInt:
 		return IndexType::UINT32;
 	default:
-		std::cerr << "ERROR: Index type not supported: " <<
+        std::cerr << "ERROR: Index type not supported: " <<
 			static_cast<uint16_t>(t) << std::endl;
 		return IndexType::UNDEFINED;
 	}
 }
 
+/**
+ * This function computes the modelMatrix out of the data given in the gltf file. It also checks, whether a modelMatrix was given.
+ * @param translation possible translation vector (default 0,0,0)
+ * @param scale possible scale vector (default 1,1,1)
+ * @param rotation possible rotation, given in quaternion (default 0,0,0,1)
+ * @param matrix possible modelmatrix (default identity)
+ * @return model Matrix as an array of floats
+ */
 std::array<float, 16> computeModelMatrix(std::array<float, 3> translation, std::array<float, 3> scale, std::array<float, 4> rotation, std::array<float, 16> matrix){
     std::array<float, 16> modelMatrix = {1,0,0,0,
                                          0,1,0,0,
@@ -106,200 +114,11 @@ std::array<float, 16> computeModelMatrix(std::array<float, 3> translation, std::
 
 }
 
-int loadMesh(const std::string &path, Mesh &mesh) {
-	fx::gltf::Document object;
-
-	try {
-		if (path.rfind(".glb", (path.length()-4)) != std::string::npos) {
-			object = fx::gltf::LoadFromBinary(path);
-		} else {
-			object = fx::gltf::LoadFromText(path);
-		}
-	} catch (const std::system_error &err) {
-		print_what(err, path);
-		return 0;
-	} catch (const std::exception &e) {
-		print_what(e, path);
-		return 0;
-	}
-
-	// TODO Temporary restriction: Only one mesh per glTF file allowed
-	// currently. Later, we want to support whole scenes with more than
-	// just meshes.
-	if (object.meshes.size() != 1) return 0;
-
-	fx::gltf::Mesh const &objectMesh = object.meshes[0];
-	// TODO We want to support more than one vertex group per mesh
-	// eventually... right now this is hard-coded to use only the first one
-	// because we only care about the example triangle and cube
-	fx::gltf::Primitive const &objectPrimitive = objectMesh.primitives[0];
-	fx::gltf::Accessor posAccessor;
-	
-	std::vector<VertexAttribute> vertexAttributes;
-	vertexAttributes.reserve(objectPrimitive.attributes.size());
-	
-	for (auto const & attrib : objectPrimitive.attributes) {
-		fx::gltf::Accessor accessor =  object.accessors[attrib.second];
-		VertexAttribute attribute;
-
-		if (attrib.first == "POSITION") {
-			attribute.type = PrimitiveType::POSITION;
-			posAccessor = accessor;
-		} else if (attrib.first == "NORMAL") {
-			attribute.type = PrimitiveType::NORMAL;
-		} else if (attrib.first == "TEXCOORD_0") {
-			attribute.type = PrimitiveType::TEXCOORD_0;
-		} else {
-			return 0;
-		}
-		
-		attribute.offset = object.bufferViews[accessor.bufferView].byteOffset;
-		attribute.length = object.bufferViews[accessor.bufferView].byteLength;
-		attribute.stride = object.bufferViews[accessor.bufferView].byteStride;
-		attribute.componentType = static_cast<ComponentType>(accessor.componentType);
-		
-		if (convertTypeToInt(accessor.type) != 10) {
-			attribute.componentCount = convertTypeToInt(accessor.type);
-		} else {
-			return 0;
-		}
-		
-		vertexAttributes.push_back(attribute);
-	}
-
-	// TODO consider the case where there is no index buffer (not all
-	// meshes have to use indexed rendering)
-	const fx::gltf::Accessor &indexAccessor = object.accessors[objectPrimitive.indices];
-	const fx::gltf::BufferView &indexBufferView = object.bufferViews[indexAccessor.bufferView];
-	const fx::gltf::Buffer &indexBuffer = object.buffers[indexBufferView.buffer];
-	
-	std::vector<uint8_t> indexBufferData;
-	indexBufferData.resize(indexBufferView.byteLength);
-	{
-		const size_t off = indexBufferView.byteOffset;
-		const void *const ptr = ((char*)indexBuffer.data.data()) + off;
-		if (!memcpy(indexBufferData.data(), ptr, indexBufferView.byteLength)) {
-			vkcv_log(LogLevel::ERROR, "Copying index buffer data");
-			return 0;
-		}
-	}
-
-	const fx::gltf::BufferView&	vertexBufferView	= object.bufferViews[posAccessor.bufferView];
-	const fx::gltf::Buffer&		vertexBuffer		= object.buffers[vertexBufferView.buffer];
-	
-	// FIXME: This only works when all vertex attributes are in one buffer
-	std::vector<uint8_t> vertexBufferData;
-	vertexBufferData.resize(vertexBuffer.byteLength);
-	{
-		const size_t off = 0;
-		const void *const ptr = ((char*)vertexBuffer.data.data()) + off;
-		if (!memcpy(vertexBufferData.data(), ptr, vertexBuffer.byteLength)) {
-			vkcv_log(LogLevel::ERROR, "Copying vertex buffer data");
-			return 0;
-		}
-	}
-
-	IndexType indexType;
-	switch(indexAccessor.componentType) {
-	case fx::gltf::Accessor::ComponentType::UnsignedByte:
-		indexType = IndexType::UINT8; break;
-	case fx::gltf::Accessor::ComponentType::UnsignedShort:
-		indexType = IndexType::UINT16; break;
-	case fx::gltf::Accessor::ComponentType::UnsignedInt:
-		indexType = IndexType::UINT32; break;
-	default:
-		vkcv_log(LogLevel::ERROR, "Index type (%u) not supported",
-				 static_cast<uint16_t>(indexAccessor.componentType));
-		return 0;
-	}
-
-	const size_t numVertexGroups = objectMesh.primitives.size();
-	
-	std::vector<VertexGroup> vertexGroups;
-	vertexGroups.reserve(numVertexGroups);
-	
-	vertexGroups.push_back({
-		static_cast<PrimitiveMode>(objectPrimitive.mode),
-		object.accessors[objectPrimitive.indices].count,
-		posAccessor.count,
-		{indexType, indexBufferData},
-		{vertexBufferData, vertexAttributes},
-		{posAccessor.min[0], posAccessor.min[1], posAccessor.min[2]},
-		{posAccessor.max[0], posAccessor.max[1], posAccessor.max[2]},
-		static_cast<uint8_t>(objectPrimitive.material)
-	});
-	
-	std::vector<Material> materials;
-	std::vector<Texture> textures;
-	std::vector<Sampler> samplers;
-
-	std::vector<int> vertexGroupsIndex;
-
-    //glm::mat4 modelMatrix = object.nodes[0].matrix;
-	for(int i = 0; i < numVertexGroups; i++){
-        vertexGroupsIndex.push_back(i);
-	}
-
-
-	mesh = {
-		object.meshes[0].name,
-        object.nodes[0].matrix,
-		vertexGroupsIndex,
-	};
-
-	// FIXME HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK
-	// fail quietly if there is no texture
-	textures.reserve(1);
-	if (object.textures.size()) {
-		const std::string mime_type("image/jpeg");
-		const fx::gltf::Texture &tex = object.textures[0];
-		const fx::gltf::Image &img = object.images[tex.source];
-#ifndef NDEBUG
-		printf("texture name=%s sampler=%u source=%u\n",
-				tex.name.c_str(), tex.sampler, tex.source);
-		printf("image   name=%s uri=%s mime=%s\n", img.name.c_str(),
-				img.uri.c_str(), img.mimeType.c_str());
-#endif
-		
-		size_t pos = path.find_last_of("/");
-		auto dir = path.substr(0, pos);
-		
-		std::string img_uri = dir + "/" + img.uri;
-		int w, h, c;
-		uint8_t *data = stbi_load(img_uri.c_str(), &w, &h, &c, 4);
-		c = 4;	// FIXME hardcoded to always have RGBA channel layout
-		if (!data) {
-			std::cerr << "ERROR loading texture image data.\n";
-			return 0;
-		}
-		const size_t byteLen = w * h * c;
-
-		std::vector<uint8_t> imgdata;
-		imgdata.resize(byteLen);
-		if (!memcpy(imgdata.data(), data, byteLen)) {
-			std::cerr << "ERROR copying texture image data.\n";
-			free(data);
-			return 0;
-		}
-		free(data);
-
-		textures.push_back({
-			0, static_cast<uint8_t>(c),
-			static_cast<uint16_t>(w), static_cast<uint16_t>(h),
-			imgdata
-		});
-	}
-	// FIXME HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK
-	return 1;
-}
-
-
 bool materialHasTexture(const Material *const m, const PBRTextureTarget t)
 {
 	return m->textureMask & bitflag(t);
 }
 
-
 int loadScene(const std::string &path, Scene &scene){
     fx::gltf::Document sceneObjects;
 
@@ -322,7 +141,6 @@ int loadScene(const std::string &path, Scene &scene){
     // file has to contain at least one mesh
     if (sceneObjects.meshes.size() == 0) return 0;
 
-
     fx::gltf::Accessor posAccessor;
     std::vector<VertexAttribute> vertexAttributes;
     std::vector<Material> materials;
@@ -334,7 +152,6 @@ int loadScene(const std::string &path, Scene &scene){
 
     Mesh mesh = {};
 
-
     for(int i = 0; i < sceneObjects.meshes.size(); i++){
         std::vector<int> vertexGroupsIndices;
         fx::gltf::Mesh const &objectMesh = sceneObjects.meshes[i];
@@ -388,13 +205,16 @@ int loadScene(const std::string &path, Scene &scene){
                     const size_t off = indexBufferView.byteOffset;
                     const void *const ptr = ((char*)indexBuffer.data.data()) + off;
                     if (!memcpy(indexBufferData.data(), ptr, indexBufferView.byteLength)) {
-                        std::cerr << "ERROR copying index buffer data.\n";
+                        vkcv_log(LogLevel::ERROR, "Copying index buffer data");
                         return 0;
                     }
                 }
 
                 indexType = getIndexType(indexAccessor.componentType);
-                if (indexType == IndexType::UNDEFINED) return 0; // TODO return vkcv::error;
+                if (indexType == IndexType::UNDEFINED){
+                    vkcv_log(LogLevel::ERROR, "Index Type undefined.");
+                    return 0;
+                }
             }
 
             const fx::gltf::BufferView&	vertexBufferView	= sceneObjects.bufferViews[posAccessor.bufferView];
@@ -416,7 +236,7 @@ int loadScene(const std::string &path, Scene &scene){
             {
                 const void *const ptr = ((char*)vertexBuffer.data.data()) + relevantBufferOffset;
                 if (!memcpy(vertexBufferData.data(), ptr, relevantBufferSize)) {
-                    std::cerr << "ERROR copying vertex buffer data.\n";
+                    vkcv_log(LogLevel::ERROR, "Copying vertex buffer data");
                     return 0;
                 }
             }
@@ -474,7 +294,7 @@ int loadScene(const std::string &path, Scene &scene){
             uint8_t *data = stbi_load(img_uri.c_str(), &w, &h, &c, 4);
             c = 4;	// FIXME hardcoded to always have RGBA channel layout
             if (!data) {
-                std::cerr << "ERROR loading texture image data.\n";
+                vkcv_log(LogLevel::ERROR, "Loading texture image data.")
                 return 0;
             }
             const size_t byteLen = w * h * c;
@@ -482,7 +302,7 @@ int loadScene(const std::string &path, Scene &scene){
             std::vector<uint8_t> imgdata;
             imgdata.resize(byteLen);
             if (!memcpy(imgdata.data(), data, byteLen)) {
-                std::cerr << "ERROR copying texture image data.\n";
+                vkcv_log(LogLevel::ERROR, "Copying texture image data")
                 free(data);
                 return 0;
             }
diff --git a/projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.bin b/projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.bin
deleted file mode 100644
index bf792432..00000000
--- a/projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.bin
+++ /dev/null
@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:389b0f08cf80a8d6efcc67fb51fc2dd1a078783719a1de516c3c462bd6dc62d2
-size 428636
diff --git a/projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.gltf b/projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.gltf
deleted file mode 100644
index 7b6aa24c..00000000
--- a/projects/first_scene/resources/Sponza/Sponta_20_keine_blumen.gltf
+++ /dev/null
@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:5e92e01711b44f4345b2c19df51fcdc69834f425277b7d7cdec9bc5f7b063f61
-size 41102
-- 
GitLab