Clarify attribute indexing documentation

[abcthomas]

The current documentation around the indicesPerMaterial attribute is very confusing as many different terms are used interchangeably.

The Readme states that the indices can be used to index into the attribute component arrays such like

submeshIndices = indicesPerMaterial[0];
attributeIndex = submeshIndices[0];
vertices[attributeIndex];
normals[attributeIndex];
textures[attributeIndex];

However the attribute arrays themselves are flattened so that each element stores an attribute component rather than a vertex full attribute. Therefore to store information for a full vertex one must index the attribute arrays once for each component of the vertex attribute (so 3 times for a position and 2 times for a texture coordinate). But it isn't clear which is the correct method of doing this either:

attributeIndex0 = submeshIndices[0];
attributeIndex1 = submeshIndices[1];
attributeIndex2 = submeshIndices[2];
Vector3 actualVertexPosition;
actualVertexPosition.x = vertices[attributeIndex0];
actualVertexPosition.y = vertices[attributeIndex1];
actualVertexPosition.z = vertices[attributeIndex2];

or

attributeIndex = submeshIndices[0];
Vector3 actualVertexPosition;
actualVertexPosition.x = vertices[attributeIndex];
actualVertexPosition.y = vertices[attributeIndex + 1];
actualVertexPosition.z = vertices[attributeIndex + 2];

I'm guessing the latter otherwise it wouldn't work with attributes of different sizes, but that's just my guess. The documentation should be clearer to specify which is correct.

What also doesn't help is the naming of the vertices attribute which is in fact vertex positions and so should be named positions, but maybe I should make a separate issue for that?

[frenchtoast747]

Hi Ashleigh, thanks for your interest in this project!

The latter example is closer to how it works. While vertex, texture, and normal attributes are the most common for a model, it's possible that an author wants to include some other attribute whose data is of an arbitrary stride length; it's up to the initialization of the data and the shader program to determine what attributes are used and how that data is arranged. The stride length (or the number of bytes in data) is also required in order to know how to correctly index. Note that the element index found in indices is not the index of the each of the attribute arrays (in JS), rather, it's the index for the underlying Vertex Buffer Object found on the graphics card. This index aligns with the objects specified in the vertexAttribPointer() calls where a specific number of components, component size, stride, and offset is required.

I've created the following diagram of a single element index that points to the parallel arrays. Do you think this would help to clarify the documentation?

Lastly, the name vertices is used to denote the Geometric Vertex. Since changing the name of the vertices attribute would be a breaking change, I would like to avoid that as much as possible. I wouldn't mind, however, updating the documentation to denote that it is indeed a geometric vertex attribute instead of an entire element.

[abcthomas]

I think the diagram definitely helps, but I would still add some information about how to use the indices Mesh provides in order to pull the correct data out of the attribute arrays that are also provided. I understand the indices are how OpenGL would index the data and that's useful as the indices can be stored and passed straight through, but it's worth clarifying that the element indices point to the first component in the provided attribute arrays.

I think adding the array indices of the individual attribute arrays of the above diagram would also help. Currently for instance the normal attribute array shows the XYZ components at element index 38, which is true. However if underneath it showed 0, 1, 2 etc that may go some way into clarifying how to index into the attribute arrays.

And I definitely understand not wanting to make a breaking change for that, it might be worth noting in the documentation though, the problem with the .obj standard is it has a strange definition for vertices. Geometric vertices are actually just points in space, with the vertices array what we're actually storing is the position of those vertices. So I think maybe just adding a little line stating they're the array stores vertex positions would be enough.

[frenchtoast747]

Will do for adding a note on vertices being position.

I will also add the following snippet for correctly indexing the individual components on the JS side:

// there are 3 components for a geometric vertex: X, Y, and Z
const NUM_COMPONENTS_FOR_VERTS = 3;
elementIdx = mesh.indices[SOME_IDX]; // e.g. 38
// in order to get the X component of the vertex component of element "38"
elementVertX = mesh.vertices[(elementIdx * NUM_COMPONENTS_FOR_VERTS) + 0]
// in order to get the Y component of the vertex component of element "38"
elementVertY = mesh.vertices[(elementIdx * NUM_COMPONENTS_FOR_VERTS) + 1]
// in order to get the Z component of the vertex component of element "38"
elementVertZ = mesh.vertices[(elementIdx * NUM_COMPONENTS_FOR_VERTS) + 2]

which is doing roughly what the shader compiler does for you when you say vert.x, vert.y, vert.z.

And likewise, to iterate over the each vertex

for (let i = 0; i < mesh.vertices.length; i += 3) {
    const x = mesh.vertices[i];
    const y = mesh.vertices[i+ 1];
    const z = mesh.vertices[i + 2];
}

[abcthomas]

Sounds good 👍

[frenchtoast747]

Hi @abcthomas, I have updated the README based on the points from the discussion here. Could you have a quick look at it and let me know what you think?

I used the same diagram but included an additional description underneath it that should hopefully describe the boxes and why X, Y, and Z are labeled with the 38th index.