microsoft/glTF-SDK

More general reading / decoding of accessor data

zeux opened this issue · 6 comments

zeux commented

I'm looking into refactoring glTF-SDK support for reading accessor data to make it more general wrt accessor value types.

Specifically, right now there's a fair amount of code dedicated to handling various forms of quantized data but all of this code is special-cased to handle individual cases covered by the spec. Like MeshPrimitiveUtils::GetTexCoords handling float, unsigned byte and unsigned short but not checking that the inputs are normalized, and not handling signed variants (which are not permitted by the spec atm so it's not a bug).

It feels like the code could be simpler if GLTFResourceReader provided smth like std::vector<float> DecodeBinaryData() that handled all format conversions and could be used in contexts where floating-point data is desired.

My ulterior motive is to use these helpers for decoding texture coordinate, position, normal and tangent data in MeshPrimitiveUtils - this would allow glTF SDK to work with quantized position/normal/tangent streams. This is currently outside of the set of types supported by specification but I'm hoping to make an extension that makes these types officially supported, and in the meantime this would allow files processed using gltfpack (http://github.com/zeux/meshoptimizer#gltfpack) to load using glTF-SDK (and, presumably by extension, using the 3D preview program in Win10).

Since the main goal here is to add support for files that are technically outside of the spec right now, I thought I'd ask first - is a PR that implement this likely to get merged? Does the implementation approach sound reasonable?

bghgary commented

I'm okay with this. @agrittmsft @chriche-ms Any thoughts?

agrittmsft commented

So this is just a refactor without any real functional changes? Sounds good to me.

zeux commented

The only functional change would entail getting (through refactoring) support for a full set of quantized types for geometry attributes without having to hard-code them (e.g. 8-bit normalized signed integers for normals) - which is currently outside of the set of types mandated by the spec but is useful to reduce the data size.

agrittmsft commented

What would be the new way to perform that type validation?

agrittmsft commented

Maybe something like this?

  1. The completely generic data handler function which you're describing
  2. A wrapper for (1) which takes std::pair<std::set, std::set> (+ maybe bool normalized?) and throws accordingly
  3. Individual wrappers for (2) for specific data types (e.g. Normals)
zeux commented

Can you clarify why the validation is necessary? OTOH I don’t think glTF SDK currently thoroughly validates the document on parse. Would help to understand the specific desirable validity criteria that apply here.