From 134282937e724f33f9cc18731024346373e6c38c Mon Sep 17 00:00:00 2001 From: 3gg <3gg@shellblade.net> Date: Wed, 4 Jan 2023 16:53:28 -0800 Subject: Simplify cleanup in the scene loader. --- gfx/src/util/scene.c | 471 +++++++++++++++++---------------------------------- 1 file changed, 156 insertions(+), 315 deletions(-) diff --git a/gfx/src/util/scene.c b/gfx/src/util/scene.c index dc97259..706e518 100644 --- a/gfx/src/util/scene.c +++ b/gfx/src/util/scene.c @@ -207,80 +207,44 @@ static size_t get_total_primitives(const cgltf_data* data) { /// /// Return an array of Buffers such that the index of each glTF buffer in /// the original array matches the same Buffer in the resulting array. -static Buffer** load_buffers( - const cgltf_data* data, RenderBackend* render_backend, - cgltf_size* num_buffers) { +static bool load_buffers( + const cgltf_data* data, RenderBackend* render_backend, Buffer** buffers) { assert(data); assert(render_backend); - assert(num_buffers); - - Buffer** buffers = 0; - - buffers = calloc(data->buffers_count, sizeof(Buffer*)); - if (!buffers) { - goto cleanup; - } + assert(buffers); for (cgltf_size i = 0; i < data->buffers_count; ++i) { const cgltf_buffer* buffer = &data->buffers[i]; assert(buffer->data); buffers[i] = gfx_make_buffer(render_backend, buffer->data, buffer->size); if (!buffers[i]) { - goto cleanup; + return false; } } - *num_buffers = data->buffers_count; - return buffers; -cleanup: - if (buffers) { - for (cgltf_size i = 0; i < data->buffers_count; ++i) { - if (buffers[i]) { - gfx_destroy_buffer(render_backend, &buffers[i]); - } - } - free(buffers); - } - *num_buffers = 0; - return 0; + return true; } /// Load tangent buffers. -static Buffer** load_tangent_buffers( - const cgltfTangentBuffer* tangent_buffers, cgltf_size num_tangent_buffers, - RenderBackend* render_backend) { - assert(tangent_buffers); +static bool load_tangent_buffers( + const cgltfTangentBuffer* cgltf_tangent_buffers, + cgltf_size num_tangent_buffers, RenderBackend* render_backend, + Buffer** tangent_buffers) { + assert(cgltf_tangent_buffers); assert(render_backend); - - Buffer** buffers = 0; - - buffers = calloc(num_tangent_buffers, sizeof(Buffer*)); - if (!buffers) { - goto cleanup; - } + assert(tangent_buffers); for (cgltf_size i = 0; i < num_tangent_buffers; ++i) { - const cgltfTangentBuffer* buffer = &tangent_buffers[i]; + const cgltfTangentBuffer* buffer = &cgltf_tangent_buffers[i]; assert(buffer->data); - buffers[i] = + tangent_buffers[i] = gfx_make_buffer(render_backend, buffer->data, buffer->size_bytes); - if (!buffers[i]) { - goto cleanup; + if (!tangent_buffers[i]) { + return false; } } - return buffers; - -cleanup: - if (buffers) { - for (cgltf_size i = 0; i < num_tangent_buffers; ++i) { - if (buffers[i]) { - gfx_destroy_buffer(render_backend, &buffers[i]); - } - } - free(buffers); - } - return 0; + return true; } /// Lazily load all textures from the glTF scene. @@ -294,24 +258,13 @@ cleanup: /// we know their colour space when loading glTF materials. /// /// Return an array of LoadTextureCmds such that the index of each cmd matches -/// the index of each glTF texture in the scene. Also return the number of -/// textures. -/// -/// Return true on success (all textures processed or no textures in the -/// scene), false otherwise. -static bool load_textures_lazy( +/// the index of each glTF texture in the scene. +static void load_textures_lazy( const cgltf_data* data, RenderBackend* render_backend, - const char* directory, LoadTextureCmd** load_texture_cmds, - cgltf_size* num_textures) { + const char* directory, LoadTextureCmd* load_texture_cmds) { assert(data); assert(render_backend); assert(load_texture_cmds); - assert(num_textures); - - *load_texture_cmds = calloc(data->textures_count, sizeof(LoadTextureCmd)); - if (!*load_texture_cmds) { - goto cleanup; - } for (cgltf_size i = 0; i < data->textures_count; ++i) { const cgltf_texture* texture = &data->textures[i]; @@ -359,7 +312,7 @@ static bool load_textures_lazy( mstring fullpath = mstring_concat_path(mstring_make(directory), mstring_make(image->uri)); - (*load_texture_cmds)[i] = (LoadTextureCmd){ + load_texture_cmds[i] = (LoadTextureCmd){ .origin = TextureFromFile, .type = LoadTexture, .colour_space = sRGB, @@ -368,16 +321,6 @@ static bool load_textures_lazy( .mipmaps = mipmaps, .data.texture.filepath = fullpath}; } - - *num_textures = data->textures_count; - return true; - -cleanup: - if (*load_texture_cmds) { - free(*load_texture_cmds); - } - *num_textures = 0; - return false; } /// Load a texture uniform. @@ -438,25 +381,15 @@ static bool load_texture_and_uniform( /// Return an array of Materials such that the index of each descriptor matches /// the index of each glTF material in the scene. Also return the number of /// materials and the textures used by them. -static Material** load_materials( +static bool load_materials( const cgltf_data* data, RenderBackend* render_backend, - LoadTextureCmd* load_texture_cmds, Texture*** textures, - cgltf_size* num_materials) { + LoadTextureCmd* load_texture_cmds, Texture** textures, + Material** materials) { assert(data); assert(render_backend); assert(load_texture_cmds); assert(textures); - assert(num_materials); - - Material** materials = calloc(data->materials_count, sizeof(Material*)); - if (!materials) { - goto cleanup; - } - - *textures = calloc(data->textures_count, sizeof(Texture*)); - if (!*textures) { - goto cleanup; - } + assert(materials); for (cgltf_size i = 0; i < data->materials_count; ++i) { const cgltf_material* mat = &data->materials[i]; @@ -495,18 +428,18 @@ static Material** load_materials( if (pbr->base_color_texture.texture) { if (!load_texture_and_uniform( data, render_backend, &pbr->base_color_texture, - BaseColorTexture, *textures, load_texture_cmds, &next_uniform, + BaseColorTexture, textures, load_texture_cmds, &next_uniform, &desc)) { - goto cleanup; + return false; } } if (pbr->metallic_roughness_texture.texture) { if (!load_texture_and_uniform( data, render_backend, &pbr->metallic_roughness_texture, - MetallicRoughnessTexture, *textures, load_texture_cmds, + MetallicRoughnessTexture, textures, load_texture_cmds, &next_uniform, &desc)) { - goto cleanup; + return false; } } } @@ -514,25 +447,25 @@ static Material** load_materials( if (mat->emissive_texture.texture) { if (!load_texture_and_uniform( data, render_backend, &mat->emissive_texture, EmissiveTexture, - *textures, load_texture_cmds, &next_uniform, &desc)) { - goto cleanup; + textures, load_texture_cmds, &next_uniform, &desc)) { + return false; } } if (mat->occlusion_texture.texture) { if (!load_texture_and_uniform( data, render_backend, &mat->occlusion_texture, - AmbientOcclusionTexture, *textures, load_texture_cmds, + AmbientOcclusionTexture, textures, load_texture_cmds, &next_uniform, &desc)) { - goto cleanup; + return false; } } if (mat->normal_texture.texture) { if (!load_texture_and_uniform( - data, render_backend, &mat->normal_texture, NormalMap, *textures, + data, render_backend, &mat->normal_texture, NormalMap, textures, load_texture_cmds, &next_uniform, &desc)) { - goto cleanup; + return false; } } @@ -541,42 +474,20 @@ static Material** load_materials( materials[i] = gfx_make_material(&desc); if (!materials[i]) { - goto cleanup; + return false; } } - *num_materials = data->materials_count; - return materials; - -cleanup: - if (materials) { - for (cgltf_size i = 0; i < data->materials_count; ++i) { - if (materials[i]) { - gfx_destroy_material(&materials[i]); - } - } - free(materials); - } - if (*textures) { - for (cgltf_size i = 0; i < data->textures_count; ++i) { - if ((*textures)[i]) { - gfx_destroy_texture(render_backend, &(*textures)[i]); - } - } - free(*textures); - *textures = 0; - } - *num_materials = 0; - return 0; + return true; } /// Load all meshes from the glTF scene. -static SceneObject** load_meshes( +static bool load_meshes( const cgltf_data* data, Gfx* gfx, Buffer** buffers, Buffer** tangent_buffers, const cgltfTangentBuffer* cgltf_tangent_buffers, cgltf_size num_tangent_buffers, Material** materials, ShaderProgram* shader, - Geometry*** geometries, Mesh*** meshes, cgltf_size* num_geometries, - cgltf_size* num_meshes, cgltf_size* num_scene_objects) { + size_t primitive_count, Geometry** geometries, Mesh** meshes, + SceneObject** scene_objects) { // Walk through the mesh primitives to create Meshes. A GLTF mesh primitive // has a material (Mesh) and vertex data (Geometry). A GLTF mesh maps to // a SceneObject. @@ -594,9 +505,7 @@ static SceneObject** load_meshes( assert(shader); assert(geometries); assert(meshes); - assert(num_geometries); - assert(num_meshes); - assert(num_scene_objects); + assert(scene_objects); if (num_tangent_buffers > 0) { assert(tangent_buffers); assert(cgltf_tangent_buffers); @@ -605,21 +514,6 @@ static SceneObject** load_meshes( // TODO: pass render_backend as argument instead of gfx. RenderBackend* render_backend = gfx_get_render_backend(gfx); - const size_t primitive_count = get_total_primitives(data); - - *geometries = calloc(primitive_count, sizeof(Geometry*)); - if (!*geometries) { - goto cleanup; - } - *meshes = calloc(primitive_count, sizeof(Mesh*)); - if (!*meshes) { - goto cleanup; - } - SceneObject** objects = calloc(data->meshes_count, sizeof(SceneObject*)); - if (!objects) { - goto cleanup; - } - // Points to the next available Mesh and also the next available Geometry. // There is one (Mesh, Geometry) pair per glTF mesh primitive. size_t next_mesh = 0; @@ -627,9 +521,9 @@ static SceneObject** load_meshes( for (cgltf_size m = 0; m < data->meshes_count; ++m) { const cgltf_mesh* mesh = &data->meshes[m]; - objects[m] = gfx_make_object(); - if (!objects[m]) { - goto cleanup; + scene_objects[m] = gfx_make_object(); + if (!scene_objects[m]) { + return false; } for (cgltf_size p = 0; p < mesh->primitives_count; ++p) { @@ -689,7 +583,7 @@ static SceneObject** load_meshes( "Unhandled accessor type %d in vertex positions", accessor->type); assert(false); - break; + return false; } // It is assumed that meshes have positions, so there is nothing to // do for the mesh permutation in this case. @@ -776,16 +670,9 @@ static SceneObject** load_meshes( assert(material_index < data->materials_count); Material* material = materials[material_index]; - // TODO: We need a better way to handle clean-up, specifically of - // materials. One is to add materials to a dynamically-allocated list or - // vector. Another is to expose some kind of scene purge that deletes the - // resources of a given scene. The latter would make clean-up much simpler - // in general, not just for materials. - - (*geometries)[next_mesh] = - gfx_make_geometry(render_backend, &geometry_desc); - if (!(*geometries)[next_mesh]) { - goto cleanup; + geometries[next_mesh] = gfx_make_geometry(render_backend, &geometry_desc); + if (!geometries[next_mesh]) { + return false; } // If the user specifies a custom shader, use that instead. @@ -802,65 +689,31 @@ static SceneObject** load_meshes( // swapping shaders, so shader caching is the most important thing here. assert(shader); - (*meshes)[next_mesh] = gfx_make_mesh(&(MeshDesc){ - .geometry = (*geometries)[next_mesh], + meshes[next_mesh] = gfx_make_mesh(&(MeshDesc){ + .geometry = geometries[next_mesh], .material = material, .shader = shader}); - gfx_add_object_mesh(objects[m], (*meshes)[next_mesh]); + if (!meshes[next_mesh]) { + return false; + } + + gfx_add_object_mesh(scene_objects[m], meshes[next_mesh]); ++next_mesh; } // glTF mesh primitive / gfx Mesh. } // glTF mesh / gfx SceneObject. - *num_geometries = primitive_count; - *num_meshes = primitive_count; - *num_scene_objects = data->meshes_count; - return objects; - -cleanup: - // Destroy resources, free pointers arrays. - if (*geometries) { - for (size_t i = 0; i < primitive_count; ++i) { - if ((*geometries)[i]) { - gfx_destroy_geometry(render_backend, &(*geometries[i])); - } - } - free(*geometries); - *geometries = 0; - } - if (*meshes) { - for (size_t i = 0; i < primitive_count; ++i) { - if ((*meshes[i])) { - gfx_destroy_mesh(&(*meshes)[i]); - } - } - free(*meshes); - *meshes = 0; - } - if (objects) { - for (cgltf_size i = 0; i < data->meshes_count; ++i) { - if (objects[i]) { - gfx_destroy_object(&objects[i]); - } - } - free(objects); - objects = 0; - } - *num_geometries = 0; - *num_meshes = 0; - *num_scene_objects = 0; - return 0; + return true; } /// Load all nodes from the glTF scene. /// /// This function ignores the many scenes and default scene of the glTF spec /// and instead just loads all nodes into a single gfx Scene. -static SceneNode** load_nodes( +static void load_nodes( const cgltf_data* data, Gfx* gfx, SceneNode* root_node, - SceneObject** scene_objects, SceneCamera*** cameras, - cgltf_size* num_cameras, cgltf_size* num_nodes) { + SceneObject** objects, SceneCamera** cameras, SceneNode** nodes) { // Note that with glTF 2.0, nodes do not form a DAG / scene graph but a // disjount union of strict trees: // @@ -876,19 +729,9 @@ static SceneNode** load_nodes( // TODO: gfx not needed? assert(gfx); assert(root_node); - assert(scene_objects); + assert(objects); assert(cameras); - assert(num_cameras); - assert(num_nodes); - - SceneNode** nodes = calloc(data->nodes_count, sizeof(SceneNode**)); - if (!nodes) { - goto cleanup; - } - *cameras = calloc(data->cameras_count, sizeof(SceneCamera**)); - if (!*cameras) { - goto cleanup; - } + assert(nodes); cgltf_size next_camera = 0; @@ -902,7 +745,7 @@ static SceneNode** load_nodes( if (node->mesh) { const cgltf_size mesh_index = node->mesh - data->meshes; assert(mesh_index < data->meshes_count); - nodes[n] = gfx_make_object_node(scene_objects[mesh_index]); + nodes[n] = gfx_make_object_node(objects[mesh_index]); } else if (node->camera) { assert(next_camera < data->cameras_count); @@ -927,8 +770,8 @@ static SceneNode** load_nodes( break; } - gfx_set_camera_camera((*cameras)[next_camera], &camera); - nodes[n] = gfx_make_camera_node((*cameras)[next_camera]); + gfx_set_camera_camera(cameras[next_camera], &camera); + nodes[n] = gfx_make_camera_node(cameras[next_camera]); ++next_camera; } else { continue; // TODO: implementation for missing node types. @@ -977,32 +820,6 @@ static SceneNode** load_nodes( gfx_set_node_parent(child_scene_node, parent_scene_node); } } - - *num_cameras = data->cameras_count; - *num_nodes = data->nodes_count; - return nodes; - -cleanup: - // Destroy resources, free pointers arrays. - if (nodes) { - for (cgltf_size i = 0; i < data->nodes_count; ++i) { - if (nodes[i]) { - gfx_destroy_node(&nodes[i]); - } - } - free(nodes); - } - if (cameras) { - for (cgltf_size i = 0; i < data->cameras_count; ++i) { - if (cameras[i]) { - gfx_destroy_camera(&(*cameras)[i]); - } - } - free(cameras); - } - *num_cameras = 0; - *num_nodes = 0; - return 0; } /// Load all scenes from the glTF file into the given gfx Scene. @@ -1015,143 +832,167 @@ static bool load_scene( cgltf_data* data, Gfx* gfx, SceneNode* root_node, const char* filepath, ShaderProgram* shader, const cgltfTangentBuffer* cgltf_tangent_buffers, cgltf_size num_tangent_buffers) { - /// In a GLTF scene, buffers can be shared among meshes, meshes among nodes, - /// etc. Each object is referenced by its index in the relevant array. Here we - /// do a button-up construction, first allocating our own graphics objects in - /// the same quantities and then re-using the GLTF indices to index these - /// arrays. + // In a GLTF scene, buffers can be shared among meshes, meshes among nodes, + // etc. Each object is referenced by its index in the relevant array. Here we + // do a button-up construction, first allocating our own graphics objects in + // the same quantities and then re-using the GLTF indices to index these + // arrays. + // + // For simplicity, this function also handles all of the cleanup. Arrays are + // allocated up front, and the helper functions construct their elements. If + // an error is encountered, the helper functions can simply return and this + // function cleans up any intermediate objects that had been created up until + // the point of failure. assert(data); assert(gfx); assert(root_node); - RenderBackend* render_backend = gfx_get_render_backend(gfx); + RenderBackend* render_backend = gfx_get_render_backend(gfx); + const size_t primitive_count = get_total_primitives(data); const mstring directory = mstring_dirname(mstring_make(filepath)); LOGD("Filepath: %s", filepath); LOGD("Directory: %s", mstring_cstring(&directory)); - Buffer** buffers = 0; Buffer** tangent_buffers = 0; - Geometry** geometries = 0; + Buffer** buffers = 0; + LoadTextureCmd* load_texture_cmds = 0; + Texture** textures = 0; Material** materials = 0; + Geometry** geometries = 0; Mesh** meshes = 0; SceneObject** scene_objects = 0; SceneCamera** scene_cameras = 0; SceneNode** scene_nodes = 0; - Texture** textures = 0; - LoadTextureCmd* load_texture_cmds = 0; - cgltf_size num_buffers = 0; - cgltf_size num_geometries = 0; - cgltf_size num_materials = 0; - cgltf_size num_meshes = 0; - cgltf_size num_scene_objects = 0; - cgltf_size num_scene_cameras = 0; - cgltf_size num_scene_nodes = 0; - cgltf_size num_textures = 0; - - // TODO: Let this function handle all the cleanup. Let the other functions - // return pass/failure booleans and the arrays as in/out parameters. This way - // we do not need the individual functions to duplicate cleanup code. - buffers = load_buffers(data, render_backend, &num_buffers); - if (!buffers) { - goto cleanup; + + tangent_buffers = calloc(num_tangent_buffers, sizeof(Buffer*)); + buffers = calloc(data->buffers_count, sizeof(Buffer*)); + textures = calloc(data->textures_count, sizeof(Texture*)); + materials = calloc(data->materials_count, sizeof(Material*)); + geometries = calloc(primitive_count, sizeof(Geometry*)); + meshes = calloc(primitive_count, sizeof(Mesh*)); + scene_objects = calloc(data->meshes_count, sizeof(SceneObject*)); + scene_cameras = calloc(data->cameras_count, sizeof(SceneCamera**)); + scene_nodes = calloc(data->nodes_count, sizeof(SceneNode**)); + // A glTF scene does not necessarily have textures. Materials can be given + // as constants, for example. + if (data->textures_count > 0) { + load_texture_cmds = calloc(data->textures_count, sizeof(LoadTextureCmd)); } - if (num_tangent_buffers > 0) { - tangent_buffers = load_tangent_buffers( - cgltf_tangent_buffers, num_tangent_buffers, render_backend); - if (!tangent_buffers) { - goto cleanup; - } + if (!buffers || !tangent_buffers || + ((data->textures_count > 0) && !load_texture_cmds) || !textures || + !materials || !geometries || !meshes || !scene_objects || + !scene_cameras || !scene_nodes) { + goto cleanup; } - if (!load_textures_lazy( - data, render_backend, mstring_cstring(&directory), &load_texture_cmds, - &num_textures)) { + if ((num_tangent_buffers > 0) && + !load_tangent_buffers( + cgltf_tangent_buffers, num_tangent_buffers, render_backend, + tangent_buffers)) { goto cleanup; } - materials = load_materials( - data, render_backend, load_texture_cmds, &textures, &num_materials); - if (!materials) { + if (!load_buffers(data, render_backend, buffers)) { goto cleanup; } - scene_objects = load_meshes( - data, gfx, buffers, tangent_buffers, cgltf_tangent_buffers, - num_tangent_buffers, materials, shader, &geometries, &meshes, - &num_geometries, &num_meshes, &num_scene_objects); - if (!scene_objects) { + load_textures_lazy( + data, render_backend, mstring_cstring(&directory), load_texture_cmds); + + if (!load_materials( + data, render_backend, load_texture_cmds, textures, materials)) { goto cleanup; } - scene_nodes = load_nodes( - data, gfx, root_node, scene_objects, &scene_cameras, &num_scene_cameras, - &num_scene_nodes); - if (!scene_nodes) { + if (!load_meshes( + data, gfx, buffers, tangent_buffers, cgltf_tangent_buffers, + num_tangent_buffers, materials, shader, primitive_count, geometries, + meshes, scene_objects)) { goto cleanup; } + load_nodes(data, gfx, root_node, scene_objects, scene_cameras, scene_nodes); + return true; cleanup: - if (buffers) { - for (cgltf_size i = 0; i < num_buffers; ++i) { - gfx_destroy_buffer(render_backend, &buffers[i]); - } - free(buffers); - } if (tangent_buffers) { for (cgltf_size i = 0; i < num_tangent_buffers; ++i) { - gfx_destroy_buffer(render_backend, &tangent_buffers[i]); + if (tangent_buffers[i]) { + gfx_destroy_buffer(render_backend, &tangent_buffers[i]); + } } free(tangent_buffers); } + if (buffers) { + for (cgltf_size i = 0; i < data->buffers_count; ++i) { + if (buffers[i]) { + gfx_destroy_buffer(render_backend, &buffers[i]); + } + } + free(buffers); + } + if (load_texture_cmds) { + free(load_texture_cmds); + } if (textures) { - for (cgltf_size i = 0; i < num_textures; ++i) { - gfx_destroy_texture(render_backend, &textures[i]); + for (cgltf_size i = 0; i < data->textures_count; ++i) { + if (textures[i]) { + gfx_destroy_texture(render_backend, &textures[i]); + } } free(textures); } if (materials) { - for (cgltf_size i = 0; i < num_materials; ++i) { - gfx_destroy_material(&materials[i]); + for (cgltf_size i = 0; i < data->materials_count; ++i) { + if (materials[i]) { + gfx_destroy_material(&materials[i]); + } } free(materials); } if (geometries) { - for (cgltf_size i = 0; i < num_geometries; ++i) { - gfx_destroy_geometry(render_backend, &geometries[i]); + for (size_t i = 0; i < primitive_count; ++i) { + if (geometries[i]) { + gfx_destroy_geometry(render_backend, &geometries[i]); + } } free(geometries); } if (meshes) { - for (cgltf_size i = 0; i < num_meshes; ++i) { - gfx_destroy_mesh(&meshes[i]); + for (size_t i = 0; i < primitive_count; ++i) { + if (meshes[i]) { + gfx_destroy_mesh(&meshes[i]); + } } free(meshes); } if (scene_objects) { - for (cgltf_size i = 0; i < num_scene_objects; ++i) { - gfx_destroy_object(&scene_objects[i]); + for (cgltf_size i = 0; i < data->meshes_count; ++i) { + if (scene_objects[i]) { + gfx_destroy_object(&scene_objects[i]); + } } free(scene_objects); } if (scene_cameras) { - for (cgltf_size i = 0; i < num_scene_cameras; ++i) { - gfx_destroy_camera(&scene_cameras[i]); + for (cgltf_size i = 0; i < data->cameras_count; ++i) { + if (scene_cameras[i]) { + gfx_destroy_camera(&scene_cameras[i]); + } } free(scene_cameras); } if (scene_nodes) { - for (cgltf_size i = 0; i < num_scene_nodes; ++i) { - gfx_destroy_node(&scene_nodes[i]); + for (cgltf_size i = 0; i < data->nodes_count; ++i) { + if (scene_nodes[i]) { + gfx_destroy_node(&scene_nodes[i]); + } } free(scene_nodes); } - if (load_texture_cmds) { - free(load_texture_cmds); - } return false; } -- cgit v1.2.3