Problem with converting the OpenGL glDrawElements function to WebGL in emscripten · Issue #4214 · emscripten-core/emscripten (original) (raw)

I'm running into an issue with how emscripten converts the OpenGL glDrawElements function to the WebGL equivalent (Glctx.drawElements). I had been getting a number of weird triangles popping around in my graphics, and I believe I’ve tracked it down to this:

In emscripten’s library_gl.js file resides the glDrawElements function that replaces the OpenGL call with the same name. Part of what this function does is set up a buffer for the indices (called GLctx.ELEMENT_ARRAY_BUFFER) and (in the GL.preDrawHandleClientVertexAttribBindings function call) it sets up a buffer for the vertices (called GLctx.ARRAY_BUFFER).

The problem is that in the GL.preDrawHandleClientVertexAttribBindings function, it is setting how large the GLctx.ARRAY_BUFFER data array will be based on the variable ‘count’, which is the size of the GLctx.ELEMENT_ARRAY_BUFFER (i.e. the array of indices). This is a problem because what the Glctx.drawElements function will actually do is use the values in the index array to determine where it needs to get its data from in the vertex array (i.e. GLctx.ARRAY_BUFFER). So if the values in the index array point to an index larger than the size of the index array, the Glctx.drawElements function will try to get data from outside the bounds of the GLctx.ARRAY_BUFFER (which just gives junk data).

An example of where the current code would have this happen (i.e. failing to produce the correct graphics) would be if you defined a triangle with an index array (or GLctx.ELEMENT_ARRAY_BUFFER) of [0, 5, 6] and a vertex array (or GLctx.ARRAY_BUFFER) containing only the position in (x,y,z) with value of
[-1,-1,-5,
1, 2, 3,
2, 3, 4,
3, 4, 5,
4, 5, 6,
1, -1, -5,
1, 1, -5].
After running through emscripten, the program would calculate the size of the index array to be 3 (since only 3 indices were given) and each position has 3 values (x, y, and z), so the GLctx.ARRAY_BUFFER would be made to hold only the first 9 values:
[-1,-1,-5,
1, 2, 3,
2, 3, 4]
However, when the WebGL function GLctx.drawElements is called, it would be looking for values in the GLctx.ARRAY_BUFFER array at 0, 5 and 6 (which should give it -1, -1, -5 and 1, -1, -5 and 1, 1, -5 respectively). But since the size of the GLctx.ARRAY_BUFFER is based on the fact that there are 3 indices (and therefore doesn’t include the values after the first 9), it will actually just be getting junk values for indices 5 and 6.

Solution:
I believe the best way to fix this (and at least the only way I can see to fix it) would be to base the size of the GLctx.ARRAY_BUFFER data array on the highest/maximum index value in the index array instead of basing it on the size of the index array. This would ensure that you would not accidentally make the GLctx.ARRAY_BUFFER data array shorter than it should be. To this end, I came up with a fix for this problem (I put a copy of this diff below). I know that this works for my code, but I believe it should also work in general. But if there’s a better or more concise way to fix this, great.

Basically what I’ve done is find the maximum index value in the index array and use that as the parameter for the GL.preDrawHandleClientVertexAttribBindings function instead of using the size of the index array (which makes the size of the GLctx.ARRAY_BUFFER data array be based on the maximum index value instead of the size of the index array).

@@ -6975,6 +6975,33 @@
   glDrawElements: function(mode, count, type, indices) {
#if FULL_ES2
     var buf;
+
+    // Calculate the maximum index number
+    var maxIndex = count;
+    switch (GL.byteSizeByType[type - GL.byteSizeByTypeRoot]) {
+        case 1: 
+            var indicesArray = HEAPU8.subarray((indices>>0), (indices>>0) + count);
+            break;
+        case 2: 
+            var indicesArray = HEAPU16.subarray((indices>>1), (indices>>1) + count);
+            break;
+        case 4:
+            var indicesArray = HEAPU32.subarray((indices>>2), (indices>>2) + count);
+            break;
+        case 8:
+            var indicesArray = HEAP64.subarray((indices>>3), (indices>>3) + count);
+            break;
+        default:
+    }
+    if (indicesArray) {
+        maxIndex = indicesArray[0];
+        for (var i = 1; i < count; i++) {
+            if (indicesArray[i] > maxIndex)
+                maxIndex = indicesArray[i];
+        }
+        maxIndex += 1; // indices start at 0, so final size is max+1
+    }
+
     if (!GL.currElementArrayBuffer) {
       var size = GL.calcBufLength(1, type, 0, count);
       buf = GL.getTempIndexBuffer(size);
@@ -6987,7 +7014,7 @@
     }
     // bind any client-side buffers
-    GL.preDrawHandleClientVertexAttribBindings(count);
+    GL.preDrawHandleClientVertexAttribBindings(maxIndex);
#endif
     GLctx.drawElements(mode, count, type, indices);