Issue 820

[emilian0]

Work in progress branch (Issue #820 ) to be merged to glTF 2.0 specs branch.
TODO:

• Verify with the team that UNSIGNED_INT should be supported as indices type.
• Decide, together with the team, if the index type should be defined in "sparse.indices.componentType" or "sparse.indices.type"

[pjcozzi]

@emilian0 since this is WIP, can you add a tasklist (like the TODOs in #826) with any remaining items?

@tparisi @lexaknyazev do you guys want to review this?

[lexaknyazev]

Tightening up:

• With Move byteStride from accessor to bufferView? #827, we've added new restriction on accessor/bufferView relations (all accessors using the same bufferView must have the same byteStride). This should be reflected in indices and values sub-objects (i.e., remove byteStride from there). Such change ensures proper alignment when indices and values use the same bufferView.
• bufferViews used for sparse data must have bufferView.target undefined.
• For more predictable unpacking process, sparse indices buffer must be sorted and it can't contain duplicate elements.

[lexaknyazev]

When accessor.type is not "SCALAR", e.g., "VEC3", how are indices stored: per component (e.g., 3 indices for each vec3) or per "value" (e.g., 1 index for each vec3)? The latter seems more effective.

[emilian0]

@lexaknyazev Indices are intended per value right now (one per VEC3 entry). What use case do you have in mind when you say it will make more sense with the index being per component? (skinning?)
Having it per component will be more granular:

• we will have to store ~*3 # indices.
• we will have more control over the entries that are zeroes.
  I see value in both, shall we have both versions, what do you think?

[lexaknyazev]

I meant per value.

[emilian0]

So @lexaknyazev it sounds like we both want indices to be per value. Thank you for clarifying that. I will re-iterate that in the specs. Will make the changes and ping you.

[emilian0]

@lexaknyazev I believe I addressed your concerns.

• I removed byte Stride from the accessor, sparse and indices arrays.
• the new language specifies that indices is a strictly increasing array of integer pointing to attributes (rather than components):
  indices: strictly increasing array of integers of size count and specific componentType that stores the indices of those attributes that deviate from the initialization value.
  I haven't address the note on "bufferView.target". I suggest to do this in a second pass, after the merge to 2.0 (when we will have a first stab at both sparse array and new bufferViews specs).
  Unless you have further comments this is ready to merge for me.
  Thanks!

[lexaknyazev]

Looks good. Couple more tweaks:

• when accessor is sparse, accessor.min and accessor.max must reflect decoded data, not initial;
• for example to be correct,
  • it must contain min and max;
  • sparse object shouldn't use the same bufferView as base accessor (it can for animation data, but not for vertex arrays because of bufferView.target);
• example uses indices.type, while in all other places it's indices.componentType.

[emilian0]

@lexaknyazev accessor range should be a union between the range in the initialization array and the range in the final (sparse) array. Does this sound reasonable to you? Rationale: a glTF implementation not supporting sparse arrays will fall back to the standard accessor implementation, we want the range (min/max) to work for both. Sounds good? (the other two were typos, thank you for catching them!)

[lexaknyazev]

Not sure about union of ranges. Why reduce bounding box efficiency for fully conformant implementation?

[emilian0]

To avoid things not rendering at all on non-conformant implementations! It is a trade off between being efficient in the fully conformant implementations vs being conservative enough so that things work in the old version. I am fine either way, it sounds like you value efficiency in conformat over fall-back on non conformant, I am happy to make the change.