Fix for zero count in glUniform family of functions

[sbc100]

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.

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

[sbc100]

The code size code here is either 2 bytes or 4 bytes of total compressed size.

However, the alternative would be add an additional count && to yet more like lines in each of these functions, which depending on the build config can also have a code size cost (due to many count && in the each function).

[kripken]

lgtm, but can we add a test for this?

[sbc100]

lgtm, but can we add a test for this?

Can I use the same excuse that was used last time not to? #16837 (comment) :)

[kripken]

I'll meet you halfway: how about a test for one of them? 😄 that would at least test the recurring pattern, and have a good chance of catching regressions assuming we regress them all at once.

[sbc100]

Test added! Confirmed that it fails without this change.

[sbc100]

OK to land now?

[juj]

I see that in miniTempWebGLInt/FloatBuffers we have

for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
  miniTempWebGLFloatBuffers[i] = miniTempWebGLFloatBuffersStorage.subarray(0, i+1);
}
for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
  miniTempWebGLIntBuffers[i] = miniTempWebGLIntBuffersStorage.subarray(0, i+1);
}

and then at site of use there is that

    if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}) {
      // avoid allocation when uploading few enough uniforms
      var view = miniTempWebGLIntBuffers[count-1];
      for (var i = 0; i < count; ++i) {
        view[i] = {{{ makeGetValue('value', '4*i', 'i32') }}};
      }
    } else

Maybe we can fix this by changing miniTempWebGLIntBuffersStorage.subarray(0, i+1); to miniTempWebGLIntBuffersStorage.subarray(0, i);, and then var view = miniTempWebGLIntBuffers[count-1]; to var view = miniTempWebGLIntBuffers[count];?

That should result in a zero-sized arraybuffer being passed to the function calls naturally?

Not sure why that original -1 offset existed, maybe some idiomatic thinking that allocating a dummy zero-sized array as first entry of the pooled buffers would have been silly, hence the offset. But that should be fine to do, and it would then allow count=0 naturally?

[sbc100]

I see that in miniTempWebGLInt/FloatBuffers we have

for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
  miniTempWebGLFloatBuffers[i] = miniTempWebGLFloatBuffersStorage.subarray(0, i+1);
}
for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}; ++i) {
  miniTempWebGLIntBuffers[i] = miniTempWebGLIntBuffersStorage.subarray(0, i+1);
}

and then at site of use there is that

    if (count <= {{{ GL_POOL_TEMP_BUFFERS_SIZE }}}) {
      // avoid allocation when uploading few enough uniforms
      var view = miniTempWebGLIntBuffers[count-1];
      for (var i = 0; i < count; ++i) {
        view[i] = {{{ makeGetValue('value', '4*i', 'i32') }}};
      }
    } else

Maybe we can fix this by changing miniTempWebGLIntBuffersStorage.subarray(0, i+1); to miniTempWebGLIntBuffersStorage.subarray(0, i);, and then var view = miniTempWebGLIntBuffers[count-1]; to var view = miniTempWebGLIntBuffers[count];?

That should result in a zero-sized arraybuffer being passed to the function calls naturally?

Not sure why that original -1 offset existed, maybe some idiomatic thinking that allocating a dummy zero-sized array as first entry of the pooled buffers would have been silly, hence the offset. But that should be fine to do, and it would then allow count=0 naturally?

Thats works great!

[sbc100]

Ping...

[juj]

Nice!