Skip to content

Commit

Permalink
Fix a shader compilation bug affecting release builds
Browse files Browse the repository at this point in the history
glShaderSource was being called with arguments which signify
null-terminated strings when some sources were not null-terminated.
  • Loading branch information
gustafla committed Aug 6, 2023
1 parent 007e6d2 commit 1734731
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 21 deletions.
16 changes: 6 additions & 10 deletions src/demo.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ typedef struct {
} fbo_t;

typedef struct {
int width;
int height;
double aspect_ratio;
int x0;
int y0;
Expand Down Expand Up @@ -99,7 +97,8 @@ static int replace_program(program_t *old, program_t new) {
void demo_reload(demo_t *demo) {
demo->programs_ok = 1;

shader_t vertex_shader = compile_shader(vertex_shader_src, "vert", NULL, 0);
shader_t vertex_shader = compile_shader(
vertex_shader_src, strlen(vertex_shader_src), "vert", NULL, 0);
shader_t fragment_shader =
compile_shader_file("shaders/shader.frag", NULL, 0);

Expand Down Expand Up @@ -185,8 +184,6 @@ demo_t *demo_init(int width, int height) {
return NULL;
}

demo->width = width;
demo->height = height;
demo->aspect_ratio = (double)width / (double)height;
demo_resize(demo, width, height);

Expand Down Expand Up @@ -391,13 +388,12 @@ void demo_render(demo_t *demo, struct sync_device *rocket, double rocket_row) {
// Animate shader reload to show feedback in debug builds
float a = fmin((SDL_GetTicks64() - demo->reload_time) / 100.f, 1.);
const GLint x0 = demo->x0 * a, x1 = demo->x1 * a, y0 = demo->y0 * a,
y1 = demo->y1 * a, w = demo->width, h = demo->height;
y1 = demo->y1 * a;
#else
const GLint x0 = demo->x0, x1 = demo->x1, y0 = demo->y0, y1 = demo->y1,
w = demo->width, h = demo->height;
const GLint x0 = demo->x0, x1 = demo->x1, y0 = demo->y0, y1 = demo->y1;
#endif
glBlitFramebuffer(0, 0, w, h, x0, y0, x1, y1, GL_COLOR_BUFFER_BIT,
GL_LINEAR);
glBlitFramebuffer(0, 0, demo->fbs[2].width, demo->fbs[2].height, x0, y0, x1,
y1, GL_COLOR_BUFFER_BIT, GL_LINEAR);

// Switch fb to keep render results in memory for feedback effects
demo->firstpass_fb_idx = alt_fb_idx;
Expand Down
30 changes: 21 additions & 9 deletions src/shader.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,14 @@ static size_t read_file(const char *filename, char **dst) {
goto cleanup;
}

*dst = (char *)malloc(len + 1);
*dst = (char *)malloc(len);
fseek(file, 0, SEEK_SET);

size_t read = fread(*dst, sizeof(char), len, file);
fclose(file);
if (read != len)
goto cleanup;

(*dst)[len] = '\0';

return len;

cleanup:
Expand Down Expand Up @@ -71,9 +69,21 @@ static GLenum type_to_enum(const char *shader_type) {
return GL_INVALID_ENUM;
}

shader_t compile_shader(const char *shader_src, const char *shader_type,
const shader_define_t *defines, size_t n_defs) {
shader_t compile_shader(const char *shader_src, size_t shader_src_len,
const char *shader_type, const shader_define_t *defines,
size_t n_defs) {
static const char *src[MAX_SRC_FRAGMENTS];
static GLint src_lens[MAX_SRC_FRAGMENTS];

// Set src_lens to all 1-bits because that will cause them to be negative.
//
// glShaderSource documentation:
//
// Each element in the length array may contain the length of the
// corresponding string (the null character is not counted as part
// of the string length) or a value less than 0 to indicate that
// the string is null terminated.
memset(src_lens, ~0, MAX_SRC_FRAGMENTS * sizeof(GLint));

shader_t ret = {0};
ret.uniforms = parse_uniforms(shader_src, &ret.uniform_count);
Expand All @@ -90,9 +100,10 @@ shader_t compile_shader(const char *shader_src, const char *shader_type,
src[i++] = "\n";
}
}
src_lens[i] = shader_src_len;
src[i++] = shader_src;

glShaderSource(ret.handle, i, src, NULL);
glShaderSource(ret.handle, i, src, src_lens);
glCompileShader(ret.handle);

GLint status;
Expand All @@ -115,8 +126,8 @@ shader_t compile_shader(const char *shader_src, const char *shader_type,
shader_t compile_shader_file(const char *filename,
const shader_define_t *defines, size_t n_defs) {
char *shader_src = NULL;

if (read_file(filename, &shader_src) == 0) {
size_t shader_src_len = read_file(filename, &shader_src);
if (shader_src_len == 0) {
return (shader_t){0};
}

Expand All @@ -128,7 +139,8 @@ shader_t compile_shader_file(const char *filename,
}
} while (ret);

shader_t shader = compile_shader(shader_src, shader_type, defines, n_defs);
shader_t shader = compile_shader(shader_src, shader_src_len, shader_type,
defines, n_defs);
#ifdef DEBUG
free(shader_src);
#endif
Expand Down
5 changes: 3 additions & 2 deletions src/shader.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ typedef struct {
size_t uniform_count;
} program_t;

shader_t compile_shader(const char *shader_src, const char *shader_type,
const shader_define_t *defines, size_t n_defs);
shader_t compile_shader(const char *shader_src, size_t shader_src_len,
const char *shader_type, const shader_define_t *defines,
size_t n_defs);
shader_t compile_shader_file(const char *filename,
const shader_define_t *defines, size_t n_defs);
program_t link_program(shader_t *shaders, size_t count);
Expand Down

0 comments on commit 1734731

Please sign in to comment.