greggman / twgl.js

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

replace error with warning #192

Closed yurivish closed 2 years ago

yurivish commented 2 years ago

Hi, thanks for TWGL!

I noticed that getNumElementsFromAttributes, called from createBufferInfoFromArrays, currently throws an error when the buffer size does not evenly divide the attribute size.

However, there are valid use cases for having an uneven division, such as reusing a large, preallocated webgl buffer to render different kinds of geometry. This PR changes the error to a warning, since this situation may be indicative of a bug in the user's code in the common case.

In my app I have a WASM core that passes data to a JS renderer, which copies it into a preallocated webgl buffer. Geometry types can differ across frames, which is how I ran into this bug — some of them did not evenly divide the buffer size, even though the buffer is intended to be large enough to hold the maximum possible amount of geometry each frame, so it would never reach the end of the buffer.

yurivish commented 2 years ago

I chatted about this with @onethreeseven, who pointed out that if I pass a third argument to createBufferInfoFromArrays that sets numElements to a (nonzero) number, this entire function invocation would be avoided:

https://github.com/yurivish/twgl.js/blob/master/src/attributes.js#L639

Perhaps the check on this line should check for null or undefined, since making a buffer with no elements for future use is a legitimate use case and right now a zero value for numElements would cause this if branch to be taken?