greggman / twgl.js

A Tiny WebGL helper Library
http://twgljs.org
MIT License
2.61k stars 258 forks source link

createUniformBlockInfo does not work with float arrays #173

Closed tuner closed 3 years ago

tuner commented 3 years ago

... and subsequently, setBlockUniforms does not work as advertised: https://twgljs.org/docs/module-twgl.html#.setBlockUniforms

There are two problems. First, the keys in the uniforms array have a [0] suffix. That is trivial to fix or workaround, though.

The second issue is more severe. WebGL appears to use the std140 layout, which is incompatible with the current approach used in twgl, which exposes the individual uniforms as ArrayBufferViews:

If the member is an array of scalars or vectors, the base alignment and array stride are set to match the base alignment of a single array element, according to rules (1), (2), and (3), and rounded up to the base alignment of a vec4.

More details can be found in the OpenGL spec 4.5, section 7.6.2.2.

Here's an example that demonstrates the issue: https://karilavikka.fi/stuff/uniformBuffer.html

I propose that UniformBlockInfo would provide setters for uniforms, just like ProgramInfo already does. The ArrayBufferViews could be retained (and deprecated) to prevent breaking changes at this point.

I can come up with a patch if you find my proposal acceptable.

Thanks!

greggman commented 3 years ago

Doh, sorry about that.

Here's a PR. #174

Do you want to look it over?

PS: Never look at the OpenGL specs. Always look at the OpenGL ES specs. They are different enough that OpenGL specs will be wrong if you're looking for WebGL info

tuner commented 3 years ago

Awesome!

There was a bug in createUniformBlockUniformSetter: https://github.com/greggman/twgl.js/pull/174/commits/a434b6c514a321237384052732975a8582d507c9#r554443354

After fixing that, at least float and vec2 arrays seem to work. I tested the setters only, though.

Thanks for the fix, the great library, and the hint about the specs!

greggman commented 3 years ago

pushed up 4.18.0

tuner commented 3 years ago

Thanks!