From 4f34ef2042f0eb22ce0b51b50c8b3ac7d53a7805 Mon Sep 17 00:00:00 2001 From: 3gg <3gg@shellblade.net> Date: Tue, 13 Feb 2024 17:47:13 -0800 Subject: Assert cleanup. --- gfx/CMakeLists.txt | 1 + gfx/src/asset/asset_cache.c | 47 +++++++----------- gfx/src/gfx_assert.h | 5 ++ gfx/src/render/buffer.c | 8 ++- gfx/src/render/framebuffer.c | 4 +- gfx/src/render/geometry.c | 9 ++-- gfx/src/render/render_backend.c | 107 ++++++++++++++++++++-------------------- gfx/src/render/renderbuffer.c | 1 + gfx/src/render/shader.c | 8 +-- gfx/src/render/shader_program.c | 21 ++++++-- gfx/src/render/texture.c | 19 ++++--- gfx/src/scene/animation.c | 72 +++++++++++---------------- gfx/src/scene/camera.c | 25 ++++------ gfx/src/scene/light.c | 13 ++--- gfx/src/scene/material.c | 3 -- gfx/src/scene/mesh.c | 3 -- gfx/src/scene/node.c | 73 +++++++++------------------ gfx/src/scene/object.c | 16 +++--- gfx/src/scene/scene.c | 13 ++--- 19 files changed, 204 insertions(+), 244 deletions(-) create mode 100644 gfx/src/gfx_assert.h diff --git a/gfx/CMakeLists.txt b/gfx/CMakeLists.txt index e5c965e..2b295d5 100644 --- a/gfx/CMakeLists.txt +++ b/gfx/CMakeLists.txt @@ -74,6 +74,7 @@ target_link_libraries(gfx PUBLIC math) target_link_libraries(gfx PRIVATE + cassert cgltf cgltf-tangents error diff --git a/gfx/src/asset/asset_cache.c b/gfx/src/asset/asset_cache.c index 0c6a8dc..2fa57ad 100644 --- a/gfx/src/asset/asset_cache.c +++ b/gfx/src/asset/asset_cache.c @@ -4,13 +4,11 @@ #include #include #include +#include #include #include -#include -#include - static Hash calc_scene_hash(const LoadSceneCmd* cmd) { assert(cmd); switch (cmd->origin) { @@ -19,7 +17,7 @@ static Hash calc_scene_hash(const LoadSceneCmd* cmd) { case AssetFromMemory: return (Hash)cmd->data; } - assert(false); + FAIL("Unhandled scene asset origin"); return 0; } @@ -59,7 +57,7 @@ static Hash calc_texture_hash(const LoadTextureCmd* cmd) { } break; } - assert(false); + FAIL("Unhandled texture asset origin"); return 0; } @@ -73,25 +71,16 @@ static Asset* lookup_cache(AssetCache* cache, Hash hash) { return 0; } -// TODO: questionable function. Make mempool_alloc() fail when out of memory. -static void insert_into_cache(AssetCache* cache, const Asset* asset) { - assert(cache); - assert(asset); - Asset* poolAsset = mempool_alloc(&cache->assets); - assert(asset); - *poolAsset = *asset; -} - static void log_scene_cache_hit(const LoadSceneCmd* cmd, Hash hash) { assert(cmd); switch (cmd->origin) { case AssetFromFile: - LOGI( + LOGD( "Found asset [%s] in cache with hash [%lu]", mstring_cstr(&cmd->filepath), hash); break; case AssetFromMemory: - LOGI("Found asset [%p] in cache with hash [%lu]", cmd->data, hash); + LOGD("Found asset [%p] in cache with hash [%lu]", cmd->data, hash); break; } } @@ -100,10 +89,10 @@ static void log_scene_loaded(const LoadSceneCmd* cmd) { assert(cmd); switch (cmd->origin) { case AssetFromFile: - LOGI("Loaded asset from file: [%s]", mstring_cstr(&cmd->filepath)); + LOGD("Loaded asset from file: [%s]", mstring_cstr(&cmd->filepath)); break; case AssetFromMemory: - LOGI("Loaded asset from memory: [%p]", cmd->data); + LOGD("Loaded asset from memory: [%p]", cmd->data); break; } } @@ -143,12 +132,11 @@ SceneNode* gfx_load_scene( // Load it, insert it into the cache, and return it. SceneNode* node = gfx_scene_load(gfx, root_node, cmd); if (node) { - insert_into_cache( - cache, &(Asset){ - .type = SceneAsset, - .hash = hash, - .data = node, - }); + *(Asset*)mempool_alloc(&cache->assets) = (Asset){ + .type = SceneAsset, + .hash = hash, + .data = node, + }; log_scene_loaded(cmd); } return node; @@ -172,12 +160,11 @@ Texture* gfx_load_texture(Gfx* gfx, const LoadTextureCmd* cmd) { RenderBackend* render_backend = gfx_get_render_backend(gfx); Texture* texture = gfx_texture_load(render_backend, cmd); if (texture) { - insert_into_cache( - cache, &(Asset){ - .type = TextureAsset, - .hash = hash, - .data = texture, - }); + *(Asset*)mempool_alloc(&cache->assets) = (Asset){ + .type = TextureAsset, + .hash = hash, + .data = texture, + }; } return texture; } diff --git a/gfx/src/gfx_assert.h b/gfx/src/gfx_assert.h new file mode 100644 index 0000000..f4b3aa5 --- /dev/null +++ b/gfx/src/gfx_assert.h @@ -0,0 +1,5 @@ +#pragma once + +#include + +#include // Include after log to use log's LOGE(). diff --git a/gfx/src/render/buffer.c b/gfx/src/render/buffer.c index 55c68cc..28877bb 100644 --- a/gfx/src/render/buffer.c +++ b/gfx/src/render/buffer.c @@ -1,6 +1,7 @@ #include "buffer.h" #include +#include #include #include @@ -18,7 +19,7 @@ static GLenum get_buffer_usage(BufferUsage usage) { case BufferDynamic: return GL_DYNAMIC_DRAW; } - assert(false); + FAIL("Unhandled buffer usage"); return GL_STATIC_DRAW; } @@ -39,21 +40,24 @@ size_t gfx_get_buffer_type_size_bytes(BufferType type) { case BufferU16: return sizeof(uint16_t); } - assert(false); + FAIL("Unhandled buffer type"); return 0; } bool gfx_init_buffer(Buffer* buffer, const BufferDesc* desc) { assert(buffer); + buffer->type = desc->type; buffer->usage = desc->usage; buffer->size_bytes = get_buffer_size_bytes(desc->type, &desc->data); const GLenum usage = get_buffer_usage(desc->usage); + glGenBuffers(1, &buffer->vbo); glBindBuffer(GL_ARRAY_BUFFER, buffer->vbo); glBufferData(GL_ARRAY_BUFFER, buffer->size_bytes, desc->data.data, usage); glBindBuffer(GL_ARRAY_BUFFER, 0); ASSERT_GL; + return true; } diff --git a/gfx/src/render/framebuffer.c b/gfx/src/render/framebuffer.c index 84ade4a..2ea2165 100644 --- a/gfx/src/render/framebuffer.c +++ b/gfx/src/render/framebuffer.c @@ -3,9 +3,9 @@ #include "renderbuffer.h" #include "texture.h" -#include +#include -#include +#include static void framebuffer_attach_colour( FrameBuffer* framebuffer, const FrameBufferAttachment* attachment) { diff --git a/gfx/src/render/geometry.c b/gfx/src/render/geometry.c index 44bfd04..8974387 100644 --- a/gfx/src/render/geometry.c +++ b/gfx/src/render/geometry.c @@ -3,6 +3,8 @@ #include "buffer.h" #include "constants.h" +#include + #include #include @@ -20,10 +22,9 @@ static GLenum primitive_type_to_gl(PrimitiveType type) { return GL_TRIANGLE_FAN; case TriangleStrip: return GL_TRIANGLE_STRIP; - default: - LOGE("primitive_type_to_gl(): missing case"); - return GL_INVALID_ENUM; } + FAIL("primitive_type_to_gl(): missing case"); + return GL_INVALID_ENUM; } /// Create a typed buffer for the buffer view if the view does not already point @@ -300,7 +301,7 @@ void gfx_update_geometry(Geometry* geometry, const GeometryDesc* desc) { } // TODO: more else { - assert(false && "TODO: gfx_update_geometry() - handle other buffer types"); + FAIL("TODO: gfx_update_geometry() - handle other buffer types"); } if (desc->num_verts != 0) { diff --git a/gfx/src/render/render_backend.c b/gfx/src/render/render_backend.c index defc164..4e783f8 100644 --- a/gfx/src/render/render_backend.c +++ b/gfx/src/render/render_backend.c @@ -2,7 +2,6 @@ #include "gl_util.h" -#include // #include #include @@ -138,10 +137,8 @@ void gfx_set_polygon_offset( Buffer* gfx_make_buffer(RenderBackend* render_backend, const BufferDesc* desc) { assert(render_backend); assert(desc); + Buffer* buffer = mempool_alloc(&render_backend->buffers); - if (!buffer) { - return 0; - } if (!gfx_init_buffer(buffer, desc)) { mempool_free(&render_backend->buffers, &buffer); return 0; @@ -152,8 +149,10 @@ Buffer* gfx_make_buffer(RenderBackend* render_backend, const BufferDesc* desc) { void gfx_destroy_buffer(RenderBackend* render_backend, Buffer** buffer) { assert(render_backend); assert(buffer); - gfx_del_buffer(*buffer); - mempool_free(&render_backend->buffers, buffer); + if (*buffer) { + gfx_del_buffer(*buffer); + mempool_free(&render_backend->buffers, buffer); + } } // ----------------------------------------------------------------------------- @@ -164,10 +163,8 @@ Geometry* gfx_make_geometry( RenderBackend* render_backend, const GeometryDesc* desc) { assert(render_backend); assert(desc); + Geometry* geometry = mempool_alloc(&render_backend->geometries); - if (!geometry) { - return 0; - } if (!gfx_init_geometry(geometry, render_backend, desc)) { mempool_free(&render_backend->geometries, &geometry); return 0; @@ -178,8 +175,11 @@ Geometry* gfx_make_geometry( void gfx_destroy_geometry(RenderBackend* render_backend, Geometry** geometry) { assert(render_backend); assert(geometry); - gfx_del_geometry(*geometry); - mempool_free(&render_backend->geometries, geometry); + + if (*geometry) { + gfx_del_geometry(*geometry); + mempool_free(&render_backend->geometries, geometry); + } } // ----------------------------------------------------------------------------- @@ -190,10 +190,8 @@ Texture* gfx_make_texture( RenderBackend* render_backend, const TextureDesc* desc) { assert(render_backend); assert(desc); + Texture* texture = mempool_alloc(&render_backend->textures); - if (!texture) { - return 0; - } if (!gfx_init_texture(texture, desc)) { mempool_free(&render_backend->textures, &texture); return 0; @@ -205,8 +203,11 @@ void gfx_destroy_texture(RenderBackend* render_backend, Texture** texture) { assert(render_backend); assert(texture); assert(*texture); - gfx_del_texture(*texture); - mempool_free(&render_backend->textures, texture); + + if (*texture) { + gfx_del_texture(*texture); + mempool_free(&render_backend->textures, texture); + } } // ----------------------------------------------------------------------------- @@ -217,10 +218,8 @@ RenderBuffer* gfx_make_renderbuffer( RenderBackend* render_backend, const RenderBufferDesc* desc) { assert(render_backend); assert(desc); + RenderBuffer* renderbuffer = mempool_alloc(&render_backend->renderbuffers); - if (!renderbuffer) { - return 0; - } if (!gfx_init_renderbuffer(renderbuffer, desc)) { mempool_free(&render_backend->renderbuffers, &renderbuffer); } @@ -232,8 +231,11 @@ void gfx_destroy_renderbuffer( assert(render_backend); assert(renderbuffer); assert(*renderbuffer); - gfx_del_renderbuffer(*renderbuffer); - mempool_free(&render_backend->renderbuffers, renderbuffer); + + if (*renderbuffer) { + gfx_del_renderbuffer(*renderbuffer); + mempool_free(&render_backend->renderbuffers, renderbuffer); + } } // ----------------------------------------------------------------------------- @@ -244,10 +246,8 @@ FrameBuffer* gfx_make_framebuffer( RenderBackend* render_backend, const FrameBufferDesc* desc) { assert(render_backend); assert(desc); + FrameBuffer* framebuffer = mempool_alloc(&render_backend->framebuffers); - if (!framebuffer) { - return 0; - } if (!gfx_init_framebuffer(framebuffer, desc)) { mempool_free(&render_backend->framebuffers, &framebuffer); return 0; @@ -260,8 +260,11 @@ void gfx_destroy_framebuffer( assert(render_backend); assert(framebuffer); assert(*framebuffer); - gfx_del_framebuffer(*framebuffer); - mempool_free(&render_backend->framebuffers, framebuffer); + + if (*framebuffer) { + gfx_del_framebuffer(*framebuffer); + mempool_free(&render_backend->framebuffers, framebuffer); + } } // ----------------------------------------------------------------------------- @@ -340,7 +343,7 @@ Shader* gfx_make_shader(RenderBackend* render_backend, const ShaderDesc* desc) { const uint64_t hash = hash_shader_desc(desc); Shader* shader = find_cached_shader(cache, hash); if (shader) { - // LOGD("Found cached shader with hash %lx", hash); + // LOGD("Found cached shader with hash [%lx]", hash); return shader; } @@ -353,25 +356,25 @@ Shader* gfx_make_shader(RenderBackend* render_backend, const ShaderDesc* desc) { return 0; } ShaderCacheEntry* entry = mempool_alloc(cache); - assert(entry); - *entry = (ShaderCacheEntry){.hash = hash, .shader = shader}; - // LOGD("Added shader with hash %lx to cache", hash); + *entry = (ShaderCacheEntry){.hash = hash, .shader = shader}; + // LOGD("Added shader with hash [%lx] to cache", hash); return shader; } void gfx_destroy_shader(RenderBackend* render_backend, Shader** shader) { assert(render_backend); assert(shader); - assert(*shader); - // Remove the shader from the cache. - ShaderCache* cache = &render_backend->shader_cache; - ShaderCacheEntry* entry = find_shader_cache_entry(cache, *shader); - assert(entry); // Must be there, shaders can't go untracked. - mempool_free(cache, &entry); + if (*shader) { + // Remove the shader from the cache. + ShaderCache* cache = &render_backend->shader_cache; + ShaderCacheEntry* entry = find_shader_cache_entry(cache, *shader); + assert(entry); // Must be there, shaders can't go untracked. + mempool_free(cache, &entry); - gfx_del_shader(*shader); - mempool_free(&render_backend->shaders, shader); + gfx_del_shader(*shader); + mempool_free(&render_backend->shaders, shader); + } } ShaderProgram* gfx_make_shader_program( @@ -384,22 +387,18 @@ ShaderProgram* gfx_make_shader_program( const uint64_t hash = hash_program_desc(desc); ShaderProgram* prog = find_cached_program(cache, hash); if (prog) { - // LOGD("Found cached shader program with hash %lx", hash); + // LOGD("Found cached shader program with hash [%lx]", hash); return prog; } prog = mempool_alloc(&render_backend->shader_programs); - if (!prog) { - return 0; - } if (!gfx_build_shader_program(prog, desc)) { mempool_free(&render_backend->shader_programs, &prog); return 0; } ShaderProgramCacheEntry* entry = mempool_alloc(cache); - assert(entry); *entry = (ShaderProgramCacheEntry){.hash = hash, .program = prog}; - // LOGD("Added shader program with hash %lx to cache", hash); + // LOGD("Added shader program with hash [%lx] to cache", hash); return prog; } @@ -407,14 +406,14 @@ void gfx_destroy_shader_program( RenderBackend* render_backend, ShaderProgram** prog) { assert(render_backend); assert(prog); - assert(*prog); - - // Remove the shader program from the cache. - ProgramCache* cache = &render_backend->program_cache; - ShaderProgramCacheEntry* entry = find_program_cache_entry(cache, *prog); - assert(entry); // Must be there, shaders can't go untracked. - mempool_free(cache, &entry); - - gfx_del_shader_program(*prog); - mempool_free(&render_backend->shader_programs, prog); + if (*prog) { + // Remove the shader program from the cache. + ProgramCache* cache = &render_backend->program_cache; + ShaderProgramCacheEntry* entry = find_program_cache_entry(cache, *prog); + assert(entry); // Must be there, shaders can't go untracked. + mempool_free(cache, &entry); + + gfx_del_shader_program(*prog); + mempool_free(&render_backend->shader_programs, prog); + } } diff --git a/gfx/src/render/renderbuffer.c b/gfx/src/render/renderbuffer.c index a2eae52..23f9524 100644 --- a/gfx/src/render/renderbuffer.c +++ b/gfx/src/render/renderbuffer.c @@ -27,6 +27,7 @@ bool gfx_init_renderbuffer( void gfx_del_renderbuffer(RenderBuffer* renderbuffer) { assert(renderbuffer); + if (renderbuffer->id) { glDeleteRenderbuffers(1, &renderbuffer->id); renderbuffer->id = 0; diff --git a/gfx/src/render/shader.c b/gfx/src/render/shader.c index d4778a2..af2f89f 100644 --- a/gfx/src/render/shader.c +++ b/gfx/src/render/shader.c @@ -1,11 +1,11 @@ #include "shader.h" #include "gl_util.h" +#include #include #include -#include #include #include @@ -15,10 +15,9 @@ static GLenum shader_type_to_gl(ShaderType type) { return GL_VERTEX_SHADER; case FragmentShader: return GL_FRAGMENT_SHADER; - default: - LOGE("shader_type_to_gl(): missing case"); - return GL_INVALID_ENUM; } + FAIL("shader_type_to_gl(): missing case"); + return GL_INVALID_ENUM; } static lstring make_defines_string(const ShaderDesc* desc) { @@ -84,6 +83,7 @@ bool gfx_compile_shader(Shader* shader, const ShaderDesc* desc) { void gfx_del_shader(Shader* shader) { assert(shader); + if (shader->id) { glDeleteShader(shader->id); shader->id = 0; diff --git a/gfx/src/render/shader_program.c b/gfx/src/render/shader_program.c index d80ee4f..3cbe48d 100644 --- a/gfx/src/render/shader_program.c +++ b/gfx/src/render/shader_program.c @@ -3,10 +3,10 @@ #include "gl_util.h" #include "shader.h" #include "texture.h" +#include #include -#include #include #include @@ -45,12 +45,14 @@ bool gfx_build_shader_program( ShaderProgram* prog, const ShaderProgramDesc* desc) { assert(prog); assert(desc); + prog->id = create_program(desc->vertex_shader->id, desc->fragment_shader->id); return prog->id != 0; } void gfx_del_shader_program(ShaderProgram* prog) { assert(prog); + if (prog->id) { glDeleteProgram(prog->id); prog->id = 0; @@ -75,6 +77,7 @@ static void set_texture_uniform( assert(prog != 0); assert(name); assert(texture); + const GLint location = glGetUniformLocation(prog, name); if (location >= 0) { glActiveTexture(GL_TEXTURE0 + texture_unit); @@ -88,6 +91,7 @@ static void set_mat4_uniform( assert(prog != 0); assert(name); assert(mats); + const GLint location = glGetUniformLocation(prog, name); if (location >= 0) { glUniformMatrix4fv(location, count, GL_FALSE, (const float*)mats); @@ -97,6 +101,7 @@ static void set_mat4_uniform( static void set_vec3_uniform(GLuint prog, const char* name, vec3 value) { assert(prog != 0); assert(name); + const GLint location = glGetUniformLocation(prog, name); if (location >= 0) { glUniform3f(location, value.x, value.y, value.z); @@ -106,6 +111,7 @@ static void set_vec3_uniform(GLuint prog, const char* name, vec3 value) { static void set_vec4_uniform(GLuint prog, const char* name, vec4 value) { assert(prog != 0); assert(name); + const GLint location = glGetUniformLocation(prog, name); if (location >= 0) { glUniform4f(location, value.x, value.y, value.z, value.w); @@ -115,6 +121,7 @@ static void set_vec4_uniform(GLuint prog, const char* name, vec4 value) { static void set_float_uniform(GLuint prog, const char* name, float value) { assert(prog != 0); assert(name); + const GLint location = glGetUniformLocation(prog, name); if (location >= 0) { glUniform1f(location, value); @@ -123,6 +130,7 @@ static void set_float_uniform(GLuint prog, const char* name, float value) { void gfx_apply_uniforms(const ShaderProgram* prog) { assert(prog); + int next_texture_unit = 0; for (int i = 0; i < prog->num_uniforms; ++i) { const ShaderUniform* uniform = &prog->uniforms[i]; @@ -160,6 +168,7 @@ static ShaderUniform* get_or_allocate_uniform( ShaderProgram* prog, const char* name) { assert(prog); assert(name); + // First search for the uniform in the list. for (int i = 0; i < prog->num_uniforms; ++i) { ShaderUniform* uniform = &prog->uniforms[i]; @@ -167,11 +176,11 @@ static ShaderUniform* get_or_allocate_uniform( return uniform; } } + // Create the uniform if it does not exist. if (prog->num_uniforms == GFX_MAX_UNIFORMS_PER_SHADER) { - LOGE("Exceeded the maximum number of uniforms per shader. Please increase " + FAIL("Exceeded the maximum number of uniforms per shader. Please increase " "this value."); - assert(false); return 0; } ShaderUniform* uniform = &prog->uniforms[prog->num_uniforms]; @@ -187,6 +196,7 @@ void gfx_set_texture_uniform( assert(prog); assert(name); assert(texture); + const GLint location = glGetUniformLocation(prog->id, name); if (location < 0) { return; @@ -203,6 +213,7 @@ void gfx_set_mat4_uniform( assert(prog); assert(name); assert(mat); + const GLint location = glGetUniformLocation(prog->id, name); if (location < 0) { return; @@ -217,6 +228,7 @@ void gfx_set_mat4_uniform( void gfx_set_vec3_uniform(ShaderProgram* prog, const char* name, vec3 value) { assert(prog); assert(name); + const GLint location = glGetUniformLocation(prog->id, name); if (location < 0) { return; @@ -231,6 +243,7 @@ void gfx_set_vec3_uniform(ShaderProgram* prog, const char* name, vec3 value) { void gfx_set_vec4_uniform(ShaderProgram* prog, const char* name, vec4 value) { assert(prog); assert(name); + const GLint location = glGetUniformLocation(prog->id, name); if (location < 0) { return; @@ -245,6 +258,7 @@ void gfx_set_vec4_uniform(ShaderProgram* prog, const char* name, vec4 value) { void gfx_set_float_uniform(ShaderProgram* prog, const char* name, float value) { assert(prog); assert(name); + // No need to store the uniform on our side if it does not exist in the // program. const GLint location = glGetUniformLocation(prog->id, name); @@ -263,6 +277,7 @@ void gfx_set_mat4_array_uniform( assert(prog); assert(name); assert(mats); + const GLint location = glGetUniformLocation(prog->id, name); if (location < 0) { return; diff --git a/gfx/src/render/texture.c b/gfx/src/render/texture.c index 489f7a2..fd1d4dd 100644 --- a/gfx/src/render/texture.c +++ b/gfx/src/render/texture.c @@ -1,9 +1,10 @@ #include "texture.h" +#include + #include #include -#include #include bool gfx_init_texture(Texture* texture, const TextureDesc* desc) { @@ -32,7 +33,7 @@ bool gfx_init_texture(Texture* texture, const TextureDesc* desc) { texture->target, levels, internal_format, desc->width, desc->height); break; default: - assert(false && "Unhandled texture dimension"); + FAIL("Unhandled texture dimension"); gfx_del_texture(texture); return false; } @@ -79,6 +80,8 @@ bool gfx_init_texture(Texture* texture, const TextureDesc* desc) { } void gfx_del_texture(Texture* texture) { + assert(texture); + if (texture->id) { glDeleteTextures(1, &texture->id); texture->id = 0; @@ -113,7 +116,7 @@ void gfx_update_texture(Texture* texture, const TextureDataDesc* desc) { } break; default: - assert(false && "Unhandled texture dimension"); + FAIL("Unhandled texture dimension"); break; } @@ -127,7 +130,7 @@ GLenum to_GL_dimension(TextureDimension dim) { case TextureCubeMap: return GL_TEXTURE_CUBE_MAP; default: - assert(false && "Unhandled TextureDimension"); + FAIL("Unhandled TextureDimension"); return GL_INVALID_ENUM; } } @@ -151,7 +154,7 @@ GLenum to_GL_internal_format(TextureFormat format) { case TextureSRGBA8: return GL_SRGB8_ALPHA8; default: - assert(false && "Unhandled TextureFormat"); + FAIL("Unhandled TextureFormat"); return GL_INVALID_ENUM; } } @@ -171,7 +174,7 @@ GLenum to_GL_format(TextureFormat format) { case TextureSRGBA8: return GL_RGBA; default: - assert(false && "Unhandled TextureFormat"); + FAIL("Unhandled TextureFormat"); return GL_INVALID_ENUM; } } @@ -189,7 +192,7 @@ GLenum to_GL_type(TextureFormat format) { case TextureSRGBA8: return GL_UNSIGNED_BYTE; default: - assert(false && "Unhandled TextureFormat"); + FAIL("Unhandled TextureFormat"); return GL_INVALID_ENUM; } } @@ -209,7 +212,7 @@ GLenum to_GL_cubemap_face(CubemapFace face) { case CubemapFaceNegZ: return GL_TEXTURE_CUBE_MAP_NEGATIVE_Z; default: - assert(false && "Unhandled CubemapFace"); + FAIL("Unhandled CubemapFace"); return GL_INVALID_ENUM; } } diff --git a/gfx/src/scene/animation.c b/gfx/src/scene/animation.c index 8d8178e..459e864 100644 --- a/gfx/src/scene/animation.c +++ b/gfx/src/scene/animation.c @@ -13,8 +13,7 @@ Joint* gfx_make_joint(const JointDesc* desc) { // The joint matrix needs to be initialized so that meshes look right even if // no animation is played. Initializing joint matrices to the identity makes // meshes appear in their bind pose. - Joint* joint = mem_alloc_joint(); - assert(joint); + Joint* joint = mem_alloc_joint(); joint->inv_bind_matrix = desc->inv_bind_matrix; joint->joint_matrix = mat4_id(); return joint; @@ -22,20 +21,20 @@ Joint* gfx_make_joint(const JointDesc* desc) { void gfx_destroy_joint(Joint** joint) { assert(joint); - assert(*joint); - if ((*joint)->parent.val) { - gfx_del_node((*joint)->parent); + if (*joint) { + if ((*joint)->parent.val) { + gfx_del_node((*joint)->parent); + } + mem_free_joint(joint); } - mem_free_joint(joint); } static Skeleton* make_skeleton(const SkeletonDesc* desc) { assert(desc); assert(desc->num_joints < GFX_MAX_NUM_JOINTS); - Skeleton* skeleton = mem_alloc_skeleton(); - assert(skeleton); + Skeleton* skeleton = mem_alloc_skeleton(); skeleton->num_joints = desc->num_joints; for (size_t i = 0; i < desc->num_joints; ++i) { skeleton->joints[i] = mem_get_node_index(desc->joints[i]); @@ -47,8 +46,7 @@ static Animation* make_animation(const AnimationDesc* desc) { assert(desc); assert(desc->num_channels < GFX_MAX_NUM_CHANNELS); - Animation* animation = mem_alloc_animation(); - assert(animation); + Animation* animation = mem_alloc_animation(); animation->name = desc->name; animation->duration = 0; animation->num_channels = desc->num_channels; @@ -86,9 +84,6 @@ Anima* gfx_make_anima(const AnimaDesc* desc) { assert(desc); Anima* anima = mem_alloc_anima(); - if (!anima) { - return 0; - } // Wire the skeletons in the same order they are given in the descriptor. Skeleton* last_skeleton = 0; @@ -97,7 +92,6 @@ Anima* gfx_make_anima(const AnimaDesc* desc) { // TODO: Here and everywhere else, I think it would simplify the code // greatly to make mem_alloc_xyz() fail if the allocation fails. At that // point the user should just bump their memory limits. - assert(skeleton); const skeleton_idx skeleton_index = mem_get_skeleton_index(skeleton); if (last_skeleton == 0) { anima->skeleton = skeleton_index; @@ -110,8 +104,7 @@ Anima* gfx_make_anima(const AnimaDesc* desc) { // Wire the animations in the same order they are given in the descriptor. Animation* last_animation = 0; for (size_t i = 0; i < desc->num_animations; ++i) { - Animation* animation = make_animation(&desc->animations[i]); - assert(animation); + Animation* animation = make_animation(&desc->animations[i]); const animation_idx animation_index = mem_get_animation_index(animation); if (last_animation == 0) { anima->animation = animation_index; @@ -126,25 +119,26 @@ Anima* gfx_make_anima(const AnimaDesc* desc) { void gfx_destroy_anima(Anima** anima) { assert(anima); - assert(*anima); - for (skeleton_idx i = (*anima)->skeleton; i.val != 0;) { - Skeleton* skeleton = mem_get_skeleton(i); - i = skeleton->next; - mem_free_skeleton(&skeleton); - } + if (*anima) { + for (skeleton_idx i = (*anima)->skeleton; i.val != 0;) { + Skeleton* skeleton = mem_get_skeleton(i); + i = skeleton->next; + mem_free_skeleton(&skeleton); + } - for (animation_idx i = (*anima)->animation; i.val != 0;) { - Animation* animation = mem_get_animation(i); - i = animation->next; - mem_free_animation(&animation); - } + for (animation_idx i = (*anima)->animation; i.val != 0;) { + Animation* animation = mem_get_animation(i); + i = animation->next; + mem_free_animation(&animation); + } - if ((*anima)->parent.val) { - gfx_del_node((*anima)->parent); - } + if ((*anima)->parent.val) { + gfx_del_node((*anima)->parent); + } - mem_free_anima(anima); + mem_free_anima(anima); + } } static Animation* find_animation(animation_idx index, const char* name) { @@ -152,7 +146,6 @@ static Animation* find_animation(animation_idx index, const char* name) { while (index.val != 0) { Animation* animation = mem_get_animation(index); - assert(animation); if (sstring_eq_cstr(animation->name, name)) { // LOGD( // "Found animation at index %u, %s - %s", index.val, @@ -171,7 +164,7 @@ bool gfx_play_animation(Anima* anima, const AnimationPlaySettings* settings) { assert(settings); // TODO: Should we animate at t=0 here to kickstart the animation? Otherwise - // the client is forced to call gfx_update_animation() to do this. + // the client is forced to call gfx_update_animation() to do this. Animation* animation = find_animation(anima->animation, settings->name); if (!animation) { return false; @@ -188,6 +181,7 @@ static void find_keyframes(const Channel* channel, R t, int* prev, int* next) { assert(channel); assert(prev); assert(next); + *prev = -1; *next = 0; while (((*next + 1) < (int)channel->num_keyframes) && @@ -275,7 +269,6 @@ static void animate_channel(const Channel* channel, R t) { t = t > channel->keyframes[next].time ? channel->keyframes[next].time : t; SceneNode* target = mem_get_node(channel->target); - assert(target); switch (channel->type) { case RotationChannel: { @@ -306,8 +299,7 @@ static void compute_joint_matrices_rec( } const SceneNode* node = mem_get_node(node_index); - assert(node); - const mat4 global_joint_transform = + const mat4 global_joint_transform = mat4_mul(parent_global_joint_transform, node->transform); // For flexibility (glTF), we allow non-joint nodes between the root of the @@ -316,7 +308,6 @@ static void compute_joint_matrices_rec( if (node->type == JointNode) { // Compute this node's joint matrix. Joint* joint = mem_get_joint(node->joint); - assert(joint); joint->joint_matrix = mat4_mul( *root_inv_global_transform, @@ -328,8 +319,7 @@ static void compute_joint_matrices_rec( while (child.val != 0) { compute_joint_matrices_rec( child, global_joint_transform, root_inv_global_transform); - node = mem_get_node(child); - assert(node); + node = mem_get_node(child); child = node->next; // Next sibling. } } @@ -381,7 +371,6 @@ void gfx_update_animation(Anima* anima, R t) { // once (and potentially other non-joint intermediate nodes). node_idx root_index = anima->parent; SceneNode* root = mem_get_node(root_index); - assert(root); // LOGD("Root: %u, child: %u", root_index.val, root->child.val); const mat4 root_global_transform = gfx_get_node_global_transform(root); const mat4 root_inv_global_transform = mat4_inverse(root_global_transform); @@ -391,8 +380,7 @@ void gfx_update_animation(Anima* anima, R t) { compute_joint_matrices_rec( child, root_global_transform, &root_inv_global_transform); SceneNode* node = mem_get_node(child); - assert(node); - child = node->next; // Next sibling. + child = node->next; // Next sibling. } } diff --git a/gfx/src/scene/camera.c b/gfx/src/scene/camera.c index df161c2..be7d806 100644 --- a/gfx/src/scene/camera.c +++ b/gfx/src/scene/camera.c @@ -5,29 +5,24 @@ #include -static void scene_camera_make(SceneCamera* camera) { - assert(camera); - camera->camera = - camera_perspective(/*fovy=*/90.0 * TO_RAD, /*aspect=*/16.0 / 9.0, - /*near=*/0.1, /*far=*/1000); -} - SceneCamera* gfx_make_camera() { SceneCamera* camera = mem_alloc_camera(); - if (!camera) { - return 0; - } - scene_camera_make(camera); + + camera->camera = camera_perspective( + /*fovy=*/90.0 * TO_RAD, /*aspect=*/16.0 / 9.0, + /*near=*/0.1, /*far=*/1000); + return camera; } void gfx_destroy_camera(SceneCamera** camera) { assert(camera); - assert(*camera); - if ((*camera)->parent.val) { - gfx_del_node((*camera)->parent); + if (*camera) { + if ((*camera)->parent.val) { + gfx_del_node((*camera)->parent); + } + mem_free_camera(camera); } - mem_free_camera(camera); } void gfx_set_camera_camera(SceneCamera* scene_camera, Camera* camera) { diff --git a/gfx/src/scene/light.c b/gfx/src/scene/light.c index 31dca77..1046c82 100644 --- a/gfx/src/scene/light.c +++ b/gfx/src/scene/light.c @@ -9,7 +9,6 @@ static void make_environment_light( Light* light, const EnvironmentLightDesc* desc) { assert(light); assert(desc); - light->type = EnvironmentLightType; light->environment.environment_map = desc->environment_map; } @@ -18,9 +17,6 @@ Light* gfx_make_light(const LightDesc* desc) { assert(desc); Light* light = mem_alloc_light(); - if (!light) { - return 0; - } switch (desc->type) { case EnvironmentLightType: @@ -37,9 +33,10 @@ Light* gfx_make_light(const LightDesc* desc) { void gfx_destroy_light(Light** light) { assert(light); - assert(*light); - if ((*light)->parent.val) { - gfx_del_node((*light)->parent); + if (*light) { + if ((*light)->parent.val) { + gfx_del_node((*light)->parent); + } + mem_free_light(light); } - mem_free_light(light); } diff --git a/gfx/src/scene/material.c b/gfx/src/scene/material.c index 6d6decb..b32d791 100644 --- a/gfx/src/scene/material.c +++ b/gfx/src/scene/material.c @@ -17,9 +17,6 @@ static void material_make(Material* material, const MaterialDesc* desc) { Material* gfx_make_material(const MaterialDesc* desc) { assert(desc); Material* material = mem_alloc_material(); - if (!material) { - return 0; - } material_make(material, desc); return material; } diff --git a/gfx/src/scene/mesh.c b/gfx/src/scene/mesh.c index 689105c..1a93bed 100644 --- a/gfx/src/scene/mesh.c +++ b/gfx/src/scene/mesh.c @@ -17,9 +17,6 @@ static void mesh_make(Mesh* mesh, const MeshDesc* desc) { Mesh* gfx_make_mesh(const MeshDesc* desc) { Mesh* mesh = mem_alloc_mesh(); - if (!mesh) { - return 0; - } mesh_make(mesh, desc); return mesh; } diff --git a/gfx/src/scene/node.c b/gfx/src/scene/node.c index 2670680..2f761a2 100644 --- a/gfx/src/scene/node.c +++ b/gfx/src/scene/node.c @@ -19,9 +19,6 @@ static void scene_node_make(SceneNode* node) { SceneNode* gfx_make_node() { SceneNode* node = mem_alloc_node(); - if (!node) { - return 0; - } scene_node_make(node); return node; } @@ -29,60 +26,45 @@ SceneNode* gfx_make_node() { SceneNode* gfx_make_anima_node(Anima* anima) { assert(anima); SceneNode* node = gfx_make_node(); - if (!node) { - return 0; - } - node->type = AnimaNode; - node->anima = mem_get_anima_index(anima); - anima->parent = mem_get_node_index(node); + node->type = AnimaNode; + node->anima = mem_get_anima_index(anima); + anima->parent = mem_get_node_index(node); return node; } SceneNode* gfx_make_camera_node(SceneCamera* camera) { assert(camera); SceneNode* node = gfx_make_node(); - if (!node) { - return 0; - } - node->type = CameraNode; - node->camera = mem_get_camera_index(camera); - camera->parent = mem_get_node_index(node); + node->type = CameraNode; + node->camera = mem_get_camera_index(camera); + camera->parent = mem_get_node_index(node); return node; } SceneNode* gfx_make_joint_node(Joint* joint) { assert(joint); SceneNode* node = gfx_make_node(); - if (!node) { - return 0; - } - node->type = JointNode; - node->joint = mem_get_joint_index(joint); - joint->parent = mem_get_node_index(node); + node->type = JointNode; + node->joint = mem_get_joint_index(joint); + joint->parent = mem_get_node_index(node); return node; } SceneNode* gfx_make_light_node(Light* light) { assert(light); SceneNode* node = gfx_make_node(); - if (!node) { - return 0; - } - node->type = LightNode; - node->light = mem_get_light_index(light); - light->parent = mem_get_node_index(node); + node->type = LightNode; + node->light = mem_get_light_index(light); + light->parent = mem_get_node_index(node); return node; } SceneNode* gfx_make_object_node(SceneObject* object) { assert(object); SceneNode* node = gfx_make_node(); - if (!node) { - return 0; - } - node->type = ObjectNode; - node->object = mem_get_object_index(object); - object->parent = mem_get_node_index(node); + node->type = ObjectNode; + node->object = mem_get_object_index(object); + object->parent = mem_get_node_index(node); return node; } @@ -131,7 +113,6 @@ static void free_node_resource(SceneNode* node) { void gfx_construct_anima_node(SceneNode* node, Anima* anima) { assert(node); assert(anima); - free_node_resource(node); node->type = AnimaNode; node->anima = mem_get_anima_index(anima); @@ -141,7 +122,6 @@ void gfx_construct_anima_node(SceneNode* node, Anima* anima) { void gfx_construct_camera_node(SceneNode* node, SceneCamera* camera) { assert(node); assert(camera); - free_node_resource(node); node->type = CameraNode; node->camera = mem_get_camera_index(camera); @@ -153,7 +133,6 @@ void gfx_construct_camera_node(SceneNode* node, SceneCamera* camera) { void gfx_construct_joint_node(SceneNode* node, Joint* joint) { assert(node); assert(joint); - free_node_resource(node); node->type = JointNode; node->joint = mem_get_joint_index(joint); @@ -163,7 +142,6 @@ void gfx_construct_joint_node(SceneNode* node, Joint* joint) { void gfx_construct_light_node(SceneNode* node, Light* light) { assert(node); assert(light); - free_node_resource(node); node->type = LightNode; node->light = mem_get_light_index(light); @@ -173,7 +151,6 @@ void gfx_construct_light_node(SceneNode* node, Light* light) { void gfx_construct_object_node(SceneNode* node, SceneObject* object) { assert(node); assert(object); - free_node_resource(node); node->type = ObjectNode; node->object = mem_get_object_index(object); @@ -199,20 +176,18 @@ static void destroy_node_rec(SceneNode* node) { void gfx_destroy_node(SceneNode** node) { assert(node); - assert(*node); - - // Since the node and the whole hierarchy under it gets destroyed, there is - // no need to individually detach every node from its hierarchy. We can simply - // detach the given node and then destroy it and its sub-hierarchy. - TREE_REMOVE(*node); - - destroy_node_rec(*node); - - *node = 0; + if (*node) { + // Since the node and the whole hierarchy under it gets destroyed, there is + // no need to individually detach every node from its hierarchy. We can + // simply detach the given node and then destroy it and its sub-hierarchy. + TREE_REMOVE(*node); + destroy_node_rec(*node); + *node = 0; + } } // TODO: Think more about ownership of nodes and resources. Should this function -// even exist? +// even exist? void gfx_del_node(node_idx index) { assert(index.val); SceneNode* node = mem_get_node(index); diff --git a/gfx/src/scene/object.c b/gfx/src/scene/object.c index 64bb5a6..68a1340 100644 --- a/gfx/src/scene/object.c +++ b/gfx/src/scene/object.c @@ -15,20 +15,18 @@ static void scene_object_make(SceneObject* object) { SceneObject* gfx_make_object() { SceneObject* object = mem_alloc_object(); - if (!object) { - return 0; - } scene_object_make(object); return object; } void gfx_destroy_object(SceneObject** object) { assert(object); - assert(*object); - if ((*object)->parent.val) { - gfx_del_node((*object)->parent); + if (*object) { + if ((*object)->parent.val) { + gfx_del_node((*object)->parent); + } + mem_free_object(object); } - mem_free_object(object); } void gfx_set_object_transform(SceneObject* object, const mat4* transform) { @@ -80,8 +78,8 @@ void gfx_set_object_skeleton(SceneObject* object, const Skeleton* skeleton) { } // TODO: Could compute just once if we changed the Object API to require an -// object descriptor up front instead of allowing the client to add meshes -// and skeletons dynamically. +// object descriptor up front instead of allowing the client to add meshes +// and skeletons dynamically. aabb3 gfx_calc_object_aabb(const SceneObject* object) { assert(object); diff --git a/gfx/src/scene/scene.c b/gfx/src/scene/scene.c index c3dae9c..a6801ba 100644 --- a/gfx/src/scene/scene.c +++ b/gfx/src/scene/scene.c @@ -7,19 +7,16 @@ Scene* gfx_make_scene(void) { Scene* scene = mem_alloc_scene(); - if (!scene) { - return 0; - } - scene->root = gfx_make_node(); - assert(scene->root); + scene->root = gfx_make_node(); return scene; } void gfx_destroy_scene(Scene** scene) { assert(scene); - assert(*scene); - gfx_destroy_node(&(*scene)->root); - mem_free_scene(scene); + if (*scene) { + gfx_destroy_node(&(*scene)->root); + mem_free_scene(scene); + } } SceneNode* gfx_get_scene_root(Scene* scene) { -- cgit v1.2.3