From 6612b36fb5ed5107e83a815650271171dc09ff55 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 20 Aug 2023 20:59:11 -0700 Subject: [PATCH 1/7] gltfpack: Properly apply Kd and d for .obj materials Also when the material is using different texture paths for diffuse and alpha, warn about the mismatch - we can't represent a separate alpha mask via standard glTF materials. --- gltf/parseobj.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/gltf/parseobj.cpp b/gltf/parseobj.cpp index 9768e8e2a..37c43f116 100644 --- a/gltf/parseobj.cpp +++ b/gltf/parseobj.cpp @@ -58,14 +58,16 @@ static cgltf_data* parseSceneObj(fastObjMesh* obj) for (unsigned int mi = 0; mi < obj->material_count; ++mi) { + const fastObjMaterial& om = obj->materials[mi]; cgltf_material& gm = data->materials[mi]; - fastObjMaterial& om = obj->materials[mi]; gm.has_pbr_metallic_roughness = true; - gm.pbr_metallic_roughness.base_color_factor[0] = 1.0f; - gm.pbr_metallic_roughness.base_color_factor[1] = 1.0f; - gm.pbr_metallic_roughness.base_color_factor[2] = 1.0f; - gm.pbr_metallic_roughness.base_color_factor[3] = 1.0f; + + gm.pbr_metallic_roughness.base_color_factor[0] = om.Kd[0]; + gm.pbr_metallic_roughness.base_color_factor[1] = om.Kd[1]; + gm.pbr_metallic_roughness.base_color_factor[2] = om.Kd[2]; + gm.pbr_metallic_roughness.base_color_factor[3] = om.d; + gm.pbr_metallic_roughness.metallic_factor = 0.0f; gm.pbr_metallic_roughness.roughness_factor = 1.0f; @@ -80,6 +82,14 @@ static cgltf_data* parseSceneObj(fastObjMesh* obj) } if (om.map_d.name) + { + if (om.map_Kd.name && strcmp(om.map_Kd.name, om.map_d.name) != 0) + fprintf(stderr, "Warning: material has different diffuse and alpha textures (Kd: %s, d: %s) and might not render correctly\n", om.map_Kd.name, om.map_d.name); + + gm.alpha_mode = cgltf_alpha_mode_blend; + } + + if (om.d < 1.0f) { gm.alpha_mode = cgltf_alpha_mode_blend; } From 239bda7a177334679e2119ede81b9cd686a930bd Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 20 Aug 2023 21:03:06 -0700 Subject: [PATCH 2/7] gltfpack: Preserve material names when parsing .obj files These will be preserved in the output glTF file, and using -km option will disable merging named materials. Also define _CRT_NONSTDC_NO_WARNINGS to avoid MSVC warnings for strdup. --- gltf/gltfpack.h | 4 ++++ gltf/parseobj.cpp | 3 +++ 2 files changed, 7 insertions(+) diff --git a/gltf/gltfpack.h b/gltf/gltfpack.h index 4b4c71ed7..b186f66fd 100644 --- a/gltf/gltfpack.h +++ b/gltf/gltfpack.h @@ -11,6 +11,10 @@ #define _CRT_SECURE_NO_WARNINGS #endif +#ifndef _CRT_NONSTDC_NO_WARNINGS +#define _CRT_NONSTDC_NO_WARNINGS +#endif + #include "../extern/cgltf.h" #include diff --git a/gltf/parseobj.cpp b/gltf/parseobj.cpp index 37c43f116..96dd53e8a 100644 --- a/gltf/parseobj.cpp +++ b/gltf/parseobj.cpp @@ -61,6 +61,9 @@ static cgltf_data* parseSceneObj(fastObjMesh* obj) const fastObjMaterial& om = obj->materials[mi]; cgltf_material& gm = data->materials[mi]; + if (om.name) + gm.name = strdup(om.name); + gm.has_pbr_metallic_roughness = true; gm.pbr_metallic_roughness.base_color_factor[0] = om.Kd[0]; From 288d3a6706ba4c4a71eebaacd8db008dde63d05e Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 21 Aug 2023 07:32:01 -0700 Subject: [PATCH 3/7] gltfpack: Parse individual objects in .obj separately We now assign a node to each individual object in .obj file; this allows us to preserve object names. When -kn is not specified, gltfpack will merge the meshes with the same materials anyway so the result should not change, but with -kn the nodes will be preserved in the output file. --- gltf/parseobj.cpp | 78 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 24 deletions(-) diff --git a/gltf/parseobj.cpp b/gltf/parseobj.cpp index 96dd53e8a..c28466d0f 100644 --- a/gltf/parseobj.cpp +++ b/gltf/parseobj.cpp @@ -98,9 +98,31 @@ static cgltf_data* parseSceneObj(fastObjMesh* obj) } } + data->nodes = (cgltf_node*)calloc(obj->object_count, sizeof(cgltf_node)); + data->nodes_count = obj->object_count; + data->scenes = (cgltf_scene*)calloc(1, sizeof(cgltf_scene)); data->scenes_count = 1; + data->scenes->nodes = (cgltf_node**)calloc(obj->object_count, sizeof(cgltf_node*)); + data->scenes->nodes_count = obj->object_count; + + for (unsigned int oi = 0; oi < obj->object_count; ++oi) + { + const fastObjGroup& og = obj->objects[oi]; + cgltf_node* node = &data->nodes[oi]; + + if (og.name) + node->name = strdup(og.name); + + node->rotation[3] = 1.0f; + node->scale[0] = 1.0f; + node->scale[1] = 1.0f; + node->scale[2] = 1.0f; + + data->scenes->nodes[oi] = node; + } + return data; } @@ -194,39 +216,47 @@ static void parseMeshObj(fastObjMesh* obj, unsigned int face_offset, unsigned in static void parseMeshesObj(fastObjMesh* obj, cgltf_data* data, std::vector& meshes) { - unsigned int face_vertex_offset = 0; - - for (unsigned int face_offset = 0; face_offset < obj->face_count; ) + for (unsigned int oi = 0; oi < obj->object_count; ++oi) { - unsigned int mi = obj->face_materials[face_offset]; + const fastObjGroup& og = obj->objects[oi]; - unsigned int face_count = 0; - unsigned int face_vertex_count = 0; - unsigned int index_count = 0; + unsigned int face_vertex_offset = og.index_offset; + unsigned int face_end_offset = og.face_offset + og.face_count; - for (unsigned int fj = face_offset; fj < obj->face_count && obj->face_materials[fj] == mi; ++fj) + for (unsigned int face_offset = og.face_offset; face_offset < face_end_offset; ) { - face_count += 1; - face_vertex_count += obj->face_vertices[fj]; - index_count += (obj->face_vertices[fj] - 2) * 3; - } + unsigned int mi = obj->face_materials[face_offset]; - meshes.push_back(Mesh()); - Mesh& mesh = meshes.back(); + unsigned int face_count = 0; + unsigned int face_vertex_count = 0; + unsigned int index_count = 0; - if (data->materials_count) - { - assert(mi < data->materials_count); - mesh.material = &data->materials[mi]; - } + for (unsigned int fj = face_offset; fj < face_end_offset && obj->face_materials[fj] == mi; ++fj) + { + face_count += 1; + face_vertex_count += obj->face_vertices[fj]; + index_count += (obj->face_vertices[fj] - 2) * 3; + } - mesh.type = cgltf_primitive_type_triangles; - mesh.targets = 0; + meshes.push_back(Mesh()); + Mesh& mesh = meshes.back(); - parseMeshObj(obj, face_offset, face_vertex_offset, face_count, face_vertex_count, index_count, mesh); + if (data->materials_count) + { + assert(mi < data->materials_count); + mesh.material = &data->materials[mi]; + } - face_offset += face_count; - face_vertex_offset += face_vertex_count; + mesh.type = cgltf_primitive_type_triangles; + mesh.targets = 0; + + mesh.nodes.push_back(&data->nodes[oi]); + + parseMeshObj(obj, face_offset, face_vertex_offset, face_count, face_vertex_count, index_count, mesh); + + face_offset += face_count; + face_vertex_offset += face_vertex_count; + } } } From d714c60040c7aa6c927cc5fdbf6c69ffe0be5c8a Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 21 Aug 2023 17:54:07 -0700 Subject: [PATCH 4/7] gltfpack: Refactor .obj parsing a little bit This change splits nodes and materials parsing and keeps mesh group parsing loop so that the code looks more similar to what we used to have when we just had one group. That will also make it easier to ensure the relationships between objects and nodes stay 1-1. --- gltf/parseobj.cpp | 96 ++++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/gltf/parseobj.cpp b/gltf/parseobj.cpp index c28466d0f..3cdc04d09 100644 --- a/gltf/parseobj.cpp +++ b/gltf/parseobj.cpp @@ -21,11 +21,8 @@ static int textureIndex(const std::vector& textures, const char* na return -1; } -static cgltf_data* parseSceneObj(fastObjMesh* obj) +static void parseMaterialsObj(fastObjMesh* obj, cgltf_data* data) { - cgltf_data* data = (cgltf_data*)calloc(1, sizeof(cgltf_data)); - data->memory.free_func = defaultFree; - std::vector textures; for (unsigned int mi = 0; mi < obj->material_count; ++mi) @@ -97,16 +94,13 @@ static cgltf_data* parseSceneObj(fastObjMesh* obj) gm.alpha_mode = cgltf_alpha_mode_blend; } } +} +static void parseNodesObj(fastObjMesh* obj, cgltf_data* data) +{ data->nodes = (cgltf_node*)calloc(obj->object_count, sizeof(cgltf_node)); data->nodes_count = obj->object_count; - data->scenes = (cgltf_scene*)calloc(1, sizeof(cgltf_scene)); - data->scenes_count = 1; - - data->scenes->nodes = (cgltf_node**)calloc(obj->object_count, sizeof(cgltf_node*)); - data->scenes->nodes_count = obj->object_count; - for (unsigned int oi = 0; oi < obj->object_count; ++oi) { const fastObjGroup& og = obj->objects[oi]; @@ -119,11 +113,16 @@ static cgltf_data* parseSceneObj(fastObjMesh* obj) node->scale[0] = 1.0f; node->scale[1] = 1.0f; node->scale[2] = 1.0f; - - data->scenes->nodes[oi] = node; } - return data; + data->scenes = (cgltf_scene*)calloc(1, sizeof(cgltf_scene)); + data->scenes_count = 1; + + data->scenes->nodes = (cgltf_node**)calloc(obj->object_count, sizeof(cgltf_node*)); + data->scenes->nodes_count = obj->object_count; + + for (unsigned int oi = 0; oi < obj->object_count; ++oi) + data->scenes->nodes[oi] = &data->nodes[oi]; } static void parseMeshObj(fastObjMesh* obj, unsigned int face_offset, unsigned int face_vertex_offset, unsigned int face_count, unsigned int face_vertex_count, unsigned int index_count, Mesh& mesh) @@ -214,49 +213,45 @@ static void parseMeshObj(fastObjMesh* obj, unsigned int face_offset, unsigned in assert(index_offset == index_count); } -static void parseMeshesObj(fastObjMesh* obj, cgltf_data* data, std::vector& meshes) +static void parseMeshGroupObj(fastObjMesh* obj, const fastObjGroup& og, cgltf_data* data, cgltf_node* node, std::vector& meshes) { - for (unsigned int oi = 0; oi < obj->object_count; ++oi) + unsigned int face_vertex_offset = og.index_offset; + unsigned int face_end_offset = og.face_offset + og.face_count; + + for (unsigned int face_offset = og.face_offset; face_offset < face_end_offset; ) { - const fastObjGroup& og = obj->objects[oi]; + unsigned int mi = obj->face_materials[face_offset]; - unsigned int face_vertex_offset = og.index_offset; - unsigned int face_end_offset = og.face_offset + og.face_count; + unsigned int face_count = 0; + unsigned int face_vertex_count = 0; + unsigned int index_count = 0; - for (unsigned int face_offset = og.face_offset; face_offset < face_end_offset; ) + for (unsigned int fj = face_offset; fj < face_end_offset && obj->face_materials[fj] == mi; ++fj) { - unsigned int mi = obj->face_materials[face_offset]; - - unsigned int face_count = 0; - unsigned int face_vertex_count = 0; - unsigned int index_count = 0; - - for (unsigned int fj = face_offset; fj < face_end_offset && obj->face_materials[fj] == mi; ++fj) - { - face_count += 1; - face_vertex_count += obj->face_vertices[fj]; - index_count += (obj->face_vertices[fj] - 2) * 3; - } + face_count += 1; + face_vertex_count += obj->face_vertices[fj]; + index_count += (obj->face_vertices[fj] - 2) * 3; + } - meshes.push_back(Mesh()); - Mesh& mesh = meshes.back(); + meshes.push_back(Mesh()); + Mesh& mesh = meshes.back(); - if (data->materials_count) - { - assert(mi < data->materials_count); - mesh.material = &data->materials[mi]; - } + if (data->materials_count) + { + assert(mi < data->materials_count); + mesh.material = &data->materials[mi]; + } - mesh.type = cgltf_primitive_type_triangles; - mesh.targets = 0; + mesh.type = cgltf_primitive_type_triangles; + mesh.targets = 0; - mesh.nodes.push_back(&data->nodes[oi]); + if (node) + mesh.nodes.push_back(node); - parseMeshObj(obj, face_offset, face_vertex_offset, face_count, face_vertex_count, index_count, mesh); + parseMeshObj(obj, face_offset, face_vertex_offset, face_count, face_vertex_count, index_count, mesh); - face_offset += face_count; - face_vertex_offset += face_vertex_count; - } + face_offset += face_count; + face_vertex_offset += face_vertex_count; } } @@ -270,9 +265,16 @@ cgltf_data* parseObj(const char* path, std::vector& meshes, const char** e return 0; } - cgltf_data* data = parseSceneObj(obj); + cgltf_data* data = (cgltf_data*)calloc(1, sizeof(cgltf_data)); + data->memory.free_func = defaultFree; + + parseMaterialsObj(obj, data); - parseMeshesObj(obj, data, meshes); + parseNodesObj(obj, data); + assert(data->nodes_count == obj->object_count); + + for (unsigned int oi = 0; oi < obj->object_count; ++oi) + parseMeshGroupObj(obj, obj->objects[oi], data, &data->nodes[oi], meshes); fast_obj_destroy(obj); From 878fc0f135292f90a9f9863fa713179a36cd914a Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 21 Aug 2023 18:18:07 -0700 Subject: [PATCH 5/7] gltfpack: Fix default scene for glTF files converted from .obj --- gltf/parseobj.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gltf/parseobj.cpp b/gltf/parseobj.cpp index 3cdc04d09..9bd91259c 100644 --- a/gltf/parseobj.cpp +++ b/gltf/parseobj.cpp @@ -118,6 +118,8 @@ static void parseNodesObj(fastObjMesh* obj, cgltf_data* data) data->scenes = (cgltf_scene*)calloc(1, sizeof(cgltf_scene)); data->scenes_count = 1; + data->scene = data->scenes; + data->scenes->nodes = (cgltf_node**)calloc(obj->object_count, sizeof(cgltf_node*)); data->scenes->nodes_count = obj->object_count; From f0130a3ff1e848c20ce0bd47923c3940d226a971 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Tue, 22 Aug 2023 07:12:21 -0700 Subject: [PATCH 6/7] gltfpack: Fixup texture URIs parsed out of .obj/.mtl Some .mtl files contain backslash-delimited paths; backslash is not valid as a delimiter in URIs and paths like this can't be opened on non-Windows platforms when textures are embedded or compressed. --- gltf/parseobj.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gltf/parseobj.cpp b/gltf/parseobj.cpp index 9bd91259c..6775df208 100644 --- a/gltf/parseobj.cpp +++ b/gltf/parseobj.cpp @@ -21,6 +21,14 @@ static int textureIndex(const std::vector& textures, const char* na return -1; } +static void fixupUri(char* uri) +{ + // Some .obj paths come with back slashes, that are invalid as URI separators and won't open on macOS/Linux when embedding textures + for (char* s = uri; *s; ++s) + if (*s == '\\') + *s = '/'; +} + static void parseMaterialsObj(fastObjMesh* obj, cgltf_data* data) { std::vector textures; @@ -38,8 +46,8 @@ static void parseMaterialsObj(fastObjMesh* obj, cgltf_data* data) for (size_t i = 0; i < textures.size(); ++i) { - data->images[i].uri = (char*)malloc(textures[i].size() + 1); - strcpy(data->images[i].uri, textures[i].c_str()); + data->images[i].uri = strdup(textures[i].c_str()); + fixupUri(data->images[i].uri); } data->textures = (cgltf_texture*)calloc(textures.size(), sizeof(cgltf_texture)); From 5dccdc3d2158a213cd66ce1486b7b8cc6e9b87a6 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Tue, 22 Aug 2023 07:16:43 -0700 Subject: [PATCH 7/7] gltfpack: Only use Kd/d from .mtl when associated maps are absent It's not fully clear what the expected behavior is when both Kd and map_Kd are specified; some online documentation claims that they are multiplied, but some .obj files in the wild have map_Kd and Kd=0, so to stay safe we now only apply Kd/d when there's no map with the same name. --- gltf/parseobj.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/gltf/parseobj.cpp b/gltf/parseobj.cpp index 6775df208..90fdc069d 100644 --- a/gltf/parseobj.cpp +++ b/gltf/parseobj.cpp @@ -71,10 +71,10 @@ static void parseMaterialsObj(fastObjMesh* obj, cgltf_data* data) gm.has_pbr_metallic_roughness = true; - gm.pbr_metallic_roughness.base_color_factor[0] = om.Kd[0]; - gm.pbr_metallic_roughness.base_color_factor[1] = om.Kd[1]; - gm.pbr_metallic_roughness.base_color_factor[2] = om.Kd[2]; - gm.pbr_metallic_roughness.base_color_factor[3] = om.d; + gm.pbr_metallic_roughness.base_color_factor[0] = 1.0f; + gm.pbr_metallic_roughness.base_color_factor[1] = 1.0f; + gm.pbr_metallic_roughness.base_color_factor[2] = 1.0f; + gm.pbr_metallic_roughness.base_color_factor[3] = 1.0f; gm.pbr_metallic_roughness.metallic_factor = 0.0f; gm.pbr_metallic_roughness.roughness_factor = 1.0f; @@ -88,6 +88,12 @@ static void parseMaterialsObj(fastObjMesh* obj, cgltf_data* data) gm.alpha_mode = (om.illum == 4 || om.illum == 6 || om.illum == 7 || om.illum == 9) ? cgltf_alpha_mode_mask : cgltf_alpha_mode_opaque; } + else + { + gm.pbr_metallic_roughness.base_color_factor[0] = om.Kd[0]; + gm.pbr_metallic_roughness.base_color_factor[1] = om.Kd[1]; + gm.pbr_metallic_roughness.base_color_factor[2] = om.Kd[2]; + } if (om.map_d.name) { @@ -96,9 +102,10 @@ static void parseMaterialsObj(fastObjMesh* obj, cgltf_data* data) gm.alpha_mode = cgltf_alpha_mode_blend; } - - if (om.d < 1.0f) + else if (om.d < 1.0f) { + gm.pbr_metallic_roughness.base_color_factor[3] = om.d; + gm.alpha_mode = cgltf_alpha_mode_blend; } }