# HG changeset patch # User Mike Becker # Date 1750584997 -7200 # Node ID 3628cc3c04831ff8838c1abc8ff06b298e1bd5a1 # Parent d3598c834f9b2e742347679bb3a9396213115831 change NULL-safety for shaders s.t. errors are reported only once per shader relates to #690 diff -r d3598c834f9b -r 3628cc3c0483 src/2d.c --- a/src/2d.c Sun Jun 22 11:15:53 2025 +0200 +++ b/src/2d.c Sun Jun 22 11:36:37 2025 +0200 @@ -64,10 +64,11 @@ asc_error("Loading rectangle shader failed."); return NULL; } + // TODO: find better way to deal with inheritance (&shader->program just looks bad) AscRectangleShader *shader = asc_shader_create(codes, sizeof(*shader)); - if (shader == NULL) { + if (asc_shader_invalid(&shader->program)) { asc_shader_free_codes(codes); - return NULL; + return (AscShaderProgram*) shader; } shader->size = glGetUniformLocation(shader->program.gl_id, "size"); if (asc_test_flag(flags, ASC_RECTANGLE_SHADER_FLAG_FILL)) { @@ -132,7 +133,8 @@ // Look up and activate shader const AscRectangleShader *shader = asc_shader_lookup_or_create( shader_ids[shader_flags], asc_rectangle_shader_create, shader_flags); - if (shader == NULL) return; + // TODO: how to remove the following cast? + if (asc_shader_invalid((AscShaderProgram*)shader)) return; asc_shader_use(&shader->program, camera); // Upload uniforms diff -r d3598c834f9b -r 3628cc3c0483 src/ascension/shader.h --- a/src/ascension/shader.h Sun Jun 22 11:15:53 2025 +0200 +++ b/src/ascension/shader.h Sun Jun 22 11:36:37 2025 +0200 @@ -108,6 +108,13 @@ */ #define ASC_SHADER_ID_USER_MAX 1'000'000'000u +/** + * A function to create a shader program. + * This is only allowed to return @c NULL when loading the code + * files from disk fails. In all other cases, this must + * at least return a valid pointer to a program for which + * asc_shader_invalid() returns @c true. + */ typedef AscShaderProgram*(*asc_shader_create_func)(int); /** @@ -127,6 +134,16 @@ void asc_shader_free_codes(AscShaderCodes codes); /** + * Checks if the shader received a correct Open GL ID. + * + * If it did not, there is something wrong (compilation or linking failed). + * + * @param program the program + * @return true if the shader was not created successfully + */ +bool asc_shader_invalid(const AscShaderProgram *program); + +/** * Creates a shader program. * * @note This function intentionally has an unspecified pointer return type diff -r d3598c834f9b -r 3628cc3c0483 src/shader.c --- a/src/shader.c Sun Jun 22 11:15:53 2025 +0200 +++ b/src/shader.c Sun Jun 22 11:36:37 2025 +0200 @@ -99,20 +99,23 @@ /** * This function links shaders into a program. * - * The ID of the returned program will be zero when something went wrong. + * The OpenGL ID of the program will be zero when something went wrong. * * @param shader the shader IDs to link * @param n the number of shaders * @param prog the struct where to store the result - * @retval zero success - * @retval non-zero failure */ -static int asc_shader_link(unsigned shader[], unsigned n, AscShaderProgram *prog) { +static void asc_shader_link(unsigned shader[], unsigned n, AscShaderProgram *prog) { + prog->gl_id = 0; GLint success; GLint id = glCreateProgram(); if (id <= 0) { + for (unsigned i = 0; i < n; i++) { + asc_dprintf("Delete shader: %u", shader[i]); + glDeleteShader(shader[i]); + } asc_error("glCreateProgram failed: %s", glGetError()); - return -1; + return; } for (unsigned i = 0; i < n; i++) { glAttachShader(id, shader[i]); @@ -120,6 +123,7 @@ glLinkProgram(id); glGetProgramiv(id, GL_LINK_STATUS, &success); for (unsigned i = 0; i < n; i++) { + asc_dprintf("Delete shader: %u", shader[i]); glDeleteShader(shader[i]); } if (success) { @@ -129,14 +133,12 @@ prog->model = glGetUniformLocation(id, "model"); prog->view = glGetUniformLocation(id, "view"); prog->projection = glGetUniformLocation(id, "projection"); - return 0; } else { char *log = malloc(1024); glGetProgramInfoLog(id, 1024, NULL, log); glDeleteProgram(id); asc_error("Linking shader program %u failed.\n%s", id, log); free(log); - return -1; } } @@ -151,32 +153,22 @@ cxFreeDefault(program); } +bool asc_shader_invalid(const AscShaderProgram *program) { + return program == NULL || program->gl_id == 0; +} + void *asc_shader_create(AscShaderCodes codes, size_t mem_size) { AscShaderProgram *prog = cxZallocDefault(mem_size); unsigned shader[2]; unsigned n = 0; - bool shader_compile_error = false; // TODO: clean up this pp mess by introducing proper nested structs if (codes.vtx) { - shader[n] = asc_shader_compile(GL_VERTEX_SHADER, codes.vtx, codes.vtx_pp, codes.vtx_pp_list, codes.vtx_pp_list_select); - shader_compile_error |= shader[n] == 0; - n++; + shader[n++] = asc_shader_compile(GL_VERTEX_SHADER, codes.vtx, codes.vtx_pp, codes.vtx_pp_list, codes.vtx_pp_list_select); } if (codes.frag) { - shader[n] = asc_shader_compile(GL_FRAGMENT_SHADER, codes.frag, codes.frag_pp, codes.frag_pp_list, codes.frag_pp_list_select); - shader_compile_error |= shader[n] == 0; - n++; + shader[n++] = asc_shader_compile(GL_FRAGMENT_SHADER, codes.frag, codes.frag_pp, codes.frag_pp_list, codes.frag_pp_list_select); } - if (shader_compile_error || asc_shader_link(shader, n, prog)) { - cxFreeDefault(prog); - prog = NULL; - } - for (unsigned i = 0; i < n; i++) { - if (shader[i] > 0) { - asc_dprintf("Delete shader: %u", shader[i]); - glDeleteShader(shader[i]); - } - } + asc_shader_link(shader, n, prog); return prog; } diff -r d3598c834f9b -r 3628cc3c0483 src/sprite.c --- a/src/sprite.c Sun Jun 22 11:15:53 2025 +0200 +++ b/src/sprite.c Sun Jun 22 11:36:37 2025 +0200 @@ -52,10 +52,11 @@ asc_error("Loading sprite shader failed."); return NULL; } + // TODO: find better way to deal with inheritance (&shader->program just looks bad) AscSpriteShader *shader = asc_shader_create(codes, sizeof(*shader)); - if (shader == NULL) { + if (asc_shader_invalid(&shader->program)) { asc_shader_free_codes(codes); - return NULL; + return (AscShaderProgram*) shader; } shader->tex = glGetUniformLocation(shader->program.gl_id, "tex"); asc_shader_free_codes(codes); @@ -99,7 +100,8 @@ sprite->texture->target == GL_TEXTURE_RECTANGLE ? asc_shader_lookup_or_create(ASC_SHADER_SPRITE_RECT, asc_sprite_shader_create, 1) : asc_shader_lookup_or_create(ASC_SHADER_SPRITE_UV, asc_sprite_shader_create, 0); - if (shader == NULL) return; + // TODO: how to remove the following cast? + if (asc_shader_invalid((AscShaderProgram*)shader)) return; asc_shader_use(&shader->program, camera); // Upload model matrix