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

EXT_mesh_gpu_instancing should support quantized quaternions #1818

Closed
zeux opened this issue May 28, 2020 · 9 comments · Fixed by #1820
Closed

EXT_mesh_gpu_instancing should support quantized quaternions #1818

zeux opened this issue May 28, 2020 · 9 comments · Fixed by #1820

Comments

@zeux
Copy link
Contributor

zeux commented May 28, 2020

EXT_mesh_gpu_instancing was just merged in #1691 but it specifies quaternions as FLOAT_VEC4:

For "rotation" the values are quaternion FLOAT_VEC4's in the order (x, y, z, w), where w is the scalar.

This is problematic for transmission size reasons. There's a substantial difference in size between a floating-point vec4 quaternion (16 bytes) and signed-normalized 16-bit quaternion (8 bytes). In the basic case I'd expect a typical instance stream to contain 12-byte positions and 16-byte rotations right now (assuming no scaling) which adds up to 28 bytes, vs 20 bytes for signed-normalized representation.

Additionally using #1702 the quantized quaternions can be compressed further with more efficiency compared to non-quantized quaternions.

gltfpack currently only implements quantized storage. Babylon.JS already supports quantized inputs, Three.JS can trivially do so as discussed on the issue.

I'd like to ask that the spec be amended to change the rotation requirements.

@emackey
Copy link
Member

emackey commented Jun 1, 2020

@ultrafishotoy Is this something that can be added?

@ultrafishotoy
Copy link
Contributor

@ultrafishotoy Is this something that can be added?

I think its just a matter of adding some language to the description line of schema.

@emackey
Copy link
Member

emackey commented Jun 1, 2020

@zeux I think you're the most qualified, would you mind opening a PR to fix this?

@donmccurdy
Copy link
Contributor

Agreed, as this is also more consistent with the existing animation spec.

@lexaknyazev
Copy link
Member

The extension spec should have a normative list of allowed element types for each known instanced semantic. Also, while we're here, the extension spec should mention that all accessors used in the same extension object must be of the same count and that count is the number of instances.

@ultrafishotoy
Copy link
Contributor

The extension spec should have a normative list of allowed element types for each known

thanks @lexaknyazev, can someone point me to some example schema that does that? I did a search of the existing schema's and can't seem find any that specify the data type of a quaternion. It seems like the correct thing to do is to remove any references to the parameter data types from the schema and maybe mention them in the readme.md??

@lexaknyazev
Copy link
Member

It seems like the correct thing to do is to remove any references to the parameter data types from the schema and maybe mention them in the readme.md?

Exactly. The core spec has the table for attribute semantics and their types. It could be used as a template.

@zeux
Copy link
Contributor Author

zeux commented Jun 2, 2020

@ultrafishotoy Let me know if you'd like me to submit a PR for this or if you're going to do it (making sure we avoid conflicts)

@ultrafishotoy
Copy link
Contributor

@ultrafishotoy Let me know if you'd like me to submit a PR for this or if you're going to do it (making sure we avoid conflicts)

Please go ahead and thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants