search

jkuhlmann / cgltf

:diamond_shape_with_a_dot_inside: Single-file glTF 2.0 loader and writer written in C99
MIT License
1.42k stars 135 forks source link

cgltf_node transform attributes are now store in double #239

Open bcomb opened 8 months ago

bcomb commented 8 months ago

Some GLTF contain huge value in translation and reading these as float cause lost in precision. Google store big translation in their 'Photorealistic 3D Tiles' data

jkuhlmann commented 8 months ago

Hey! The tests failed unfortunately. Also, do you think it would make sense to combine this with the changes in #229?

zeux commented 8 months ago

Related: #228 outlines the caveats around compatibility with an unconditional change like this. That said, #229 has a much wider scope that technically isn't required there. Maybe a good middleground would be something like cgltf_xfloat that's only used for transform components, still defaults to float (unlike this change), can be configured via a macro? That would keep backwards compatibility but also have a narrow scope unlike #229.

bcomb commented 8 months ago

Hi, I've fix the test.

Technically, all attributes of the JSON text block should be read in double precision; the GLTF specification does not specify a storage of floating-point numbers in 32-bit. In practice, I do not see the necessity of transforming everything into double precision except for the transform related components.

I understand the compatibility caveat of this kind of modification, but the required modification is still very straightforward. However, if it seems essential for CGLTF, the solution proposed by @zeux looks good to me (cgltf_xfloat used only for the transform related attributes and a CGLTF_USE_DOUBLE_PRECISION_TRANSFORM macro to control activation)