From cfc8eee13c3afc3948ffd219436a9054f8641227 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Susanne=20D=C3=B6tsch?= <susannedoetsch@uni-koblenz.de>
Date: Fri, 25 Jun 2021 18:14:25 +0200
Subject: [PATCH] [#79]

Finished some of the ToDos
---
 .../include/vkcv/asset/asset_loader.hpp       |  51 +-
 .../src/vkcv/asset/asset_loader.cpp           | 446 +++++++++++-------
 2 files changed, 281 insertions(+), 216 deletions(-)

diff --git a/modules/asset_loader/include/vkcv/asset/asset_loader.hpp b/modules/asset_loader/include/vkcv/asset/asset_loader.hpp
index 8a080d31..b833268c 100644
--- a/modules/asset_loader/include/vkcv/asset/asset_loader.hpp
+++ b/modules/asset_loader/include/vkcv/asset/asset_loader.hpp
@@ -9,6 +9,7 @@
 #include <vector>
 #include <array>
 #include <cstdint>
+#include <filesystem>
 
 /** These macros define limits of the following structs. Implementations can
  * test against these limits when performing sanity checks. The main constraint
@@ -64,52 +65,6 @@ enum class IndexType : uint8_t { UNDEFINED=0, UINT8=1, UINT16=2, UINT32=3 };
  * directly translated to Vulkan. The vkcv::asset::Sampler struct defined here
  * adheres to the Vulkan spec, having alerady translated the flags from glTF to
  * Vulkan. All values here can directly be passed to VkSamplerCreateInfo.
- * If the glTF doesn't define samplers, we use the defaults defined by fx-gltf.
- * The following are details about the glTF/OpenGL to Vulkan translation.
- * magFilter (VkFilter?):
- * 	GL_NEAREST -> VK_FILTER_NEAREST
- * 	GL_LINEAR -> VK_FILTER_LINEAR
- * minFilter (VkFilter?):
- * mipmapMode (VkSamplerMipmapMode?):
- * Vulkans minFilter and mipmapMode combined correspond to OpenGLs
- * GL_minFilter_MIPMAP_mipmapMode:
- * 	GL_NEAREST_MIPMAP_NEAREST:
- * 		minFilter=VK_FILTER_NEAREST
- * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
- * 	GL_LINEAR_MIPMAP_NEAREST:
- * 		minFilter=VK_FILTER_LINEAR
- * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
- * 	GL_NEAREST_MIPMAP_LINEAR:
- * 		minFilter=VK_FILTER_NEAREST
- * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_LINEAR
- * 	GL_LINEAR_MIPMAP_LINEAR:
- * 		minFilter=VK_FILTER_LINEAR
- * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_LINEAR
- * The modes of GL_LINEAR and GL_NEAREST have to be emulated using
- * mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST with specific minLOD and maxLOD:
- * 	GL_LINEAR:
- * 		minFilter=VK_FILTER_LINEAR
- * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
- * 		minLOD=0, maxLOD=0.25
- * 	GL_NEAREST:
- * 		minFilter=VK_FILTER_NEAREST
- * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
- * 		minLOD=0, maxLOD=0.25
- * Setting maxLOD=0 causes magnification to always be performed (using
- * the defined magFilter), this may be valid if the min- and magFilter
- * are equal, otherwise it won't be the expected behaviour from OpenGL
- * and glTF; instead using maxLod=0.25 allows the minFilter to be
- * performed while still always rounding to the base level.
- * With other modes, minLOD and maxLOD default to:
- * 	minLOD=0
- * 	maxLOD=VK_LOD_CLAMP_NONE
- * wrapping:
- * gltf has wrapS, wrapT with {clampToEdge, MirroredRepeat, Repeat} while
- * Vulkan has addressModeU, addressModeV, addressModeW with values
- * VK_SAMPLER_ADDRESS_MODE_{REPEAT,MIRRORED_REPEAT,CLAMP_TO_EDGE,
- * 			    CAMP_TO_BORDER,MIRROR_CLAMP_TO_EDGE}
- * Translation from glTF to Vulkan is straight forward for the 3 existing
- * modes, default is repeat, the other modes aren't available.
  * */
 typedef struct {
 	int minFilter, magFilter;
@@ -130,7 +85,7 @@ typedef struct {
  * materials.*/
 typedef struct {
 	uint16_t textureMask;	// bit mask with active texture targets
-	// Indices into the Array.textures array
+	// Indices into the Scene.textures vector
 	int baseColor, metalRough, normal, occlusion, emissive;
 	// Scaling factors for each texture target
 	struct { float r, g, b, a; } baseColorFactor;
@@ -246,7 +201,7 @@ typedef struct {
  * @param scene is a reference to a Scene struct that will be filled with the
  * 	content of the glTF file being loaded.
  * */
-int loadScene(const std::string &path, Scene &scene);
+int loadScene(const std::filesystem::path &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 a61fd76b..3e2c7eeb 100644
--- a/modules/asset_loader/src/vkcv/asset/asset_loader.cpp
+++ b/modules/asset_loader/src/vkcv/asset/asset_loader.cpp
@@ -11,6 +11,7 @@
 #include <vkcv/Logger.hpp>
 #include <algorithm>
 
+
 namespace vkcv::asset {
 
 /**
@@ -33,7 +34,6 @@ void recurseExceptionPrint(const std::exception& e, const std::string &path)
  * @param type	The accessor type
  * @return	An unsigned integer count
  */
-// TODO add cases for matrices (or maybe change the type in the struct itself)
 uint8_t getCompCount(const fx::gltf::Accessor::Type type) {
 	switch (type) {
 	case fx::gltf::Accessor::Type::Scalar :
@@ -44,6 +44,12 @@ uint8_t getCompCount(const fx::gltf::Accessor::Type type) {
 		return 3;
 	case fx::gltf::Accessor::Type::Vec4 :
 		return 4;
+	case fx::gltf::Accessor::Type::Mat2 :
+		return 4;
+	case fx::gltf::Accessor::Type::Mat3 :
+		return 9;
+	case fx::gltf::Accessor::Type::Mat4 :
+		return 16;
 	case fx::gltf::Accessor::Type::None :
 	default: return ASSET_ERROR;
 	}
@@ -247,9 +253,52 @@ int translateSamplerMode(const fx::gltf::Sampler::WrappingMode mode)
 }
 
 /**
- * TODO document
- * Most of the documenting comment above the struct {...} Sampler definition in
- * the header could be moved here.
+ * If the glTF doesn't define samplers, we use the defaults defined by fx-gltf.
+ * The following are details about the glTF/OpenGL to Vulkan translation.
+ * magFilter (VkFilter?):
+ * 	GL_NEAREST -> VK_FILTER_NEAREST
+ * 	GL_LINEAR -> VK_FILTER_LINEAR
+ * minFilter (VkFilter?):
+ * mipmapMode (VkSamplerMipmapMode?):
+ * Vulkans minFilter and mipmapMode combined correspond to OpenGLs
+ * GL_minFilter_MIPMAP_mipmapMode:
+ * 	GL_NEAREST_MIPMAP_NEAREST:
+ * 		minFilter=VK_FILTER_NEAREST
+ * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
+ * 	GL_LINEAR_MIPMAP_NEAREST:
+ * 		minFilter=VK_FILTER_LINEAR
+ * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
+ * 	GL_NEAREST_MIPMAP_LINEAR:
+ * 		minFilter=VK_FILTER_NEAREST
+ * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_LINEAR
+ * 	GL_LINEAR_MIPMAP_LINEAR:
+ * 		minFilter=VK_FILTER_LINEAR
+ * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_LINEAR
+ * The modes of GL_LINEAR and GL_NEAREST have to be emulated using
+ * mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST with specific minLOD and maxLOD:
+ * 	GL_LINEAR:
+ * 		minFilter=VK_FILTER_LINEAR
+ * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
+ * 		minLOD=0, maxLOD=0.25
+ * 	GL_NEAREST:
+ * 		minFilter=VK_FILTER_NEAREST
+ * 		mipmapMode=VK_SAMPLER_MIPMAP_MODE_NEAREST
+ * 		minLOD=0, maxLOD=0.25
+ * Setting maxLOD=0 causes magnification to always be performed (using
+ * the defined magFilter), this may be valid if the min- and magFilter
+ * are equal, otherwise it won't be the expected behaviour from OpenGL
+ * and glTF; instead using maxLod=0.25 allows the minFilter to be
+ * performed while still always rounding to the base level.
+ * With other modes, minLOD and maxLOD default to:
+ * 	minLOD=0
+ * 	maxLOD=VK_LOD_CLAMP_NONE
+ * wrapping:
+ * gltf has wrapS, wrapT with {clampToEdge, MirroredRepeat, Repeat} while
+ * Vulkan has addressModeU, addressModeV, addressModeW with values
+ * VK_SAMPLER_ADDRESS_MODE_{REPEAT,MIRRORED_REPEAT,CLAMP_TO_EDGE,
+ * 			    CAMP_TO_BORDER,MIRROR_CLAMP_TO_EDGE}
+ * Translation from glTF to Vulkan is straight forward for the 3 existing
+ * modes, default is repeat, the other modes aren't available.
  */
 int translateSampler(const fx::gltf::Sampler &src, vkcv::asset::Sampler &dst)
 {
@@ -298,9 +347,189 @@ int translateSampler(const fx::gltf::Sampler &src, vkcv::asset::Sampler &dst)
 	return ASSET_SUCCESS;
 }
 
-int loadScene(const std::string &path, Scene &scene){
-    fx::gltf::Document sceneObjects;
+/**
+ * TODO document
+ */
+int createVertexGroups(fx::gltf::Mesh const& objectMesh,
+	fx::gltf::Document &sceneObjects, 
+	std::vector<VertexGroup> &vertexGroups,
+	std::vector<int> &vertexGroupsIndices,
+	int &groupCount) {
+
+	const size_t numVertexGroups = objectMesh.primitives.size();
+	vertexGroups.reserve(numVertexGroups);
+
+	for (int j = 0; j < objectMesh.primitives.size(); j++) {
+		fx::gltf::Primitive const& objectPrimitive = objectMesh.primitives[j];
+		std::vector<VertexAttribute> vertexAttributes;
+		vertexAttributes.reserve(objectPrimitive.attributes.size());
+
+		if (createVertexAttributes(objectPrimitive.attributes,
+			sceneObjects.accessors,
+			sceneObjects.bufferViews,
+			vertexAttributes)
+			!= ASSET_SUCCESS) {
+			vkcv_log(LogLevel::ERROR, "Failed to get vertex attributes");
+			return ASSET_ERROR;
+		}
+		// The accessor for the position attribute is used for
+		// 1) getting the vertex buffer view which is only needed to get
+		//    the vertex buffer
+		// 2) getting the vertex count for the VertexGroup
+		// 3) getting the min/max of the bounding box for the VertexGroup
+		fx::gltf::Accessor posAccessor;
+		for (auto const& attrib : objectPrimitive.attributes) {
+			if (attrib.first == "POSITION") {
+				posAccessor = sceneObjects.accessors[attrib.second];
+				break;
+			}
+		}
+
+		IndexType indexType;
+		std::vector<uint8_t> indexBufferData = {};
+		if (objectPrimitive.indices >= 0) { // if there is no index buffer, -1 is returned from fx-gltf
+			const fx::gltf::Accessor& indexAccessor = sceneObjects.accessors[objectPrimitive.indices];
+			const fx::gltf::BufferView& indexBufferView = sceneObjects.bufferViews[indexAccessor.bufferView];
+			const fx::gltf::Buffer& indexBuffer = sceneObjects.buffers[indexBufferView.buffer];
+
+			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 ASSET_ERROR;
+				}
+			}
+
+			indexType = getIndexType(indexAccessor.componentType);
+			if (indexType == IndexType::UNDEFINED) {
+				vkcv_log(LogLevel::ERROR, "Index Type undefined or not supported.");
+				return ASSET_ERROR;
+			}
+		}
+
+		const fx::gltf::BufferView& vertexBufferView = sceneObjects.bufferViews[posAccessor.bufferView];
+		const fx::gltf::Buffer& vertexBuffer = sceneObjects.buffers[vertexBufferView.buffer];
+
+		// only copy relevant part of vertex data
+		uint32_t relevantBufferOffset = std::numeric_limits<uint32_t>::max();
+		uint32_t relevantBufferEnd = 0;
+		for (const auto& attribute : vertexAttributes) {
+			relevantBufferOffset = std::min(attribute.offset, relevantBufferOffset);
+			const uint32_t attributeEnd = attribute.offset + attribute.length;
+			relevantBufferEnd = std::max(relevantBufferEnd, attributeEnd);    // TODO: need to incorporate stride?
+		}
+		const uint32_t relevantBufferSize = relevantBufferEnd - relevantBufferOffset;
+
+		// FIXME: This only works when all vertex attributes are in one buffer
+		std::vector<uint8_t> vertexBufferData;
+		vertexBufferData.resize(relevantBufferSize);
+		{
+			const void* const ptr = ((char*)vertexBuffer.data.data()) + relevantBufferOffset;
+			if (!memcpy(vertexBufferData.data(), ptr, relevantBufferSize)) {
+				vkcv_log(LogLevel::ERROR, "Copying vertex buffer data");
+				return ASSET_ERROR;
+			}
+		}
+
+		// make vertex attributes relative to copied section
+		for (auto& attribute : vertexAttributes) {
+			attribute.offset -= relevantBufferOffset;
+		}
+
+		vertexGroups.push_back({
+			static_cast<PrimitiveMode>(objectPrimitive.mode),
+			sceneObjects.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)
+			});
+
+		vertexGroupsIndices.push_back(groupCount);
+		groupCount++;
+	}
+	return ASSET_SUCCESS;
+}
+
+/**
+ * TODO document
+ */
+void generateTextureMask(fx::gltf::Material &material, uint16_t &textureMask) {
+	if (material.pbrMetallicRoughness.baseColorTexture.index > -1) {
+		textureMask |= bitflag(asset::PBRTextureTarget::baseColor);
+	}
+	if (material.pbrMetallicRoughness.metallicRoughnessTexture.index > -1) {
+		textureMask |= bitflag(asset::PBRTextureTarget::metalRough);
+	}
+	if (material.normalTexture.index > -1) {
+		textureMask |= bitflag(asset::PBRTextureTarget::normal);
+	}
+	if (material.occlusionTexture.index > -1) {
+		textureMask |= bitflag(asset::PBRTextureTarget::occlusion);
+	}
+	if (material.emissiveTexture.index > -1) {
+		textureMask |= bitflag(asset::PBRTextureTarget::emissive);
+	}
+}
+
+/**
+ * TODO document
+ */
+int createMaterial(fx::gltf::Document &sceneObjects, std::vector<Material> &materials) {
+	if (sceneObjects.materials.size() > 0) {
+		materials.reserve(sceneObjects.materials.size());
+
+		for (int l = 0; l < sceneObjects.materials.size(); l++) {
+			fx::gltf::Material material = sceneObjects.materials[l];
+			uint16_t textureMask = 0;
+			generateTextureMask(material, textureMask);
+			// TODO When constructing the the vkcv::asset::Material  we need to
+			// test what kind of texture targets it has and then define the
+			// textureMask for it.
+			// Also, what does the fx::gltf::Material do with indices of
+			// texture targets that don't exist? Maybe we shouldn't set the
+			// index for eb. the normalTexture if there is no normal texture...
+			// It may be a good idea to create an extra function for creating a
+			// material and adding it to the materials array instead of trying
+			// to fit it all into one push_back({...}) call.
+			materials.push_back({
+			textureMask,	
+			material.pbrMetallicRoughness.baseColorTexture.index,
+			material.pbrMetallicRoughness.metallicRoughnessTexture.index,
+			material.normalTexture.index,
+			material.occlusionTexture.index,
+			material.emissiveTexture.index,
+			{
+				material.pbrMetallicRoughness.baseColorFactor[0],
+				material.pbrMetallicRoughness.baseColorFactor[1],
+				material.pbrMetallicRoughness.baseColorFactor[2],
+				material.pbrMetallicRoughness.baseColorFactor[3]
+			},
+			material.pbrMetallicRoughness.metallicFactor,
+			material.pbrMetallicRoughness.roughnessFactor,
+			material.normalTexture.scale,
+			material.occlusionTexture.strength,
+			{
+				material.emissiveFactor[0],
+				material.emissiveFactor[1],
+				material.emissiveFactor[2]
+			}
+			});
+		}
+	}
+	else {
+		return ASSET_ERROR;
+	}
+	return ASSET_SUCCESS;
+}
 
+int loadScene(const std::filesystem::path &path, Scene &scene){
+    fx::gltf::Document sceneObjects;
+	/*
     try {
         if (path.rfind(".glb", (path.length()-4)) != std::string::npos) {
             sceneObjects = fx::gltf::LoadFromBinary(path);
@@ -313,12 +542,31 @@ int loadScene(const std::string &path, Scene &scene){
     } catch (const std::exception &e) {
         recurseExceptionPrint(e, path);
         return ASSET_ERROR;
-    }
+    }*/
+
+	try {
+		if ( path.extension() == ".glb") {
+			sceneObjects = fx::gltf::LoadFromBinary(path.string());
+		}
+		else {
+			sceneObjects = fx::gltf::LoadFromText(path.string());
+		}
+	}
+	catch (const std::system_error& err) {
+		recurseExceptionPrint(err, path.string());
+		return ASSET_ERROR;
+	}
+	catch (const std::exception& e) {
+		recurseExceptionPrint(e, path.string());
+		return ASSET_ERROR;
+	}
+
     // TODO use std::filesystem::path instead of std::string for path/uri.
     // Using simple strings and assuming the path separator symbol to be "/" is
-    // not safe across different operating systems.
-    size_t pos = path.find_last_of("/");
-    auto dir = path.substr(0, pos);
+    // not safe across different operating systems. --erledigt
+    //size_t pos = path.find_last_of("/");
+    //auto dir = path.substr(0, pos);
+	auto dir = path.parent_path().string();
 
     // file has to contain at least one mesh
     if (sceneObjects.meshes.size() == 0) return ASSET_ERROR;
@@ -328,132 +576,34 @@ int loadScene(const std::string &path, Scene &scene){
     std::vector<Sampler> samplers;
     std::vector<Mesh> meshes;
     std::vector<VertexGroup> vertexGroups;
-    int groupCount = 0;
+	int groupCount = 0;
 
     for(int i = 0; i < sceneObjects.meshes.size(); i++){
         std::vector<int> vertexGroupsIndices;
         fx::gltf::Mesh const &objectMesh = sceneObjects.meshes[i];
 
-	// TODO This loop should be moved to its own function just like the
-	// code for loading textures and getting vertex attributes.
-        for(int j = 0; j < objectMesh.primitives.size(); j++){
-            fx::gltf::Primitive const &objectPrimitive = objectMesh.primitives[j];
-	    std::vector<VertexAttribute> vertexAttributes;
-            vertexAttributes.reserve(objectPrimitive.attributes.size());
-
-	    if (createVertexAttributes(objectPrimitive.attributes,
-				    sceneObjects.accessors,
-				    sceneObjects.bufferViews,
-				    vertexAttributes)
-			    != ASSET_SUCCESS) {
-		    vkcv_log(LogLevel::ERROR, "Failed to get vertex attributes");
-		    return ASSET_ERROR;
-	    }
-
-	    // The accessor for the position attribute is used for
-	    // 1) getting the vertex buffer view which is only needed to get
-	    //    the vertex buffer
-	    // 2) getting the vertex count for the VertexGroup
-	    // 3) getting the min/max of the bounding box for the VertexGroup
-	    fx::gltf::Accessor posAccessor;
-	    for (auto const & attrib : objectPrimitive.attributes) {
-		    if (attrib.first == "POSITION") {
-			    posAccessor = sceneObjects.accessors[attrib.second];
-			    break;
-		    }
-	    }
-
-            IndexType indexType;
-            std::vector<uint8_t> indexBufferData = {};
-            if (objectPrimitive.indices >= 0){ // if there is no index buffer, -1 is returned from fx-gltf
-                const fx::gltf::Accessor &indexAccessor = sceneObjects.accessors[objectPrimitive.indices];
-                const fx::gltf::BufferView &indexBufferView = sceneObjects.bufferViews[indexAccessor.bufferView];
-                const fx::gltf::Buffer &indexBuffer = sceneObjects.buffers[indexBufferView.buffer];
-
-                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 ASSET_ERROR;
-                    }
-                }
-
-                indexType = getIndexType(indexAccessor.componentType);
-                if (indexType == IndexType::UNDEFINED){
-                    vkcv_log(LogLevel::ERROR, "Index Type undefined or not supported.");
-                    return ASSET_ERROR;
-                }
-            }
-
-            const fx::gltf::BufferView&	vertexBufferView = sceneObjects.bufferViews[posAccessor.bufferView];
-            const fx::gltf::Buffer& vertexBuffer = sceneObjects.buffers[vertexBufferView.buffer];
-
-            // only copy relevant part of vertex data
-            uint32_t relevantBufferOffset = std::numeric_limits<uint32_t>::max();
-            uint32_t relevantBufferEnd = 0;
-            for (const auto &attribute : vertexAttributes) {
-                relevantBufferOffset = std::min(attribute.offset, relevantBufferOffset);
-                const uint32_t attributeEnd = attribute.offset + attribute.length;
-                relevantBufferEnd = std::max(relevantBufferEnd, attributeEnd);    // TODO: need to incorporate stride?
-            }
-            const uint32_t relevantBufferSize = relevantBufferEnd - relevantBufferOffset;
-
-            // FIXME: This only works when all vertex attributes are in one buffer
-            std::vector<uint8_t> vertexBufferData;
-            vertexBufferData.resize(relevantBufferSize);
-            {
-                const void *const ptr = ((char*)vertexBuffer.data.data()) + relevantBufferOffset;
-                if (!memcpy(vertexBufferData.data(), ptr, relevantBufferSize)) {
-                    vkcv_log(LogLevel::ERROR, "Copying vertex buffer data");
-                    return ASSET_ERROR;
-                }
-            }
-
-            // make vertex attributes relative to copied section
-            for (auto &attribute : vertexAttributes) {
-                attribute.offset -= relevantBufferOffset;
-            }
-
-	    // FIXME This does the same in each iteration of the loop and
-	    // should be moved outside the loop
-            const size_t numVertexGroups = objectMesh.primitives.size();
-            vertexGroups.reserve(numVertexGroups);
-
-            vertexGroups.push_back({
-                static_cast<PrimitiveMode>(objectPrimitive.mode),
-                sceneObjects.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)
-            });
-
-            vertexGroupsIndices.push_back(groupCount);
-            groupCount++;
-        }
+		if (createVertexGroups(objectMesh, sceneObjects, vertexGroups, vertexGroupsIndices, groupCount) != ASSET_SUCCESS) {
+			vkcv_log(LogLevel::ERROR, "Failed to get Vertex Groups!");
+			return ASSET_ERROR;
+		}
+		
 
-	Mesh mesh = {};
+		Mesh mesh = {};
         mesh.name = sceneObjects.meshes[i].name;
         mesh.vertexGroups = vertexGroupsIndices;
 
         meshes.push_back(mesh);
     }
 
-    // TODO Are we sure that every node has a mesh defined? What if there is a
-    // node with just a Camera? Will it have a default value for the mesh index
-    // like 0? In that case we'd overwrite the model matrix of the mesh at
-    // Scene.meshes[0].modelMatrix...
-    // I'm not 100% certain, but this looks wrong to me, please double-check!
+    // This only works if the node has a mesh and it only loads the meshes and ignores cameras and lights
     for(int m = 0; m < sceneObjects.nodes.size(); m++) {
-        meshes[sceneObjects.nodes[m].mesh].modelMatrix = computeModelMatrix(sceneObjects.nodes[m].translation,
-                                                                            sceneObjects.nodes[m].scale,
-                                                                            sceneObjects.nodes[m].rotation,
-                                                                            sceneObjects.nodes[m].matrix);
-    }
+		if (sceneObjects.nodes[m].mesh > -1) {
+			meshes[sceneObjects.nodes[m].mesh].modelMatrix = computeModelMatrix(sceneObjects.nodes[m].translation,
+																				sceneObjects.nodes[m].scale,
+																				sceneObjects.nodes[m].rotation,
+																				sceneObjects.nodes[m].matrix);
+		}
+	}
 
     if (createTextures(sceneObjects.textures, sceneObjects.images, dir, textures) != ASSET_SUCCESS) {
 	    size_t missing = sceneObjects.textures.size() - textures.size();
@@ -461,51 +611,11 @@ int loadScene(const std::string &path, Scene &scene){
 			    missing, path.c_str());
     }
 
-    // TODO creating the materials should be moved to its own function just
-    // like with textures, vertex groups and vertex attributes before
-    if (sceneObjects.materials.size() > 0){
-        materials.reserve(sceneObjects.materials.size());
-
-        for (int l = 0; l < sceneObjects.materials.size(); l++){
-            fx::gltf::Material material = sceneObjects.materials[l];
-	    // TODO When constructing the the vkcv::asset::Material  we need to
-	    // test what kind of texture targets it has and then define the
-	    // textureMask for it.
-	    // Also, what does the fx::gltf::Material do with indices of
-	    // texture targets that don't exist? Maybe we shouldn't set the
-	    // index for eb. the normalTexture if there is no normal texture...
-	    // It may be a good idea to create an extra function for creating a
-	    // material and adding it to the materials array instead of trying
-	    // to fit it all into one push_back({...}) call.
-	    //
-	    // Example for using the bitmask: If a normal texture is there,
-	    // modify the materials textureMask like this:
-	    //     mat.textureMask |= bitflag(asset::PBRTextureTarget::normal);
-            materials.push_back({
-               0,	// TODO set this mask
-               material.pbrMetallicRoughness.baseColorTexture.index,
-               material.pbrMetallicRoughness.metallicRoughnessTexture.index,
-               material.normalTexture.index,
-               material.occlusionTexture.index,
-               material.emissiveTexture.index,
-               {
-                   material.pbrMetallicRoughness.baseColorFactor[0],
-                   material.pbrMetallicRoughness.baseColorFactor[1],
-                   material.pbrMetallicRoughness.baseColorFactor[2],
-                   material.pbrMetallicRoughness.baseColorFactor[3]
-               },
-               material.pbrMetallicRoughness.metallicFactor,
-               material.pbrMetallicRoughness.roughnessFactor,
-               material.normalTexture.scale,
-               material.occlusionTexture.strength,
-               {
-                   material.emissiveFactor[0],
-                   material.emissiveFactor[1],
-                   material.emissiveFactor[2]
-               }
-            });
-        }
-    }
+
+	if (createMaterial(sceneObjects, materials) != ASSET_SUCCESS) {
+		vkcv_log(LogLevel::ERROR, "Failed to get Materials!");
+		return ASSET_ERROR;
+	}
 
     samplers.reserve(sceneObjects.samplers.size());
     for (const auto &it : sceneObjects.samplers) {
-- 
GitLab