Skip to content

Commit b963a9b

Browse files
authored
Fix a crash in glUniform* miniTempWebGL*Buffers optimization (#22130)
library_webgl.js attempts to use a pool of temp float or int buffers for uniform uploads up to 288 elements, however it would only allocate 288 temp buffers, so 0 through 287 would be fine but 288 would crash (miniTempWebGLFloatBuffers[288] would be undefined). Confirmed manually that any one of the six "Just at the optimization limit" calls would crash before, and with the fix does not. Fix for the change in PR #21573
1 parent b0ca7ae commit b963a9b

File tree

2 files changed

+49
-3
lines changed

2 files changed

+49
-3
lines changed

src/library_webgl.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ var LibraryGL = {
2828

2929
$miniTempWebGLFloatBuffers: [],
3030
$miniTempWebGLFloatBuffers__postset: `var miniTempWebGLFloatBuffersStorage = new Float32Array({{{ GL_POOL_TEMP_BUFFERS_SIZE }}});
31-
for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
31+
// Create GL_POOL_TEMP_BUFFERS_SIZE+1 temporary buffers, for uploads of size 0 through GL_POOL_TEMP_BUFFERS_SIZE inclusive
32+
for (/**@suppress{duplicate}*/var i = 0; i <= {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
3233
miniTempWebGLFloatBuffers[i] = miniTempWebGLFloatBuffersStorage.subarray(0, i);
3334
}`,
3435

3536
$miniTempWebGLIntBuffers: [],
3637
$miniTempWebGLIntBuffers__postset: `var miniTempWebGLIntBuffersStorage = new Int32Array({{{ GL_POOL_TEMP_BUFFERS_SIZE }}});
37-
for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
38+
// Create GL_POOL_TEMP_BUFFERS_SIZE+1 temporary buffers, for uploads of size 0 through GL_POOL_TEMP_BUFFERS_SIZE inclusive
39+
for (/**@suppress{duplicate}*/var i = 0; i <= {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
3840
miniTempWebGLIntBuffers[i] = miniTempWebGLIntBuffersStorage.subarray(0, i);
3941
}`,
4042

test/browser/webgl_draw_triangle_with_uniform_color.c

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,56 @@ int main() {
9999
glEnableVertexAttribArray(0);
100100
glEnableVertexAttribArray(1);
101101

102+
// Spot-test for miniTempWebGLFloatBuffers optimization in library_webgl.js.
103+
// Note there WILL be GL errors from this, but the test doesn't check for them
104+
// so it won't cause it to fail.
105+
#define GL_POOL_TEMP_BUFFERS_SIZE 288
106+
{
107+
GLfloat fdata[GL_POOL_TEMP_BUFFERS_SIZE + 4] = {};
108+
// Just under the optimization limit (should use unoptimized codepath)
109+
glUniform4fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 4 - 1, fdata);
110+
glUniform2fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 2 - 1, fdata);
111+
glUniform1fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE - 1, fdata);
112+
// Just at the optimization limit (should use optimized codepath)
113+
glUniform4fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 4, fdata);
114+
glUniform2fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 2, fdata);
115+
glUniform1fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE, fdata);
116+
// Just over the optimization limit (should use optimized codepath)
117+
glUniform4fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 4 + 1, fdata);
118+
glUniform2fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 2 + 1, fdata);
119+
glUniform1fv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE + 1, fdata);
120+
121+
GLint idata[GL_POOL_TEMP_BUFFERS_SIZE + 4] = {};
122+
// Just under the optimization limit (should use unoptimized codepath)
123+
glUniform4iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 4 - 1, idata);
124+
glUniform2iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 2 - 1, idata);
125+
glUniform1iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE - 1, idata);
126+
// Just at the optimization limit (should use optimized codepath)
127+
glUniform4iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 4, idata);
128+
glUniform2iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 2, idata);
129+
glUniform1iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE, idata);
130+
// Just over the optimization limit (should use optimized codepath)
131+
glUniform4iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 4 + 1, idata);
132+
glUniform2iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE / 2 + 1, idata);
133+
glUniform1iv(glGetUniformLocation(program, "color2"), GL_POOL_TEMP_BUFFERS_SIZE + 1, idata);
134+
}
135+
136+
// Actual upload for the rest of the test (overwrites the previous one).
102137
float color2[4] = { 0.0f, 1.f, 0.0f, 1.0f };
103138
glUniform4fv(glGetUniformLocation(program, "color2"), 1, color2);
104139

105140
// Test that passing zero for the size paramater does not cause error
106141
// https://github.com/emscripten-core/emscripten/issues/21567
107-
glUniform4fv(glGetUniformLocation(program, "color2"), 0, color2);
142+
// (These are zero-sized, so shouldn't overwrite anything.)
143+
{
144+
GLfloat fdata[4] = {};
145+
glUniform4fv(glGetUniformLocation(program, "color2"), 0, fdata);
146+
glUniform4fv(glGetUniformLocation(program, "color2"), 0, NULL);
147+
148+
GLint idata[4] = {};
149+
glUniform4iv(glGetUniformLocation(program, "color2"), 0, idata);
150+
glUniform4iv(glGetUniformLocation(program, "color2"), 0, NULL);
151+
}
108152

109153
glClearColor(0.3f,0.3f,0.3f,1);
110154
glClear(GL_COLOR_BUFFER_BIT);

0 commit comments

Comments
 (0)