Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support writing sparse accessors #351

Closed
donmccurdy opened this issue Sep 10, 2021 · 5 comments · Fixed by #793
Closed

Support writing sparse accessors #351

donmccurdy opened this issue Sep 10, 2021 · 5 comments · Fixed by #793
Labels
feature New enhancement or request package:core
Milestone

Comments

@donmccurdy
Copy link
Owner

Sparse accessors are useful in certain cases, it may be helpful to have an API in /core for indicating which accessors should be written as sparse. Identifying candidate accessors could be left for user control or a helper in /functions.

@donmccurdy
Copy link
Owner Author

donmccurdy commented Feb 27, 2022

@prideout
Copy link

prideout commented Jun 6, 2022

Nice, this will be a neat feature, especially since the popular exporters don't support sparse encoding. We've seen several models with morphing that have lots of zeroes, they would be definitely be helped by sparse encoding.

@donmccurdy
Copy link
Owner Author

donmccurdy commented Jan 10, 2023

I'm thinking of several ways this could work —

Current, before support for sparse arrays.

accessor
  .setType('SCALAR')
  .setArray(new Float32Array([0, 0, 0, 0, 1, 0, 0]));

1. Boolean, supports only 0-filled base array. Full backward-compatibility — the array is kept unpacked in memory and can be read and modified by code normally. At export, the serializer generates sparse indices and values.

accessor
  .setType('SCALAR')
  .setArray(new Float32Array([0, 0, 0, 0, 1, 0, 0]))
  .setSparse(true);

2. Explicit indices and values, supports distinct base, indices, and values arrays. Breaking change, and adds some non-trivial complexity to all code that reads or writes accessor values. This is how glTF specification actually represents sparse data, and is how the data would be written into a serialized file at export in any case.

accessor
  .setType('SCALAR')
  .setArray(null) // implicit 0-filled array. optional.
  .setSparseIndices(new Uint32Array([4]))
  .setSparseValues(new Float32Array([1]));

3. Explicit base, supports distinct base vs. final values. Breaking change, but more limited complexity for the rest of the codebase than (2). Code that reads accessor values can remain the same, but code that writes accessor values may need to update setSparseBase, e.g. to ensure the arrays remain equal in length.

accessor
  .setType('SCALAR')
  .setArray(new Float32Array([0, 0, 0, 0, 1, 0, 0]))
  .setSparse(true)
  .setSparseBase(null); // implicit 0-filled array. optional.

I might prefer (3), for a few reasons:

  1. Supports all uses of sparse accessors in the glTF specification
  2. Backwards-compatible for readers
  3. Can add support incrementally, starting with (1)

The one disadvantage I see is a higher memory footprint compared to (2). However, the common case using a zero-filled array retains the same memory footprint it has today, so no regression here. And because all engines I'm aware of will have to unpack this data before GPU upload anyway, I don't think supporting sparse data sizes that won't fit in memory is an important enough goal to complicate the 'normal' accessor API, which (2) would certainly do.

@donmccurdy
Copy link
Owner Author

FYI @hybridherbst, since you'd expressed an interest in #289.

@donmccurdy
Copy link
Owner Author

Work in progress toward (1), in:

Probably won't keep going beyond (1) for now, but this could be extended to support (3) later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New enhancement or request package:core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants