change NULL-safety for shaders s.t. errors are reported only once per shader default tip

Sun, 22 Jun 2025 11:36:37 +0200

author
Mike Becker <universe@uap-core.de>
date
Sun, 22 Jun 2025 11:36:37 +0200
changeset 163
3628cc3c0483
parent 162
d3598c834f9b

change NULL-safety for shaders s.t. errors are reported only once per shader

relates to #690

src/2d.c file | annotate | diff | comparison | revisions
src/ascension/shader.h file | annotate | diff | comparison | revisions
src/shader.c file | annotate | diff | comparison | revisions
src/sprite.c file | annotate | diff | comparison | revisions
--- 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
--- 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
--- 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;
 }
 
--- 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

mercurial