From 3e0912017389f843dcb8313def7f6daeb7727bf4 Mon Sep 17 00:00:00 2001 From: Cassie Jones Date: Mon, 27 Jan 2020 16:07:41 -0500 Subject: [PATCH] Compile shaders using source lengths The previous implementation relied on glShaderSource inferring source lengths when the lengths weren't specified. This relies on the sources being properly null-terminated, however, which isn't the case due to file loading changes which now use pointer + length. This could cause intermittent crashes. Changing this on the shader side meant adding some extra arguments for passing around shader source lengths. For most of the other cases, where we're using string literals as the sources, we can just specify -1 as the length, since OpenGL will calculate the string length for you any time the length is negative. --- src/api/l_graphics.c | 20 ++++++++++++++------ src/modules/graphics/opengl.c | 20 +++++++++++++------- src/modules/graphics/shader.c | 12 ++++++------ src/modules/graphics/shader.h | 4 ++-- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/api/l_graphics.c b/src/api/l_graphics.c index 7d1b912d..4ae06985 100644 --- a/src/api/l_graphics.c +++ b/src/api/l_graphics.c @@ -1400,23 +1400,26 @@ static int l_lovrGraphicsNewModel(lua_State* L) { return 1; } -static const char* luax_checkshadersource(lua_State* L, int index) { +static const char* luax_checkshadersource(lua_State* L, int index, int *outLength) { if (lua_isnoneornil(L, index)) { return NULL; } Blob* blob = luax_totype(L, index, Blob); if (blob) { + *outLength = blob->size; return blob->data; } size_t length; const char* source = luaL_checklstring(L, index, &length); if (memchr(source, '\n', MIN(1024, length))) { + *outLength = length; return source; } else { void* contents = luax_readfile(source, &length); lovrAssert(contents, "Could not read shader from file '%s'", source); + *outLength = length; return contents; } } @@ -1488,8 +1491,10 @@ static int l_lovrGraphicsNewShader(lua_State* L) { lovrShaderSetFloats(shader, "lovrLightColor", (float[4]) { 1.f, 1.f, 1.f, 1.f }, 0, 4); } } else { - const char* vertexSource = luax_checkshadersource(L, 1); - const char* fragmentSource = luax_checkshadersource(L, 2); + int vertexSourceLength; + const char* vertexSource = luax_checkshadersource(L, 1, &vertexSourceLength); + int fragmentSourceLength; + const char* fragmentSource = luax_checkshadersource(L, 2, &fragmentSourceLength); if (lua_istable(L, 3)) { lua_getfield(L, 3, "flags"); @@ -1501,7 +1506,9 @@ static int l_lovrGraphicsNewShader(lua_State* L) { lua_pop(L, 1); } - shader = lovrShaderCreateGraphics(vertexSource, fragmentSource, flags, flagCount, multiview); + shader = lovrShaderCreateGraphics( + vertexSource, vertexSourceLength, fragmentSource, fragmentSourceLength, + flags, flagCount, multiview); } luax_pushtype(L, Shader, shader); @@ -1510,7 +1517,8 @@ static int l_lovrGraphicsNewShader(lua_State* L) { } static int l_lovrGraphicsNewComputeShader(lua_State* L) { - const char* source = luax_checkshadersource(L, 1); + int sourceLength; + const char* source = luax_checkshadersource(L, 1, &sourceLength); ShaderFlag flags[MAX_SHADER_FLAGS]; uint32_t flagCount = 0; @@ -1520,7 +1528,7 @@ static int l_lovrGraphicsNewComputeShader(lua_State* L) { lua_pop(L, 1); } - Shader* shader = lovrShaderCreateCompute(source, flags, flagCount); + Shader* shader = lovrShaderCreateCompute(source, sourceLength, flags, flagCount); luax_pushtype(L, Shader, shader); lovrRelease(Shader, shader); return 1; diff --git a/src/modules/graphics/opengl.c b/src/modules/graphics/opengl.c index af8ff442..e7232876 100644 --- a/src/modules/graphics/opengl.c +++ b/src/modules/graphics/opengl.c @@ -1900,9 +1900,9 @@ void lovrBufferDiscard(Buffer* buffer) { // Shader -static GLuint compileShader(GLenum type, const char** sources, int count) { +static GLuint compileShader(GLenum type, const char** sources, int* lengths, int count) { GLuint shader = glCreateShader(type); - glShaderSource(shader, count, sources, NULL); + glShaderSource(shader, count, sources, lengths); glCompileShader(shader); int isShaderCompiled; @@ -2175,7 +2175,7 @@ static char* lovrShaderGetFlagCode(ShaderFlag* flags, uint32_t flagCount) { return code; } -Shader* lovrShaderInitGraphics(Shader* shader, const char* vertexSource, const char* fragmentSource, ShaderFlag* flags, uint32_t flagCount, bool multiview) { +Shader* lovrShaderInitGraphics(Shader* shader, const char* vertexSource, int vertexSourceLength, const char* fragmentSource, int fragmentSourceLength, ShaderFlag* flags, uint32_t flagCount, bool multiview) { #if defined(LOVR_WEBGL) || defined(LOVR_GLES) const char* version = "#version 300 es\n"; const char* precision[2] = { "precision highp float;\nprecision highp int;\n", "precision mediump float;\nprecision mediump int;\n" }; @@ -2197,12 +2197,16 @@ Shader* lovrShaderInitGraphics(Shader* shader, const char* vertexSource, const c // Vertex vertexSource = vertexSource == NULL ? lovrUnlitVertexShader : vertexSource; const char* vertexSources[] = { version, singlepass[0], precision[0], flagSource ? flagSource : "", lovrShaderVertexPrefix, vertexSource, lovrShaderVertexSuffix }; - GLuint vertexShader = compileShader(GL_VERTEX_SHADER, vertexSources, sizeof(vertexSources) / sizeof(vertexSources[0])); + int vertexSourceLengths[] = { -1, -1, -1, -1, -1, vertexSourceLength, -1 }; + size_t vertexSourceCount = sizeof(vertexSources) / sizeof(vertexSources[0]); + GLuint vertexShader = compileShader(GL_VERTEX_SHADER, vertexSources, vertexSourceLengths, vertexSourceCount); // Fragment fragmentSource = fragmentSource == NULL ? lovrUnlitFragmentShader : fragmentSource; const char* fragmentSources[] = { version, singlepass[1], precision[1], flagSource ? flagSource : "", lovrShaderFragmentPrefix, fragmentSource, lovrShaderFragmentSuffix }; - GLuint fragmentShader = compileShader(GL_FRAGMENT_SHADER, fragmentSources, sizeof(fragmentSources) / sizeof(fragmentSources[0])); + int fragmentSourceLengths[] = { -1, -1, -1, -1, -1, fragmentSourceLength, -1 }; + size_t fragmentSourceCount = sizeof(fragmentSources) / sizeof(fragmentSources[0]); + GLuint fragmentShader = compileShader(GL_FRAGMENT_SHADER, fragmentSources, fragmentSourceLengths, fragmentSourceCount); free(flagSource); @@ -2253,14 +2257,16 @@ Shader* lovrShaderInitGraphics(Shader* shader, const char* vertexSource, const c return shader; } -Shader* lovrShaderInitCompute(Shader* shader, const char* source, ShaderFlag* flags, uint32_t flagCount) { +Shader* lovrShaderInitCompute(Shader* shader, const char* source, int length, ShaderFlag* flags, uint32_t flagCount) { #ifdef LOVR_WEBGL lovrThrow("Compute shaders are not supported on this system"); #else lovrAssert(GLAD_GL_ARB_compute_shader, "Compute shaders are not supported on this system"); char* flagSource = lovrShaderGetFlagCode(flags, flagCount); const char* sources[] = { lovrShaderComputePrefix, flagSource ? flagSource : "", source, lovrShaderComputeSuffix }; - GLuint computeShader = compileShader(GL_COMPUTE_SHADER, sources, sizeof(sources) / sizeof(sources[0])); + int lengths[] = { -1, -1, length, -1 }; + size_t count = sizeof(sources) / sizeof(sources[0]); + GLuint computeShader = compileShader(GL_COMPUTE_SHADER, sources, lengths, count); free(flagSource); GLuint program = glCreateProgram(); glAttachShader(program, computeShader); diff --git a/src/modules/graphics/shader.c b/src/modules/graphics/shader.c index 49d710ed..ec27f075 100644 --- a/src/modules/graphics/shader.c +++ b/src/modules/graphics/shader.c @@ -63,12 +63,12 @@ static const char* getUniformTypeName(const Uniform* uniform) { Shader* lovrShaderInitDefault(Shader* shader, DefaultShader type, ShaderFlag* flags, uint32_t flagCount) { switch (type) { - case SHADER_UNLIT: return lovrShaderInitGraphics(shader, NULL, NULL, flags, flagCount, true); - case SHADER_STANDARD: return lovrShaderInitGraphics(shader, lovrStandardVertexShader, lovrStandardFragmentShader, flags, flagCount, true); - case SHADER_CUBE: return lovrShaderInitGraphics(shader, lovrCubeVertexShader, lovrCubeFragmentShader, flags, flagCount, true); - case SHADER_PANO: return lovrShaderInitGraphics(shader, lovrCubeVertexShader, lovrPanoFragmentShader, flags, flagCount, true); - case SHADER_FONT: return lovrShaderInitGraphics(shader, NULL, lovrFontFragmentShader, flags, flagCount, true); - case SHADER_FILL: return lovrShaderInitGraphics(shader, lovrFillVertexShader, NULL, flags, flagCount, true); + case SHADER_UNLIT: return lovrShaderInitGraphics(shader, NULL, -1, NULL, -1, flags, flagCount, true); + case SHADER_STANDARD: return lovrShaderInitGraphics(shader, lovrStandardVertexShader, -1, lovrStandardFragmentShader, -1, flags, flagCount, true); + case SHADER_CUBE: return lovrShaderInitGraphics(shader, lovrCubeVertexShader, -1, lovrCubeFragmentShader, -1, flags, flagCount, true); + case SHADER_PANO: return lovrShaderInitGraphics(shader, lovrCubeVertexShader, -1, lovrPanoFragmentShader, -1, flags, flagCount, true); + case SHADER_FONT: return lovrShaderInitGraphics(shader, NULL, -1, lovrFontFragmentShader, -1, flags, flagCount, true); + case SHADER_FILL: return lovrShaderInitGraphics(shader, lovrFillVertexShader, -1, NULL, -1, flags, flagCount, true); default: lovrThrow("Unknown default shader type"); return NULL; } } diff --git a/src/modules/graphics/shader.h b/src/modules/graphics/shader.h index 7476ade1..40f8eed7 100644 --- a/src/modules/graphics/shader.h +++ b/src/modules/graphics/shader.h @@ -123,8 +123,8 @@ typedef struct Shader { // Shader -Shader* lovrShaderInitGraphics(Shader* shader, const char* vertexSource, const char* fragmentSource, ShaderFlag* flags, uint32_t flagCount, bool multiview); -Shader* lovrShaderInitCompute(Shader* shader, const char* source, ShaderFlag* flags, uint32_t flagCount); +Shader* lovrShaderInitGraphics(Shader* shader, const char* vertexSource, int vertexSourceLength, const char* fragmentSource, int fragmentSourceLength, ShaderFlag* flags, uint32_t flagCount, bool multiview); +Shader* lovrShaderInitCompute(Shader* shader, const char* source, int length, ShaderFlag* flags, uint32_t flagCount); Shader* lovrShaderInitDefault(Shader* shader, DefaultShader type, ShaderFlag* flags, uint32_t flagCount); #define lovrShaderCreateGraphics(...) lovrShaderInitGraphics(lovrAlloc(Shader), __VA_ARGS__) #define lovrShaderCreateCompute(...) lovrShaderInitCompute(lovrAlloc(Shader), __VA_ARGS__)