Repository: zeux/meshoptimizer (language: cpp)

Issue:
gltfpack: Keep `EXT_mesh_gpu_instancing`
### Discussed in https://github.com/zeux/meshoptimizer/discussions/680

<div type='discussions-op-text'>

<sup>Originally posted by **BoyBaykiller** April 22, 2024</sup>
I am running `gltfpack -v -mi -noq -tc -i .\SimpleInstancing.gltf -o .\Compressed\Output.gltf` over [this](https://github.com/KhronosGroup/glTF-Sample-Models/tree/main/2.0/SimpleInstancing/glTF) gltf model.
It uses the EXT_mesh_gpu_instancing extension to define a bunch of instanced cubes. I can render it correctly as it, but after running it through the command the EXT_mesh_gpu_instancing extension is removed, resulting in only a single cube being rendered.</div>


Discussion / hints:

Fix patch:
diff --git a/extern/cgltf.h b/extern/cgltf.h
index 17dc0ca5d..ac392aba4 100644
--- a/extern/cgltf.h
+++ b/extern/cgltf.h
@@ -1697,7 +1697,20 @@ cgltf_result cgltf_validate(cgltf_data* data)
 	{
 		if (data->nodes[i].weights && data->nodes[i].mesh)
 		{
-			CGLTF_ASSERT_IF (data->nodes[i].mesh->primitives_count && data->nodes[i].mesh->primitives[0].targets_count != data->nodes[i].weights_count, cgltf_result_invalid_gltf);
+			CGLTF_ASSERT_IF(data->nodes[i].mesh->primitives_count && data->nodes[i].mesh->primitives[0].targets_count != data->nodes[i].weights_count, cgltf_result_invalid_gltf);
+		}
+
+		if (data->nodes[i].has_mesh_gpu_instancing)
+		{
+			CGLTF_ASSERT_IF(data->nodes[i].mesh == NULL, cgltf_result_invalid_gltf);
+			CGLTF_ASSERT_IF(data->nodes[i].mesh_gpu_instancing.attributes_count == 0, cgltf_result_invalid_gltf);
+
+			cgltf_accessor* first = data->nodes[i].mesh_gpu_instancing.attributes[0].data;
+
+			for (cgltf_size k = 0; k < data->nodes[i].mesh_gpu_instancing.attributes_count; ++k)
+			{
+				CGLTF_ASSERT_IF(data->nodes[i].mesh_gpu_instancing.attributes[k].data->count != first->count, cgltf_result_invalid_gltf);
+			}
 		}
 	}
 
diff --git a/gltf/README.md b/gltf/README.md
index a6be16405..ffe6d1e12 100644
--- a/gltf/README.md
+++ b/gltf/README.md
@@ -83,6 +83,7 @@ gltfpack supports most Khronos extensions and some multi-vendor extensions in th
 - KHR_mesh_quantization
 - KHR_texture_basisu
 - KHR_texture_transform
+- EXT_mesh_gpu_instancing
 - EXT_meshopt_compression
 
 Even if the source file does not use extensions, gltfpack may use some extensions in the output file either by default or when certain options are used:
diff --git a/gltf/gltfpack.cpp b/gltf/gltfpack.cpp
index 2db6eff77..dee7a930a 100644
--- a/gltf/gltfpack.cpp
+++ b/gltf/gltfpack.cpp
@@ -281,7 +281,10 @@ static void process(cgltf_data* data, const char* input_path, const char* output
 	for (size_t i = 0; i < meshes.size(); ++i)
 	{
 		Mesh& mesh = meshes[i];
-		assert(mesh.instances.empty());
+
+		// mesh is already instanced, skip
+		if (!mesh.instances.empty())
+			continue;
 
 		// mesh is already world space, skip
 		if (mesh.nodes.empty())
@@ -664,9 +667,6 @@ static void process(cgltf_data* data, const char* input_path, const char* output
 
 		append(json_meshes, "}");
 
-		assert(mesh.nodes.empty() || mesh.instances.empty());
-		ext_instancing = ext_instancing || !mesh.instances.empty();
-
 		if (mesh.nodes.size())
 		{
 			for (size_t j = 0; j < mesh.nodes.size(); ++j)
@@ -691,7 +691,8 @@ static void process(cgltf_data* data, const char* input_path, const char* output
 				}
 			}
 		}
-		else if (mesh.instances.size())
+
+		if (mesh.instances.size())
 		{
 			assert(mesh.scene >= 0);
 			comma(json_roots[mesh.scene]);
@@ -704,7 +705,8 @@ static void process(cgltf_data* data, const char* input_path, const char* output
 
 			node_offset++;
 		}
-		else
+
+		if (mesh.nodes.empty() && mesh.instances.empty())
 		{
 			assert(mesh.scene >= 0);
 			comma(json_roots[mesh.scene]);
@@ -716,6 +718,7 @@ static void process(cgltf_data* data, const char* input_path, const char* output
 		}
 
 		mesh_offset++;
+		ext_instancing = ext_instancing || !mesh.instances.empty();
 
 		// skip all meshes that we've written in this iteration
 		assert(pi > i);
diff --git a/gltf/parsegltf.cpp b/gltf/parsegltf.cpp
index 91f0c020f..f83356b70 100644
--- a/gltf/parsegltf.cpp
+++ b/gltf/parsegltf.cpp
@@ -275,6 +275,54 @@ static void parseMeshesGltf(cgltf_data* data, std::vector<Mesh>& meshes, std::ve
 	}
 }
 
+static void parseMeshInstancesGltf(std::vector<Transform>& instances, cgltf_node* node)
+{
+	cgltf_accessor* translation = NULL;
+	cgltf_accessor* rotation = NULL;
+	cgltf_accessor* scale = NULL;
+
+	for (size_t i = 0; i < node->mesh_gpu_instancing.attributes_count; ++i)
+	{
+		cgltf_attribute& attr = node->mesh_gpu_instancing.attributes[i];
+
+		if (strcmp(attr.name, "TRANSLATION") == 0 && attr.data->type == cgltf_type_vec3)
+			translation = attr.data;
+		else if (strcmp(attr.name, "ROTATION") == 0 && attr.data->type == cgltf_type_vec4)
+			rotation = attr.data;
+		else if (strcmp(attr.name, "SCALE") == 0 && attr.data->type == cgltf_type_vec3)
+			scale = attr.data;
+	}
+
+	size_t count = node->mesh_gpu_instancing.attributes[0].data->count;
+
+	instances.reserve(instances.size() + count);
+
+	cgltf_node instance = {};
+	instance.parent = node;
+	instance.has_translation = translation != NULL;
+	instance.has_rotation = rotation != NULL;
+	instance.has_scale = scale != NULL;
+	instance.rotation[3] = 1.f;
+	instance.scale[0] = 1.f;
+	instance.scale[1] = 1.f;
+	instance.scale[2] = 1.f;
+
+	for (size_t i = 0; i < count; ++i)
+	{
+		if (translation)
+			cgltf_accessor_read_float(translation, i, instance.translation, sizeof(float));
+		if (rotation)
+			cgltf_accessor_read_float(rotation, i, instance.rotation, sizeof(float));
+		if (scale)
+			cgltf_accessor_read_float(scale, i, instance.scale, sizeof(float));
+
+		Transform xf;
+		cgltf_node_transform_world(&instance, xf.data);
+
+		instances.push_back(xf);
+	}
+}
+
 static void parseMeshNodesGltf(cgltf_data* data, std::vector<Mesh>& meshes, const std::vector<std::pair<size_t, size_t> >& mesh_remap)
 {
 	for (size_t i = 0; i < data->nodes_count; ++i)
@@ -289,7 +337,7 @@ static void parseMeshNodesGltf(cgltf_data* data, std::vector<Mesh>& meshes, cons
 		{
 			Mesh* mesh = &meshes[mi];
 
-			if (!mesh->nodes.empty() && mesh->skin != node.skin)
+			if (mesh->skin != node.skin && (!mesh->nodes.empty() || !mesh->instances.empty()))
 			{
 				// this should be extremely rare - if the same mesh is used with different skins, we need to duplicate it
 				// in this case we don't spend any effort on keeping the number of duplicates to the minimum, because this
@@ -298,8 +346,16 @@ static void parseMeshNodesGltf(cgltf_data* data, std::vector<Mesh>& meshes, cons
 				mesh = &meshes.back();
 			}
 
-			mesh->nodes.push_back(&node);
-			mesh->skin = node.skin;
+			if (node.has_mesh_gpu_instancing)
+			{
+				mesh->scene = 0; // we need to assign scene index since instances are attached to a scene; for now we assume 0
+				parseMeshInstancesGltf(mesh->instances, &node);
+			}
+			else
+			{
+				mesh->skin = node.skin;
+				mesh->nodes.push_back(&node);
+			}
 		}
 	}
 
@@ -308,7 +364,7 @@ static void parseMeshNodesGltf(cgltf_data* data, std::vector<Mesh>& meshes, cons
 		Mesh& mesh = meshes[i];
 
 		// because the rest of gltfpack assumes that empty nodes array = world-space mesh, we need to filter unused meshes
-		if (mesh.nodes.empty())
+		if (mesh.nodes.empty() && mesh.instances.empty())
 		{
 			mesh.streams.clear();
 			mesh.indices.clear();
@@ -530,8 +586,6 @@ static cgltf_data* parseGltf(cgltf_data* data, cgltf_result result, std::vector<
 		*error = getError(result, data);
 	else if (requiresExtension(data, "KHR_draco_mesh_compression"))
 		*error = "file requires Draco mesh compression support";
-	else if (requiresExtension(data, "EXT_mesh_gpu_instancing"))
-		*error = "file requires mesh instancing support";
 	else if (needsDummyBuffers(data))
 		*error = "buffer has no data";
 
@@ -543,6 +597,8 @@ static cgltf_data* parseGltf(cgltf_data* data, cgltf_result result, std::vector<
 
 	if (requiresExtension(data, "KHR_mesh_quantization"))
 		fprintf(stderr, "Warning: file uses quantized geometry; repacking may result in increased quantization error\n");
+	if (requiresExtension(data, "EXT_mesh_gpu_instancing") && data->scenes_count > 1)
+		fprintf(stderr, "Warning: file uses instancing and has more than one scene; results may be incorrect\n");
 
 	std::vector<std::pair<size_t, size_t> > mesh_remap;
 


Test patch: