Skip to content

Commit 65b26e3

Browse files
sbc100sbc100
authored
Fix for zero count in glUniform family of functions (#21573)
The previous fix for this was in #16837, but I looks like that only covered the new "garbage-free" webgl2 path. When old webgl1 path was still using the zero count value. For example the following line was unguarded: ``` var view = miniTempWebGLFloatBuffers[4 * count - 1]; ``` This recently resurfaced because I introduced `WEBGL_USE_GARBAGE_FREE_APIS` which is currently disabled for memories larger 2gb. This meant that users with large memories were forces onto the old path where the bug still existed. Rather than adding yet more `count &&` prefixes, this patch simply adds a single early return at the top of each function. As part of this change I resurrected `test_webgl_draw_triangle_with_uniform_color.c` which has not actually be used since #20925. Fixes: #21567
1 parent 8b18092 commit 65b26e3

File tree

3 files changed

+27
-13
lines changed

3 files changed

+27
-13
lines changed

‎src/library_webgl.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ var LibraryGL = {
2929
$miniTempWebGLFloatBuffers: [],
3030
$miniTempWebGLFloatBuffers__postset: `var miniTempWebGLFloatBuffersStorage = new Float32Array({{{ GL_POOL_TEMP_BUFFERS_SIZE }}});
3131
for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
32-
miniTempWebGLFloatBuffers[i] = miniTempWebGLFloatBuffersStorage.subarray(0, i+1);
32+
miniTempWebGLFloatBuffers[i] = miniTempWebGLFloatBuffersStorage.subarray(0, i);
3333
}`,
3434

3535
$miniTempWebGLIntBuffers: [],
3636
$miniTempWebGLIntBuffers__postset: `var miniTempWebGLIntBuffersStorage = new Int32Array({{{ GL_POOL_TEMP_BUFFERS_SIZE }}});
3737
for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
38-
miniTempWebGLIntBuffers[i] = miniTempWebGLIntBuffersStorage.subarray(0, i+1);
38+
miniTempWebGLIntBuffers[i] = miniTempWebGLIntBuffersStorage.subarray(0, i);
3939
}`,
4040

4141
$heapObjectForWebGLType: (type) => {
@@ -2439,7 +2439,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
24392439
#if GL_POOL_TEMP_BUFFERS
24402440
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}) {
24412441
// avoid allocation when uploading few enough uniforms
2442-
var view = miniTempWebGLIntBuffers[count-1];
2442+
var view = miniTempWebGLIntBuffers[count];
24432443
for (var i = 0; i < count; ++i) {
24442444
view[i] = {{{ makeGetValue('value', '4*i', 'i32') }}};
24452445
}
@@ -2480,7 +2480,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
24802480
#if GL_POOL_TEMP_BUFFERS
24812481
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 2 }}}) {
24822482
// avoid allocation when uploading few enough uniforms
2483-
var view = miniTempWebGLIntBuffers[2*count-1];
2483+
var view = miniTempWebGLIntBuffers[2*count];
24842484
for (var i = 0; i < 2*count; i += 2) {
24852485
view[i] = {{{ makeGetValue('value', '4*i', 'i32') }}};
24862486
view[i+1] = {{{ makeGetValue('value', '4*i+4', 'i32') }}};
@@ -2522,7 +2522,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
25222522
#if GL_POOL_TEMP_BUFFERS
25232523
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 3 }}}) {
25242524
// avoid allocation when uploading few enough uniforms
2525-
var view = miniTempWebGLIntBuffers[3*count-1];
2525+
var view = miniTempWebGLIntBuffers[3*count];
25262526
for (var i = 0; i < 3*count; i += 3) {
25272527
view[i] = {{{ makeGetValue('value', '4*i', 'i32') }}};
25282528
view[i+1] = {{{ makeGetValue('value', '4*i+4', 'i32') }}};
@@ -2565,7 +2565,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
25652565
#if GL_POOL_TEMP_BUFFERS
25662566
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 4 }}}) {
25672567
// avoid allocation when uploading few enough uniforms
2568-
var view = miniTempWebGLIntBuffers[4*count-1];
2568+
var view = miniTempWebGLIntBuffers[4*count];
25692569
for (var i = 0; i < 4*count; i += 4) {
25702570
view[i] = {{{ makeGetValue('value', '4*i', 'i32') }}};
25712571
view[i+1] = {{{ makeGetValue('value', '4*i+4', 'i32') }}};
@@ -2609,7 +2609,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
26092609
#if GL_POOL_TEMP_BUFFERS
26102610
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}) {
26112611
// avoid allocation when uploading few enough uniforms
2612-
var view = miniTempWebGLFloatBuffers[count-1];
2612+
var view = miniTempWebGLFloatBuffers[count];
26132613
for (var i = 0; i < count; ++i) {
26142614
view[i] = {{{ makeGetValue('value', '4*i', 'float') }}};
26152615
}
@@ -2650,7 +2650,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
26502650
#if GL_POOL_TEMP_BUFFERS
26512651
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 2 }}}) {
26522652
// avoid allocation when uploading few enough uniforms
2653-
var view = miniTempWebGLFloatBuffers[2*count-1];
2653+
var view = miniTempWebGLFloatBuffers[2*count];
26542654
for (var i = 0; i < 2*count; i += 2) {
26552655
view[i] = {{{ makeGetValue('value', '4*i', 'float') }}};
26562656
view[i+1] = {{{ makeGetValue('value', '4*i+4', 'float') }}};
@@ -2692,7 +2692,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
26922692
#if GL_POOL_TEMP_BUFFERS
26932693
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 3 }}}) {
26942694
// avoid allocation when uploading few enough uniforms
2695-
var view = miniTempWebGLFloatBuffers[3*count-1];
2695+
var view = miniTempWebGLFloatBuffers[3*count];
26962696
for (var i = 0; i < 3*count; i += 3) {
26972697
view[i] = {{{ makeGetValue('value', '4*i', 'float') }}};
26982698
view[i+1] = {{{ makeGetValue('value', '4*i+4', 'float') }}};
@@ -2735,7 +2735,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
27352735
#if GL_POOL_TEMP_BUFFERS
27362736
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 4 }}}) {
27372737
// avoid allocation when uploading few enough uniforms
2738-
var view = miniTempWebGLFloatBuffers[4*count-1];
2738+
var view = miniTempWebGLFloatBuffers[4*count];
27392739
// hoist the heap out of the loop for size and for pthreads+growth.
27402740
var heap = HEAPF32;
27412741
value = {{{ getHeapOffset('value', 'float') }}};
@@ -2783,7 +2783,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
27832783
#if GL_POOL_TEMP_BUFFERS
27842784
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 4 }}}) {
27852785
// avoid allocation when uploading few enough uniforms
2786-
var view = miniTempWebGLFloatBuffers[4*count-1];
2786+
var view = miniTempWebGLFloatBuffers[4*count];
27872787
for (var i = 0; i < 4*count; i += 4) {
27882788
view[i] = {{{ makeGetValue('value', '4*i', 'float') }}};
27892789
view[i+1] = {{{ makeGetValue('value', '4*i+4', 'float') }}};
@@ -2827,7 +2827,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
28272827
#if GL_POOL_TEMP_BUFFERS
28282828
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 9 }}}) {
28292829
// avoid allocation when uploading few enough uniforms
2830-
var view = miniTempWebGLFloatBuffers[9*count-1];
2830+
var view = miniTempWebGLFloatBuffers[9*count];
28312831
for (var i = 0; i < 9*count; i += 9) {
28322832
view[i] = {{{ makeGetValue('value', '4*i', 'float') }}};
28332833
view[i+1] = {{{ makeGetValue('value', '4*i+4', 'float') }}};
@@ -2876,7 +2876,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
28762876
#if GL_POOL_TEMP_BUFFERS
28772877
if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE / 16 }}}) {
28782878
// avoid allocation when uploading few enough uniforms
2879-
var view = miniTempWebGLFloatBuffers[16*count-1];
2879+
var view = miniTempWebGLFloatBuffers[16*count];
28802880
// hoist the heap out of the loop for size and for pthreads+growth.
28812881
var heap = HEAPF32;
28822882
value = {{{ getHeapOffset('value', 'float') }}};

‎test/browser/webgl_draw_triangle_with_uniform_color.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ int main() {
101101

102102
float color2[4] = { 0.0f, 1.f, 0.0f, 1.0f };
103103
glUniform4fv(glGetUniformLocation(program, "color2"), 1, color2);
104+
105+
// Test that passing zero for the size paramater does not cause error
106+
// https://github.com/emscripten-core/emscripten/issues/21567
107+
glUniform4fv(glGetUniformLocation(program, "color2"), 0, color2);
108+
104109
glClearColor(0.3f,0.3f,0.3f,1);
105110
glClear(GL_COLOR_BUFFER_BIT);
106111
glDrawArrays(GL_TRIANGLES, 0, 3);

‎test/test_browser.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4500,6 +4500,15 @@ def test_webgl_offscreen_framebuffer_state_restoration(self):
45004500
cmd = args + ['-lGL', '-sOFFSCREEN_FRAMEBUFFER', '-DEXPLICIT_SWAP=1']
45014501
self.btest_exit('webgl_offscreen_framebuffer_swap_with_bad_state.c', args=cmd)
45024502

4503+
@parameterized({
4504+
'': ([],),
4505+
'es2': (['-sFULL_ES2'],),
4506+
'es3': (['-sFULL_ES3'],),
4507+
})
4508+
@requires_graphics_hardware
4509+
def test_webgl_draw_triangle_with_uniform_color(self, args):
4510+
self.btest_exit('webgl_draw_triangle_with_uniform_color.c', args=args)
4511+
45034512
# Tests that using an array of structs in GL uniforms works.
45044513
@requires_graphics_hardware
45054514
def test_webgl_array_of_structs_uniform(self):

0 commit comments

Comments
 (0)